Merge pull request #2958 from dolthub/nicktobey/enum

Correctly map enum values when altering an enum column.

Author: nicktobey

enginetest/queries/script_queries.go

@@ -181,7 +181,7 @@ CREATE TABLE teams (
 			},
 			{
 				Query:       "alter table xy modify y enum('a')",
-				ExpectedErr: types.ErrConvertingToEnum,
+				ExpectedErr: types.ErrDataTruncatedForColumn,
 			},
 		},
 	},
@@ -7706,6 +7706,7 @@ where
 		Name: "preserve enums through alter statements",
 		SetUpScript: []string{
 			"create table t (i int primary key, e enum('a', 'b', 'c'));",
+			"insert ignore into t values (0, 'error');",
 			"insert into t values (1, 'a');",
 			"insert into t values (2, 'b');",
 			"insert into t values (3, 'c');",
@@ -7714,6 +7715,7 @@ where
 			{
 				Query: "select i, e, e + 0 from t;",
 				Expected: []sql.Row{
+					{0, "", float64(0)},
 					{1, "a", float64(1)},
 					{2, "b", float64(2)},
 					{3, "c", float64(3)},
@@ -7728,6 +7730,7 @@ where
 			{
 				Query: "select i, e, e + 0 from t;",
 				Expected: []sql.Row{
+					{0, "", float64(0)},
 					{1, "a", float64(2)},
 					{2, "b", float64(3)},
 					{3, "c", float64(1)},
@@ -7742,6 +7745,7 @@ where
 			{
 				Query: "select i, e, e + 0 from t;",
 				Expected: []sql.Row{
+					{0, "", float64(0)},
 					{1, "a", float64(2)},
 					{2, "b", float64(3)},
 					{3, "c", float64(4)},
@@ -7756,14 +7760,30 @@ where
 			{
 				Query: "select i, e, e + 0 from t;",
 				Expected: []sql.Row{
+					{0, "", float64(0)},
 					{1, "a", float64(2)},
 					{2, "b", float64(3)},
 					{3, "c", float64(4)},
 				},
 			},
+			{
+				Query: "alter table t modify column e enum('a', 'b', 'c');",
+				Expected: []sql.Row{
+					{types.NewOkResult(0)},
+				},
+			},
+			{
+				Query: "select i, e, e + 0 from t;",
+				Expected: []sql.Row{
+					{0, "", float64(0)},
+					{1, "a", float64(1)},
+					{2, "b", float64(2)},
+					{3, "c", float64(3)},
+				},
+			},
 			{
 				Query:       "alter table t modify column e enum('abc');",
-				ExpectedErr: types.ErrConvertingToEnum,
+				ExpectedErr: types.ErrDataTruncatedForColumn,
 			},
 		},
 	},

sql/rowexec/ddl_iters.go

@@ -545,7 +545,6 @@ func (i *modifyColumnIter) rewriteTable(ctx *sql.Context, rwt sql.RewritableTabl
 		return false, err
 	}
 
-	newColIdx := newSch.IndexOf(newCol.Name, newCol.Source)
 	rowIter := sql.NewTableRowIter(ctx, rwt, partitions)
 	for {
 		r, err := rowIter.Next(ctx)
@@ -557,24 +556,27 @@ func (i *modifyColumnIter) rewriteTable(ctx *sql.Context, rwt sql.RewritableTabl
 			return false, err
 		}
 
+		// remap old enum values to new enum values
+		if isOldEnum && isNewEnum && r[oldColIdx] != nil {
+			oldIdx := int(r[oldColIdx].(uint16))
+			// 0 values in enums are error values. They are preserved during remapping.
+			if oldIdx != 0 {
+				oldStr, _ := oldEnum.At(oldIdx)
+				newIdx := newEnum.IndexOf(oldStr)
+				if newIdx == -1 {
+					return false, types.ErrDataTruncatedForColumn.New(newCol.Name)
+				}
+				r[oldColIdx] = uint16(newIdx)
+			}
+		}
+
 		newRow, err := projectRowWithTypes(ctx, newSch, projections, r)
 		if err != nil {
 			_ = inserter.DiscardChanges(ctx, err)
 			_ = inserter.Close(ctx)
 			return false, err
 		}
 
-		// remap old enum values to new enum values
-		if isOldEnum && isNewEnum && newRow[newColIdx] != nil {
-			oldIdx := int(newRow[newColIdx].(uint16))
-			oldStr, _ := oldEnum.At(oldIdx)
-			newIdx := newEnum.IndexOf(oldStr)
-			if newIdx == -1 {
-				return false, types.ErrDataTruncatedForColumn.New(newCol.Name)
-			}
-			newRow[newColIdx] = uint16(newIdx)
-		}
-
 		err = i.validateNullability(ctx, newSch, newRow)
 		if err != nil {
 			_ = inserter.DiscardChanges(ctx, err)