diff --git a/ddl/column.go b/ddl/column.go index 6c22425a6784e..3a0df926f62f1 100644 --- a/ddl/column.go +++ b/ddl/column.go @@ -197,7 +197,7 @@ func onAddColumn(d *ddlCtx, t *meta.Meta, job *model.Job) (ver int64, err error) // none -> delete only job.SchemaState = model.StateDeleteOnly columnInfo.State = model.StateDeleteOnly - ver, err = updateVersionAndTableInfo(t, job, tblInfo, originalState != columnInfo.State) + ver, err = updateVersionAndTableInfoWithCheck(t, job, tblInfo, originalState != columnInfo.State) case model.StateDeleteOnly: // delete only -> write only job.SchemaState = model.StateWriteOnly @@ -257,7 +257,7 @@ func onDropColumn(t *meta.Meta, job *model.Job) (ver int64, _ error) { return ver, errors.Trace(err) } } - ver, err = updateVersionAndTableInfo(t, job, tblInfo, originalState != colInfo.State) + ver, err = updateVersionAndTableInfoWithCheck(t, job, tblInfo, originalState != colInfo.State) case model.StateWriteOnly: // write only -> delete only job.SchemaState = model.StateDeleteOnly @@ -452,7 +452,7 @@ func (w *worker) doModifyColumn(t *meta.Meta, job *model.Job, newCol *model.Colu } } - ver, err = updateVersionAndTableInfo(t, job, tblInfo, true) + ver, err = updateVersionAndTableInfoWithCheck(t, job, tblInfo, true) if err != nil { // Modified the type definition of 'null' to 'not null' before this, so rollBack the job when an error occurs. job.State = model.JobStateRollingback @@ -561,7 +561,7 @@ func modifyColumnFromNull2NotNull(w *worker, t *meta.Meta, dbInfo *model.DBInfo, // Prevent this field from inserting null values. tblInfo.Columns[oldCol.Offset].Flag |= mysql.PreventNullInsertFlag - ver, err = updateVersionAndTableInfo(t, job, tblInfo, true) + ver, err = updateVersionAndTableInfoWithCheck(t, job, tblInfo, true) return ver, errors.Trace(err) } diff --git a/ddl/db_test.go b/ddl/db_test.go index 998c960041404..1bcdcca44fff5 100644 --- a/ddl/db_test.go +++ b/ddl/db_test.go @@ -1526,6 +1526,13 @@ func (s *testDBSuite2) TestDropColumn(c *C) { } } + // Test for drop partition table column. + s.tk.MustExec("drop table if exists t1") + s.tk.MustExec("create table t1 (a int,b int) partition by hash(a) partitions 4;") + _, err := s.tk.Exec("alter table t1 drop column a") + c.Assert(err, NotNil) + c.Assert(err.Error(), Equals, "[expression:1054]Unknown column 'a' in 'expression'") + s.tk.MustExec("drop database drop_col_db") } @@ -2750,3 +2757,34 @@ func (s *testDBSuite2) TestAlterShardRowIDBits(c *C) { c.Assert(err, NotNil) c.Assert(err.Error(), Equals, "[autoid:1467]Failed to read auto-increment value from storage engine") } + +func (s *testDBSuite2) TestDDLWithInvalidTableInfo(c *C) { + s.tk = testkit.NewTestKit(c, s.store) + tk := s.tk + + tk.MustExec("use test") + tk.MustExec("drop table if exists t") + defer tk.MustExec("drop table if exists t") + // Test create with invalid expression. + _, err := s.tk.Exec(`CREATE TABLE t ( + c0 int(11) , + c1 int(11), + c2 decimal(16,4) GENERATED ALWAYS AS ((case when (c0 = 0) then 0 when (c0 > 0) then (c1 / c0) end)) + );`) + c.Assert(err, NotNil) + c.Assert(err.Error(), Equals, "[parser:1064]You have an error in your SQL syntax; check the manual that corresponds to your TiDB version for the right syntax to use line 1 column 55 near \"THEN (`c1` / `c0`) END)\" ") + + tk.MustExec("create table t (a bigint, b int, c int generated always as (b+1)) partition by hash(a) partitions 4;") + // Test drop partition column. + _, err = tk.Exec("alter table t drop column a;") + c.Assert(err, NotNil) + c.Assert(err.Error(), Equals, "[expression:1054]Unknown column 'a' in 'expression'") + // Test modify column with invalid expression. + _, err = tk.Exec("alter table t modify column c int GENERATED ALWAYS AS ((case when (a = 0) then 0 when (a > 0) then (b / a) end));") + c.Assert(err, NotNil) + c.Assert(err.Error(), Equals, "[parser:1064]You have an error in your SQL syntax; check the manual that corresponds to your TiDB version for the right syntax to use line 1 column 53 near \"THEN (`b` / `a`) END)\" ") + // Test add column with invalid expression. + _, err = tk.Exec("alter table t add column d int GENERATED ALWAYS AS ((case when (a = 0) then 0 when (a > 0) then (b / a) end));") + c.Assert(err, NotNil) + c.Assert(err.Error(), Equals, "[parser:1064]You have an error in your SQL syntax; check the manual that corresponds to your TiDB version for the right syntax to use line 1 column 53 near \"THEN (`b` / `a`) END)\" ") +} diff --git a/ddl/ddl_api.go b/ddl/ddl_api.go index 1d7d5bb3c1dc8..f94454c93d24b 100644 --- a/ddl/ddl_api.go +++ b/ddl/ddl_api.go @@ -1114,6 +1114,12 @@ func (d *ddl) CreateTableWithLike(ctx sessionctx.Context, ident, referIdent ast. return errors.Trace(err) } +// checkTableInfoValid uses to check table info valid. This is used to validate table info. +func checkTableInfoValid(tblInfo *model.TableInfo) error { + _, err := tables.TableFromMeta(nil, tblInfo) + return err +} + func buildTableInfoWithLike(ident ast.Ident, referTblInfo *model.TableInfo) model.TableInfo { tblInfo := *referTblInfo // Check non-public column and adjust column offset. @@ -1248,6 +1254,12 @@ func (d *ddl) CreateTable(ctx sessionctx.Context, s *ast.CreateTableStmt) (err e if err != nil { return errors.Trace(err) } + tbInfo.State = model.StatePublic + err = checkTableInfoValid(tbInfo) + if err != nil { + return err + } + tbInfo.State = model.StateNone job := &model.Job{ SchemaID: schema.ID, diff --git a/ddl/index.go b/ddl/index.go index a613d06026138..1aaab3e0ce5af 100644 --- a/ddl/index.go +++ b/ddl/index.go @@ -283,7 +283,7 @@ func (w *worker) onCreateIndex(d *ddlCtx, t *meta.Meta, job *model.Job) (ver int // none -> delete only job.SchemaState = model.StateDeleteOnly indexInfo.State = model.StateDeleteOnly - ver, err = updateVersionAndTableInfo(t, job, tblInfo, originalState != indexInfo.State) + ver, err = updateVersionAndTableInfoWithCheck(t, job, tblInfo, originalState != indexInfo.State) case model.StateDeleteOnly: // delete only -> write only job.SchemaState = model.StateWriteOnly diff --git a/ddl/table.go b/ddl/table.go index 07444aac471e9..47fe0d5609e4f 100644 --- a/ddl/table.go +++ b/ddl/table.go @@ -69,7 +69,7 @@ func onCreateTable(d *ddlCtx, t *meta.Meta, job *model.Job) (ver int64, _ error) // none -> public tbInfo.State = model.StatePublic tbInfo.UpdateTS = t.StartTS - err = t.CreateTableOrView(schemaID, tbInfo) + err = createTableOrViewWithCheck(t, job, schemaID, tbInfo) if err != nil { return ver, errors.Trace(err) } @@ -82,6 +82,15 @@ func onCreateTable(d *ddlCtx, t *meta.Meta, job *model.Job) (ver int64, _ error) } } +func createTableOrViewWithCheck(t *meta.Meta, job *model.Job, schemaID int64, tbInfo *model.TableInfo) error { + err := checkTableInfoValid(tbInfo) + if err != nil { + job.State = model.JobStateCancelled + return errors.Trace(err) + } + return t.CreateTableOrView(schemaID, tbInfo) +} + func onCreateView(d *ddlCtx, t *meta.Meta, job *model.Job) (ver int64, _ error) { schemaID := job.SchemaID tbInfo := &model.TableInfo{} @@ -122,7 +131,7 @@ func onCreateView(d *ddlCtx, t *meta.Meta, job *model.Job) (ver int64, _ error) return ver, errors.Trace(err) } } - err = t.CreateTableOrView(schemaID, tbInfo) + err = createTableOrViewWithCheck(t, job, schemaID, tbInfo) if err != nil { return ver, errors.Trace(err) } @@ -802,6 +811,18 @@ func checkTableNotExistsFromStore(t *meta.Meta, schemaID int64, tableName string return nil } +// updateVersionAndTableInfoWithCheck checks table info validate and updates the schema version and the table information +func updateVersionAndTableInfoWithCheck(t *meta.Meta, job *model.Job, tblInfo *model.TableInfo, shouldUpdateVer bool) ( + ver int64, err error) { + err = checkTableInfoValid(tblInfo) + if err != nil { + job.State = model.JobStateCancelled + return ver, errors.Trace(err) + } + return updateVersionAndTableInfo(t, job, tblInfo, shouldUpdateVer) + +} + // updateVersionAndTableInfo updates the schema version and the table information. func updateVersionAndTableInfo(t *meta.Meta, job *model.Job, tblInfo *model.TableInfo, shouldUpdateVer bool) ( ver int64, err error) {