Skip to content

Commit

Permalink
insert ignore to enum column truncates data (#2774)
Browse files Browse the repository at this point in the history
  • Loading branch information
jennifersp authored Dec 2, 2024
1 parent 6efed41 commit ef1a969
Show file tree
Hide file tree
Showing 5 changed files with 51 additions and 8 deletions.
39 changes: 39 additions & 0 deletions enginetest/queries/insert_queries.go
Original file line number Diff line number Diff line change
Expand Up @@ -2702,6 +2702,28 @@ var InsertIgnoreScripts = []ScriptTest{
},
},
},
{
// /~https://github.com/dolthub/dolt/issues/8611
Name: "issue 8611: insert ignore on enum type column",
SetUpScript: []string{
"create table test_table (x int auto_increment primary key, y enum('hello','bye'))",
},
Assertions: []ScriptTestAssertion{
{
Query: "insert into test_table values (1, 'invalid'), (2, 'comparative politics'), (3, null)",
ExpectedErr: types.ErrConvertingToEnum, // TODO: should be ErrDataTruncatedForColumn
},
{
Query: "insert ignore into test_table values (1, 'invalid'), (2, 'bye'), (3, null)",
Expected: []sql.Row{{types.OkResult{RowsAffected: 3}}},
//ExpectedWarning: mysql.ERWarnDataTruncated, // TODO: incorrect code
},
{
Query: "select * from test_table",
Expected: []sql.Row{{1, ""}, {2, "bye"}, {3, nil}},
},
},
},
}

var IgnoreWithDuplicateUniqueKeyKeylessScripts = []ScriptTest{
Expand Down Expand Up @@ -2958,4 +2980,21 @@ var InsertBrokenScripts = []ScriptTest{
},
},
},
{
// /~https://github.com/dolthub/dolt/issues/8617
Name: "INSERT INTO with ENUM NOT NULL",
SetUpScript: []string{
"CREATE TABLE test (pk BIGINT PRIMARY KEY NOT NULL, v1 ENUM('a','b','c') NOT NULL);",
},
Assertions: []ScriptTestAssertion{
{
Query: "INSERT INTO test (pk) VALUES (1);",
Expected: []sql.Row{{types.NewOkResult(1)}},
},
{
Query: "select * from t2;",
Expected: []sql.Row{{1, "a"}},
},
},
},
}
2 changes: 1 addition & 1 deletion enginetest/queries/script_queries.go
Original file line number Diff line number Diff line change
Expand Up @@ -4368,7 +4368,7 @@ CREATE TABLE tab3 (
time.Date(0, 1, 1, 0, 0, 0, 0, time.UTC),
time.Date(0, 1, 1, 0, 0, 0, 0, time.UTC),
0,
"first",
"",
"",
},
},
Expand Down
2 changes: 1 addition & 1 deletion sql/rowexec/ddl_iters.go
Original file line number Diff line number Diff line change
Expand Up @@ -570,7 +570,7 @@ func (i *modifyColumnIter) rewriteTable(ctx *sql.Context, rwt sql.RewritableTabl
oldStr, _ := oldEnum.At(oldIdx)
newIdx := newEnum.IndexOf(oldStr)
if newIdx == -1 {
return false, fmt.Errorf("data truncated for column %s", newCol.Name)
return false, types.ErrDataTruncatedForColumn.New(newCol.Name)
}
newRow[newColIdx] = uint16(newIdx)
}
Expand Down
13 changes: 9 additions & 4 deletions sql/types/enum.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@ const (
var (
ErrConvertingToEnum = errors.NewKind("value %v is not valid for this Enum")

ErrDataTruncatedForColumn = errors.NewKind("Data truncated for column '%s'")

enumValueType = reflect.TypeOf(uint16(0))
)

Expand Down Expand Up @@ -278,15 +280,18 @@ func (t EnumType) CollationCoercibility(ctx *sql.Context) (collation sql.Collati

// Zero implements Type interface.
func (t EnumType) Zero() interface{} {
// / If an ENUM column is declared NOT NULL, its default value is the first element of the list of permitted values.
return uint16(1)
// TODO: If an ENUM column is declared NOT NULL, its default value is the first element of the list of permitted values.
return uint16(0)
}

// At implements EnumType interface.
func (t EnumType) At(index int) (string, bool) {
// / The elements listed in the column specification are assigned index numbers, beginning with 1.
// The elements listed in the column specification are assigned index numbers, beginning with 1.
index -= 1
if index < 0 || index >= len(t.indexToVal) {
if index <= -1 {
// for index zero, the value is empty. It's used for insert ignore.
return "", true
} else if index >= len(t.indexToVal) {
return "", false
}
return t.indexToVal[index], true
Expand Down
3 changes: 1 addition & 2 deletions sql/types/enum_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,6 @@ func TestEnumConvert(t *testing.T) {
{[]string{"0", "1", "2"}, sql.Collation_Default, "2", "2", false},

{[]string{"one", "two"}, sql.Collation_Default, 3, nil, true},
{[]string{"one", "two"}, sql.Collation_Default, 0, nil, true},
{[]string{"one", "two"}, sql.Collation_Default, "three", nil, true},
{[]string{"one", "two"}, sql.Collation_Default, time.Date(2019, 12, 12, 12, 12, 12, 0, time.UTC), nil, true},
}
Expand Down Expand Up @@ -199,7 +198,7 @@ func TestEnumZero(t *testing.T) {
typ := MustCreateEnumType(test.vals, sql.Collation_Default)
v, ok := typ.Zero().(uint16)
assert.True(t, ok)
assert.Equal(t, uint16(1), v)
assert.Equal(t, uint16(0), v)
})
}
}

0 comments on commit ef1a969

Please sign in to comment.