From 7e332236130fc023b81125a059e3b5306b8d82bf Mon Sep 17 00:00:00 2001 From: Harshit Gangal Date: Mon, 18 Mar 2024 11:24:33 +0530 Subject: [PATCH 1/4] test: added e2e fk test for multi target delete Signed-off-by: Harshit Gangal --- go/test/endtoend/vtgate/foreignkey/fk_test.go | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/go/test/endtoend/vtgate/foreignkey/fk_test.go b/go/test/endtoend/vtgate/foreignkey/fk_test.go index a83059454ad..a1619970b5c 100644 --- a/go/test/endtoend/vtgate/foreignkey/fk_test.go +++ b/go/test/endtoend/vtgate/foreignkey/fk_test.go @@ -704,6 +704,22 @@ func TestFkScenarios(t *testing.T) { "select * from fk_multicol_t17 order by id", "select * from fk_multicol_t19 order by id", }, + }, { + name: "Multi Target Delete success", + dataQueries: []string{ + "insert into fk_multicol_t15(id, cola, colb) values (1, 7, 1), (2, 9, 1), (3, 11, 1), (4, 13, 1)", + "insert into fk_multicol_t16(id, cola, colb) values (1, 7, 1), (2, 9, 1), (3, 11, 1), (4, 13, 1)", + "insert into fk_multicol_t17(id, cola, colb) values (1, 7, 1), (2, 9, 1), (3, 11, 1)", + "insert into fk_multicol_t18(id, cola, colb) values (1, 7, 1), (3, 11, 1)", + "insert into fk_multicol_t19(id, cola, colb) values (1, 7, 1), (2, 9, 1)", + }, + dmlQuery: "delete fk_multicol_t15, fk_multicol_t17 from fk_multicol_t15 join fk_multicol_t17 where fk_multicol_t15.id = fk_multicol_t17.id", + assertionQueries: []string{ + "select * from fk_multicol_t15 order by id", + "select * from fk_multicol_t16 order by id", + "select * from fk_multicol_t17 order by id", + "select * from fk_multicol_t19 order by id", + }, }, { name: "Delete with limit success", dataQueries: []string{ From ebcd8ca39864c15b16e578188fdf23fcb687869c Mon Sep 17 00:00:00 2001 From: Harshit Gangal Date: Mon, 18 Mar 2024 13:51:54 +0530 Subject: [PATCH 2/4] allow multi target delete with foreign key Signed-off-by: Harshit Gangal --- go/vt/vtgate/planbuilder/operators/delete.go | 6 +- .../testdata/foreignkey_cases.json | 125 +++++++++++++++++- 2 files changed, 124 insertions(+), 7 deletions(-) diff --git a/go/vt/vtgate/planbuilder/operators/delete.go b/go/vt/vtgate/planbuilder/operators/delete.go index bac61c51126..d2c0bc25e6a 100644 --- a/go/vt/vtgate/planbuilder/operators/delete.go +++ b/go/vt/vtgate/planbuilder/operators/delete.go @@ -94,9 +94,6 @@ func createOperatorFromDelete(ctx *plancontext.PlanningContext, deleteStmt *sqlp func deleteWithInputPlanningRequired(childFks []vindexes.ChildFKInfo, deleteStmt *sqlparser.Delete) bool { if len(deleteStmt.Targets) > 1 { - if len(childFks) > 0 { - panic(vterrors.VT12001("multi table delete with foreign keys")) - } return true } // If there are no foreign keys, we don't need to use delete with input. @@ -354,6 +351,7 @@ func updateQueryGraphWithSource(ctx *plancontext.PlanningContext, input Operator func createFkCascadeOpForDelete(ctx *plancontext.PlanningContext, parentOp Operator, delStmt *sqlparser.Delete, childFks []vindexes.ChildFKInfo, deletedTbl *vindexes.Table) Operator { var fkChildren []*FkChild var selectExprs []sqlparser.SelectExpr + tblName := delStmt.Targets[0] for _, fk := range childFks { // Any RESTRICT type foreign keys that arrive here, // are cross-shard/cross-keyspace RESTRICT cases, which we don't currently support. @@ -363,7 +361,7 @@ func createFkCascadeOpForDelete(ctx *plancontext.PlanningContext, parentOp Opera // We need to select all the parent columns for the foreign key constraint, to use in the update of the child table. var offsets []int - offsets, selectExprs = addColumns(ctx, fk.ParentColumns, selectExprs, deletedTbl.GetTableName()) + offsets, selectExprs = addColumns(ctx, fk.ParentColumns, selectExprs, tblName) fkChildren = append(fkChildren, createFkChildForDelete(ctx, fk, offsets)) diff --git a/go/vt/vtgate/planbuilder/testdata/foreignkey_cases.json b/go/vt/vtgate/planbuilder/testdata/foreignkey_cases.json index 68f49f41e64..9a8c9f6ef77 100644 --- a/go/vt/vtgate/planbuilder/testdata/foreignkey_cases.json +++ b/go/vt/vtgate/planbuilder/testdata/foreignkey_cases.json @@ -3151,8 +3151,8 @@ "Name": "unsharded_fk_allow", "Sharded": false }, - "FieldQuery": "select u_tbl6.col6 from u_tbl6 as u where 1 != 1", - "Query": "select u_tbl6.col6 from u_tbl6 as u where u.id in ::dml_vals for update", + "FieldQuery": "select u.col6 from u_tbl6 as u where 1 != 1", + "Query": "select u.col6 from u_tbl6 as u where u.id in ::dml_vals for update", "Table": "u_tbl6" }, { @@ -3805,7 +3805,126 @@ { "comment": "multi table delete on foreign key enabled tables", "query": "delete u, m from u_tbl6 u join u_tbl5 m on u.col = m.col where u.col2 = 4 and m.col3 = 6", - "plan": "VT12001: unsupported: multi table delete with foreign keys" + "plan": { + "QueryType": "DELETE", + "Original": "delete u, m from u_tbl6 u join u_tbl5 m on u.col = m.col where u.col2 = 4 and m.col3 = 6", + "Instructions": { + "OperatorType": "DMLWithInput", + "TargetTabletType": "PRIMARY", + "Offset": [ + "0:[0]", + "1:[1]" + ], + "Inputs": [ + { + "OperatorType": "Route", + "Variant": "Unsharded", + "Keyspace": { + "Name": "unsharded_fk_allow", + "Sharded": false + }, + "FieldQuery": "select u.id, m.id from u_tbl6 as u, u_tbl5 as m where 1 != 1", + "Query": "select u.id, m.id from u_tbl6 as u, u_tbl5 as m where u.col2 = 4 and m.col3 = 6 and u.col = m.col for update", + "Table": "u_tbl5, u_tbl6" + }, + { + "OperatorType": "FkCascade", + "Inputs": [ + { + "InputName": "Selection", + "OperatorType": "Route", + "Variant": "Unsharded", + "Keyspace": { + "Name": "unsharded_fk_allow", + "Sharded": false + }, + "FieldQuery": "select u.col6 from u_tbl6 as u where 1 != 1", + "Query": "select u.col6 from u_tbl6 as u where u.id in ::dml_vals for update", + "Table": "u_tbl6" + }, + { + "InputName": "CascadeChild-1", + "OperatorType": "Delete", + "Variant": "Unsharded", + "Keyspace": { + "Name": "unsharded_fk_allow", + "Sharded": false + }, + "TargetTabletType": "PRIMARY", + "BvName": "fkc_vals", + "Cols": [ + 0 + ], + "Query": "delete from u_tbl8 where (col8) in ::fkc_vals", + "Table": "u_tbl8" + }, + { + "InputName": "Parent", + "OperatorType": "Delete", + "Variant": "Unsharded", + "Keyspace": { + "Name": "unsharded_fk_allow", + "Sharded": false + }, + "TargetTabletType": "PRIMARY", + "Query": "delete from u_tbl6 as u where u.id in ::dml_vals", + "Table": "u_tbl6" + } + ] + }, + { + "OperatorType": "FkCascade", + "Inputs": [ + { + "InputName": "Selection", + "OperatorType": "Route", + "Variant": "Unsharded", + "Keyspace": { + "Name": "unsharded_fk_allow", + "Sharded": false + }, + "FieldQuery": "select m.col6 from u_tbl5 as m where 1 != 1", + "Query": "select m.col6 from u_tbl5 as m where m.id in ::dml_vals for update", + "Table": "u_tbl5" + }, + { + "InputName": "CascadeChild-1", + "OperatorType": "Delete", + "Variant": "Unsharded", + "Keyspace": { + "Name": "unsharded_fk_allow", + "Sharded": false + }, + "TargetTabletType": "PRIMARY", + "BvName": "fkc_vals1", + "Cols": [ + 0 + ], + "Query": "delete from u_tbl8 where (col8) in ::fkc_vals1", + "Table": "u_tbl8" + }, + { + "InputName": "Parent", + "OperatorType": "Delete", + "Variant": "Unsharded", + "Keyspace": { + "Name": "unsharded_fk_allow", + "Sharded": false + }, + "TargetTabletType": "PRIMARY", + "Query": "delete from u_tbl5 as m where m.id in ::dml_vals", + "Table": "u_tbl5" + } + ] + } + ] + }, + "TablesUsed": [ + "unsharded_fk_allow.u_tbl5", + "unsharded_fk_allow.u_tbl6", + "unsharded_fk_allow.u_tbl8" + ] + } }, { "comment": "update with limit with foreign keys", From 03aa52e89b4eb95c5a907dbaff849c9057620b5b Mon Sep 17 00:00:00 2001 From: Harshit Gangal Date: Mon, 18 Mar 2024 14:50:21 +0530 Subject: [PATCH 3/4] fk planning to get only target child foreign keys Signed-off-by: Harshit Gangal --- .../vtgate/foreignkey/fk_fuzz_test.go | 6 ++- go/vt/vtgate/planbuilder/operators/delete.go | 5 ++ .../testdata/foreignkey_cases.json | 52 ++++--------------- go/vt/vtgate/semantics/semantic_state.go | 10 ++-- 4 files changed, 26 insertions(+), 47 deletions(-) diff --git a/go/test/endtoend/vtgate/foreignkey/fk_fuzz_test.go b/go/test/endtoend/vtgate/foreignkey/fk_fuzz_test.go index d430c31e35a..379af1f1631 100644 --- a/go/test/endtoend/vtgate/foreignkey/fk_fuzz_test.go +++ b/go/test/endtoend/vtgate/foreignkey/fk_fuzz_test.go @@ -214,7 +214,11 @@ func (fz *fuzzer) generateMultiDeleteDMLQuery() string { tableId2 := rand.Intn(len(fkTables)) idValue := 1 + rand.Intn(fz.maxValForId) setVarFkChecksVal := fz.getSetVarFkChecksVal() - query := fmt.Sprintf("delete %v%v from %v join %v using (id) where %v.id = %v", setVarFkChecksVal, fkTables[tableId], fkTables[tableId], fkTables[tableId2], fkTables[tableId], idValue) + target := fkTables[tableId] + if rand.Intn(2)%2 == 0 { + target += ", " + fkTables[tableId2] + } + query := fmt.Sprintf("delete %v%v from %v join %v using (id) where %v.id = %v", setVarFkChecksVal, target, fkTables[tableId], fkTables[tableId2], fkTables[tableId], idValue) return query } diff --git a/go/vt/vtgate/planbuilder/operators/delete.go b/go/vt/vtgate/planbuilder/operators/delete.go index d2c0bc25e6a..453a503ced1 100644 --- a/go/vt/vtgate/planbuilder/operators/delete.go +++ b/go/vt/vtgate/planbuilder/operators/delete.go @@ -84,6 +84,11 @@ func createOperatorFromDelete(ctx *plancontext.PlanningContext, deleteStmt *sqlp } } + var err error + childFks, err = ctx.SemTable.GetChildForeignKeysForTable(deleteStmt.Targets[0]) + if err != nil { + panic(err) + } // If there are no foreign key constraints, then we don't need to do anything special. if len(childFks) == 0 { return op diff --git a/go/vt/vtgate/planbuilder/testdata/foreignkey_cases.json b/go/vt/vtgate/planbuilder/testdata/foreignkey_cases.json index 9a8c9f6ef77..ae82f075d08 100644 --- a/go/vt/vtgate/planbuilder/testdata/foreignkey_cases.json +++ b/go/vt/vtgate/planbuilder/testdata/foreignkey_cases.json @@ -3873,49 +3873,15 @@ ] }, { - "OperatorType": "FkCascade", - "Inputs": [ - { - "InputName": "Selection", - "OperatorType": "Route", - "Variant": "Unsharded", - "Keyspace": { - "Name": "unsharded_fk_allow", - "Sharded": false - }, - "FieldQuery": "select m.col6 from u_tbl5 as m where 1 != 1", - "Query": "select m.col6 from u_tbl5 as m where m.id in ::dml_vals for update", - "Table": "u_tbl5" - }, - { - "InputName": "CascadeChild-1", - "OperatorType": "Delete", - "Variant": "Unsharded", - "Keyspace": { - "Name": "unsharded_fk_allow", - "Sharded": false - }, - "TargetTabletType": "PRIMARY", - "BvName": "fkc_vals1", - "Cols": [ - 0 - ], - "Query": "delete from u_tbl8 where (col8) in ::fkc_vals1", - "Table": "u_tbl8" - }, - { - "InputName": "Parent", - "OperatorType": "Delete", - "Variant": "Unsharded", - "Keyspace": { - "Name": "unsharded_fk_allow", - "Sharded": false - }, - "TargetTabletType": "PRIMARY", - "Query": "delete from u_tbl5 as m where m.id in ::dml_vals", - "Table": "u_tbl5" - } - ] + "OperatorType": "Delete", + "Variant": "Unsharded", + "Keyspace": { + "Name": "unsharded_fk_allow", + "Sharded": false + }, + "TargetTabletType": "PRIMARY", + "Query": "delete from u_tbl5 as m where m.id in ::dml_vals", + "Table": "u_tbl5" } ] }, diff --git a/go/vt/vtgate/semantics/semantic_state.go b/go/vt/vtgate/semantics/semantic_state.go index a0bf0624044..9ba1fa43de1 100644 --- a/go/vt/vtgate/semantics/semantic_state.go +++ b/go/vt/vtgate/semantics/semantic_state.go @@ -198,9 +198,13 @@ func (st *SemTable) GetChildForeignKeysForTargets() (fks []vindexes.ChildFKInfo) return fks } -// GetChildForeignKeysForTableSet gets the child foreign keys as a list for the specified TableSet. -func (st *SemTable) GetChildForeignKeysForTableSet(ts TableSet) []vindexes.ChildFKInfo { - return st.childForeignKeysInvolved[ts] +// GetChildForeignKeysForTable gets the child foreign keys as a list for the specified TableName. +func (st *SemTable) GetChildForeignKeysForTable(tbl sqlparser.TableName) ([]vindexes.ChildFKInfo, error) { + ts, err := st.GetTargetTableSetForTableName(tbl) + if err != nil { + return nil, err + } + return st.childForeignKeysInvolved[ts], nil } // GetChildForeignKeysList gets the child foreign keys as a list. From 15f57ba7d2ff909bd8296cc938f9d05efe430247 Mon Sep 17 00:00:00 2001 From: Harshit Gangal Date: Tue, 19 Mar 2024 20:37:25 +0530 Subject: [PATCH 4/4] test: update rand function Signed-off-by: Harshit Gangal --- go/test/endtoend/vtgate/foreignkey/fk_fuzz_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go/test/endtoend/vtgate/foreignkey/fk_fuzz_test.go b/go/test/endtoend/vtgate/foreignkey/fk_fuzz_test.go index 898a4f178d7..df0b427af9e 100644 --- a/go/test/endtoend/vtgate/foreignkey/fk_fuzz_test.go +++ b/go/test/endtoend/vtgate/foreignkey/fk_fuzz_test.go @@ -215,7 +215,7 @@ func (fz *fuzzer) generateMultiDeleteDMLQuery() string { idValue := 1 + rand.IntN(fz.maxValForId) setVarFkChecksVal := fz.getSetVarFkChecksVal() target := fkTables[tableId] - if rand.Intn(2)%2 == 0 { + if rand.IntN(2)%2 == 0 { target += ", " + fkTables[tableId2] } query := fmt.Sprintf("delete %v%v from %v join %v using (id) where %v.id = %v", setVarFkChecksVal, target, fkTables[tableId], fkTables[tableId2], fkTables[tableId], idValue)