-
-
Notifications
You must be signed in to change notification settings - Fork 58
Support for using TEXT as a foreign key target #1260
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
2b62558
dc944fe
2713aec
6d61577
509be11
38c5b55
24130f3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -15,7 +15,12 @@ | |
| package analyzer | ||
|
|
||
| import ( | ||
| "strings" | ||
|
|
||
| "github.com/dolthub/go-mysql-server/sql" | ||
| "github.com/dolthub/go-mysql-server/sql/analyzer" | ||
| "github.com/dolthub/go-mysql-server/sql/plan" | ||
| "github.com/dolthub/vitess/go/sqltypes" | ||
| ) | ||
|
|
||
| // IDs are basically arbitrary, we just need to ensure that they do not conflict with existing IDs | ||
|
|
@@ -86,6 +91,49 @@ func Init() { | |
| analyzer.Rule{Id: ruleId_AddDomainConstraintsToCasts, Apply: AddDomainConstraintsToCasts}, | ||
| analyzer.Rule{Id: ruleId_ReplaceNode, Apply: ReplaceNode}, | ||
| analyzer.Rule{Id: ruleId_InsertContextRootFinalizer, Apply: InsertContextRootFinalizer}) | ||
|
|
||
| initEngine() | ||
| } | ||
|
|
||
| func initEngine() { | ||
| plan.ValidateForeignKeyDefinition = validateForeignKeyDefinition | ||
| } | ||
|
|
||
| // validateForeignKeyDefinition validates that the given foreign key definition is valid for creation | ||
| func validateForeignKeyDefinition(ctx *sql.Context, fkDef sql.ForeignKeyConstraint, cols map[string]*sql.Column, parentCols map[string]*sql.Column) error { | ||
| // TODO: this check is too permissive, we should be doing some type checks here | ||
| for i := range fkDef.Columns { | ||
| col := cols[strings.ToLower(fkDef.Columns[i])] | ||
| parentCol := parentCols[strings.ToLower(fkDef.ParentColumns[i])] | ||
| if !foreignKeyComparableTypes(ctx, col.Type, parentCol.Type) { | ||
| return sql.ErrForeignKeyColumnTypeMismatch.New(fkDef.Columns[i], fkDef.ParentColumns[i]) | ||
| } | ||
| } | ||
| return nil | ||
| } | ||
|
|
||
| // foreignKeyComparableTypes returns whether the two given types are able to be used as parent/child columns in a | ||
| // foreign key. | ||
| func foreignKeyComparableTypes(ctx *sql.Context, type1 sql.Type, type2 sql.Type) bool { | ||
| if !type1.Equals(type2) { | ||
| // There seems to be a special case where CHAR/VARCHAR/BINARY/VARBINARY can have unequal lengths. | ||
| // Have not tested every type nor combination, but this seems specific to those 4 types. | ||
| if type1.Type() == type2.Type() { | ||
| switch type1.Type() { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Adding on to my other comment, we aren't too strict on ensuring that these are returned correctly, considering they don't map to all of our types so it's not really possible to do so. Instead, it's better to check for these specific types. You could even use I think |
||
| case sqltypes.Char, sqltypes.VarChar, sqltypes.Binary, sqltypes.VarBinary: | ||
| type1String := type1.(sql.StringType) | ||
| type2String := type2.(sql.StringType) | ||
| if type1String.Collation().CharacterSet() != type2String.Collation().CharacterSet() { | ||
| return false | ||
| } | ||
| default: | ||
| return false | ||
| } | ||
| } else { | ||
| return false | ||
| } | ||
| } | ||
| return true | ||
| } | ||
|
|
||
| // insertAnalyzerRules inserts the given rule(s) before or after the given analyzer.RuleId, returning an updated slice. | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These can be
DoltgresTypes rather thansql.Type