ddl : Fix generated column can refer only to generated columns defined prior to it. (#11549)

ddl/db_integration_test.go

@@ -598,6 +598,31 @@ func (s *testIntegrationSuite7) TestNullGeneratedColumn(c *C) {
 	tk.MustExec("drop table t")
 }
 
+func (s *testIntegrationSuite7) TestDependedGeneratedColumnPrior2GeneratedColumn(c *C) {
+	tk := testkit.NewTestKit(c, s.store)
+	tk.MustExec("use test")
+	tk.MustExec("drop table if exists t")
+	tk.MustExec("CREATE TABLE `t` (" +
+		"`a` int(11) DEFAULT NULL," +
+		"`b` int(11) GENERATED ALWAYS AS (`a` + 1) VIRTUAL," +
+		"`c` int(11) GENERATED ALWAYS AS (`b` + 1) VIRTUAL" +
+		") ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_bin")
+	// should check unknown column first, then the prior ones.
+	sql := "alter table t add column d int as (c + f + 1) first"
+	assertErrorCode(c, tk, sql, mysql.ErrBadField)
+
+	// depended generated column should be prior to generated column self
+	sql = "alter table t add column d int as (c+1) first"
+	assertErrorCode(c, tk, sql, mysql.ErrGeneratedColumnNonPrior)
+
+	// correct case
+	tk.MustExec("alter table t add column d int as (c+1) after c")
+
+	// check position nil case
+	tk.MustExec("alter table t add column(e int as (c+1))")
+	tk.MustExec("drop table if exists t")
+}
+
 func (s *testIntegrationSuite9) TestChangingCharsetToUtf8(c *C) {
 	tk := testkit.NewTestKit(c, s.store)
 

ddl/ddl_api.go

@@ -2049,9 +2049,8 @@ func (d *ddl) AddColumn(ctx sessionctx.Context, ti ast.Ident, spec *ast.AlterTab
 	}
 
 	// If new column is a generated column, do validation.
-	// NOTE: Because now we can only append columns to table,
-	// we don't need check whether the column refers other
-	// generated columns occurring later in table.
+	// NOTE: we do check whether the column refers other generated
+	// columns occurring later in a table, but we don't handle the col offset.
 	for _, option := range specNewColumn.Options {
 		if option.Tp == ast.ColumnOptionGenerated {
 			if err := checkIllegalFn4GeneratedColumn(specNewColumn.Name.Name.L, option.Expr); err != nil {
@@ -2066,8 +2065,17 @@ func (d *ddl) AddColumn(ctx sessionctx.Context, ti ast.Ident, spec *ast.AlterTab
 			if err = checkAutoIncrementRef(specNewColumn.Name.Name.L, dependColNames, t.Meta()); err != nil {
 				return errors.Trace(err)
 			}
+			duplicateColNames := make(map[string]struct{}, len(dependColNames))
+			for k := range dependColNames {
+				duplicateColNames[k] = struct{}{}
+			}
+			cols := t.Cols()
+
+			if err = checkDependedColExist(dependColNames, cols); err != nil {
+				return errors.Trace(err)
+			}
 
-			if err = checkDependedColExist(dependColNames, t.Cols()); err != nil {
+			if err = verifyColumnGenerationSingle(duplicateColNames, cols, spec.Position); err != nil {
 				return errors.Trace(err)
 			}
 		}

ddl/generated_column.go

@@ -50,8 +50,26 @@ func verifyColumnGeneration(colName2Generation map[string]columnGenerationInDDL,
 	return nil
 }
 
+// verifyColumnGenerationSingle is for ADD GENERATED COLUMN, we just need verify one column itself.
+func verifyColumnGenerationSingle(dependColNames map[string]struct{}, cols []*table.Column, position *ast.ColumnPosition) error {
+	// Since the added column does not exist yet, we should derive it's offset from ColumnPosition.
+	pos, err := findPositionRelativeColumn(cols, position)
+	if err != nil {
+		return errors.Trace(err)
+	}
+	// should check unknown column first, then the prior ones.
+	for _, col := range cols {
+		if _, ok := dependColNames[col.Name.L]; ok {
+			if col.IsGenerated() && col.Offset >= pos {
+				// Generated column can refer only to generated columns defined prior to it.
+				return errGeneratedColumnNonPrior.GenWithStackByArgs()
+			}
+		}
+	}
+	return nil
+}
+
 // checkDependedColExist ensure all depended columns exist.
-//
 // NOTE: this will MODIFY parameter `dependCols`.
 func checkDependedColExist(dependCols map[string]struct{}, cols []*table.Column) error {
 	for _, col := range cols {
@@ -65,6 +83,34 @@ func checkDependedColExist(dependCols map[string]struct{}, cols []*table.Column)
 	return nil
 }
 
+// findPositionRelativeColumn returns a pos relative to added generated column position.
+func findPositionRelativeColumn(cols []*table.Column, pos *ast.ColumnPosition) (int, error) {
+	position := len(cols)
+	// Get the column position, default is cols's length means appending.
+	// For "alter table ... add column(...)", the position will be nil.
+	// For "alter table ... add column ... ", the position will be default one.
+	if pos == nil {
+		return position, nil
+	}
+	if pos.Tp == ast.ColumnPositionFirst {
+		position = 0
+	} else if pos.Tp == ast.ColumnPositionAfter {
+		var col *table.Column
+		for _, c := range cols {
+			if c.Name.L == pos.RelativeColumn.Name.L {
+				col = c
+				break
+			}
+		}
+		if col == nil {
+			return -1, ErrBadField.GenWithStackByArgs(pos.RelativeColumn, "generated column function")
+		}
+		// Inserted position is after the mentioned column.
+		position = col.Offset + 1
+	}
+	return position, nil
+}
+
 // findDependedColumnNames returns a set of string, which indicates
 // the names of the columns that are depended by colDef.
 func findDependedColumnNames(colDef *ast.ColumnDef) (generated bool, colsMap map[string]struct{}) {