From df27883b591fbf6e3d4bcae23093daf3133470b4 Mon Sep 17 00:00:00 2001 From: crazycs520 Date: Mon, 26 Sep 2022 13:05:50 +0800 Subject: [PATCH 1/6] *: add foreign key constraint check when execute update statement Signed-off-by: crazycs520 --- executor/builder.go | 4 + executor/foreign_key.go | 12 ++ executor/foreign_key_test.go | 234 +++++++++++++++++++++++++++ executor/update.go | 14 +- planner/core/common_plans.go | 2 + planner/core/foreign_key.go | 83 ++++++++++ planner/core/logical_plan_builder.go | 4 + planner/core/point_get_plan.go | 5 + 8 files changed, 357 insertions(+), 1 deletion(-) diff --git a/executor/builder.go b/executor/builder.go index efbaf0fab95a0..3972869802e66 100644 --- a/executor/builder.go +++ b/executor/builder.go @@ -2248,6 +2248,10 @@ func (b *executorBuilder) buildUpdate(v *plannercore.Update) Executor { tblColPosInfos: v.TblColPosInfos, assignFlag: assignFlag, } + updateExec.fkChecks, b.err = buildTblID2FKCheckExecs(b.ctx, tblID2table, v.FKChecks) + if b.err != nil { + return nil + } return updateExec } diff --git a/executor/foreign_key.go b/executor/foreign_key.go index ee49dd06f77b3..cc265f757e7ef 100644 --- a/executor/foreign_key.go +++ b/executor/foreign_key.go @@ -51,6 +51,18 @@ type FKCheckExec struct { checkRowsCache map[string]bool } +func buildTblID2FKCheckExecs(sctx sessionctx.Context, tblID2Table map[int64]table.Table, tblID2FKChecks map[int64][]*plannercore.FKCheck) (map[int64][]*FKCheckExec, error) { + var err error + fkChecks := make(map[int64][]*FKCheckExec) + for tid, tbl := range tblID2Table { + fkChecks[tid], err = buildFKCheckExecs(sctx, tbl, tblID2FKChecks[tid]) + if err != nil { + return nil, err + } + } + return fkChecks, nil +} + func buildFKCheckExecs(sctx sessionctx.Context, tbl table.Table, fkChecks []*plannercore.FKCheck) ([]*FKCheckExec, error) { fkCheckExecs := make([]*FKCheckExec, 0, len(fkChecks)) for _, fkCheck := range fkChecks { diff --git a/executor/foreign_key_test.go b/executor/foreign_key_test.go index 2e68042a5aa49..83838c99a21d2 100644 --- a/executor/foreign_key_test.go +++ b/executor/foreign_key_test.go @@ -349,6 +349,52 @@ func TestForeignKeyCheckAndLock(t *testing.T) { err := tk.ExecToErr("commit") require.Error(t, err) require.Contains(t, err.Error(), "Write conflict") + + // Test update child table + tk.MustExec("insert into t1 (id, name) values (1, 'a'), (2, 'b');") + tk.MustExec("insert into t2 (a, name) values (1, 'a');") + tk.MustExec("begin optimistic") + tk.MustExec("update t2 set a=2 where a = 1") + tk2.MustExec("delete from t1 where id = 2") + err = tk.ExecToErr("commit") + require.Error(t, err) + require.Contains(t, err.Error(), "Write conflict") + tk.MustQuery("select id, name from t1 order by name").Check(testkit.Rows("1 a")) + tk.MustQuery("select a, name from t2 order by name").Check(testkit.Rows("1 a")) + + // Test in pessimistic txn + tk.MustExec("delete from t2") + // Test insert child table + tk.MustExec("begin pessimistic") + tk.MustExec("insert into t2 (a, name) values (1, 'a');") + var wg sync.WaitGroup + wg.Add(1) + go func() { + defer wg.Done() + err := tk2.ExecToErr("update t1 set id = 2 where id = 1") + require.Error(t, err) + require.Equal(t, "[planner:1451]Cannot delete or update a parent row: a foreign key constraint fails (`test`.`t2`, CONSTRAINT `fk` FOREIGN KEY (`a`) REFERENCES `t1` (`id`))", err.Error()) + }() + tk.MustExec("commit") + wg.Wait() + tk.MustQuery("select id, name from t1 order by name").Check(testkit.Rows("1 a")) + tk.MustQuery("select a, name from t2 order by name").Check(testkit.Rows("1 a")) + + // Test update child table + tk.MustExec("insert into t1 (id, name) values (2, 'b');") + tk.MustExec("begin pessimistic") + tk.MustExec("update t2 set a=2 where a = 1") + wg.Add(1) + go func() { + defer wg.Done() + err := tk2.ExecToErr("update t1 set id = 3 where id = 2") + require.Error(t, err) + require.Equal(t, "[planner:1451]Cannot delete or update a parent row: a foreign key constraint fails (`test`.`t2`, CONSTRAINT `fk` FOREIGN KEY (`a`) REFERENCES `t1` (`id`))", err.Error()) + }() + tk.MustExec("commit") + wg.Wait() + tk.MustQuery("select id, name from t1 order by name").Check(testkit.Rows("1 a", "2 b")) + tk.MustQuery("select a, name from t2 order by name").Check(testkit.Rows("2 a")) } } @@ -443,6 +489,22 @@ func TestForeignKey(t *testing.T) { tk.MustExec("insert into t2 (id, a, b) values (2, 22, 222);") tk.MustGetDBError("insert into t3 (id, a, b) values (1, 1, 1)", plannercore.ErrNoReferencedRow2) tk.MustGetDBError("insert into t3 (id, a, b) values (2, 3, 2)", plannercore.ErrNoReferencedRow2) + tk.MustExec("insert into t3 (id, a, b) values (0, 1, 2);") + tk.MustExec("insert into t3 (id, a, b) values (1, 2, 2);") + tk.MustGetDBError("update t3 set a=3 where a=1", plannercore.ErrNoReferencedRow2) + tk.MustGetDBError("update t3 set b=4 where id=1", plannercore.ErrNoReferencedRow2) + + // Test table has been referenced by more than tables. + tk.MustExec("drop table if exists t3,t2,t1;") + tk.MustExec("create table t1 (id int, a int, b int, primary key (id));") + tk.MustExec("create table t2 (b int, a int, id int, primary key (a), foreign key (a) references t1(id));") + tk.MustExec("create table t3 (b int, a int, id int, primary key (a), foreign key (a) references t1(id));") + tk.MustExec("insert into t1 (id, a, b) values (1, 1, 1);") + tk.MustExec("insert into t2 (id, a, b) values (1, 1, 1);") + tk.MustExec("insert into t3 (id, a, b) values (1, 1, 1);") + tk.MustGetDBError(" update t1 set id=2 where id = 1", plannercore.ErrRowIsReferenced2) + tk.MustExec(" update t1 set a=2 where id = 1") + tk.MustExec(" update t1 set b=2 where id = 1") } func TestForeignKeyConcurrentInsertChildTable(t *testing.T) { @@ -472,3 +534,175 @@ func TestForeignKeyConcurrentInsertChildTable(t *testing.T) { } wg.Wait() } + +func TestForeignKeyOnUpdateChildTable(t *testing.T) { + store := testkit.CreateMockStore(t) + tk := testkit.NewTestKit(t, store) + tk.MustExec("set @@global.tidb_enable_foreign_key=1") + tk.MustExec("set @@foreign_key_checks=1") + tk.MustExec("use test") + + for _, ca := range foreignKeyTestCase1 { + tk.MustExec("drop table if exists t2;") + tk.MustExec("drop table if exists t1;") + for _, sql := range ca.prepareSQLs { + tk.MustExec(sql) + } + tk.MustExec("insert into t1 (id, a, b) values (1, 11, 21),(2, 12, 22), (3, 13, 23), (4, 14, 24)") + tk.MustExec("insert into t2 (id, a, b, name) values (1, 11, 21, 'a')") + + sqls := []string{ + "update t2 set a=100, b = 200 where id = 1", + "update t2 set a=a+10, b = b+20 where a = 11", + "update t2 set a=a+100, b = b+200", + "update t2 set a=12, b = 23 where id = 1", + } + for _, sqlStr := range sqls { + err := tk.ExecToErr(sqlStr) + require.NotNil(t, err) + require.True(t, plannercore.ErrNoReferencedRow2.Equal(err)) + } + tk.MustExec("update t2 set a=12, b = 22 where id = 1") + tk.MustQuery("select id, a, b, name from t2 order by id").Check(testkit.Rows("1 12 22 a")) + if !ca.notNull { + tk.MustExec("update t2 set a=null, b = 22 where a = 12 ") + tk.MustExec("update t2 set b = null where b = 22 ") + tk.MustQuery("select id, a, b, name from t2 order by id").Check(testkit.Rows("1 a")) + } + tk.MustExec("update t2 set a=13, b=23 where id = 1") + tk.MustQuery("select id, a, b, name from t2").Check(testkit.Rows("1 13 23 a")) + + // Test In txn. + tk.MustExec("delete from t2") + tk.MustExec("delete from t1") + tk.MustExec("insert into t1 (id, a, b) values (1, 11, 21),(2, 12, 22), (3, 13, 23), (4, 14, 24)") + tk.MustExec("insert into t2 (id, a, b, name) values (1, 11, 21, 'a')") + tk.MustExec("begin") + tk.MustExec("update t2 set a=12, b=22 where id=1") + tk.MustExec("rollback") + + tk.MustExec("begin") + tk.MustExec("delete from t1 where id=2") + err := tk.ExecToErr("update t2 set a=12, b=22 where id=1") + require.NotNil(t, err) + require.True(t, plannercore.ErrNoReferencedRow2.Equal(err)) + tk.MustExec("update t2 set a=13, b=23 where id=1") + tk.MustExec("insert into t1 (id, a, b) values (5, 15, 25)") + tk.MustExec("update t2 set a=15, b=25 where id=1") + tk.MustExec("delete from t1 where id=1") + err = tk.ExecToErr("update t2 set a=11, b=21 where id=1") + require.NotNil(t, err) + require.True(t, plannercore.ErrNoReferencedRow2.Equal(err)) + tk.MustExec("commit") + tk.MustQuery("select id, a, b, name from t2").Check(testkit.Rows("1 15 25 a")) + } + + // Case-9: test primary key is handle and contain foreign key column. + tk.MustExec("drop table if exists t2;") + tk.MustExec("drop table if exists t1;") + tk.MustExec("set @@tidb_enable_clustered_index=0;") + tk.MustExec("create table t1 (id int, a int, b int, primary key (id));") + tk.MustExec("create table t2 (b int, a int, id int, name varchar(10), primary key (a), foreign key fk(a) references t1(id));") + tk.MustExec("insert into t1 (id, a, b) values (1, 11, 21),(2, 12, 22), (3, 13, 23), (4, 14, 24)") + tk.MustExec("insert into t2 (id, a, b, name) values (11, 1, 21, 'a')") + tk.MustExec("update t2 set a = 2 where id = 11") + tk.MustQuery("select id, a, b, name from t2 order by id").Check(testkit.Rows("11 2 21 a")) + tk.MustExec("update t2 set a = 3 where id = 11") + tk.MustQuery("select id, a, b, name from t2 order by id").Check(testkit.Rows("11 3 21 a")) + tk.MustExec("update t2 set b=b+1 where id = 11") + tk.MustQuery("select id, a, b , name from t2 order by id").Check(testkit.Rows("11 3 22 a")) + tk.MustExec("update t2 set id = 1 where id = 11") + tk.MustQuery("select id, a, b, name from t2 order by id").Check(testkit.Rows("1 3 22 a")) + err := tk.ExecToErr("update t2 set a = 10 where id = 1") + require.NotNil(t, err) + require.True(t, plannercore.ErrNoReferencedRow2.Equal(err)) + tk.MustQuery("select id, a, b, name from t2 order by id").Check(testkit.Rows("1 3 22 a")) + + // Test In txn. + tk.MustExec("delete from t2") + tk.MustExec("delete from t1") + tk.MustExec("insert into t1 (id, a, b) values (1, 11, 21),(2, 12, 22), (3, 13, 23), (4, 14, 24)") + tk.MustExec("insert into t2 (id, a, b, name) values (1, 1, 21, 'a')") + tk.MustExec("begin") + tk.MustExec("update t2 set a=2, b=22 where id=1") + tk.MustExec("rollback") + + tk.MustExec("begin") + tk.MustExec("delete from t1 where id=2") + err = tk.ExecToErr("update t2 set a=2, b=22 where id=1") + require.NotNil(t, err) + require.True(t, plannercore.ErrNoReferencedRow2.Equal(err)) + tk.MustExec("update t2 set a=3, b=23 where id=1") + tk.MustExec("insert into t1 (id, a, b) values (5, 15, 25)") + tk.MustExec("update t2 set a=5, b=25 where id=1") + tk.MustExec("delete from t1 where id=1") + err = tk.ExecToErr("update t2 set a=1, b=21 where id=1") + require.NotNil(t, err) + require.True(t, plannercore.ErrNoReferencedRow2.Equal(err)) + tk.MustExec("commit") + tk.MustQuery("select id, a, b, name from t2").Check(testkit.Rows("1 5 25 a")) +} + +func TestForeignKeyOnUpdateParentTableCheck(t *testing.T) { + store := testkit.CreateMockStore(t) + tk := testkit.NewTestKit(t, store) + tk.MustExec("set @@global.tidb_enable_foreign_key=1") + tk.MustExec("set @@foreign_key_checks=1") + tk.MustExec("use test") + + for _, ca := range foreignKeyTestCase1 { + tk.MustExec("drop table if exists t2;") + tk.MustExec("drop table if exists t1;") + for _, sql := range ca.prepareSQLs { + tk.MustExec(sql) + } + if !ca.notNull { + tk.MustExec("insert into t1 (id, a, b) values (1, 11, 21),(2, 12, 22), (3, 13, 23), (4, 14, 24), (5, 15, null), (6, null, 26), (7, null, null);") + tk.MustExec("insert into t2 (id, a, b, name) values (1, 11, 21, 'a'), (5, 15, null, 'e'), (6, null, 26, 'f'), (7, null, null, 'g');") + + tk.MustExec("update t1 set a=a+100, b = b+200 where id = 2") + tk.MustExec("update t1 set a=a+1000, b = b+2000 where a = 13 or b=222") + tk.MustExec("update t1 set a=a+10000, b = b+20000 where id = 5 or a is null or b is null") + tk.MustQuery("select id, a, b from t1 order by id").Check(testkit.Rows("1 11 21", "2 1112 2222", "3 1013 2023", "4 14 24", "5 10015 ", "6 20026", "7 ")) + tk.MustQuery("select id, a, b, name from t2 order by id").Check(testkit.Rows("1 11 21 a", "5 15 e", "6 26 f", "7 g")) + + err := tk.ExecToErr("update t1 set a=a+10, b = b+20 where id = 1 or a = 1112 or b = 24") + require.NotNil(t, err) + require.True(t, plannercore.ErrRowIsReferenced2.Equal(err)) + tk.MustQuery("select id, a, b from t1 order by id").Check(testkit.Rows("1 11 21", "2 1112 2222", "3 1013 2023", "4 14 24", "5 10015 ", "6 20026", "7 ")) + tk.MustQuery("select id, a, b, name from t2 order by id").Check(testkit.Rows("1 11 21 a", "5 15 e", "6 26 f", "7 g")) + } else { + tk.MustExec("insert into t1 (id, a, b) values (1, 11, 21),(2, 12, 22), (3, 13, 23), (4, 14, 24)") + tk.MustExec("insert into t2 (id, a, b, name) values (1, 11, 21, 'a');") + + tk.MustExec("update t1 set a=a+100, b = b+200 where id = 2") + tk.MustExec("update t1 set a=a+1000, b = b+2000 where a = 13 or b=222") + tk.MustQuery("select id, a, b from t1 order by id").Check(testkit.Rows("1 11 21", "2 1112 2222", "3 1013 2023", "4 14 24")) + tk.MustQuery("select id, a, b, name from t2 order by id").Check(testkit.Rows("1 11 21 a")) + + err := tk.ExecToErr("update t1 set a=a+10, b = b+20 where id = 1 or a = 1112 or b = 24") + require.NotNil(t, err) + require.True(t, plannercore.ErrRowIsReferenced2.Equal(err)) + tk.MustQuery("select id, a, b from t1 order by id").Check(testkit.Rows("1 11 21", "2 1112 2222", "3 1013 2023", "4 14 24")) + tk.MustQuery("select id, a, b, name from t2 order by id").Check(testkit.Rows("1 11 21 a")) + } + + } + + // Case-9: test primary key is handle and contain foreign key column. + tk.MustExec("drop table if exists t2;") + tk.MustExec("drop table if exists t1;") + tk.MustExec("set @@tidb_enable_clustered_index=0;") + tk.MustExec("create table t1 (id int, a int, b int, primary key (id));") + tk.MustExec("create table t2 (b int, a int, id int, name varchar(10), primary key (a), foreign key fk(a) references t1(id));") + tk.MustExec("insert into t1 (id, a, b) values (1, 11, 21),(2, 12, 22), (3, 13, 23), (4, 14, 24)") + tk.MustExec("insert into t2 (id, a, b, name) values (11, 1, 21, 'a')") + tk.MustExec("update t1 set id = id + 100 where id =2 or a = 13") + tk.MustQuery("select id, a, b from t1 order by id").Check(testkit.Rows("1 11 21", "4 14 24", "102 12 22", "103 13 23")) + + err := tk.ExecToErr("update t1 set id = id+10 where id = 1 or b = 24") + require.NotNil(t, err) + require.True(t, plannercore.ErrRowIsReferenced2.Equal(err)) + tk.MustQuery("select id, a, b from t1 order by id").Check(testkit.Rows("1 11 21", "4 14 24", "102 12 22", "103 13 23")) + tk.MustQuery("select id, a, b, name from t2 order by id").Check(testkit.Rows("11 1 21 a")) +} diff --git a/executor/update.go b/executor/update.go index 463bf92a40a28..c34d452cf2eb7 100644 --- a/executor/update.go +++ b/executor/update.go @@ -66,6 +66,8 @@ type UpdateExec struct { tableUpdatable []bool changed []bool matches []bool + // fkChecks contains the foreign key checkers. the map is tableID -> []*FKCheckExec + fkChecks map[int64][]*FKCheckExec } // prepare `handles`, `tableUpdatable`, `changed` to avoid re-computations. @@ -191,7 +193,8 @@ func (e *UpdateExec) exec(ctx context.Context, schema *expression.Schema, row, n flags := bAssignFlag[content.Start:content.End] // Update row - changed, err1 := updateRecord(ctx, e.ctx, handle, oldData, newTableData, flags, tbl, false, e.memTracker, nil) + fkChecks := e.fkChecks[content.TblID] + changed, err1 := updateRecord(ctx, e.ctx, handle, oldData, newTableData, flags, tbl, false, e.memTracker, fkChecks) if err1 == nil { _, exist := e.updatedRowKeys[content.Start].Get(handle) memDelta := e.updatedRowKeys[content.Start].Set(handle, changed) @@ -537,3 +540,12 @@ func (e *updateRuntimeStats) Merge(other execdetails.RuntimeStats) { func (e *updateRuntimeStats) Tp() int { return execdetails.TpUpdateRuntimeStats } + +// GetFKChecks implements WithForeignKeyTrigger interface. +func (e *UpdateExec) GetFKChecks() []*FKCheckExec { + fkChecks := []*FKCheckExec{} + for _, fkcs := range e.fkChecks { + fkChecks = append(fkChecks, fkcs...) + } + return fkChecks +} diff --git a/planner/core/common_plans.go b/planner/core/common_plans.go index 988f4f54c8d72..b5eaf32eeb60b 100644 --- a/planner/core/common_plans.go +++ b/planner/core/common_plans.go @@ -346,6 +346,8 @@ type Update struct { PartitionedTable []table.PartitionedTable tblID2Table map[int64]table.Table + + FKChecks map[int64][]*FKCheck } // Delete represents a delete plan. diff --git a/planner/core/foreign_key.go b/planner/core/foreign_key.go index 5cb7305a746f9..31fb1c7f3d16d 100644 --- a/planner/core/foreign_key.go +++ b/planner/core/foreign_key.go @@ -77,6 +77,49 @@ func (p *Insert) buildOnDuplicateUpdateColumns() map[string]struct{} { return m } +func (updt *Update) buildOnUpdateFKChecks(ctx sessionctx.Context, is infoschema.InfoSchema, tblID2table map[int64]table.Table) (map[int64][]*FKCheck, error) { + if !ctx.GetSessionVars().ForeignKeyChecks { + return nil, nil + } + tblID2UpdateColumns := updt.buildTbl2UpdateColumns() + fkChecks := make(map[int64][]*FKCheck) + for tid, tbl := range tblID2table { + tblInfo := tbl.Meta() + dbInfo, exist := is.SchemaByTable(tblInfo) + if !exist { + return nil, infoschema.ErrDatabaseNotExists + } + updateCols := tblID2UpdateColumns[tid] + if len(updateCols) == 0 { + continue + } + referredFKChecks, err := buildOnUpdateReferredFKChecks(is, dbInfo.Name.L, tblInfo, updateCols) + if err != nil { + return nil, err + } + if len(referredFKChecks) > 0 { + fkChecks[tid] = append(fkChecks[tid], referredFKChecks...) + } + for _, fk := range tblInfo.ForeignKeys { + if fk.Version < 1 { + continue + } + if !isMapContainAnyCols(updateCols, fk.Cols...) { + continue + } + failedErr := ErrNoReferencedRow2.FastGenByArgs(fk.String(dbInfo.Name.L, tblInfo.Name.L)) + fkCheck, err := buildFKCheckOnModifyChildTable(is, fk, failedErr) + if err != nil { + return nil, err + } + if fkCheck != nil { + fkChecks[tid] = append(fkChecks[tid], fkCheck) + } + } + } + return fkChecks, nil +} + func buildOnUpdateReferredFKChecks(is infoschema.InfoSchema, dbName string, tblInfo *model.TableInfo, updateCols map[string]struct{}) ([]*FKCheck, error) { referredFKs := is.GetTableReferredForeignKeys(dbName, tblInfo.Name.L) fkChecks := make([]*FKCheck, 0, len(referredFKs)) @@ -95,6 +138,46 @@ func buildOnUpdateReferredFKChecks(is infoschema.InfoSchema, dbName string, tblI return fkChecks, nil } +func (updt *Update) buildTbl2UpdateColumns() map[int64]map[string]struct{} { + colsInfo := make([]*model.ColumnInfo, len(updt.SelectPlan.Schema().Columns)) + for _, content := range updt.TblColPosInfos { + tbl := updt.tblID2Table[content.TblID] + for i, c := range tbl.WritableCols() { + colsInfo[content.Start+i] = c.ColumnInfo + } + } + tblID2UpdateColumns := make(map[int64]map[string]struct{}) + for tid := range updt.tblID2Table { + tblID2UpdateColumns[tid] = make(map[string]struct{}) + } + for _, assign := range updt.OrderedList { + col := colsInfo[assign.Col.Index] + for _, content := range updt.TblColPosInfos { + if assign.Col.Index >= content.Start && assign.Col.Index < content.End { + tblID2UpdateColumns[content.TblID][col.Name.L] = struct{}{} + break + } + } + } + for tid, tbl := range updt.tblID2Table { + updateCols := tblID2UpdateColumns[tid] + if len(updateCols) == 0 { + continue + } + for _, col := range tbl.WritableCols() { + if !col.IsGenerated() || !col.GeneratedStored { + continue + } + for depCol := range col.Dependences { + if _, ok := updateCols[depCol]; ok { + tblID2UpdateColumns[tid][col.Name.L] = struct{}{} + } + } + } + } + return tblID2UpdateColumns +} + func isMapContainAnyCols(colsMap map[string]struct{}, cols ...model.CIStr) bool { for _, col := range cols { _, exist := colsMap[col.L] diff --git a/planner/core/logical_plan_builder.go b/planner/core/logical_plan_builder.go index e25548ca541db..2806e73c5ef70 100644 --- a/planner/core/logical_plan_builder.go +++ b/planner/core/logical_plan_builder.go @@ -5376,8 +5376,12 @@ func (b *PlanBuilder) buildUpdate(ctx context.Context, update *ast.UpdateStmt) ( tblID2table[id], _ = b.is.TableByID(id) } updt.TblColPosInfos, err = buildColumns2Handle(updt.OutputNames(), tblID2Handle, tblID2table, true) + if err != nil { + return nil, err + } updt.PartitionedTable = b.partitionedTable updt.tblID2Table = tblID2table + updt.FKChecks, err = updt.buildOnUpdateFKChecks(b.ctx, b.is, tblID2table) return updt, err } diff --git a/planner/core/point_get_plan.go b/planner/core/point_get_plan.go index b400144f873bf..10c4050d76e6a 100644 --- a/planner/core/point_get_plan.go +++ b/planner/core/point_get_plan.go @@ -1454,6 +1454,11 @@ func buildPointUpdatePlan(ctx sessionctx.Context, pointPlan PhysicalPlan, dbName updatePlan.PartitionedTable = append(updatePlan.PartitionedTable, pt) } } + fkChecks, err := updatePlan.buildOnUpdateFKChecks(ctx, is, updatePlan.tblID2Table) + if err != nil { + return nil + } + updatePlan.FKChecks = fkChecks return updatePlan } From d18b5409af7e6d07289d1b99b3961383d6a346d6 Mon Sep 17 00:00:00 2001 From: crazycs520 Date: Mon, 26 Sep 2022 14:39:40 +0800 Subject: [PATCH 2/6] tiny refactor Signed-off-by: crazycs520 --- executor/update.go | 14 ++++---------- planner/core/foreign_key.go | 14 ++++---------- planner/core/logical_plan_builder.go | 12 ++++++++++++ 3 files changed, 20 insertions(+), 20 deletions(-) diff --git a/executor/update.go b/executor/update.go index c34d452cf2eb7..e10875404b912 100644 --- a/executor/update.go +++ b/executor/update.go @@ -245,13 +245,7 @@ func (e *UpdateExec) Next(ctx context.Context, req *chunk.Chunk) error { func (e *UpdateExec) updateRows(ctx context.Context) (int, error) { fields := retTypes(e.children[0]) - colsInfo := make([]*table.Column, len(fields)) - for _, content := range e.tblColPosInfos { - tbl := e.tblID2table[content.TblID] - for i, c := range tbl.WritableCols() { - colsInfo[content.Start+i] = c - } - } + colsInfo := plannercore.GetUpdateColumnsInfo(e.tblID2table, e.tblColPosInfos, len(fields)) globalRowIdx := 0 chk := newFirstChunk(e.children[0]) if !e.allAssignmentsAreConstant { @@ -543,9 +537,9 @@ func (e *updateRuntimeStats) Tp() int { // GetFKChecks implements WithForeignKeyTrigger interface. func (e *UpdateExec) GetFKChecks() []*FKCheckExec { - fkChecks := []*FKCheckExec{} - for _, fkcs := range e.fkChecks { - fkChecks = append(fkChecks, fkcs...) + fkChecks := make([]*FKCheckExec, 0, len(e.fkChecks)) + for _, fkc := range e.fkChecks { + fkChecks = append(fkChecks, fkc...) } return fkChecks } diff --git a/planner/core/foreign_key.go b/planner/core/foreign_key.go index 31fb1c7f3d16d..188dfa126736c 100644 --- a/planner/core/foreign_key.go +++ b/planner/core/foreign_key.go @@ -139,21 +139,15 @@ func buildOnUpdateReferredFKChecks(is infoschema.InfoSchema, dbName string, tblI } func (updt *Update) buildTbl2UpdateColumns() map[int64]map[string]struct{} { - colsInfo := make([]*model.ColumnInfo, len(updt.SelectPlan.Schema().Columns)) - for _, content := range updt.TblColPosInfos { - tbl := updt.tblID2Table[content.TblID] - for i, c := range tbl.WritableCols() { - colsInfo[content.Start+i] = c.ColumnInfo - } - } + colsInfo := GetUpdateColumnsInfo(updt.tblID2Table, updt.TblColPosInfos, len(updt.SelectPlan.Schema().Columns)) tblID2UpdateColumns := make(map[int64]map[string]struct{}) - for tid := range updt.tblID2Table { - tblID2UpdateColumns[tid] = make(map[string]struct{}) - } for _, assign := range updt.OrderedList { col := colsInfo[assign.Col.Index] for _, content := range updt.TblColPosInfos { if assign.Col.Index >= content.Start && assign.Col.Index < content.End { + if _, ok := tblID2UpdateColumns[content.TblID]; !ok { + tblID2UpdateColumns[content.TblID] = make(map[string]struct{}) + } tblID2UpdateColumns[content.TblID][col.Name.L] = struct{}{} break } diff --git a/planner/core/logical_plan_builder.go b/planner/core/logical_plan_builder.go index 2806e73c5ef70..2ec4b4c7c849d 100644 --- a/planner/core/logical_plan_builder.go +++ b/planner/core/logical_plan_builder.go @@ -5385,6 +5385,18 @@ func (b *PlanBuilder) buildUpdate(ctx context.Context, update *ast.UpdateStmt) ( return updt, err } +// GetUpdateColumnsInfo get the update columns info. +func GetUpdateColumnsInfo(tblID2Table map[int64]table.Table, tblColPosInfos TblColPosInfoSlice, size int) []*table.Column { + colsInfo := make([]*table.Column, size) + for _, content := range tblColPosInfos { + tbl := tblID2Table[content.TblID] + for i, c := range tbl.WritableCols() { + colsInfo[content.Start+i] = c + } + } + return colsInfo +} + type tblUpdateInfo struct { name string pkUpdated bool From a513dac25ecfec11e65586ebab54050ab53910f5 Mon Sep 17 00:00:00 2001 From: crazycs520 Date: Mon, 26 Sep 2022 15:03:11 +0800 Subject: [PATCH 3/6] tiny refactor Signed-off-by: crazycs520 --- planner/core/foreign_key.go | 42 ++++++++++++++++++++++++------------- 1 file changed, 27 insertions(+), 15 deletions(-) diff --git a/planner/core/foreign_key.go b/planner/core/foreign_key.go index 188dfa126736c..1523065bb553d 100644 --- a/planner/core/foreign_key.go +++ b/planner/core/foreign_key.go @@ -100,21 +100,12 @@ func (updt *Update) buildOnUpdateFKChecks(ctx sessionctx.Context, is infoschema. if len(referredFKChecks) > 0 { fkChecks[tid] = append(fkChecks[tid], referredFKChecks...) } - for _, fk := range tblInfo.ForeignKeys { - if fk.Version < 1 { - continue - } - if !isMapContainAnyCols(updateCols, fk.Cols...) { - continue - } - failedErr := ErrNoReferencedRow2.FastGenByArgs(fk.String(dbInfo.Name.L, tblInfo.Name.L)) - fkCheck, err := buildFKCheckOnModifyChildTable(is, fk, failedErr) - if err != nil { - return nil, err - } - if fkCheck != nil { - fkChecks[tid] = append(fkChecks[tid], fkCheck) - } + childFKChecks, err := buildOnUpdateChildFKChecks(is, dbInfo.Name.L, tblInfo, updateCols) + if err != nil { + return nil, err + } + if len(childFKChecks) > 0 { + fkChecks[tid] = append(fkChecks[tid], childFKChecks...) } } return fkChecks, nil @@ -138,6 +129,27 @@ func buildOnUpdateReferredFKChecks(is infoschema.InfoSchema, dbName string, tblI return fkChecks, nil } +func buildOnUpdateChildFKChecks(is infoschema.InfoSchema, dbName string, tblInfo *model.TableInfo, updateCols map[string]struct{}) ([]*FKCheck, error) { + fkChecks := make([]*FKCheck, 0, len(tblInfo.ForeignKeys)) + for _, fk := range tblInfo.ForeignKeys { + if fk.Version < 1 { + continue + } + if !isMapContainAnyCols(updateCols, fk.Cols...) { + continue + } + failedErr := ErrNoReferencedRow2.FastGenByArgs(fk.String(dbName, tblInfo.Name.L)) + fkCheck, err := buildFKCheckOnModifyChildTable(is, fk, failedErr) + if err != nil { + return nil, err + } + if fkCheck != nil { + fkChecks = append(fkChecks, fkCheck) + } + } + return fkChecks, nil +} + func (updt *Update) buildTbl2UpdateColumns() map[int64]map[string]struct{} { colsInfo := GetUpdateColumnsInfo(updt.tblID2Table, updt.TblColPosInfos, len(updt.SelectPlan.Schema().Columns)) tblID2UpdateColumns := make(map[int64]map[string]struct{}) From 7c9ca0bf0f87b5e79e9beae8d62df6b233eda5bd Mon Sep 17 00:00:00 2001 From: crazycs520 Date: Mon, 26 Sep 2022 15:26:18 +0800 Subject: [PATCH 4/6] fix lint Signed-off-by: crazycs520 --- executor/foreign_key_test.go | 43 ++++++++---------------------------- 1 file changed, 9 insertions(+), 34 deletions(-) diff --git a/executor/foreign_key_test.go b/executor/foreign_key_test.go index 83838c99a21d2..dd4096a4be985 100644 --- a/executor/foreign_key_test.go +++ b/executor/foreign_key_test.go @@ -558,9 +558,7 @@ func TestForeignKeyOnUpdateChildTable(t *testing.T) { "update t2 set a=12, b = 23 where id = 1", } for _, sqlStr := range sqls { - err := tk.ExecToErr(sqlStr) - require.NotNil(t, err) - require.True(t, plannercore.ErrNoReferencedRow2.Equal(err)) + tk.MustGetDBError(sqlStr, plannercore.ErrNoReferencedRow2) } tk.MustExec("update t2 set a=12, b = 22 where id = 1") tk.MustQuery("select id, a, b, name from t2 order by id").Check(testkit.Rows("1 12 22 a")) @@ -583,16 +581,12 @@ func TestForeignKeyOnUpdateChildTable(t *testing.T) { tk.MustExec("begin") tk.MustExec("delete from t1 where id=2") - err := tk.ExecToErr("update t2 set a=12, b=22 where id=1") - require.NotNil(t, err) - require.True(t, plannercore.ErrNoReferencedRow2.Equal(err)) + tk.MustGetDBError("update t2 set a=12, b=22 where id=1", plannercore.ErrNoReferencedRow2) tk.MustExec("update t2 set a=13, b=23 where id=1") tk.MustExec("insert into t1 (id, a, b) values (5, 15, 25)") tk.MustExec("update t2 set a=15, b=25 where id=1") tk.MustExec("delete from t1 where id=1") - err = tk.ExecToErr("update t2 set a=11, b=21 where id=1") - require.NotNil(t, err) - require.True(t, plannercore.ErrNoReferencedRow2.Equal(err)) + tk.MustGetDBError("update t2 set a=11, b=21 where id=1", plannercore.ErrNoReferencedRow2) tk.MustExec("commit") tk.MustQuery("select id, a, b, name from t2").Check(testkit.Rows("1 15 25 a")) } @@ -613,9 +607,7 @@ func TestForeignKeyOnUpdateChildTable(t *testing.T) { tk.MustQuery("select id, a, b , name from t2 order by id").Check(testkit.Rows("11 3 22 a")) tk.MustExec("update t2 set id = 1 where id = 11") tk.MustQuery("select id, a, b, name from t2 order by id").Check(testkit.Rows("1 3 22 a")) - err := tk.ExecToErr("update t2 set a = 10 where id = 1") - require.NotNil(t, err) - require.True(t, plannercore.ErrNoReferencedRow2.Equal(err)) + tk.MustGetDBError("update t2 set a = 10 where id = 1", plannercore.ErrNoReferencedRow2) tk.MustQuery("select id, a, b, name from t2 order by id").Check(testkit.Rows("1 3 22 a")) // Test In txn. @@ -629,16 +621,12 @@ func TestForeignKeyOnUpdateChildTable(t *testing.T) { tk.MustExec("begin") tk.MustExec("delete from t1 where id=2") - err = tk.ExecToErr("update t2 set a=2, b=22 where id=1") - require.NotNil(t, err) - require.True(t, plannercore.ErrNoReferencedRow2.Equal(err)) + tk.MustGetDBError("update t2 set a=2, b=22 where id=1", plannercore.ErrNoReferencedRow2) tk.MustExec("update t2 set a=3, b=23 where id=1") tk.MustExec("insert into t1 (id, a, b) values (5, 15, 25)") tk.MustExec("update t2 set a=5, b=25 where id=1") tk.MustExec("delete from t1 where id=1") - err = tk.ExecToErr("update t2 set a=1, b=21 where id=1") - require.NotNil(t, err) - require.True(t, plannercore.ErrNoReferencedRow2.Equal(err)) + tk.MustGetDBError("update t2 set a=1, b=21 where id=1", plannercore.ErrNoReferencedRow2) tk.MustExec("commit") tk.MustQuery("select id, a, b, name from t2").Check(testkit.Rows("1 5 25 a")) } @@ -649,7 +637,6 @@ func TestForeignKeyOnUpdateParentTableCheck(t *testing.T) { tk.MustExec("set @@global.tidb_enable_foreign_key=1") tk.MustExec("set @@foreign_key_checks=1") tk.MustExec("use test") - for _, ca := range foreignKeyTestCase1 { tk.MustExec("drop table if exists t2;") tk.MustExec("drop table if exists t1;") @@ -665,30 +652,21 @@ func TestForeignKeyOnUpdateParentTableCheck(t *testing.T) { tk.MustExec("update t1 set a=a+10000, b = b+20000 where id = 5 or a is null or b is null") tk.MustQuery("select id, a, b from t1 order by id").Check(testkit.Rows("1 11 21", "2 1112 2222", "3 1013 2023", "4 14 24", "5 10015 ", "6 20026", "7 ")) tk.MustQuery("select id, a, b, name from t2 order by id").Check(testkit.Rows("1 11 21 a", "5 15 e", "6 26 f", "7 g")) - - err := tk.ExecToErr("update t1 set a=a+10, b = b+20 where id = 1 or a = 1112 or b = 24") - require.NotNil(t, err) - require.True(t, plannercore.ErrRowIsReferenced2.Equal(err)) + tk.MustGetDBError("update t1 set a=a+10, b = b+20 where id = 1 or a = 1112 or b = 24", plannercore.ErrRowIsReferenced2) tk.MustQuery("select id, a, b from t1 order by id").Check(testkit.Rows("1 11 21", "2 1112 2222", "3 1013 2023", "4 14 24", "5 10015 ", "6 20026", "7 ")) tk.MustQuery("select id, a, b, name from t2 order by id").Check(testkit.Rows("1 11 21 a", "5 15 e", "6 26 f", "7 g")) } else { tk.MustExec("insert into t1 (id, a, b) values (1, 11, 21),(2, 12, 22), (3, 13, 23), (4, 14, 24)") tk.MustExec("insert into t2 (id, a, b, name) values (1, 11, 21, 'a');") - tk.MustExec("update t1 set a=a+100, b = b+200 where id = 2") tk.MustExec("update t1 set a=a+1000, b = b+2000 where a = 13 or b=222") tk.MustQuery("select id, a, b from t1 order by id").Check(testkit.Rows("1 11 21", "2 1112 2222", "3 1013 2023", "4 14 24")) tk.MustQuery("select id, a, b, name from t2 order by id").Check(testkit.Rows("1 11 21 a")) - - err := tk.ExecToErr("update t1 set a=a+10, b = b+20 where id = 1 or a = 1112 or b = 24") - require.NotNil(t, err) - require.True(t, plannercore.ErrRowIsReferenced2.Equal(err)) + tk.MustGetDBError("update t1 set a=a+10, b = b+20 where id = 1 or a = 1112 or b = 24", plannercore.ErrRowIsReferenced2) tk.MustQuery("select id, a, b from t1 order by id").Check(testkit.Rows("1 11 21", "2 1112 2222", "3 1013 2023", "4 14 24")) tk.MustQuery("select id, a, b, name from t2 order by id").Check(testkit.Rows("1 11 21 a")) } - } - // Case-9: test primary key is handle and contain foreign key column. tk.MustExec("drop table if exists t2;") tk.MustExec("drop table if exists t1;") @@ -699,10 +677,7 @@ func TestForeignKeyOnUpdateParentTableCheck(t *testing.T) { tk.MustExec("insert into t2 (id, a, b, name) values (11, 1, 21, 'a')") tk.MustExec("update t1 set id = id + 100 where id =2 or a = 13") tk.MustQuery("select id, a, b from t1 order by id").Check(testkit.Rows("1 11 21", "4 14 24", "102 12 22", "103 13 23")) - - err := tk.ExecToErr("update t1 set id = id+10 where id = 1 or b = 24") - require.NotNil(t, err) - require.True(t, plannercore.ErrRowIsReferenced2.Equal(err)) + tk.MustGetDBError("update t1 set id = id+10 where id = 1 or b = 24", plannercore.ErrRowIsReferenced2) tk.MustQuery("select id, a, b from t1 order by id").Check(testkit.Rows("1 11 21", "4 14 24", "102 12 22", "103 13 23")) tk.MustQuery("select id, a, b, name from t2 order by id").Check(testkit.Rows("11 1 21 a")) } From 02c1090a89133c28ebae8af82ab9618588658dc3 Mon Sep 17 00:00:00 2001 From: crazycs520 Date: Wed, 28 Sep 2022 18:05:28 +0800 Subject: [PATCH 5/6] add comment Signed-off-by: crazycs520 --- planner/core/foreign_key.go | 1 + 1 file changed, 1 insertion(+) diff --git a/planner/core/foreign_key.go b/planner/core/foreign_key.go index 1523065bb553d..b628687869762 100644 --- a/planner/core/foreign_key.go +++ b/planner/core/foreign_key.go @@ -87,6 +87,7 @@ func (updt *Update) buildOnUpdateFKChecks(ctx sessionctx.Context, is infoschema. tblInfo := tbl.Meta() dbInfo, exist := is.SchemaByTable(tblInfo) if !exist { + // Normally, it should never happen. Just check here to avoid panic here. return nil, infoschema.ErrDatabaseNotExists } updateCols := tblID2UpdateColumns[tid] From dac9943093d8754f442d16dd84d708dd20d54994 Mon Sep 17 00:00:00 2001 From: crazycs520 Date: Wed, 28 Sep 2022 19:30:56 +0800 Subject: [PATCH 6/6] address comment Signed-off-by: crazycs520 --- planner/core/foreign_key.go | 13 +++++++------ planner/core/logical_plan_builder.go | 2 +- planner/core/point_get_plan.go | 3 +-- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/planner/core/foreign_key.go b/planner/core/foreign_key.go index b628687869762..651bd5b897e4a 100644 --- a/planner/core/foreign_key.go +++ b/planner/core/foreign_key.go @@ -77,9 +77,9 @@ func (p *Insert) buildOnDuplicateUpdateColumns() map[string]struct{} { return m } -func (updt *Update) buildOnUpdateFKChecks(ctx sessionctx.Context, is infoschema.InfoSchema, tblID2table map[int64]table.Table) (map[int64][]*FKCheck, error) { +func (updt *Update) buildOnUpdateFKChecks(ctx sessionctx.Context, is infoschema.InfoSchema, tblID2table map[int64]table.Table) error { if !ctx.GetSessionVars().ForeignKeyChecks { - return nil, nil + return nil } tblID2UpdateColumns := updt.buildTbl2UpdateColumns() fkChecks := make(map[int64][]*FKCheck) @@ -88,7 +88,7 @@ func (updt *Update) buildOnUpdateFKChecks(ctx sessionctx.Context, is infoschema. dbInfo, exist := is.SchemaByTable(tblInfo) if !exist { // Normally, it should never happen. Just check here to avoid panic here. - return nil, infoschema.ErrDatabaseNotExists + return infoschema.ErrDatabaseNotExists } updateCols := tblID2UpdateColumns[tid] if len(updateCols) == 0 { @@ -96,20 +96,21 @@ func (updt *Update) buildOnUpdateFKChecks(ctx sessionctx.Context, is infoschema. } referredFKChecks, err := buildOnUpdateReferredFKChecks(is, dbInfo.Name.L, tblInfo, updateCols) if err != nil { - return nil, err + return err } if len(referredFKChecks) > 0 { fkChecks[tid] = append(fkChecks[tid], referredFKChecks...) } childFKChecks, err := buildOnUpdateChildFKChecks(is, dbInfo.Name.L, tblInfo, updateCols) if err != nil { - return nil, err + return err } if len(childFKChecks) > 0 { fkChecks[tid] = append(fkChecks[tid], childFKChecks...) } } - return fkChecks, nil + updt.FKChecks = fkChecks + return nil } func buildOnUpdateReferredFKChecks(is infoschema.InfoSchema, dbName string, tblInfo *model.TableInfo, updateCols map[string]struct{}) ([]*FKCheck, error) { diff --git a/planner/core/logical_plan_builder.go b/planner/core/logical_plan_builder.go index 84bd3c2322f50..4b864a00deb8c 100644 --- a/planner/core/logical_plan_builder.go +++ b/planner/core/logical_plan_builder.go @@ -5381,7 +5381,7 @@ func (b *PlanBuilder) buildUpdate(ctx context.Context, update *ast.UpdateStmt) ( } updt.PartitionedTable = b.partitionedTable updt.tblID2Table = tblID2table - updt.FKChecks, err = updt.buildOnUpdateFKChecks(b.ctx, b.is, tblID2table) + err = updt.buildOnUpdateFKChecks(b.ctx, b.is, tblID2table) return updt, err } diff --git a/planner/core/point_get_plan.go b/planner/core/point_get_plan.go index a5506fa1e660f..e33c16d3923a1 100644 --- a/planner/core/point_get_plan.go +++ b/planner/core/point_get_plan.go @@ -1546,11 +1546,10 @@ func buildPointUpdatePlan(ctx sessionctx.Context, pointPlan PhysicalPlan, dbName updatePlan.PartitionedTable = append(updatePlan.PartitionedTable, pt) } } - fkChecks, err := updatePlan.buildOnUpdateFKChecks(ctx, is, updatePlan.tblID2Table) + err := updatePlan.buildOnUpdateFKChecks(ctx, is, updatePlan.tblID2Table) if err != nil { return nil } - updatePlan.FKChecks = fkChecks return updatePlan }