ddl: fix create and alter check constraints problems (#47633)

close #47567, close #47631, close #47632

pkg/ddl/constraint.go

@@ -29,19 +29,14 @@ import (
  "github.com/pingcap/tidb/pkg/parser/model"
  "github.com/pingcap/tidb/pkg/parser/mysql"
  "github.com/pingcap/tidb/pkg/sessionctx"
- "github.com/pingcap/tidb/pkg/table"
  "github.com/pingcap/tidb/pkg/util/dbterror"
  "github.com/pingcap/tidb/pkg/util/sqlexec"
 )
 
 func (w *worker) onAddCheckConstraint(d *ddlCtx, t *meta.Meta, job *model.Job) (ver int64, err error) {
  // Handle the rolling back job.
  if job.IsRollingback() {
- ver, err = onDropCheckConstraint(d, t, job)
- if err != nil {
- return ver, errors.Trace(err)
- }
- return ver, nil
+ return rollingBackAddConstraint(d, t, job)
  }
 
  failpoint.Inject("errorBeforeDecodeArgs", func(val failpoint.Value) {
@@ -84,23 +79,37 @@ func (w *worker) onAddCheckConstraint(d *ddlCtx, t *meta.Meta, job *model.Job) (
  constraintInfoInMeta = constraintInfoInJob
  }
 
- originalState := constraintInfoInMeta.State
+ // If not enforced, add it directly.
+ if !constraintInfoInMeta.Enforced {
+ constraintInfoInMeta.State = model.StatePublic
+ ver, err = updateVersionAndTableInfo(d, t, job, tblInfo, true)
+ if err != nil {
+ return ver, errors.Trace(err)
+ }
+ // Finish this job.
+ job.FinishTableJob(model.JobStateDone, model.StatePublic, ver, tblInfo)
+ return ver, nil
+ }
+
  switch constraintInfoInMeta.State {
  case model.StateNone:
  job.SchemaState = model.StateWriteOnly
  constraintInfoInMeta.State = model.StateWriteOnly
- ver, err = updateVersionAndTableInfoWithCheck(d, t, job, tblInfo, originalState != constraintInfoInMeta.State)
+ ver, err = updateVersionAndTableInfoWithCheck(d, t, job, tblInfo, true)
  case model.StateWriteOnly:
  job.SchemaState = model.StateWriteReorganization
  constraintInfoInMeta.State = model.StateWriteReorganization
- ver, err = updateVersionAndTableInfoWithCheck(d, t, job, tblInfo, originalState != constraintInfoInMeta.State)
+ ver, err = updateVersionAndTableInfoWithCheck(d, t, job, tblInfo, true)
  case model.StateWriteReorganization:
- err = w.verifyRemainRecordsForCheckConstraint(dbInfo, tblInfo, constraintInfoInMeta, job)
+ err = w.verifyRemainRecordsForCheckConstraint(dbInfo, tblInfo, constraintInfoInMeta)
  if err != nil {
+ if dbterror.ErrCheckConstraintIsViolated.Equal(err) {
+ job.State = model.JobStateRollingback
+ }
  return ver, errors.Trace(err)
  }
  constraintInfoInMeta.State = model.StatePublic
- ver, err = updateVersionAndTableInfo(d, t, job, tblInfo, originalState != constraintInfoInMeta.State)
+ ver, err = updateVersionAndTableInfo(d, t, job, tblInfo, true)
  if err != nil {
  return ver, errors.Trace(err)
  }
@@ -151,12 +160,11 @@ func onDropCheckConstraint(d *ddlCtx, t *meta.Meta, job *model.Job) (ver int64,
  return ver, errors.Trace(err)
  }
 
- originalState := constraintInfo.State
  switch constraintInfo.State {
  case model.StatePublic:
  job.SchemaState = model.StateWriteOnly
  constraintInfo.State = model.StateWriteOnly
- ver, err = updateVersionAndTableInfoWithCheck(d, t, job, tblInfo, originalState != constraintInfo.State)
+ ver, err = updateVersionAndTableInfoWithCheck(d, t, job, tblInfo, true)
  case model.StateWriteOnly:
  // write only state constraint will still take effect to check the newly inserted data.
  // So the dependent column shouldn't be dropped even in this intermediate state.
@@ -167,16 +175,11 @@ func onDropCheckConstraint(d *ddlCtx, t *meta.Meta, job *model.Job) (ver int64,
  tblInfo.Constraints = append(tblInfo.Constraints[0:i], tblInfo.Constraints[i+1:]...)
  }
  }
- ver, err = updateVersionAndTableInfo(d, t, job, tblInfo, originalState != constraintInfo.State)
+ ver, err = updateVersionAndTableInfo(d, t, job, tblInfo, true)
  if err != nil {
  return ver, errors.Trace(err)
  }
- // Finish this job.
- if job.IsRollingback() {
- job.FinishTableJob(model.JobStateRollbackDone, model.StateNone, ver, tblInfo)
- } else {
- job.FinishTableJob(model.JobStateDone, model.StateNone, ver, tblInfo)
- }
+ job.FinishTableJob(model.JobStateDone, model.StateNone, ver, tblInfo)
  default:
  err = dbterror.ErrInvalidDDLJob.GenWithStackByArgs("constraint", tblInfo.State)
  }
@@ -212,29 +215,38 @@ func (w *worker) onAlterCheckConstraint(d *ddlCtx, t *meta.Meta, job *model.Job)
  return ver, errors.Trace(err)
  }
 
+ if job.IsRollingback() {
+ return rollingBackAlterConstraint(d, t, job)
+ }
+
+ // Current State is desired.
+ if constraintInfo.State == model.StatePublic && constraintInfo.Enforced == enforced {
+ job.FinishTableJob(model.JobStateDone, model.StatePublic, ver, tblInfo)
+ return
+ }
+
  // enforced will fetch table data and check the constraint.
  if enforced {
- originalState := constraintInfo.State
  switch constraintInfo.State {
  case model.StatePublic:
  job.SchemaState = model.StateWriteReorganization
  constraintInfo.State = model.StateWriteReorganization
  constraintInfo.Enforced = enforced
- ver, err = updateVersionAndTableInfoWithCheck(d, t, job, tblInfo, originalState != constraintInfo.State)
+ ver, err = updateVersionAndTableInfoWithCheck(d, t, job, tblInfo, true)
  case model.StateWriteReorganization:
  job.SchemaState = model.StateWriteOnly
  constraintInfo.State = model.StateWriteOnly
- ver, err = updateVersionAndTableInfoWithCheck(d, t, job, tblInfo, originalState != constraintInfo.State)
+ ver, err = updateVersionAndTableInfoWithCheck(d, t, job, tblInfo, true)
  case model.StateWriteOnly:
- err = w.verifyRemainRecordsForCheckConstraint(dbInfo, tblInfo, constraintInfo, job)
+ err = w.verifyRemainRecordsForCheckConstraint(dbInfo, tblInfo, constraintInfo)
  if err != nil {
- if !table.ErrCheckConstraintViolated.Equal(err) {
- return ver, errors.Trace(err)
+ if dbterror.ErrCheckConstraintIsViolated.Equal(err) {
+ job.State = model.JobStateRollingback
  }
- constraintInfo.Enforced = !enforced
+ return ver, errors.Trace(err)
  }
  constraintInfo.State = model.StatePublic
- ver, err = updateVersionAndTableInfoWithCheck(d, t, job, tblInfo, originalState != constraintInfo.State)
+ ver, err = updateVersionAndTableInfoWithCheck(d, t, job, tblInfo, true)
  if err != nil {
  return ver, errors.Trace(err)
  }
@@ -336,7 +348,7 @@ func findDependentColsInExpr(expr ast.ExprNode) map[string]struct{} {
  return colsMap
 }
 
-func (w *worker) verifyRemainRecordsForCheckConstraint(dbInfo *model.DBInfo, tableInfo *model.TableInfo, constr *model.ConstraintInfo, job *model.Job) error {
+func (w *worker) verifyRemainRecordsForCheckConstraint(dbInfo *model.DBInfo, tableInfo *model.TableInfo, constr *model.ConstraintInfo) error {
  // Inject a fail-point to skip the remaining records check.
  failpoint.Inject("mockVerifyRemainDataSuccess", func(val failpoint.Value) {
  if val.(bool) {
@@ -363,8 +375,6 @@ func (w *worker) verifyRemainRecordsForCheckConstraint(dbInfo *model.DBInfo, tab
  }
  rowCount := len(rows)
  if rowCount != 0 {
- // If check constraint fail, the job state should be changed to canceled, otherwise it will tracked in.
- job.State = model.JobStateCancelled
  return dbterror.ErrCheckConstraintIsViolated.GenWithStackByArgs(constr.Name.L)
  }
  return nil

pkg/ddl/rollingback.go

@@ -498,15 +498,16 @@ func convertJob2RollbackJob(w *worker, d *ddlCtx, t *meta.Meta, job *model.Job)
  model.ActionModifyTableCharsetAndCollate,
  model.ActionModifySchemaCharsetAndCollate, model.ActionRepairTable,
  model.ActionModifyTableAutoIdCache, model.ActionAlterIndexVisibility,
- model.ActionModifySchemaDefaultPlacement,
- model.ActionRecoverSchema, model.ActionAlterCheckConstraint:
+ model.ActionModifySchemaDefaultPlacement, model.ActionRecoverSchema:
  ver, err = cancelOnlyNotHandledJob(job, model.StateNone)
  case model.ActionMultiSchemaChange:
  err = rollingBackMultiSchemaChange(job)
  case model.ActionAddCheckConstraint:
  ver, err = rollingBackAddConstraint(d, t, job)
  case model.ActionDropCheckConstraint:
  ver, err = rollingBackDropConstraint(t, job)
+ case model.ActionAlterCheckConstraint:
+ ver, err = rollingBackAlterConstraint(d, t, job)
  default:
  job.State = model.JobStateCancelled
  err = dbterror.ErrCancelledDDLJob
@@ -554,7 +555,6 @@ func convertJob2RollbackJob(w *worker, d *ddlCtx, t *meta.Meta, job *model.Job)
 }
 
 func rollingBackAddConstraint(d *ddlCtx, t *meta.Meta, job *model.Job) (ver int64, err error) {
- job.State = model.JobStateRollingback
  _, tblInfo, constrInfoInMeta, _, err := checkAddCheckConstraint(t, job)
  if err != nil {
  return ver, errors.Trace(err)
@@ -565,18 +565,17 @@ func rollingBackAddConstraint(d *ddlCtx, t *meta.Meta, job *model.Job) (ver int6
  job.State = model.JobStateCancelled
  return ver, dbterror.ErrCancelledDDLJob
  }
- // Add constraint has stored constraint info into meta, that means the job has at least
- // arrived write only state.
- originalState := constrInfoInMeta.State
- constrInfoInMeta.State = model.StateWriteOnly
- job.SchemaState = model.StateWriteOnly
-
- job.Args = []interface{}{constrInfoInMeta.Name}
- ver, err = updateVersionAndTableInfo(d, t, job, tblInfo, originalState != constrInfoInMeta.State)
- if err != nil {
- return ver, errors.Trace(err)
+ for i, constr := range tblInfo.Constraints {
+ if constr.Name.L == constrInfoInMeta.Name.L {
+ tblInfo.Constraints = append(tblInfo.Constraints[0:i], tblInfo.Constraints[i+1:]...)
+ break
+ }
  }
- return ver, dbterror.ErrCancelledDDLJob
+ if job.IsRollingback() {
+ job.State = model.JobStateRollbackDone
+ }
+ ver, err = updateVersionAndTableInfo(d, t, job, tblInfo, true)
+ return ver, errors.Trace(err)
 }
 
 func rollingBackDropConstraint(t *meta.Meta, job *model.Job) (ver int64, err error) {
@@ -594,3 +593,25 @@ func rollingBackDropConstraint(t *meta.Meta, job *model.Job) (ver int64, err err
  job.State = model.JobStateRunning
  return ver, nil
 }
+
+func rollingBackAlterConstraint(d *ddlCtx, t *meta.Meta, job *model.Job) (ver int64, err error) {
+ _, tblInfo, constraintInfo, enforced, err := checkAlterCheckConstraint(t, job)
+ if err != nil {
+ return ver, errors.Trace(err)
+ }
+
+ // StatePublic means when the job is not running yet.
+ if constraintInfo.State == model.StatePublic {
+ job.State = model.JobStateCancelled
+ return ver, dbterror.ErrCancelledDDLJob
+ }
+
+ // Only alter check constraints ENFORCED can get here.
+ constraintInfo.Enforced = !enforced
+ constraintInfo.State = model.StatePublic
+ if job.IsRollingback() {
+ job.State = model.JobStateRollbackDone
+ }
+ ver, err = updateVersionAndTableInfoWithCheck(d, t, job, tblInfo, true)
+ return ver, errors.Trace(err)
+}

tests/integrationtest/r/ddl/constraint.result

@@ -720,7 +720,6 @@ insert into t values(1), (2), (3);
 alter table t add constraint check(a < 2);
 Error 3819 (HY000): Check constraint 't_chk_1' is violated.
 alter table t add constraint check(a < 2) not enforced;
-Error 3819 (HY000): Check constraint 't_chk_1' is violated.
 drop table if exists t;
 set @@global.tidb_enable_check_constraint = 1;
 create table t(a int not null check(a>0), b int, constraint haha check(a < b), check(a<b+1));
@@ -789,4 +788,35 @@ t CREATE TABLE `t` (
 CONSTRAINT `haha` CHECK ((`a` < `b`)) /*!80016 NOT ENFORCED */,
 CONSTRAINT `t_chk_1` CHECK ((`a` > 0))
 ) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_bin
+drop table if exists t;
+set @@global.tidb_enable_check_constraint = 1;
+CREATE TABLE `t` (`a` int(11) DEFAULT NULL);
+show create table t;
+Table Create Table
+t CREATE TABLE `t` (
+ `a` int(11) DEFAULT NULL
+) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_bin
+insert t values(1);
+select * from t;
+a
+1
+alter table t ADD CONSTRAINT chk CHECK (a > 1) ENFORCED;
+Error 3819 (HY000): Check constraint 'chk' is violated.
+alter table t ADD CONSTRAINT chk CHECK (a > 1) ENFORCED;
+Error 3819 (HY000): Check constraint 'chk' is violated.
+alter table t ADD CONSTRAINT chk CHECK (a > 1) NOT ENFORCED;
+ALTER TABLE t ALTER CONSTRAINT chk ENFORCED;
+Error 3819 (HY000): Check constraint 'chk' is violated.
+show create table t;
+Table Create Table
+t CREATE TABLE `t` (
+ `a` int(11) DEFAULT NULL,
+CONSTRAINT `chk` CHECK ((`a` > 1)) /*!80016 NOT ENFORCED */
+) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_bin
+alter table t drop CONSTRAINT chk;
+show create table t;
+Table Create Table
+t CREATE TABLE `t` (
+ `a` int(11) DEFAULT NULL
+) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_bin
 set @@global.tidb_enable_check_constraint = 0;

tests/integrationtest/t/ddl/constraint.test

@@ -658,4 +658,23 @@ show create table t;
 # _, err = tk.Exec("alter table t alter constraint haha enforced")
 # require.Errorf(t, err, "[table:3819]Check constraint 'haha' is violated.")
 
-set @@global.tidb_enable_check_constraint = 0;
+# Related issue TiDB#47567, #47631 and #47632.
+# TestCheckConstraintIssue47567
+drop table if exists t;
+set @@global.tidb_enable_check_constraint = 1;
+CREATE TABLE `t` (`a` int(11) DEFAULT NULL);
+show create table t;
+insert t values(1);
+select * from t;
+-- error 3940
+alter table t ADD CONSTRAINT chk CHECK (a > 1) ENFORCED;
+-- error 3940
+alter table t ADD CONSTRAINT chk CHECK (a > 1) ENFORCED;
+alter table t ADD CONSTRAINT chk CHECK (a > 1) NOT ENFORCED;
+-- error 3940
+ALTER TABLE t ALTER CONSTRAINT chk ENFORCED;
+show create table t;
+alter table t drop CONSTRAINT chk;
+show create table t;
+
+set @@global.tidb_enable_check_constraint = 0;