Skip to content

Commit c33a07d

Browse files
elianddbclaude
andcommitted
Fix mixed string type foreign key constraints
Fixes dolthub/dolt#9494 MySQL allows mixed string types in foreign key constraints: - CHAR can reference VARCHAR and vice versa - BINARY can reference VARBINARY and vice versa This change updates the foreignKeyComparableTypes function to allow these mixed type combinations by adding a new compatibleStringTypes helper function that follows existing codebase naming patterns and includes proper nil safety checks. The fix enables 6 previously skipped tests that validate the mixed string type functionality. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
1 parent 21301de commit c33a07d

File tree

2 files changed

+37
-7
lines changed

2 files changed

+37
-7
lines changed

enginetest/queries/foreign_key_queries.go

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2807,21 +2807,18 @@ var CreateForeignKeyTests = []ScriptTest{
28072807
ExpectedErrStr: "string 'abc' is too large for column 'c'",
28082808
},
28092809
{
2810-
Skip: true,
28112810
Query: "create table child_varchar_10 (vc varchar(10), foreign key (vc) references parent(c));",
28122811
Expected: []sql.Row{
28132812
{types.NewOkResult(0)},
28142813
},
28152814
},
28162815
{
2817-
Skip: true,
28182816
Query: "insert into child_varchar_10 values ('abc');",
28192817
Expected: []sql.Row{
28202818
{types.NewOkResult(1)},
28212819
},
28222820
},
28232821
{
2824-
Skip: true,
28252822
Query: "insert into child_varchar_10 values ('abcdefghij');",
28262823
ExpectedErr: sql.ErrForeignKeyChildViolation,
28272824
},
@@ -2850,21 +2847,18 @@ var CreateForeignKeyTests = []ScriptTest{
28502847
ExpectedErrStr: "string 'abc' is too large for column 'b'",
28512848
},
28522849
{
2853-
Skip: true,
28542850
Query: "create table child_varbinary_10 (vb varbinary(10), foreign key (vb) references parent(b));",
28552851
Expected: []sql.Row{
28562852
{types.NewOkResult(0)},
28572853
},
28582854
},
28592855
{
2860-
Skip: true,
28612856
Query: "insert into child_varbinary_10 values ('abc');",
28622857
Expected: []sql.Row{
28632858
{types.NewOkResult(1)},
28642859
},
28652860
},
28662861
{
2867-
Skip: true,
28682862
Query: "insert into child_varbinary_10 values ('abcdefghij');",
28692863
ExpectedErr: sql.ErrForeignKeyChildViolation,
28702864
},

sql/plan/alter_foreign_key.go

Lines changed: 37 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -670,12 +670,48 @@ func foreignKeyComparableTypes(ctx *sql.Context, type1 sql.Type, type2 sql.Type)
670670
return false
671671
}
672672
} else {
673-
return false
673+
// MySQL allows mixed string types in foreign key constraints:
674+
// - CHAR can reference VARCHAR and vice versa
675+
// - BINARY can reference VARBINARY and vice versa
676+
if compatibleStringTypes(type1, type2) {
677+
type1String := type1.(sql.StringType)
678+
type2String := type2.(sql.StringType)
679+
if type1String.Collation().CharacterSet() != type2String.Collation().CharacterSet() {
680+
return false
681+
}
682+
} else {
683+
return false
684+
}
674685
}
675686
}
676687
return true
677688
}
678689

690+
// compatibleStringTypes checks if two different string types are compatible for foreign key constraints.
691+
// MySQL allows mixed string types within the same category:
692+
// - Character types: CHAR and VARCHAR
693+
// - Binary types: BINARY and VARBINARY
694+
func compatibleStringTypes(type1 sql.Type, type2 sql.Type) bool {
695+
if type1 == nil || type2 == nil {
696+
return false
697+
}
698+
699+
t1 := type1.Type()
700+
t2 := type2.Type()
701+
702+
// Check if both are character string types (CHAR/VARCHAR)
703+
if (t1 == sqltypes.Char || t1 == sqltypes.VarChar) && (t2 == sqltypes.Char || t2 == sqltypes.VarChar) {
704+
return true
705+
}
706+
707+
// Check if both are binary string types (BINARY/VARBINARY)
708+
if (t1 == sqltypes.Binary || t1 == sqltypes.VarBinary) && (t2 == sqltypes.Binary || t2 == sqltypes.VarBinary) {
709+
return true
710+
}
711+
712+
return false
713+
}
714+
679715
// exprsAreIndexPrefix returns whether the given expressions are a prefix of the given index expressions
680716
func exprsAreIndexPrefix(exprs, indexExprs []string) bool {
681717
if len(exprs) > len(indexExprs) {

0 commit comments

Comments
 (0)