diff --git a/ddl/db_integration_test.go b/ddl/db_integration_test.go index 8880fc9e4d707..9dbb6eec33e24 100644 --- a/ddl/db_integration_test.go +++ b/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) diff --git a/ddl/ddl_api.go b/ddl/ddl_api.go index e1597411c03b7..8f011b37635ac 100644 --- a/ddl/ddl_api.go +++ b/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) } } diff --git a/ddl/generated_column.go b/ddl/generated_column.go index c0550a292efc3..dab4e0b3f50a1 100644 --- a/ddl/generated_column.go +++ b/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{}) {