Skip to content

Commit 0a764ce

Browse files
elianddbclaude
andcommitted
Fix enum 0 value validation to respect STRICT_TRANS_TABLES mode
Previously, enum columns would always reject 0 values and other invalid values with ErrConvertingToEnum, regardless of SQL mode. This change implements proper MySQL-compatible behavior: - In STRICT_TRANS_TABLES mode: Invalid enum values (including 0) return ErrDataTruncatedForColumn error - In non-strict mode: Invalid enum values are converted to empty string (index 0) - INSERT IGNORE: Now properly handles ErrDataTruncatedForColumn as an ignorable error, converting invalid values to empty string with warnings Changes made: - Modified EnumType.Convert() to check SQL mode and handle invalid values accordingly - Added special handling for integer 0 in strict mode - Ensured uint16(0) is always allowed as internal enum storage value - Added ErrDataTruncatedForColumn to IgnorableErrors list for INSERT IGNORE - Updated unit tests to reflect correct MySQL-compatible behavior - Added integration tests for enum zero validation in both strict and non-strict modes Fixes dolthub/dolt#9425 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
1 parent 1af0f6e commit 0a764ce

File tree

4 files changed

+70
-4
lines changed

4 files changed

+70
-4
lines changed

enginetest/queries/insert_queries.go

Lines changed: 38 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2836,7 +2836,7 @@ var InsertIgnoreScripts = []ScriptTest{
28362836
Assertions: []ScriptTestAssertion{
28372837
{
28382838
Query: "insert into test_table values (1, 'invalid'), (2, 'comparative politics'), (3, null)",
2839-
ExpectedErr: types.ErrConvertingToEnum, // TODO: should be ErrDataTruncatedForColumn
2839+
ExpectedErr: types.ErrDataTruncatedForColumn,
28402840
},
28412841
{
28422842
Query: "insert ignore into test_table values (1, 'invalid'), (2, 'bye'), (3, null)",
@@ -2849,6 +2849,43 @@ var InsertIgnoreScripts = []ScriptTest{
28492849
},
28502850
},
28512851
},
2852+
{
2853+
// https://github.com/dolthub/dolt/issues/9425
2854+
Name: "issue 9425: 0 value is not allowed for enum in strict mode",
2855+
SetUpScript: []string{
2856+
"create table enum_zero_test (id int auto_increment primary key, enum_col enum('apple','banana','cherry'))",
2857+
},
2858+
Assertions: []ScriptTestAssertion{
2859+
{
2860+
Query: "select @@sql_mode",
2861+
Expected: []sql.Row{{"NO_ENGINE_SUBSTITUTION,ONLY_FULL_GROUP_BY,STRICT_TRANS_TABLES"}},
2862+
},
2863+
{
2864+
Query: "insert into enum_zero_test (enum_col) values (0)",
2865+
ExpectedErr: types.ErrDataTruncatedForColumn,
2866+
},
2867+
{
2868+
Query: "insert into enum_zero_test (enum_col) values ('invalid')",
2869+
ExpectedErr: types.ErrDataTruncatedForColumn,
2870+
},
2871+
{
2872+
Query: "set sql_mode = ''",
2873+
Expected: []sql.Row{{}},
2874+
},
2875+
{
2876+
Query: "insert into enum_zero_test (enum_col) values (0)",
2877+
Expected: []sql.Row{{types.OkResult{RowsAffected: 1, InsertID: 1}}},
2878+
},
2879+
{
2880+
Query: "insert into enum_zero_test (enum_col) values ('invalid')",
2881+
Expected: []sql.Row{{types.OkResult{RowsAffected: 1, InsertID: 2}}},
2882+
},
2883+
{
2884+
Query: "select * from enum_zero_test",
2885+
Expected: []sql.Row{{1, ""}, {2, ""}},
2886+
},
2887+
},
2888+
},
28522889
}
28532890

28542891
var IgnoreWithDuplicateUniqueKeyKeylessScripts = []ScriptTest{

sql/plan/insert.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import (
2222
"github.com/dolthub/go-mysql-server/sql"
2323
"github.com/dolthub/go-mysql-server/sql/expression"
2424
"github.com/dolthub/go-mysql-server/sql/transform"
25+
"github.com/dolthub/go-mysql-server/sql/types"
2526
)
2627

2728
// ErrInsertIntoNotSupported is thrown when a table doesn't support inserts
@@ -54,6 +55,7 @@ var IgnorableErrors = []*errors.Kind{sql.ErrInsertIntoNonNullableProvidedNull,
5455
sql.ErrDuplicateEntry,
5556
sql.ErrUniqueKeyViolation,
5657
sql.ErrCheckConstraintViolated,
58+
types.ErrDataTruncatedForColumn,
5759
}
5860

5961
// InsertInto is the top level node for INSERT INTO statements. It has a source for rows and a destination to insert

sql/types/enum.go

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -165,6 +165,15 @@ func (t EnumType) Convert(ctx context.Context, v interface{}) (interface{}, sql.
165165
switch value := v.(type) {
166166
case int:
167167
if _, ok := t.At(value); ok {
168+
// Special handling for index 0 in strict mode
169+
if value == 0 {
170+
if sqlCtx, ok := ctx.(*sql.Context); ok {
171+
sqlMode := sql.LoadSqlMode(sqlCtx)
172+
if sqlMode.ModeEnabled(sql.StrictTransTables) {
173+
return nil, sql.OutOfRange, ErrDataTruncatedForColumn.New("")
174+
}
175+
}
176+
}
168177
return uint16(value), sql.InRange, nil
169178
}
170179
case uint:
@@ -176,6 +185,10 @@ func (t EnumType) Convert(ctx context.Context, v interface{}) (interface{}, sql.
176185
case int16:
177186
return t.Convert(ctx, int(value))
178187
case uint16:
188+
// uint16(0) is always allowed as it represents the enum empty value used by INSERT IGNORE
189+
if value == 0 {
190+
return uint16(0), sql.InRange, nil
191+
}
179192
return t.Convert(ctx, int(value))
180193
case int32:
181194
return t.Convert(ctx, int(value))
@@ -204,6 +217,19 @@ func (t EnumType) Convert(ctx context.Context, v interface{}) (interface{}, sql.
204217
return t.Convert(ctx, string(value))
205218
}
206219

220+
// Check if we're in strict mode and handle invalid enum values appropriately
221+
// But only for cases where we might want to return the empty value (0)
222+
// For other invalid values, always return an error
223+
if sqlCtx, ok := ctx.(*sql.Context); ok {
224+
sqlMode := sql.LoadSqlMode(sqlCtx)
225+
if sqlMode.ModeEnabled(sql.StrictTransTables) {
226+
return nil, sql.OutOfRange, ErrDataTruncatedForColumn.New("")
227+
}
228+
// In non-strict mode, return empty string (index 0) for invalid enum values
229+
return uint16(0), sql.InRange, nil
230+
}
231+
232+
// If we can't determine SQL mode (e.g., test contexts), use the original error
207233
return nil, sql.InRange, ErrConvertingToEnum.New(v)
208234
}
209235

sql/types/enum_test.go

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -138,9 +138,10 @@ func TestEnumConvert(t *testing.T) {
138138
{[]string{"0", "1", "2"}, sql.Collation_Default, "3", "2", false},
139139
{[]string{"0", "1", "2"}, sql.Collation_Default, "2", "2", false},
140140

141-
{[]string{"one", "two"}, sql.Collation_Default, 3, nil, true},
142-
{[]string{"one", "two"}, sql.Collation_Default, "three", nil, true},
143-
{[]string{"one", "two"}, sql.Collation_Default, time.Date(2019, 12, 12, 12, 12, 12, 0, time.UTC), nil, true},
141+
// In non-strict mode (empty context), invalid enum values are converted to empty value (index 0)
142+
{[]string{"one", "two"}, sql.Collation_Default, 3, "", false},
143+
{[]string{"one", "two"}, sql.Collation_Default, "three", "", false},
144+
{[]string{"one", "two"}, sql.Collation_Default, time.Date(2019, 12, 12, 12, 12, 12, 0, time.UTC), "", false},
144145
}
145146

146147
for _, test := range tests {

0 commit comments

Comments
 (0)