Skip to content

Commit

Permalink
ddl: disallow modifying the generated expression of stored or… (pingc…
Browse files Browse the repository at this point in the history 
…ap#11068)
  • Loading branch information
@tangenta @winkyao
tangenta authored and winkyao committed Jul 4, 2019
1 parent aafe172 commit 8dff08a
Show file tree
Hide file tree
Showing 4 changed files with 126 additions and 31 deletions.
80 changes: 76 additions & 4 deletions ddl/db_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2121,13 +2121,17 @@ func (s *testDBSuite3) TestGeneratedColumnDDL(c *C) {
}

// Check alter table modify/change generated column.
s.tk.MustExec(`alter table test_gv_ddl modify column c bigint as (b+200) stored`)
modStoredColErrMsg := "[ddl:3106]'modifying a stored column' is not supported for generated columns."
_, err := s.tk.Exec(`alter table test_gv_ddl modify column c bigint as (b+200) stored`)
c.Assert(err, NotNil)
c.Assert(err.Error(), Equals, modStoredColErrMsg)

result = s.tk.MustQuery(`DESC test_gv_ddl`)
result.Check(testkit.Rows(`a int(11) YES <nil> `, `b int(11) YES <nil> VIRTUAL GENERATED`, `c bigint(20) YES <nil> STORED GENERATED`))
result.Check(testkit.Rows(`a int(11) YES <nil> `, `b int(11) YES <nil> VIRTUAL GENERATED`, `c int(11) YES <nil> STORED GENERATED`))

s.tk.MustExec(`alter table test_gv_ddl change column b b bigint as (a+100) virtual`)
result = s.tk.MustQuery(`DESC test_gv_ddl`)
result.Check(testkit.Rows(`a int(11) YES <nil> `, `b bigint(20) YES <nil> VIRTUAL GENERATED`, `c bigint(20) YES <nil> STORED GENERATED`))
result.Check(testkit.Rows(`a int(11) YES <nil> `, `b bigint(20) YES <nil> VIRTUAL GENERATED`, `c int(11) YES <nil> STORED GENERATED`))

s.tk.MustExec(`alter table test_gv_ddl change column c cnew bigint`)
result = s.tk.MustQuery(`DESC test_gv_ddl`)
Expand Down Expand Up @@ -2720,7 +2724,75 @@ func (s *testDBSuite5) TestAddIndexForGeneratedColumn(c *C) {
s.tk.MustExec("admin check table gcai_table")
}

func (s *testDBSuite6) TestIssue9100(c *C) {
func (s *testDBSuite5) TestModifyGeneratedColumn(c *C) {
tk := testkit.NewTestKit(c, s.store)
tk.MustExec("create database if not exists test;")
tk.MustExec("use test")
modIdxColErrMsg := "[ddl:3106]'modifying an indexed column' is not supported for generated columns."
modStoredColErrMsg := "[ddl:3106]'modifying a stored column' is not supported for generated columns."

// Modify column with single-col-index.
tk.MustExec("drop table if exists t1;")
tk.MustExec("create table t1 (a int, b int as (a+1), index idx(b));")
tk.MustExec("insert into t1 set a=1;")
_, err := tk.Exec("alter table t1 modify column b int as (a+2);")
c.Assert(err, NotNil)
c.Assert(err.Error(), Equals, modIdxColErrMsg)
tk.MustExec("drop index idx on t1;")
tk.MustExec("alter table t1 modify b int as (a+2);")
tk.MustQuery("select * from t1").Check(testkit.Rows("1 3"))

// Modify column with multi-col-index.
tk.MustExec("drop table t1;")
tk.MustExec("create table t1 (a int, b int as (a+1), index idx(a, b));")
tk.MustExec("insert into t1 set a=1;")
_, err = tk.Exec("alter table t1 modify column b int as (a+2);")
c.Assert(err, NotNil)
c.Assert(err.Error(), Equals, modIdxColErrMsg)
tk.MustExec("drop index idx on t1;")
tk.MustExec("alter table t1 modify b int as (a+2);")
tk.MustQuery("select * from t1").Check(testkit.Rows("1 3"))

// Modify column with stored status to a different expression.
tk.MustExec("drop table t1;")
tk.MustExec("create table t1 (a int, b int as (a+1) stored);")
tk.MustExec("insert into t1 set a=1;")
_, err = tk.Exec("alter table t1 modify column b int as (a+2) stored;")
c.Assert(err, NotNil)
c.Assert(err.Error(), Equals, modStoredColErrMsg)

// Modify column with stored status to the same expression.
tk.MustExec("drop table t1;")
tk.MustExec("create table t1 (a int, b int as (a+1) stored);")
tk.MustExec("insert into t1 set a=1;")
tk.MustExec("alter table t1 modify column b bigint as (a+1) stored;")
tk.MustExec("alter table t1 modify column b bigint as (a + 1) stored;")
tk.MustQuery("select * from t1").Check(testkit.Rows("1 2"))

// Modify column with index to the same expression.
tk.MustExec("drop table t1;")
tk.MustExec("create table t1 (a int, b int as (a+1), index idx(b));")
tk.MustExec("insert into t1 set a=1;")
tk.MustExec("alter table t1 modify column b bigint as (a+1);")
tk.MustExec("alter table t1 modify column b bigint as (a + 1);")
tk.MustQuery("select * from t1").Check(testkit.Rows("1 2"))

// Modify column from non-generated to stored generated.
tk.MustExec("drop table t1;")
tk.MustExec("create table t1 (a int, b int);")
_, err = tk.Exec("alter table t1 modify column b bigint as (a+1) stored;")
c.Assert(err, NotNil)
c.Assert(err.Error(), Equals, modStoredColErrMsg)

// Modify column from stored generated to non-generated.
tk.MustExec("drop table t1;")
tk.MustExec("create table t1 (a int, b int as (a+1) stored);")
tk.MustExec("insert into t1 set a=1;")
tk.MustExec("alter table t1 modify column b int;")
tk.MustQuery("select * from t1").Check(testkit.Rows("1 2"))
}

func (s *testDBSuite4) TestIssue9100(c *C) {
tk := testkit.NewTestKit(c, s.store)
tk.MustExec("use test_db")
tk.MustExec("create table employ (a int, b int) partition by range (b) (partition p0 values less than (1));")
Expand Down
16 changes: 1 addition & 15 deletions ddl/ddl_api.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2551,7 +2551,7 @@ func (d *ddl) getModifiableColumnJob(ctx sessionctx.Context, ident ast.Ident, or
}

// As same with MySQL, we don't support modifying the stored status for generated columns.
if err = checkModifyGeneratedColumn(t.Cols(), col, newCol); err != nil {
if err = checkModifyGeneratedColumn(t, col, newCol, specNewColumn); err != nil {
return nil, errors.Trace(err)
}

Expand Down Expand Up @@ -2604,20 +2604,6 @@ func (d *ddl) ModifyColumn(ctx sessionctx.Context, ident ast.Ident, spec *ast.Al
return ErrWrongTableName.GenWithStackByArgs(specNewColumn.Name.Table.O)
}

// If the modified column is generated, check whether it refers to any auto-increment columns.
for _, option := range specNewColumn.Options {
if option.Tp == ast.ColumnOptionGenerated {
_, t, err := d.getSchemaAndTableByIdent(ctx, ident)
if err != nil {
return errors.Trace(err)
}
_, dependColNames := findDependedColumnNames(specNewColumn)
if err := checkAutoIncrementRef(specNewColumn.Name.Name.L, dependColNames, t.Meta()); err != nil {
return errors.Trace(err)
}
}
}

originalColName := specNewColumn.Name.Name
job, err := d.getModifiableColumnJob(ctx, ident, originalColName, spec)
if err != nil {
Expand Down
59 changes: 48 additions & 11 deletions ddl/generated_column.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ func verifyColumnGeneration(colName2Generation map[string]columnGenerationInDDL,
if attr, ok := colName2Generation[depCol]; ok {
if attr.generated && attribute.position <= attr.position {
// A generated column definition can refer to other
// generated columns occurring earilier in the table.
// generated columns occurring earlier in the table.
err := errGeneratedColumnNonPrior.GenWithStackByArgs()
return errors.Trace(err)
}
Expand Down Expand Up @@ -109,19 +109,18 @@ func (c *generatedColumnChecker) Leave(inNode ast.Node) (node ast.Node, ok bool)
// 1. the modification can't change stored status;
// 2. if the new is generated, check its refer rules.
// 3. check if the modified expr contains non-deterministic functions
func checkModifyGeneratedColumn(originCols []*table.Column, oldCol, newCol *table.Column) error {
// 4. check whether new column refers to any auto-increment columns.
// 5. check if the new column is indexed or stored
func checkModifyGeneratedColumn(tbl table.Table, oldCol, newCol *table.Column, newColDef *ast.ColumnDef) error {
// rule 1.
var stored = [2]bool{false, false}
var cols = [2]*table.Column{oldCol, newCol}
for i, col := range cols {
if !col.IsGenerated() || col.GeneratedStored {
stored[i] = true
}
}
if stored[0] != stored[1] {
oldColIsStored := !oldCol.IsGenerated() || oldCol.GeneratedStored
newColIsStored := !newCol.IsGenerated() || newCol.GeneratedStored
if oldColIsStored != newColIsStored {
return errUnsupportedOnGeneratedColumn.GenWithStackByArgs("Changing the STORED status")
}

// rule 2.
originCols := tbl.Cols()
var colName2Generation = make(map[string]columnGenerationInDDL, len(originCols))
for i, column := range originCols {
// We can compare the pointers simply.
Expand Down Expand Up @@ -158,11 +157,21 @@ func checkModifyGeneratedColumn(originCols []*table.Column, oldCol, newCol *tabl
}
}

// rule 3
if newCol.IsGenerated() {
// rule 3.
if err := checkIllegalFn4GeneratedColumn(newCol.Name.L, newCol.GeneratedExpr); err != nil {
return errors.Trace(err)
}

// rule 4.
if err := checkGeneratedWithAutoInc(tbl.Meta(), newColDef); err != nil {
return errors.Trace(err)
}

// rule 5.
if err := checkIndexOrStored(tbl, oldCol, newCol); err != nil {
return errors.Trace(err)
}
}
return nil
}
Expand Down Expand Up @@ -198,6 +207,34 @@ func checkIllegalFn4GeneratedColumn(colName string, expr ast.ExprNode) error {
return nil
}

// Check whether newColumnDef refers to any auto-increment columns.
func checkGeneratedWithAutoInc(tableInfo *model.TableInfo, newColumnDef *ast.ColumnDef) error {
_, dependColNames := findDependedColumnNames(newColumnDef)
if err := checkAutoIncrementRef(newColumnDef.Name.Name.L, dependColNames, tableInfo); err != nil {
return errors.Trace(err)
}
return nil
}

func checkIndexOrStored(tbl table.Table, oldCol, newCol *table.Column) error {
if oldCol.GeneratedExprString == newCol.GeneratedExprString {
return nil
}

if newCol.GeneratedStored {
return errUnsupportedOnGeneratedColumn.GenWithStackByArgs("modifying a stored column")
}

for _, idx := range tbl.Indices() {
for _, col := range idx.Meta().Columns {
if col.Name.L == newCol.Name.L {
return errUnsupportedOnGeneratedColumn.GenWithStackByArgs("modifying an indexed column")
}
}
}
return nil
}

// checkAutoIncrementRef checks if an generated column depends on an auto-increment column and raises an error if so.
// See https://dev.mysql.com/doc/refman/5.7/en/create-table-generated-columns.html for details.
func checkAutoIncrementRef(name string, dependencies map[string]struct{}, tbInfo *model.TableInfo) error {
Expand Down
2 changes: 1 addition & 1 deletion table/tables/tables.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1047,7 +1047,7 @@ var (
recordPrefixSep = []byte("_r")
)

// FindIndexByColName implements table.Table FindIndexByColName interface.
// FindIndexByColName returns a public table index containing only one column named `name`.
func FindIndexByColName(t table.Table, name string) table.Index {
for _, idx := range t.Indices() {
// only public index can be read.
Expand Down

0 comments on commit 8dff08a

Please sign in to comment.