Merge pull request #3076 from dolthub/elianddb/9425-enum-zero-strict-mode

elianddb · web-flow · commit 8b0896dfd96f · 2025-07-10T10:41:31.000-07:00
dolthub/dolt#9425 - Fix enum zero validation in strict mode
diff --git a/enginetest/queries/script_queries.go b/enginetest/queries/script_queries.go
@@ -8465,23 +8465,20 @@ where
 				ExpectedErr: sql.ErrIncompatibleDefaultType,
 			},
 			{
-				Skip:        true,
 				Query:       "create table bad (e enum('a') default 0);",
-				ExpectedErr: sql.ErrIncompatibleDefaultType,
+				ExpectedErr: sql.ErrInvalidColumnDefaultValue,
 			},
 			{
 				Query:       "create table bad (e enum('a') default '');",
 				ExpectedErr: sql.ErrIncompatibleDefaultType,
 			},
 			{
-				Skip:        true,
 				Query:       "create table bad (e enum('a') default '1');",
-				ExpectedErr: sql.ErrIncompatibleDefaultType,
+				ExpectedErr: sql.ErrInvalidColumnDefaultValue,
 			},
 			{
-				Skip:        true,
 				Query:       "create table bad (e enum('a') default 1);",
-				ExpectedErr: sql.ErrIncompatibleDefaultType,
+				ExpectedErr: sql.ErrInvalidColumnDefaultValue,
 			},
 
 			{
@@ -8688,22 +8685,79 @@ where
 	},
 	{
 		// This is with STRICT_TRANS_TABLES or STRICT_ALL_TABLES in sql_mode
-		Skip:    true,
 		Name:    "enums with zero",
 		Dialect: "mysql",
 		SetUpScript: []string{
+			"SET sql_mode = 'STRICT_TRANS_TABLES';",
 			"create table t (e enum('a', 'b', 'c'));",
 		},
 		Assertions: []ScriptTestAssertion{
 			{
-				Query: "insert into t values (0);",
-				// TODO should be truncated error, but this is the error we throw for empty string
-				ExpectedErrStr: "is not valid for this Enum",
+				Query:          "insert into t values (0);",
+				ExpectedErrStr: "Data truncated for column 'e' at row 1",
+			},
+			{
+				Query:          "insert into t values ('a'), (0), ('b');",
+				ExpectedErrStr: "Data truncated for column 'e' at row 2",
 			},
 			{
 				Query:       "create table tt (e enum('a', 'b', 'c') default 0)",
 				ExpectedErr: sql.ErrInvalidColumnDefaultValue,
 			},
+			{
+				Query: "create table et (e enum('a', 'b', '', 'c'));",
+				Expected: []sql.Row{
+					{types.NewOkResult(0)},
+				},
+			},
+			{
+				Query:          "insert into et values (0);",
+				ExpectedErrStr: "Data truncated for column 'e' at row 1",
+			},
+		},
+	},
+	{
+		Name:    "enums with zero strict all tables",
+		Dialect: "mysql",
+		SetUpScript: []string{
+			"SET sql_mode = 'STRICT_ALL_TABLES';",
+			"create table t (e enum('a', 'b', 'c'));",
+		},
+		Assertions: []ScriptTestAssertion{
+			{
+				Query:          "insert into t values (0);",
+				ExpectedErrStr: "Data truncated for column 'e' at row 1",
+			},
+			{
+				Query:          "insert into t values ('a'), (0), ('b');",
+				ExpectedErrStr: "Data truncated for column 'e' at row 2",
+			},
+			{
+				Query:       "create table tt (e enum('a', 'b', 'c') default 0)",
+				ExpectedErr: sql.ErrInvalidColumnDefaultValue,
+			},
+		},
+	},
+	{
+		Name:    "enums with zero non-strict mode",
+		Dialect: "mysql",
+		SetUpScript: []string{
+			"SET sql_mode = '';",
+			"create table t (e enum('a', 'b', 'c'));",
+		},
+		Assertions: []ScriptTestAssertion{
+			{
+				Query: "insert into t values (0);",
+				Expected: []sql.Row{
+					{types.NewOkResult(1)},
+				},
+			},
+			{
+				Query: "select * from t;",
+				Expected: []sql.Row{
+					{""},
+				},
+			},
 		},
 	},
 	{
diff --git a/sql/analyzer/resolve_column_defaults.go b/sql/analyzer/resolve_column_defaults.go
@@ -278,9 +278,46 @@ func validateColumnDefault(ctx *sql.Context, col *sql.Column, colDefault *sql.Co
 		return err
 	}
 
+	if enumType, isEnum := col.Type.(sql.EnumType); isEnum && colDefault.IsLiteral() {
+		if err = validateEnumLiteralDefault(enumType, colDefault, col.Name, ctx); err != nil {
+			return err
+		}
+	}
+
 	return nil
 }
 
+// validateEnumLiteralDefault validates enum literal defaults more strictly than runtime conversions
+// MySQL doesn't allow numeric index references for literal enum defaults
+func validateEnumLiteralDefault(enumType sql.EnumType, colDefault *sql.ColumnDefaultValue, columnName string, ctx *sql.Context) error {
+	val, err := colDefault.Expr.Eval(ctx, nil)
+	if err != nil {
+		return err
+	}
+
+	switch v := val.(type) {
+	case string:
+		// For string values, check if it's a direct enum value match
+		enumValues := enumType.Values()
+		for _, enumVal := range enumValues {
+			if enumVal == v {
+				return nil // Valid enum value
+			}
+		}
+		// String doesn't match any enum value, return appropriate error
+		if v == "" {
+			return sql.ErrIncompatibleDefaultType.New()
+		}
+		return sql.ErrInvalidColumnDefaultValue.New(columnName)
+	case int, int8, int16, int32, int64, uint, uint8, uint16, uint32, uint64:
+		// MySQL doesn't allow numeric enum indices as literal defaults
+		return sql.ErrInvalidColumnDefaultValue.New(columnName)
+	default:
+		// Other types not supported for enum defaults
+		return sql.ErrIncompatibleDefaultType.New()
+	}
+}
+
 func stripTableNamesFromDefault(e *expression.Wrapper) (sql.Expression, transform.TreeIdentity, error) {
 	newDefault, ok := e.Unwrap().(*sql.ColumnDefaultValue)
 	if !ok {
diff --git a/sql/planbuilder/ddl.go b/sql/planbuilder/ddl.go
@@ -1418,6 +1418,15 @@ func (b *Builder) tableSpecToSchema(inScope, outScope *scope, db sql.Database, t
 	}
 
 	for i, def := range defaults {
+		// Early validation for enum default 0 to catch it before conversion
+		if def != nil && types.IsEnum(schema[i].Type) {
+			if lit, ok := def.(*ast.SQLVal); ok {
+				if lit.Type == ast.IntVal && string(lit.Val) == "0" {
+					b.handleErr(sql.ErrInvalidColumnDefaultValue.New(schema[i].Name))
+				}
+			}
+		}
+
 		schema[i].Default = b.convertDefaultExpression(outScope, def, schema[i].Type, schema[i].Nullable)
 		err := validateDefaultExprs(schema[i])
 		if err != nil {
diff --git a/sql/types/enum.go b/sql/types/enum.go
@@ -165,6 +165,10 @@ func (t EnumType) Convert(ctx context.Context, v interface{}) (interface{}, sql.
 
 	switch value := v.(type) {
 	case int:
+		// MySQL rejects 0 values in strict mode regardless of enum definition
+		if value == 0 && t.validateScrictMode(ctx) {
+			return nil, sql.OutOfRange, ErrConvertingToEnum.New(value)
+		}
 		if _, ok := t.At(value); ok {
 			return uint16(value), sql.InRange, nil
 		}
@@ -177,7 +181,10 @@ func (t EnumType) Convert(ctx context.Context, v interface{}) (interface{}, sql.
 	case int16:
 		return t.Convert(ctx, int(value))
 	case uint16:
-		return t.Convert(ctx, int(value))
+		// uint16 values are stored enum indices - allow them without strict mode validation
+		if _, ok := t.At(int(value)); ok {
+			return value, sql.InRange, nil
+		}
 	case int32:
 		return t.Convert(ctx, int(value))
 	case uint32:
@@ -208,6 +215,15 @@ func (t EnumType) Convert(ctx context.Context, v interface{}) (interface{}, sql.
 	return nil, sql.InRange, ErrConvertingToEnum.New(v)
 }
 
+// validateScrictMode checks if STRICT_TRANS_TABLES or STRICT_ALL_TABLES is enabled
+func (t EnumType) validateScrictMode(ctx context.Context) bool {
+	if sqlCtx, ok := ctx.(*sql.Context); ok {
+		sqlMode := sql.LoadSqlMode(sqlCtx)
+		return sqlMode.ModeEnabled("STRICT_TRANS_TABLES") || sqlMode.ModeEnabled("STRICT_ALL_TABLES")
+	}
+	return false
+}
+
 // Equals implements the Type interface.
 func (t EnumType) Equals(otherType sql.Type) bool {
 	if ot, ok := otherType.(EnumType); ok && t.collation.Equals(ot.collation) && len(t.idxToVal) == len(ot.idxToVal) {
