Skip to content

Commit 028d9ca

Browse files
angelamayxieangelamayxie
authored
Merge pull request #3134 from dolthub/angela/conversions
Properly convert enum/set columns to string columns with alter table
2 parents 6bc9ba7 + bb8cef2 commit 028d9ca

File tree

7 files changed

+87
-28
lines changed

7 files changed

+87
-28
lines changed

‎enginetest/queries/script_queries.go‎

Lines changed: 66 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9224,7 +9224,6 @@ where
92249224
},
92259225
},
92269226
{
9227-
Skip: true,
92289227
Query: "select e from t where e like 'a%' order by e;",
92299228
Expected: []sql.Row{
92309229
{"abc"},
@@ -9389,6 +9388,39 @@ where
93899388
},
93909389
},
93919390
},
9391+
{
9392+
Name: "Convert enum columns to string columns with alter table",
9393+
Dialect: "mysql",
9394+
SetUpScript: []string{
9395+
"create table t(pk int primary key, c0 enum('a', 'b', 'c'));",
9396+
"insert into t values(0, 'a'), (1, 'b'), (2, 'c');",
9397+
"alter table t modify column c0 varchar(100);",
9398+
},
9399+
Assertions: []ScriptTestAssertion{
9400+
{
9401+
Query: "select * from t",
9402+
Expected: []sql.Row{{0, "a"}, {1, "b"}, {2, "c"}},
9403+
},
9404+
},
9405+
},
9406+
{
9407+
// https://github.com/dolthub/dolt/issues/9613
9408+
Skip: true,
9409+
Name: "Convert enum columns to string columns when copying table",
9410+
Dialect: "mysql",
9411+
SetUpScript: []string{
9412+
"create table t(pk int primary key, c0 enum('a', 'b', 'c'));",
9413+
"insert into t values(0, 'a'), (1, 'b'), (2, 'c');",
9414+
"create table tt(pk int primary key, c0 varchar(10))",
9415+
"insert into tt select * from t",
9416+
},
9417+
Assertions: []ScriptTestAssertion{
9418+
{
9419+
Query: "select * from tt",
9420+
Expected: []sql.Row{{0, "a"}, {1, "b"}, {2, "c"}},
9421+
},
9422+
},
9423+
},
93929424
{
93939425
Name: "enums with foreign keys",
93949426
Dialect: "mysql",
@@ -10001,6 +10033,39 @@ where
1000110033
},
1000210034
},
1000310035
},
10036+
{
10037+
Name: "Convert set columns to string columns with alter table",
10038+
Dialect: "mysql",
10039+
SetUpScript: []string{
10040+
"create table t(pk int primary key, c0 set('abc', 'def','ghi'))",
10041+
"insert into t values(0, 'abc,def'), (1, 'def'), (2, 'ghi');",
10042+
"alter table t modify column c0 varchar(100);",
10043+
},
10044+
Assertions: []ScriptTestAssertion{
10045+
{
10046+
Query: "select * from t",
10047+
Expected: []sql.Row{{0, "abc,def"}, {1, "def"}, {2, "ghi"}},
10048+
},
10049+
},
10050+
},
10051+
{
10052+
// https://github.com/dolthub/dolt/issues/9613
10053+
Skip: true,
10054+
Name: "Convert set columns to string columns when copying table",
10055+
Dialect: "mysql",
10056+
SetUpScript: []string{
10057+
"create table t(pk int primary key, c0 set('abc', 'def','ghi'))",
10058+
"insert into t values(0, 'abc,def'), (1, 'def'), (2, 'ghi');",
10059+
"create table tt(pk int primary key, c0 varchar(10))",
10060+
"insert into tt select * from t",
10061+
},
10062+
Assertions: []ScriptTestAssertion{
10063+
{
10064+
Query: "select * from tt",
10065+
Expected: []sql.Row{{0, "abc,def"}, {1, "def"}, {2, "ghi"}},
10066+
},
10067+
},
10068+
},
1000410069
{
1000510070
Name: "set with duplicates",
1000610071
Dialect: "mysql",

‎memory/table.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1436,7 +1436,8 @@ func (t *Table) ModifyColumn(ctx *sql.Context, columnName string, column *sql.Co
14361436
var oldRowWithoutVal sql.Row
14371437
oldRowWithoutVal = append(oldRowWithoutVal, row[:oldIdx]...)
14381438
oldRowWithoutVal = append(oldRowWithoutVal, row[oldIdx+1:]...)
1439-
newVal, inRange, err := column.Type.Convert(ctx, row[oldIdx])
1439+
oldType := data.schema.Schema[oldIdx].Type
1440+
newVal, inRange, err := types.TypeAwareConversion(ctx, row[oldIdx], oldType, column.Type)
14401441
if err != nil {
14411442
if sql.ErrNotMatchingSRID.Is(err) {
14421443
err = sql.ErrNotMatchingSRIDWithColName.New(columnName, err)

‎sql/expression/case.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,7 @@ func (c *Case) Eval(ctx *sql.Context, row sql.Row) (interface{}, error) {
136136
}
137137
// When unable to convert to the type of the case, return the original value
138138
// A common error here is "Out of bounds value for decimal type"
139-
if ret, err := types.TypeAwareConversion(ctx, bval, b.Value.Type(), t); err == nil {
139+
if ret, inRange, err := types.TypeAwareConversion(ctx, bval, b.Value.Type(), t); inRange && err == nil {
140140
return ret, nil
141141
}
142142
return bval, nil
@@ -150,7 +150,7 @@ func (c *Case) Eval(ctx *sql.Context, row sql.Row) (interface{}, error) {
150150
}
151151
// When unable to convert to the type of the case, return the original value
152152
// A common error here is "Out of bounds value for decimal type"
153-
if ret, err := types.TypeAwareConversion(ctx, val, c.Else.Type(), t); err == nil {
153+
if ret, inRange, err := types.TypeAwareConversion(ctx, val, c.Else.Type(), t); inRange && err == nil {
154154
return ret, nil
155155
}
156156
return val, nil

‎sql/expression/comparison.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -188,7 +188,7 @@ func (c *comparison) castLeftAndRight(ctx *sql.Context, left, right interface{})
188188
if r, inRange, err := leftType.Convert(ctx, right); inRange && err == nil {
189189
return left, r, leftType, nil
190190
} else {
191-
l, err := types.TypeAwareConversion(ctx, left, leftType, rightType)
191+
l, _, err := types.TypeAwareConversion(ctx, left, leftType, rightType)
192192
if err != nil {
193193
return nil, nil, nil, err
194194
}
@@ -200,7 +200,7 @@ func (c *comparison) castLeftAndRight(ctx *sql.Context, left, right interface{})
200200
if l, inRange, err := rightType.Convert(ctx, left); inRange && err == nil {
201201
return l, right, rightType, nil
202202
} else {
203-
r, err := types.TypeAwareConversion(ctx, right, rightType, leftType)
203+
r, _, err := types.TypeAwareConversion(ctx, right, rightType, leftType)
204204
if err != nil {
205205
return nil, nil, nil, err
206206
}

‎sql/expression/convert.go‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -291,10 +291,7 @@ func convertValue(ctx *sql.Context, val interface{}, castTo string, originType s
291291
}
292292
switch strings.ToLower(castTo) {
293293
case ConvertToBinary:
294-
if types.IsSet(originType) || types.IsEnum(originType) {
295-
val, _ = types.TypeAwareConversion(ctx, val, originType, types.LongText)
296-
}
297-
b, _, err := types.LongBlob.Convert(ctx, val)
294+
b, _, err := types.TypeAwareConversion(ctx, val, originType, types.LongBlob)
298295
if err != nil {
299296
return nil, nil
300297
}
@@ -312,8 +309,7 @@ func convertValue(ctx *sql.Context, val interface{}, castTo string, originType s
312309
}
313310
return truncateConvertedValue(b, typeLength)
314311
case ConvertToChar, ConvertToNChar:
315-
val, _ = types.TypeAwareConversion(ctx, val, originType, types.LongText)
316-
s, _, err := types.LongText.Convert(ctx, val)
312+
s, _, err := types.TypeAwareConversion(ctx, val, originType, types.LongText)
317313
if err != nil {
318314
return nil, nil
319315
}

‎sql/rowexec/ddl_iters.go‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -570,7 +570,7 @@ func (i *modifyColumnIter) rewriteTable(ctx *sql.Context, rwt sql.RewritableTabl
570570
}
571571
}
572572

573-
newRow, err := projectRowWithTypes(ctx, newSch, projections, r)
573+
newRow, err := projectRowWithTypes(ctx, targetSchema, newSch, projections, r)
574574
if err != nil {
575575
_ = inserter.DiscardChanges(ctx, err)
576576
_ = inserter.Close(ctx)
@@ -905,21 +905,21 @@ func (i *loggingKeyValueIter) Close(ctx *sql.Context) error {
905905

906906
// projectRowWithTypes projects the row given with the projections given and additionally converts them to the
907907
// corresponding types found in the schema given, using the standard type conversion logic.
908-
func projectRowWithTypes(ctx *sql.Context, sch sql.Schema, projections []sql.Expression, r sql.Row) (sql.Row, error) {
908+
func projectRowWithTypes(ctx *sql.Context, oldSchema, newSchema sql.Schema, projections []sql.Expression, r sql.Row) (sql.Row, error) {
909909
newRow, err := ProjectRow(ctx, projections, r)
910910
if err != nil {
911911
return nil, err
912912
}
913913

914914
for i := range newRow {
915-
converted, inRange, err := sch[i].Type.Convert(ctx, newRow[i])
915+
converted, inRange, err := types.TypeAwareConversion(ctx, newRow[i], oldSchema[i].Type, newSchema[i].Type)
916916
if err != nil {
917917
if sql.ErrNotMatchingSRID.Is(err) {
918-
err = sql.ErrNotMatchingSRIDWithColName.New(sch[i].Name, err)
918+
err = sql.ErrNotMatchingSRIDWithColName.New(newSchema[i].Name, err)
919919
}
920920
return nil, err
921921
} else if !inRange {
922-
return nil, sql.ErrValueOutOfRange.New(newRow[i], sch[i].Type)
922+
return nil, sql.ErrValueOutOfRange.New(newRow[i], newSchema[i].Type)
923923
}
924924
newRow[i] = converted
925925
}

‎sql/types/conversion.go‎

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -738,19 +738,16 @@ func GeneralizeTypes(a, b sql.Type) sql.Type {
738738
// TypeAwareConversion converts a value to a specified type, with awareness of the value's original type. This is
739739
// necessary because some types, such as EnumType and SetType, are stored as ints and require information from the
740740
// original type to properly convert to strings.
741-
func TypeAwareConversion(ctx *sql.Context, val interface{}, originalType sql.Type, convertedType sql.Type) (interface{}, error) {
741+
func TypeAwareConversion(ctx *sql.Context, val interface{}, originalType sql.Type, convertedType sql.Type) (interface{}, sql.ConvertInRange, error) {
742742
if val == nil {
743-
return nil, nil
743+
return nil, sql.InRange, nil
744744
}
745-
var converted interface{}
746745
var err error
747-
if IsTextOnly(convertedType) {
748-
converted, _, err = ConvertToCollatedString(ctx, val, originalType)
749-
} else {
750-
converted, _, err = convertedType.Convert(ctx, val)
751-
}
752-
if err != nil {
753-
return nil, err
746+
if (IsEnum(originalType) || IsSet(originalType)) && IsText(convertedType) {
747+
val, _, err = ConvertToCollatedString(ctx, val, originalType)
748+
if err != nil {
749+
return nil, sql.OutOfRange, err
750+
}
754751
}
755-
return converted, nil
752+
return convertedType.Convert(ctx, val)
756753
}

0 commit comments

Comments
 (0)