Merge pull request #3094 from dolthub/elianddb/9496-decimal-foreign-keys

  dolthub/dolt#9496 - Fix DECIMAL foreign key constraint validation to match MySQL behavior

Author: elianddb

enginetest/queries/script_queries.go

@@ -10508,6 +10508,8 @@ where
 		SetUpScript: []string{
 			"create table parent (d decimal(4, 2) primary key);",
 			"insert into parent values (1.23), (45.67), (78.9);",
+			"create table parent_multi (d1 decimal(4,2), d2 decimal(3,1), primary key (d1, d2));",
+			"insert into parent_multi values (1.23, 4.5), (45.67, 78.9), (99.99, 0.1);",
 		},
 		Assertions: []ScriptTestAssertion{
 			{
@@ -10528,43 +10530,60 @@ where
 					{types.NewOkResult(3)},
 				},
 			},
-
 			{
-				Skip:  true,
+				Query:       "insert into child_dec_4_2 values (99.99);",
+				ExpectedErr: sql.ErrForeignKeyChildViolation,
+			},
+			{
 				Query: "create table child_dec_4_1 (d decimal(4,1), foreign key (d) references parent (d));",
 				Expected: []sql.Row{
 					{types.NewOkResult(0)},
 				},
 			},
 			{
-				Skip:        true,
 				Query:       "insert into child_dec_4_1 values (78.9);",
-				ExpectedErr: sql.ErrForeignKeyParentViolation,
+				ExpectedErr: sql.ErrForeignKeyChildViolation,
+			},
+			{
+				Query:       "insert into child_dec_4_1 values (99.9);",
+				ExpectedErr: sql.ErrForeignKeyChildViolation,
 			},
-
 			{
-				Skip:  true,
 				Query: "create table child_dec_3_2 (d decimal(3,2), foreign key (d) references parent (d));",
 				Expected: []sql.Row{
 					{types.NewOkResult(0)},
 				},
 			},
 			{
-				Skip:        true,
-				Query:       "insert into child_dec_3_2 values (1.23);",
-				ExpectedErr: sql.ErrForeignKeyParentViolation,
+				Query: "insert into child_dec_3_2 values (1.23);",
+				Expected: []sql.Row{
+					{types.NewOkResult(1)},
+				},
 			},
 			{
-				Skip:  true,
 				Query: "create table child_dec_65_30 (d decimal(65,30), foreign key (d) references parent (d));",
 				Expected: []sql.Row{
 					{types.NewOkResult(0)},
 				},
 			},
 			{
-				Skip:        true,
 				Query:       "insert into child_dec_65_30 values (1.23);",
-				ExpectedErr: sql.ErrForeignKeyParentViolation,
+				ExpectedErr: sql.ErrForeignKeyChildViolation,
+			},
+			{
+				Query: "create table child_multi_4_2_3_1 (d1 decimal(4,2), d2 decimal(3,1), foreign key (d1, d2) references parent_multi (d1, d2));",
+				Expected: []sql.Row{
+					{types.NewOkResult(0)},
+				}},
+			{
+				Query: "insert into child_multi_4_2_3_1 values (1.23, 4.5), (45.67, 78.9), (NULL, NULL);",
+				Expected: []sql.Row{
+					{types.NewOkResult(3)},
+				},
+			},
+			{
+				Query:       "insert into child_multi_4_2_3_1 values (1.23, 9.9);",
+				ExpectedErr: sql.ErrForeignKeyChildViolation,
 			},
 		},
 	},

sql/plan/alter_foreign_key.go

@@ -659,6 +659,10 @@ func foreignKeyComparableTypes(ctx *sql.Context, type1 sql.Type, type2 sql.Type)
 				// Enum types can reference each other in foreign keys regardless of their string values.
 				// MySQL allows enum foreign keys to match based on underlying numeric values.
 				return true
+			case sqltypes.Decimal:
+				// MySQL allows decimal foreign keys with different precision/scale
+				// The foreign key constraint validation will handle the actual value comparison
+				return true
 			default:
 				return false
 			}

sql/plan/foreign_key_editor.go

@@ -512,6 +512,10 @@ func (reference *ForeignKeyReferenceHandler) CheckReference(ctx *sql.Context, ro
 		return err
 	}
 	if err == nil {
+		// We have a parent row, but check for type-specific validation
+		if validationErr := reference.validateDecimalConstraints(row); validationErr != nil {
+			return validationErr
+		}
 		// We have a parent row so throw no error
 		return nil
 	}
@@ -539,6 +543,39 @@ func (reference *ForeignKeyReferenceHandler) CheckReference(ctx *sql.Context, ro
 		reference.ForeignKey.ParentTable, reference.RowMapper.GetKeyString(row))
 }
 
+// validateDecimalConstraints checks that decimal foreign key columns have compatible scales.
+func (reference *ForeignKeyReferenceHandler) validateDecimalConstraints(row sql.Row) error {
+	if reference.RowMapper.Index == nil {
+		return nil
+	}
+	indexColumnTypes := reference.RowMapper.Index.ColumnExpressionTypes()
+	for parentIdx, parentCol := range indexColumnTypes {
+		if parentIdx >= len(reference.RowMapper.IndexPositions) {
+			break
+		}
+		parentType := parentCol.Type
+		childColIdx := reference.RowMapper.IndexPositions[parentIdx]
+		childType := reference.RowMapper.SourceSch[childColIdx].Type
+		childDecimal, ok := childType.(sql.DecimalType)
+		if !ok {
+			continue
+		}
+		parentDecimal, ok := parentType.(sql.DecimalType)
+		if !ok {
+			continue
+		}
+		if childDecimal.Scale() != parentDecimal.Scale() {
+			return sql.ErrForeignKeyChildViolation.New(
+				reference.ForeignKey.Name,
+				reference.ForeignKey.Table,
+				reference.ForeignKey.ParentTable,
+				reference.RowMapper.GetKeyString(row),
+			)
+		}
+	}
+	return nil
+}
+
 // CheckTable checks that every row in the table has an index entry in the referenced table.
 func (reference *ForeignKeyReferenceHandler) CheckTable(ctx *sql.Context, tbl sql.ForeignKeyTable) error {
 	partIter, err := tbl.Partitions(ctx)