Fix alter NULL enum value panic (#2750)

* Test for alter NULL enum value

* delete old bench

* error for enum truncation

* fix test

Author: max-hoffman

enginetest/queries/script_queries.go

@@ -120,6 +120,22 @@ var ScriptTests = []ScriptTest{
 			},
 		},
 	},
+	{
+		Name: "alter nil enum",
+		SetUpScript: []string{
+			"create table xy (x int primary key, y enum ('a', 'b'));",
+			"insert into xy values (0, NULL),(1, 'b')",
+		},
+		Assertions: []ScriptTestAssertion{
+			{
+				Query: "alter table xy modify y enum('a','b','c')",
+			},
+			{
+				Query:       "alter table xy modify y enum('a')",
+				ExpectedErr: sql.ErrEnumTypeTruncated,
+			},
+		},
+	},
 	{
 		Name: "issue 7958, update join uppercase table name validation",
 		SetUpScript: []string{
@@ -7494,8 +7510,8 @@ where
 				},
 			},
 			{
-				Query:          "alter table t modify column e enum('abc');",
-				ExpectedErrStr: "value 2 is not valid for this Enum",
+				Query:       "alter table t modify column e enum('abc');",
+				ExpectedErr: sql.ErrEnumTypeTruncated,
 			},
 		},
 	},

sql/analyzer/validate_create_table.go

@@ -470,6 +470,15 @@ func validateModifyColumn(ctx *sql.Context, initialSch sql.Schema, schema sql.Sc
 		return nil, err
 	}
 
+	if e1, ok := newCol.Type.(sql.EnumType); ok {
+		oldCol := initialSch[initialSch.IndexOfColName(oldColName)]
+		if e2, ok := oldCol.Type.(sql.EnumType); ok {
+			if len(e1.Values()) < len(e2.Values()) {
+				return nil, sql.ErrEnumTypeTruncated.New()
+			}
+		}
+	}
+
 	// TODO: When a column is being modified, we should ideally check that any existing table check constraints
 	//       are still valid (e.g. if the column type changed) and throw an error if they are invalidated.
 	//       That would be consistent with MySQL behavior.

sql/errors.go

@@ -924,6 +924,8 @@ var (
 	ErrInvalidTypeForLimit = errors.NewKind("invalid limit. expected %T, found %T")
 
 	ErrColumnSpecifiedTwice = errors.NewKind("column '%v' specified twice")
+
+	ErrEnumTypeTruncated = errors.NewKind("new enum type change truncates value")
 )
 
 // CastSQLError returns a *mysql.SQLError with the error code and in some cases, also a SQL state, populated for the

sql/rowexec/ddl_iters.go

@@ -565,7 +565,7 @@ func (i *modifyColumnIter) rewriteTable(ctx *sql.Context, rwt sql.RewritableTabl
 		}
 
 		// remap old enum values to new enum values
-		if isOldEnum && isNewEnum {
+		if isOldEnum && isNewEnum && newRow[newColIdx] != nil {
 			oldIdx := int(newRow[newColIdx].(uint16))
 			oldStr, _ := oldEnum.At(oldIdx)
 			newIdx := newEnum.IndexOf(oldStr)

sql/rowexec/merge_join.go

@@ -180,7 +180,7 @@ func (i *mergeJoinIter) Next(ctx *sql.Context) (sql.Row, error) {
 	// We use two variables to manage the lookahead state management.
 	// |matchedleft| is a forward-looking indicator of whether the current left
 	// row has satisfied a join condition. It is reset to false when we
-	// increment left. |matchincleft| is true when the most recent call to
+	// increment left. |matchingleft| is true when the most recent call to
 	// |incmatch| incremented the left row. The two vars combined let us
 	// lookahead during msSelect to 1) identify proper nullified row matches,
 	// and 2) maintain forward-looking state for the next |i.fullrow|.