Skip to content

Commit f464bd8

Browse files
elianddbclaude
andcommitted
Refactor foreign key type validation for better maintainability
- Optimized validation logic with a cleaner flag-based approach - Improved code structure with a single return point - Enhanced documentation with detailed comments on type requirements - Added TODO explaining the current TIME precision limitation - Verified with foreign key tests to ensure correct behavior 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
1 parent 1e01ee0 commit f464bd8

File tree

1 file changed

+18
-18
lines changed

1 file changed

+18
-18
lines changed

sql/plan/foreign_key_editor.go

Lines changed: 18 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -550,8 +550,8 @@ func (reference *ForeignKeyReferenceHandler) CheckReference(ctx *sql.Context, ro
550550
reference.ForeignKey.ParentTable, reference.RowMapper.GetKeyString(row))
551551
}
552552

553-
// validateColumnTypeConstraints validates that column types meet MySQL foreign key requirements.
554-
// Centralizes type validation for decimal scale matching and exact time type precision matching.
553+
// validateColumnTypeConstraints enforces MySQL-compatible foreign key type validation
554+
// between child and parent columns in a foreign key relationship.
555555
func (reference *ForeignKeyReferenceHandler) validateColumnTypeConstraints(ctx *sql.Context, childRow sql.Row, parentRow sql.Row) error {
556556
mapper := reference.RowMapper
557557
if mapper.Index == nil {
@@ -565,24 +565,24 @@ func (reference *ForeignKeyReferenceHandler) validateColumnTypeConstraints(ctx *
565565

566566
parentType := parentCol.Type
567567
childType := mapper.SourceSch[mapper.IndexPositions[parentIdx]].Type
568-
569-
// Check for constraint violations
570568
hasViolation := false
571569

572-
// Decimal scale must match
573-
if childDecimal, ok := childType.(sql.DecimalType); ok {
574-
if parentDecimal, ok := parentType.(sql.DecimalType); ok {
575-
hasViolation = childDecimal.Scale() != parentDecimal.Scale()
576-
}
577-
}
578-
579-
// Time types must match exactly (including precision)
580-
if !hasViolation {
581-
isChildTime := types.IsTime(childType) || types.IsTimespan(childType)
582-
isParentTime := types.IsTime(parentType) || types.IsTimespan(parentType)
583-
if isChildTime && isParentTime {
584-
hasViolation = !childType.Equals(parentType)
585-
}
570+
// For decimal types, scales must match exactly
571+
childDecimal, childOk := childType.(sql.DecimalType)
572+
parentDecimal, parentOk := parentType.(sql.DecimalType)
573+
if childOk && parentOk {
574+
hasViolation = childDecimal.Scale() != parentDecimal.Scale()
575+
}
576+
577+
// For time types, require exact type matching (including precision)
578+
// TODO: The TIME type currently normalizes all precisions to TIME(6) internally,
579+
// which means TIME and TIME(n) are all treated as TIME(6). This prevents proper
580+
// precision validation between different TIME types in foreign keys.
581+
// See time.go:50-53 - "TIME is implemented as TIME(6)."
582+
isChildTime := types.IsTime(childType) || types.IsTimespan(childType)
583+
isParentTime := types.IsTime(parentType) || types.IsTimespan(parentType)
584+
if isChildTime && isParentTime {
585+
hasViolation = hasViolation || !childType.Equals(parentType)
586586
}
587587

588588
if hasViolation {

0 commit comments

Comments
 (0)