From 986f00c2c6c129ac2cdabec6da6b3a7d4a37760b Mon Sep 17 00:00:00 2001 From: Harshit Gangal Date: Wed, 20 Mar 2024 14:23:56 +0530 Subject: [PATCH] Foreign Key: Add support for multi target delete (#15504) Signed-off-by: Harshit Gangal --- .../vtgate/foreignkey/fk_fuzz_test.go | 6 +- go/test/endtoend/vtgate/foreignkey/fk_test.go | 16 ++++ go/vt/vtgate/planbuilder/operators/delete.go | 11 ++- .../testdata/foreignkey_cases.json | 91 ++++++++++++++++++- go/vt/vtgate/semantics/semantic_state.go | 10 +- 5 files changed, 123 insertions(+), 11 deletions(-) diff --git a/go/test/endtoend/vtgate/foreignkey/fk_fuzz_test.go b/go/test/endtoend/vtgate/foreignkey/fk_fuzz_test.go index 2f1a703b54a..df0b427af9e 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/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{ diff --git a/go/vt/vtgate/planbuilder/operators/delete.go b/go/vt/vtgate/planbuilder/operators/delete.go index bac61c51126..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 @@ -94,9 +99,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 +356,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 +366,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..ae82f075d08 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,92 @@ { "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": "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", 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.