From 825b59d1a0ef5dcb150d7c763b5981dbf0e0c25f Mon Sep 17 00:00:00 2001 From: crazycs520 Date: Tue, 1 Nov 2022 17:06:52 +0800 Subject: [PATCH 1/7] executor: use generate ast instead SQL for foreign key cascade Signed-off-by: crazycs520 --- executor/fktest/foreign_key_test.go | 69 +++++---- executor/foreign_key.go | 214 ++++++++++++---------------- planner/core/foreign_key.go | 43 +++++- 3 files changed, 168 insertions(+), 158 deletions(-) diff --git a/executor/fktest/foreign_key_test.go b/executor/fktest/foreign_key_test.go index ac20a0c43c9df..4afb5e81f0c5b 100644 --- a/executor/fktest/foreign_key_test.go +++ b/executor/fktest/foreign_key_test.go @@ -15,6 +15,7 @@ package fk_test import ( + "context" "fmt" "strconv" "strings" @@ -24,10 +25,14 @@ import ( "github.com/pingcap/tidb/executor" "github.com/pingcap/tidb/kv" + "github.com/pingcap/tidb/parser/ast" + "github.com/pingcap/tidb/parser/format" "github.com/pingcap/tidb/parser/model" + "github.com/pingcap/tidb/parser/mysql" plannercore "github.com/pingcap/tidb/planner/core" "github.com/pingcap/tidb/testkit" "github.com/pingcap/tidb/types" + "github.com/pingcap/tidb/util/sqlexec" "github.com/stretchr/testify/require" ) @@ -1247,44 +1252,50 @@ func TestForeignKeyOnDeleteCascade2(t *testing.T) { tk.MustQuery("select count(*) from t2").Check(testkit.Rows("0")) } -func TestForeignKeyGenerateCascadeSQL(t *testing.T) { - fk := &model.FKInfo{ - Cols: []model.CIStr{model.NewCIStr("c0"), model.NewCIStr("c1")}, - } +func TestForeignKeyGenerateCascadeAST(t *testing.T) { + store := testkit.CreateMockStore(t) + tk := testkit.NewTestKit(t, store) + tk.MustExec("use test;") fkValues := [][]types.Datum{ {types.NewDatum(1), types.NewDatum("a")}, {types.NewDatum(2), types.NewDatum("b")}, } - sql, err := executor.GenCascadeDeleteSQL(model.NewCIStr("test"), model.NewCIStr("t"), model.NewCIStr(""), fk, fkValues) - require.NoError(t, err) - require.Equal(t, "DELETE FROM `test`.`t` WHERE (`c0`, `c1`) IN ((1,'a'), (2,'b'))", sql) - - sql, err = executor.GenCascadeDeleteSQL(model.NewCIStr("test"), model.NewCIStr("t"), model.NewCIStr("idx"), fk, fkValues) - require.NoError(t, err) - require.Equal(t, "DELETE FROM `test`.`t` USE INDEX(`idx`) WHERE (`c0`, `c1`) IN ((1,'a'), (2,'b'))", sql) - - sql, err = executor.GenCascadeSetNullSQL(model.NewCIStr("test"), model.NewCIStr("t"), model.NewCIStr(""), fk, fkValues) - require.NoError(t, err) - require.Equal(t, "UPDATE `test`.`t` SET `c0` = NULL, `c1` = NULL WHERE (`c0`, `c1`) IN ((1,'a'), (2,'b'))", sql) - - sql, err = executor.GenCascadeSetNullSQL(model.NewCIStr("test"), model.NewCIStr("t"), model.NewCIStr("idx"), fk, fkValues) - require.NoError(t, err) - require.Equal(t, "UPDATE `test`.`t` USE INDEX(`idx`) SET `c0` = NULL, `c1` = NULL WHERE (`c0`, `c1`) IN ((1,'a'), (2,'b'))", sql) - + cols := []*model.ColumnInfo{ + {ID: 1, Name: model.NewCIStr("a"), FieldType: *types.NewFieldType(mysql.TypeLonglong)}, + {ID: 2, Name: model.NewCIStr("name"), FieldType: *types.NewFieldType(mysql.TypeVarchar)}, + } + restoreFn := func(stmt ast.StmtNode) string { + var sb strings.Builder + fctx := format.NewRestoreCtx(format.DefaultRestoreFlags, &sb) + err := stmt.Restore(fctx) + require.NoError(t, err) + return sb.String() + } + checkStmtFn := func(stmt ast.StmtNode, sql string) { + exec, ok := tk.Session().(sqlexec.RestrictedSQLExecutor) + require.True(t, ok) + expectedStmt, err := exec.ParseWithParams(context.Background(), sql) + require.NoError(t, err) + require.Equal(t, restoreFn(expectedStmt), restoreFn(stmt)) + } + var stmt ast.StmtNode + stmt = executor.GenCascadeDeleteAST(model.NewCIStr("test"), model.NewCIStr("t2"), model.NewCIStr(""), cols, fkValues) + checkStmtFn(stmt, "delete from test.t2 where (a,name) in ((1,'a'), (2,'b'))") + stmt = executor.GenCascadeDeleteAST(model.NewCIStr("test"), model.NewCIStr("t2"), model.NewCIStr("idx"), cols, fkValues) + checkStmtFn(stmt, "delete from test.t2 use index(idx) where (a,name) in ((1,'a'), (2,'b'))") + stmt = executor.GenCascadeSetNullAST(model.NewCIStr("test"), model.NewCIStr("t2"), model.NewCIStr(""), cols, fkValues) + checkStmtFn(stmt, "update test.t2 set a = null, name = null where (a,name) in ((1,'a'), (2,'b'))") + stmt = executor.GenCascadeSetNullAST(model.NewCIStr("test"), model.NewCIStr("t2"), model.NewCIStr("idx"), cols, fkValues) + checkStmtFn(stmt, "update test.t2 use index(idx) set a = null, name = null where (a,name) in ((1,'a'), (2,'b'))") newValue1 := []types.Datum{types.NewDatum(10), types.NewDatum("aa")} couple := &executor.UpdatedValuesCouple{ NewValues: newValue1, OldValuesList: fkValues, } - sql, err = executor.GenCascadeUpdateSQL(model.NewCIStr("test"), model.NewCIStr("t"), model.NewCIStr(""), fk, couple) - require.NoError(t, err) - require.Equal(t, "UPDATE `test`.`t` SET `c0` = 10, `c1` = 'aa' WHERE (`c0`, `c1`) IN ((1,'a'), (2,'b'))", sql) - - newValue2 := []types.Datum{types.NewDatum(nil), types.NewDatum(nil)} - couple.NewValues = newValue2 - sql, err = executor.GenCascadeUpdateSQL(model.NewCIStr("test"), model.NewCIStr("t"), model.NewCIStr("idx"), fk, couple) - require.NoError(t, err) - require.Equal(t, "UPDATE `test`.`t` USE INDEX(`idx`) SET `c0` = NULL, `c1` = NULL WHERE (`c0`, `c1`) IN ((1,'a'), (2,'b'))", sql) + stmt = executor.GenCascadeUpdateAST(model.NewCIStr("test"), model.NewCIStr("t2"), model.NewCIStr(""), cols, couple) + checkStmtFn(stmt, "update test.t2 set a = 10, name = 'aa' where (a,name) in ((1,'a'), (2,'b'))") + stmt = executor.GenCascadeUpdateAST(model.NewCIStr("test"), model.NewCIStr("t2"), model.NewCIStr("idx"), cols, couple) + checkStmtFn(stmt, "update test.t2 use index(idx) set a = 10, name = 'aa' where (a,name) in ((1,'a'), (2,'b'))") } func TestForeignKeyOnDeleteSetNull(t *testing.T) { diff --git a/executor/foreign_key.go b/executor/foreign_key.go index ea45e9bd394b5..1f71a8a6e46f1 100644 --- a/executor/foreign_key.go +++ b/executor/foreign_key.go @@ -15,12 +15,12 @@ package executor import ( - "bytes" "context" "sync/atomic" "github.com/pingcap/errors" "github.com/pingcap/tidb/kv" + "github.com/pingcap/tidb/parser/ast" "github.com/pingcap/tidb/parser/model" "github.com/pingcap/tidb/planner" plannercore "github.com/pingcap/tidb/planner/core" @@ -29,9 +29,9 @@ import ( "github.com/pingcap/tidb/table" "github.com/pingcap/tidb/tablecodec" "github.com/pingcap/tidb/types" + driver "github.com/pingcap/tidb/types/parser_driver" "github.com/pingcap/tidb/util/codec" "github.com/pingcap/tidb/util/set" - "github.com/pingcap/tidb/util/sqlexec" "github.com/tikv/client-go/v2/txnkv/txnsnapshot" ) @@ -65,6 +65,8 @@ type FKCascadeExec struct { referredFK *model.ReferredFKInfo childTable *model.TableInfo fk *model.FKInfo + fkCols []*model.ColumnInfo + fkIdx *model.IndexInfo // On delete statement, fkValues stores the delete foreign key values. // On update statement and the foreign key cascade is `SET NULL`, fkValues stores the old foreign key values. fkValues [][]types.Datum @@ -577,6 +579,8 @@ func (b *executorBuilder) buildFKCascadeExec(tbl table.Table, fkCascade *planner referredFK: fkCascade.ReferredFK, childTable: fkCascade.ChildTable.Meta(), fk: fkCascade.FK, + fkCols: fkCascade.FKCols, + fkIdx: fkCascade.FKIdx, fkUpdatedValuesMap: make(map[string]*UpdatedValuesCouple), }, nil } @@ -634,51 +638,37 @@ func (fkc *FKCascadeExec) buildFKCascadePlan(ctx context.Context) (plannercore.P return nil, nil } var indexName model.CIStr - indexForFK := model.FindIndexByColumns(fkc.childTable, fkc.fk.Cols...) - if indexForFK != nil { - indexName = indexForFK.Name + if fkc.fkIdx != nil { + indexName = fkc.fkIdx.Name } - var sqlStr string - var err error + var stmtNode ast.StmtNode switch fkc.tp { case plannercore.FKCascadeOnDelete: fkValues := fkc.fetchOnDeleteOrUpdateFKValues() switch model.ReferOptionType(fkc.fk.OnDelete) { case model.ReferOptionCascade: - sqlStr, err = GenCascadeDeleteSQL(fkc.referredFK.ChildSchema, fkc.childTable.Name, indexName, fkc.fk, fkValues) + stmtNode = GenCascadeDeleteAST(fkc.referredFK.ChildSchema, fkc.childTable.Name, indexName, fkc.fkCols, fkValues) case model.ReferOptionSetNull: - sqlStr, err = GenCascadeSetNullSQL(fkc.referredFK.ChildSchema, fkc.childTable.Name, indexName, fkc.fk, fkValues) + stmtNode = GenCascadeSetNullAST(fkc.referredFK.ChildSchema, fkc.childTable.Name, indexName, fkc.fkCols, fkValues) } case plannercore.FKCascadeOnUpdate: switch model.ReferOptionType(fkc.fk.OnUpdate) { case model.ReferOptionCascade: couple := fkc.fetchUpdatedValuesCouple() if couple != nil && len(couple.NewValues) != 0 { - sqlStr, err = GenCascadeUpdateSQL(fkc.referredFK.ChildSchema, fkc.childTable.Name, indexName, fkc.fk, couple) + stmtNode = GenCascadeUpdateAST(fkc.referredFK.ChildSchema, fkc.childTable.Name, indexName, fkc.fkCols, couple) } case model.ReferOptionSetNull: fkValues := fkc.fetchOnDeleteOrUpdateFKValues() - sqlStr, err = GenCascadeSetNullSQL(fkc.referredFK.ChildSchema, fkc.childTable.Name, indexName, fkc.fk, fkValues) + stmtNode = GenCascadeSetNullAST(fkc.referredFK.ChildSchema, fkc.childTable.Name, indexName, fkc.fkCols, fkValues) } } - if err != nil { - return nil, err + if stmtNode == nil { + return nil, errors.Errorf("generate foreign key cascade ast failed, %v", fkc.tp) } - if sqlStr == "" { - return nil, errors.Errorf("generate foreign key cascade sql failed, %v", fkc.tp) - } - sctx := fkc.b.ctx - exec, ok := sctx.(sqlexec.RestrictedSQLExecutor) - if !ok { - return nil, nil - } - stmtNode, err := exec.ParseWithParams(ctx, sqlStr) - if err != nil { - return nil, err - } ret := &plannercore.PreprocessorReturn{} - err = plannercore.Preprocess(ctx, sctx, stmtNode, plannercore.WithPreprocessorReturn(ret), plannercore.InitTxnContextProvider) + err := plannercore.Preprocess(ctx, sctx, stmtNode, plannercore.WithPreprocessorReturn(ret), plannercore.InitTxnContextProvider) if err != nil { return nil, err } @@ -717,120 +707,100 @@ func (fkc *FKCascadeExec) fetchUpdatedValuesCouple() *UpdatedValuesCouple { return nil } -// GenCascadeDeleteSQL uses to generate cascade delete SQL, export for test. -func GenCascadeDeleteSQL(schema, table, idx model.CIStr, fk *model.FKInfo, fkValues [][]types.Datum) (string, error) { - buf := bytes.NewBuffer(make([]byte, 0, 48+8*len(fkValues))) - buf.WriteString("DELETE FROM `") - buf.WriteString(schema.L) - buf.WriteString("`.`") - buf.WriteString(table.L) - buf.WriteString("`") - if idx.L != "" { - // Add use index to make sure the optimizer will use index instead of full table scan. - buf.WriteString(" USE INDEX(`") - buf.WriteString(idx.L) - buf.WriteString("`)") - } - err := genCascadeSQLWhereCondition(buf, fk, fkValues) - if err != nil { - return "", err +// GenCascadeDeleteAST uses to generate cascade delete ast, export for test. +func GenCascadeDeleteAST(schema, table, idx model.CIStr, cols []*model.ColumnInfo, fkValues [][]types.Datum) *ast.DeleteStmt { + deleteStmt := &ast.DeleteStmt{ + TableRefs: genTableRefsAST(schema, table, idx), + Where: genWhereConditionAst(cols, fkValues), } - return buf.String(), nil + return deleteStmt } -// GenCascadeSetNullSQL uses to generate foreign key `SET NULL` SQL, export for test. -func GenCascadeSetNullSQL(schema, table, idx model.CIStr, fk *model.FKInfo, fkValues [][]types.Datum) (string, error) { - newValues := make([]types.Datum, len(fk.Cols)) - for i := range fk.Cols { +// GenCascadeSetNullAST uses to generate foreign key `SET NULL` ast, export for test. +func GenCascadeSetNullAST(schema, table, idx model.CIStr, cols []*model.ColumnInfo, fkValues [][]types.Datum) *ast.UpdateStmt { + newValues := make([]types.Datum, len(cols)) + for i := range cols { newValues[i] = types.NewDatum(nil) } couple := &UpdatedValuesCouple{ NewValues: newValues, OldValuesList: fkValues, } - return GenCascadeUpdateSQL(schema, table, idx, fk, couple) + return GenCascadeUpdateAST(schema, table, idx, cols, couple) } -// GenCascadeUpdateSQL uses to generate cascade update SQL, export for test. -func GenCascadeUpdateSQL(schema, table, idx model.CIStr, fk *model.FKInfo, couple *UpdatedValuesCouple) (string, error) { - buf := bytes.NewBuffer(nil) - buf.WriteString("UPDATE `") - buf.WriteString(schema.L) - buf.WriteString("`.`") - buf.WriteString(table.L) - buf.WriteString("`") - if idx.L != "" { - // Add use index to make sure the optimizer will use index instead of full table scan. - buf.WriteString(" USE INDEX(`") - buf.WriteString(idx.L) - buf.WriteString("`)") - } - buf.WriteString(" SET ") - for i, col := range fk.Cols { - if i > 0 { - buf.WriteString(", ") - } - buf.WriteString("`" + col.L) - buf.WriteString("` = ") - val, err := genFKValueString(couple.NewValues[i]) - if err != nil { - return "", err +// GenCascadeUpdateAST uses to generate cascade update ast, export for test. +func GenCascadeUpdateAST(schema, table, idx model.CIStr, cols []*model.ColumnInfo, couple *UpdatedValuesCouple) *ast.UpdateStmt { + list := make([]*ast.Assignment, 0, len(cols)) + for i, col := range cols { + v := &driver.ValueExpr{Datum: couple.NewValues[i]} + v.Type = col.FieldType + assignment := &ast.Assignment{ + Column: &ast.ColumnName{Name: col.Name}, + Expr: v, } - buf.WriteString(val) + list = append(list, assignment) } - err := genCascadeSQLWhereCondition(buf, fk, couple.OldValuesList) - if err != nil { - return "", err + updateStmt := &ast.UpdateStmt{ + TableRefs: genTableRefsAST(schema, table, idx), + Where: genWhereConditionAst(cols, couple.OldValuesList), + List: list, } - return buf.String(), nil + return updateStmt } -func genCascadeSQLWhereCondition(buf *bytes.Buffer, fk *model.FKInfo, fkValues [][]types.Datum) error { - buf.WriteString(" WHERE (") - for i, col := range fk.Cols { - if i > 0 { - buf.WriteString(", ") - } - buf.WriteString("`" + col.L + "`") - } - buf.WriteString(") IN (") - for i, vs := range fkValues { - if i > 0 { - buf.WriteString(", (") - } else { - buf.WriteString("(") - } - for i := range vs { - val, err := genFKValueString(vs[i]) - if err != nil { - return err - } - if i > 0 { - buf.WriteString(",") +func genTableRefsAST(schema, table, idx model.CIStr) *ast.TableRefsClause { + tn := &ast.TableName{Schema: schema, Name: table} + if idx.L != "" { + tn.IndexHints = []*ast.IndexHint{{ + IndexNames: []model.CIStr{idx}, + HintType: ast.HintUse, + HintScope: ast.HintForScan, + }} + } + join := &ast.Join{Left: &ast.TableSource{Source: tn}} + return &ast.TableRefsClause{TableRefs: join} +} + +func genWhereConditionAst(cols []*model.ColumnInfo, fkValues [][]types.Datum) ast.ExprNode { + if len(cols) > 1 { + return genWhereConditionAstForMultiColumn(cols, fkValues) + } + valueList := make([]ast.ExprNode, 0, len(fkValues)) + for _, fkVals := range fkValues { + v := &driver.ValueExpr{Datum: fkVals[0]} + v.Type = cols[0].FieldType + row := &ast.ParenthesesExpr{Expr: v} + valueList = append(valueList, row) + } + return &ast.PatternInExpr{ + Expr: &ast.ParenthesesExpr{Expr: &ast.ColumnNameExpr{Name: &ast.ColumnName{Name: cols[0].Name}}}, + List: valueList, + } +} + +func genWhereConditionAstForMultiColumn(cols []*model.ColumnInfo, fkValues [][]types.Datum) ast.ExprNode { + var colValues []ast.ExprNode + for i := range cols { + col := &ast.ColumnNameExpr{Name: &ast.ColumnName{Name: cols[i].Name}} + colValues = append(colValues, col) + } + valueList := make([]ast.ExprNode, 0, len(fkValues)) + for _, fkVals := range fkValues { + values := make([]ast.ExprNode, len(fkVals)) + for i, v := range fkVals { + values[i] = &driver.ValueExpr{ + TexprNode: ast.TexprNode{ + Type: cols[i].FieldType, + }, + Datum: v, } - buf.WriteString(val) } - buf.WriteString(")") + row := &ast.RowExpr{Values: values} + valueList = append(valueList, row) } - buf.WriteString(")") - return nil -} - -func genFKValueString(v types.Datum) (string, error) { - switch v.Kind() { - case types.KindNull: - return "NULL", nil - case types.KindMysqlBit: - return v.GetBinaryLiteral().ToBitLiteralString(true), nil - } - val, err := v.ToString() - if err != nil { - return "", err - } - switch v.Kind() { - case types.KindInt64, types.KindUint64, types.KindFloat32, types.KindFloat64, types.KindMysqlDecimal: - return val, nil - default: - return "'" + val + "'", nil + return &ast.PatternInExpr{ + Expr: &ast.RowExpr{Values: colValues}, + List: valueList, } } diff --git a/planner/core/foreign_key.go b/planner/core/foreign_key.go index 8c5b03384ae6e..e428d74e023b6 100644 --- a/planner/core/foreign_key.go +++ b/planner/core/foreign_key.go @@ -17,6 +17,7 @@ package core import ( "unsafe" + "github.com/pingcap/errors" "github.com/pingcap/tidb/infoschema" "github.com/pingcap/tidb/parser/model" "github.com/pingcap/tidb/parser/mysql" @@ -45,6 +46,8 @@ type FKCascade struct { ReferredFK *model.ReferredFKInfo ChildTable table.Table FK *model.FKInfo + FKCols []*model.ColumnInfo + FKIdx *model.IndexInfo } // FKCascadeType indicates in which (delete/update) statements. @@ -294,13 +297,8 @@ func buildOnDeleteOrUpdateFKTrigger(is infoschema.InfoSchema, referredFK *model. } switch fkReferOption { case model.ReferOptionCascade, model.ReferOptionSetNull: - fkCascade := &FKCascade{ - Tp: tp, - ReferredFK: referredFK, - ChildTable: childTable, - FK: fk, - } - return nil, fkCascade, nil + fkCascade, err := buildFKCascade(tp, referredFK, childTable, fk) + return nil, fkCascade, err default: fkCheck, err := buildFKCheckForReferredFK(childTable, fk, referredFK) return fkCheck, nil, err @@ -390,3 +388,34 @@ func buildFKCheck(tbl table.Table, cols []model.CIStr, failedErr error) (*FKChec FailedErr: failedErr, }, nil } + +func buildFKCascade(tp FKCascadeType, referredFK *model.ReferredFKInfo, childTable table.Table, fk *model.FKInfo) (*FKCascade, error) { + cols := make([]*model.ColumnInfo, len(fk.Cols)) + childTableColumns := childTable.Meta().Columns + for i, c := range fk.Cols { + col := model.FindColumnInfo(childTableColumns, c.L) + if col == nil { + return nil, errors.Errorf("foreign key column %s is not found in table %s", c.L, childTable.Meta().Name) + } + cols[i] = col + } + fkCascade := &FKCascade{ + Tp: tp, + ReferredFK: referredFK, + ChildTable: childTable, + FK: fk, + FKCols: cols, + } + if childTable.Meta().PKIsHandle && len(cols) == 1 { + refColInfo := model.FindColumnInfo(childTableColumns, cols[0].Name.L) + if refColInfo != nil && mysql.HasPriKeyFlag(refColInfo.GetFlag()) { + return fkCascade, nil + } + } + indexForFK := model.FindIndexByColumns(childTable.Meta(), fk.Cols...) + if indexForFK == nil { + return nil, errors.Errorf("Missing index for '%s' foreign key columns in the table '%s'", fk.Name, childTable.Meta().Name) + } + fkCascade.FKIdx = indexForFK + return fkCascade, nil +} From 5737e9e31ae1cda1ac98cdd4da6d637c73303340 Mon Sep 17 00:00:00 2001 From: crazycs520 Date: Wed, 2 Nov 2022 10:31:46 +0800 Subject: [PATCH 2/7] fix ci lint Signed-off-by: crazycs520 --- executor/foreign_key.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/executor/foreign_key.go b/executor/foreign_key.go index e8b7994e38d52..67a27c32fc6bf 100644 --- a/executor/foreign_key.go +++ b/executor/foreign_key.go @@ -789,10 +789,10 @@ func genWhereConditionAst(cols []*model.ColumnInfo, fkValues [][]types.Datum) as } func genWhereConditionAstForMultiColumn(cols []*model.ColumnInfo, fkValues [][]types.Datum) ast.ExprNode { - var colValues []ast.ExprNode + colValues := make([]ast.ExprNode, len(cols)) for i := range cols { col := &ast.ColumnNameExpr{Name: &ast.ColumnName{Name: cols[i].Name}} - colValues = append(colValues, col) + colValues[i] = col } valueList := make([]ast.ExprNode, 0, len(fkValues)) for _, fkVals := range fkValues { From f95a2b99d3fe0f8d353401654bdd70a68ea8012f Mon Sep 17 00:00:00 2001 From: crazycs520 Date: Thu, 3 Nov 2022 14:15:23 +0800 Subject: [PATCH 3/7] address comment Signed-off-by: crazycs520 --- executor/foreign_key.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/executor/foreign_key.go b/executor/foreign_key.go index 67a27c32fc6bf..4d76d493a7aab 100644 --- a/executor/foreign_key.go +++ b/executor/foreign_key.go @@ -676,8 +676,7 @@ func (fkc *FKCascadeExec) buildFKCascadePlan(ctx context.Context) (plannercore.P return nil, errors.Errorf("generate foreign key cascade ast failed, %v", fkc.tp) } sctx := fkc.b.ctx - ret := &plannercore.PreprocessorReturn{} - err := plannercore.Preprocess(ctx, sctx, stmtNode, plannercore.WithPreprocessorReturn(ret), plannercore.InitTxnContextProvider) + err := plannercore.Preprocess(ctx, sctx, stmtNode) if err != nil { return nil, err } From bd435a522f044d4cfdd0358cb5cdbcb166df0635 Mon Sep 17 00:00:00 2001 From: crazycs520 Date: Thu, 3 Nov 2022 14:37:23 +0800 Subject: [PATCH 4/7] add comment Signed-off-by: crazycs520 --- executor/adapter.go | 10 ++++++++++ executor/foreign_key.go | 2 ++ 2 files changed, 12 insertions(+) diff --git a/executor/adapter.go b/executor/adapter.go index 45ab87e2cd409..abd40408e0df1 100644 --- a/executor/adapter.go +++ b/executor/adapter.go @@ -611,6 +611,16 @@ func (a *ExecStmt) handleForeignKeyTrigger(ctx context.Context, e Executor, dept return nil } +// handleForeignKeyCascade uses to execute foreign key cascade behaviour, the progress is: +// 1. Build delete/update executor for foreign key on delete/update behaviour. +// a. Construct delete/update AST. We used to try generated SQL string first and then parse the SQL to get AST, +// but we need convert Datum to string, there may be some risks here, so we chose to construct AST directly. +// b. Build plan by the delete/update AST. +// c. Build executor by the delete/update plan. +// 2. Execute the delete/update executor. +// 3. Close the executor. +// 4. `StmtCommit` to commit the kv change to transaction mem-buffer. +// 5. If the foreign key cascade behaviour has more fk value need to be cascaded, go to step 1. func (a *ExecStmt) handleForeignKeyCascade(ctx context.Context, fkc *FKCascadeExec, depth int) error { if len(fkc.fkValues) == 0 && len(fkc.fkUpdatedValuesMap) == 0 { return nil diff --git a/executor/foreign_key.go b/executor/foreign_key.go index 4d76d493a7aab..80a4175c8ef25 100644 --- a/executor/foreign_key.go +++ b/executor/foreign_key.go @@ -640,6 +640,8 @@ func (fkc *FKCascadeExec) buildExecutor(ctx context.Context) (Executor, error) { return e, fkc.b.err } +// maxHandleFKValueInOneCascade uses to limit the max handle fk value in one cascade executor, +// this is to avoid performance issue, see: https://github.com/pingcap/tidb/issues/38631 var maxHandleFKValueInOneCascade = 1024 func (fkc *FKCascadeExec) buildFKCascadePlan(ctx context.Context) (plannercore.Plan, error) { From 0dd154da40ebb36248026db15cf6fcbc81c0027d Mon Sep 17 00:00:00 2001 From: crazycs520 Date: Thu, 3 Nov 2022 21:41:09 +0800 Subject: [PATCH 5/7] address comment for eliminate ParenthesesExpr when threr are only 1 fk column Signed-off-by: crazycs520 --- executor/fktest/foreign_key_test.go | 7 +++++++ executor/foreign_key.go | 14 +++++--------- 2 files changed, 12 insertions(+), 9 deletions(-) diff --git a/executor/fktest/foreign_key_test.go b/executor/fktest/foreign_key_test.go index 4c54b8ffc3cfb..c704494ffb7f5 100644 --- a/executor/fktest/foreign_key_test.go +++ b/executor/fktest/foreign_key_test.go @@ -1305,6 +1305,13 @@ func TestForeignKeyGenerateCascadeAST(t *testing.T) { checkStmtFn(stmt, "update test.t2 set a = 10, name = 'aa' where (a,name) in ((1,'a'), (2,'b'))") stmt = executor.GenCascadeUpdateAST(model.NewCIStr("test"), model.NewCIStr("t2"), model.NewCIStr("idx"), cols, couple) checkStmtFn(stmt, "update test.t2 use index(idx) set a = 10, name = 'aa' where (a,name) in ((1,'a'), (2,'b'))") + // Test for 1 fk column. + fkValues = [][]types.Datum{{types.NewDatum(1)}, {types.NewDatum(2)}} + cols = []*model.ColumnInfo{{ID: 1, Name: model.NewCIStr("a"), FieldType: *types.NewFieldType(mysql.TypeLonglong)}} + stmt = executor.GenCascadeDeleteAST(model.NewCIStr("test"), model.NewCIStr("t2"), model.NewCIStr(""), cols, fkValues) + checkStmtFn(stmt, "delete from test.t2 where a in (1,2)") + stmt = executor.GenCascadeDeleteAST(model.NewCIStr("test"), model.NewCIStr("t2"), model.NewCIStr("idx"), cols, fkValues) + checkStmtFn(stmt, "delete from test.t2 use index(idx) where a in (1,2)") } func TestForeignKeyOnDeleteSetNull(t *testing.T) { diff --git a/executor/foreign_key.go b/executor/foreign_key.go index 8b26763fd5759..62dbb400537e2 100644 --- a/executor/foreign_key.go +++ b/executor/foreign_key.go @@ -781,11 +781,10 @@ func genWhereConditionAst(cols []*model.ColumnInfo, fkValues [][]types.Datum) as for _, fkVals := range fkValues { v := &driver.ValueExpr{Datum: fkVals[0]} v.Type = cols[0].FieldType - row := &ast.ParenthesesExpr{Expr: v} - valueList = append(valueList, row) + valueList = append(valueList, v) } return &ast.PatternInExpr{ - Expr: &ast.ParenthesesExpr{Expr: &ast.ColumnNameExpr{Name: &ast.ColumnName{Name: cols[0].Name}}}, + Expr: &ast.ColumnNameExpr{Name: &ast.ColumnName{Name: cols[0].Name}}, List: valueList, } } @@ -800,12 +799,9 @@ func genWhereConditionAstForMultiColumn(cols []*model.ColumnInfo, fkValues [][]t for _, fkVals := range fkValues { values := make([]ast.ExprNode, len(fkVals)) for i, v := range fkVals { - values[i] = &driver.ValueExpr{ - TexprNode: ast.TexprNode{ - Type: cols[i].FieldType, - }, - Datum: v, - } + val := &driver.ValueExpr{Datum: v} + val.Type = cols[i].FieldType + values[i] = val } row := &ast.RowExpr{Values: values} valueList = append(valueList, row) From afc6881e41ea733f983117f9ef6d2aff989914b9 Mon Sep 17 00:00:00 2001 From: crazycs520 Date: Thu, 3 Nov 2022 21:44:12 +0800 Subject: [PATCH 6/7] refine comment Signed-off-by: crazycs520 --- executor/adapter.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/executor/adapter.go b/executor/adapter.go index abd40408e0df1..c4007a09213e4 100644 --- a/executor/adapter.go +++ b/executor/adapter.go @@ -614,7 +614,8 @@ func (a *ExecStmt) handleForeignKeyTrigger(ctx context.Context, e Executor, dept // handleForeignKeyCascade uses to execute foreign key cascade behaviour, the progress is: // 1. Build delete/update executor for foreign key on delete/update behaviour. // a. Construct delete/update AST. We used to try generated SQL string first and then parse the SQL to get AST, -// but we need convert Datum to string, there may be some risks here, so we chose to construct AST directly. +// but we need convert Datum to string, there may be some risks here, since assert_eq(datum_a, parse(datum_a.toString())) may be broken. +// so we chose to construct AST directly. // b. Build plan by the delete/update AST. // c. Build executor by the delete/update plan. // 2. Execute the delete/update executor. From c45b645e0be147310f4b15b5c3d2f9be5bfbbe15 Mon Sep 17 00:00:00 2001 From: crazycs520 Date: Fri, 4 Nov 2022 10:47:54 +0800 Subject: [PATCH 7/7] make bazel_prepare Signed-off-by: crazycs520 --- executor/BUILD.bazel | 1 + executor/fktest/BUILD.bazel | 4 ++++ 2 files changed, 5 insertions(+) diff --git a/executor/BUILD.bazel b/executor/BUILD.bazel index c7957510b1297..787a7ed8429f2 100644 --- a/executor/BUILD.bazel +++ b/executor/BUILD.bazel @@ -150,6 +150,7 @@ go_library( "//telemetry", "//tidb-binlog/node", "//types", + "//types/parser_driver", "//util", "//util/admin", "//util/bitmap", diff --git a/executor/fktest/BUILD.bazel b/executor/fktest/BUILD.bazel index 40237466542e5..dbdae1843edaf 100644 --- a/executor/fktest/BUILD.bazel +++ b/executor/fktest/BUILD.bazel @@ -13,10 +13,14 @@ go_test( "//executor", "//kv", "//meta/autoid", + "//parser/ast", + "//parser/format", "//parser/model", + "//parser/mysql", "//planner/core", "//testkit", "//types", + "//util/sqlexec", "@com_github_stretchr_testify//require", "@com_github_tikv_client_go_v2//tikv", "@org_uber_go_goleak//:goleak",