From 19af51c23889d769bf47b9f12ba3bc4cc0532d63 Mon Sep 17 00:00:00 2001 From: Manan Gupta <35839558+GuptaManan100@users.noreply.github.com> Date: Thu, 1 Feb 2024 12:56:30 +0530 Subject: [PATCH] Add support for delete planning with limits in presence of foreign keys (#15097) Signed-off-by: Manan Gupta Signed-off-by: Harshit Gangal Co-authored-by: Harshit Gangal --- .../vtgate/foreignkey/fk_fuzz_test.go | 122 ++++++------ go/test/endtoend/vtgate/foreignkey/fk_test.go | 30 +++ go/vt/vtgate/engine/delete_with_input.go | 3 + go/vt/vtgate/planbuilder/operators/delete.go | 60 +++++- .../testdata/foreignkey_cases.json | 188 +++++++++++++++++- .../testdata/foreignkey_checks_on_cases.json | 76 ++++++- 6 files changed, 411 insertions(+), 68 deletions(-) diff --git a/go/test/endtoend/vtgate/foreignkey/fk_fuzz_test.go b/go/test/endtoend/vtgate/foreignkey/fk_fuzz_test.go index d738d699811..8ff1d660537 100644 --- a/go/test/endtoend/vtgate/foreignkey/fk_fuzz_test.go +++ b/go/test/endtoend/vtgate/foreignkey/fk_fuzz_test.go @@ -186,8 +186,12 @@ func (fz *fuzzer) generateSingleDeleteDMLQuery() string { tableId := rand.Intn(len(fkTables)) idValue := 1 + rand.Intn(fz.maxValForId) setVarFkChecksVal := fz.getSetVarFkChecksVal() - query := fmt.Sprintf("delete %vfrom %v where id = %v", setVarFkChecksVal, fkTables[tableId], idValue) - return query + delWithLimit := rand.Intn(2) + if delWithLimit == 0 { + return fmt.Sprintf("delete %vfrom %v where id = %v", setVarFkChecksVal, fkTables[tableId], idValue) + } + limitCount := rand.Intn(3) + return fmt.Sprintf("delete %vfrom %v order by id limit %v", setVarFkChecksVal, fkTables[tableId], limitCount) } // generateMultiDeleteDMLQuery generates a DELETE query using 2 tables from the parameters for the fuzzer. @@ -604,61 +608,65 @@ func TestFkFuzzTest(t *testing.T) { insertShare int deleteShare int updateShare int - }{{ - name: "Single Thread - Only Inserts", - concurrency: 1, - timeForTesting: 5 * time.Second, - maxValForCol: 5, - maxValForId: 10, - insertShare: 100, - deleteShare: 0, - updateShare: 0, - }, { - name: "Single Thread - Balanced Inserts and Deletes", - concurrency: 1, - timeForTesting: 5 * time.Second, - maxValForCol: 5, - maxValForId: 10, - insertShare: 50, - deleteShare: 50, - updateShare: 0, - }, { - name: "Single Thread - Balanced Inserts and Updates", - concurrency: 1, - timeForTesting: 5 * time.Second, - maxValForCol: 5, - maxValForId: 10, - insertShare: 50, - deleteShare: 0, - updateShare: 50, - }, { - name: "Single Thread - Balanced Inserts, Updates and Deletes", - concurrency: 1, - timeForTesting: 5 * time.Second, - maxValForCol: 5, - maxValForId: 10, - insertShare: 50, - deleteShare: 50, - updateShare: 50, - }, { - name: "Multi Thread - Only Inserts", - concurrency: 30, - timeForTesting: 5 * time.Second, - maxValForCol: 5, - maxValForId: 30, - insertShare: 100, - deleteShare: 0, - updateShare: 0, - }, { - name: "Multi Thread - Balanced Inserts, Updates and Deletes", - concurrency: 30, - timeForTesting: 5 * time.Second, - maxValForCol: 5, - maxValForId: 30, - insertShare: 50, - deleteShare: 50, - updateShare: 50, - }} + }{ + { + name: "Single Thread - Only Inserts", + concurrency: 1, + timeForTesting: 5 * time.Second, + maxValForCol: 5, + maxValForId: 10, + insertShare: 100, + deleteShare: 0, + updateShare: 0, + }, { + name: "Single Thread - Balanced Inserts and Deletes", + concurrency: 1, + timeForTesting: 5 * time.Second, + maxValForCol: 5, + maxValForId: 10, + insertShare: 50, + deleteShare: 50, + updateShare: 0, + }, { + name: "Single Thread - Balanced Inserts and Updates", + concurrency: 1, + timeForTesting: 5 * time.Second, + maxValForCol: 5, + maxValForId: 10, + insertShare: 50, + deleteShare: 0, + updateShare: 50, + }, + { + name: "Single Thread - Balanced Inserts, Updates and Deletes", + concurrency: 1, + timeForTesting: 5 * time.Second, + maxValForCol: 5, + maxValForId: 10, + insertShare: 50, + deleteShare: 50, + updateShare: 50, + }, + { + name: "Multi Thread - Only Inserts", + concurrency: 30, + timeForTesting: 5 * time.Second, + maxValForCol: 5, + maxValForId: 30, + insertShare: 100, + deleteShare: 0, + updateShare: 0, + }, { + name: "Multi Thread - Balanced Inserts, Updates and Deletes", + concurrency: 30, + timeForTesting: 5 * time.Second, + maxValForCol: 5, + maxValForId: 30, + insertShare: 50, + deleteShare: 50, + updateShare: 50, + }, + } valTrue := true valFalse := false diff --git a/go/test/endtoend/vtgate/foreignkey/fk_test.go b/go/test/endtoend/vtgate/foreignkey/fk_test.go index 97b1e7202e0..ef8a67c071f 100644 --- a/go/test/endtoend/vtgate/foreignkey/fk_test.go +++ b/go/test/endtoend/vtgate/foreignkey/fk_test.go @@ -687,6 +687,36 @@ func TestFkScenarios(t *testing.T) { "select * from fk_t17 order by id", "select * from fk_t19 order by id", }, + }, { + name: "Delete with limit success", + dataQueries: []string{ + "insert into fk_t15(id, col) values (1, 7), (2, 9)", + "insert into fk_t16(id, col) values (1, 7), (2, 9)", + "insert into fk_t17(id, col) values (1, 7)", + "insert into fk_t19(id, col) values (1, 7)", + }, + dmlQuery: "delete from fk_t15 order by id limit 1", + assertionQueries: []string{ + "select * from fk_t15 order by id", + "select * from fk_t16 order by id", + "select * from fk_t17 order by id", + "select * from fk_t19 order by id", + }, + }, { + name: "Delete with limit 0 success", + dataQueries: []string{ + "insert into fk_t15(id, col) values (1, 7), (2, 9)", + "insert into fk_t16(id, col) values (1, 7), (2, 9)", + "insert into fk_t17(id, col) values (1, 7)", + "insert into fk_t19(id, col) values (1, 7)", + }, + dmlQuery: "delete from fk_t15 order by id limit 0", + assertionQueries: []string{ + "select * from fk_t15 order by id", + "select * from fk_t16 order by id", + "select * from fk_t17 order by id", + "select * from fk_t19 order by id", + }, }, } diff --git a/go/vt/vtgate/engine/delete_with_input.go b/go/vt/vtgate/engine/delete_with_input.go index 9a0168b946b..5b80dcce10c 100644 --- a/go/vt/vtgate/engine/delete_with_input.go +++ b/go/vt/vtgate/engine/delete_with_input.go @@ -62,6 +62,9 @@ func (del *DeleteWithInput) TryExecute(ctx context.Context, vcursor VCursor, bin if err != nil { return nil, err } + if inputRes == nil || len(inputRes.Rows) == 0 { + return &sqltypes.Result{}, nil + } var bv *querypb.BindVariable if len(del.OutputCols) == 1 { diff --git a/go/vt/vtgate/planbuilder/operators/delete.go b/go/vt/vtgate/planbuilder/operators/delete.go index 189ae5ff2fc..82ab6fe09ca 100644 --- a/go/vt/vtgate/planbuilder/operators/delete.go +++ b/go/vt/vtgate/planbuilder/operators/delete.go @@ -21,6 +21,7 @@ import ( "vitess.io/vitess/go/vt/sqlparser" "vitess.io/vitess/go/vt/vterrors" + "vitess.io/vitess/go/vt/vtgate/engine" "vitess.io/vitess/go/vt/vtgate/planbuilder/plancontext" "vitess.io/vitess/go/vt/vtgate/semantics" "vitess.io/vitess/go/vt/vtgate/vindexes" @@ -78,8 +79,15 @@ func (d *Delete) ShortDescription() string { } func createOperatorFromDelete(ctx *plancontext.PlanningContext, deleteStmt *sqlparser.Delete) (op Operator) { - delClone := sqlparser.CloneRefOfDelete(deleteStmt) + childFks := ctx.SemTable.GetChildForeignKeysForTable(deleteStmt.Targets[0]) + + // If the delete statement has a limit and has foreign keys, we will use a DeleteWithInput + // operator wherein we do a selection first and use that output for the subsequent deletes. + if len(childFks) > 0 && deleteStmt.Limit != nil { + return deletePlanningForLimitFk(ctx, deleteStmt) + } + delClone := sqlparser.CloneRefOfDelete(deleteStmt) delOp := createDeleteOperator(ctx, deleteStmt) op = delOp @@ -90,19 +98,57 @@ func createOperatorFromDelete(ctx *plancontext.PlanningContext, deleteStmt *sqlp } } - childFks := ctx.SemTable.GetChildForeignKeysForTable(deleteStmt.Targets[0]) - // If there are no foreign key constraints, then we don't need to do anything. + // If there are no foreign key constraints, then we don't need to do anything special. if len(childFks) == 0 { return op } - // If the delete statement has a limit, we don't support it yet. - if delClone.Limit != nil { - panic(vterrors.VT12001("foreign keys management at vitess with limit")) - } return createFkCascadeOpForDelete(ctx, op, delClone, childFks, delOp.Target.VTable) } +func deletePlanningForLimitFk(ctx *plancontext.PlanningContext, del *sqlparser.Delete) Operator { + delClone := ctx.SemTable.Clone(del).(*sqlparser.Delete) + del.Limit = nil + del.OrderBy = nil + + selectStmt := &sqlparser.Select{ + From: delClone.TableExprs, + Where: delClone.Where, + OrderBy: delClone.OrderBy, + Limit: delClone.Limit, + Lock: sqlparser.ForUpdateLock, + } + ts := ctx.SemTable.Targets[del.Targets[0].Name] + ti, err := ctx.SemTable.TableInfoFor(ts) + if err != nil { + panic(vterrors.VT13001(err.Error())) + } + vTbl := ti.GetVindexTable() + + var leftComp sqlparser.ValTuple + cols := make([]*sqlparser.ColName, 0, len(vTbl.PrimaryKey)) + for _, col := range vTbl.PrimaryKey { + colName := sqlparser.NewColName(col.String()) + selectStmt.SelectExprs = append(selectStmt.SelectExprs, aeWrap(colName)) + cols = append(cols, colName) + leftComp = append(leftComp, colName) + ctx.SemTable.Recursive[colName] = ts + } + // optimize for case when there is only single column on left hand side. + var lhs sqlparser.Expr = leftComp + if len(leftComp) == 1 { + lhs = leftComp[0] + } + compExpr := sqlparser.NewComparisonExpr(sqlparser.InOp, lhs, sqlparser.ListArg(engine.DmVals), nil) + del.Where = sqlparser.NewWhere(sqlparser.WhereClause, compExpr) + + return &DeleteWithInput{ + Delete: createOperatorFromDelete(ctx, del), + Source: createOperatorFromSelect(ctx, selectStmt), + cols: cols, + } +} + func createDeleteOperator(ctx *plancontext.PlanningContext, del *sqlparser.Delete) *Delete { op := crossJoin(ctx, del.TableExprs) diff --git a/go/vt/vtgate/planbuilder/testdata/foreignkey_cases.json b/go/vt/vtgate/planbuilder/testdata/foreignkey_cases.json index e0b30bb21b5..0bd8a58e21b 100644 --- a/go/vt/vtgate/planbuilder/testdata/foreignkey_cases.json +++ b/go/vt/vtgate/planbuilder/testdata/foreignkey_cases.json @@ -1246,9 +1246,81 @@ "plan": "VT09016: Cannot delete or update a parent row: a foreign key constraint fails" }, { - "comment": "delete in a table with limit - disallowed", + "comment": "delete in a table with limit", "query": "delete from u_tbl2 limit 2", - "plan": "VT12001: unsupported: foreign keys management at vitess with limit" + "plan": { + "QueryType": "DELETE", + "Original": "delete from u_tbl2 limit 2", + "Instructions": { + "OperatorType": "DeleteWithInput", + "TargetTabletType": "PRIMARY", + "Offset": [ + 0 + ], + "Inputs": [ + { + "OperatorType": "Route", + "Variant": "Unsharded", + "Keyspace": { + "Name": "unsharded_fk_allow", + "Sharded": false + }, + "FieldQuery": "select id from u_tbl2 where 1 != 1", + "Query": "select id from u_tbl2 limit 2 for update", + "Table": "u_tbl2" + }, + { + "OperatorType": "FkCascade", + "Inputs": [ + { + "InputName": "Selection", + "OperatorType": "Route", + "Variant": "Unsharded", + "Keyspace": { + "Name": "unsharded_fk_allow", + "Sharded": false + }, + "FieldQuery": "select u_tbl2.col2 from u_tbl2 where 1 != 1", + "Query": "select u_tbl2.col2 from u_tbl2 where id in ::dm_vals for update", + "Table": "u_tbl2" + }, + { + "InputName": "CascadeChild-1", + "OperatorType": "Update", + "Variant": "Unsharded", + "Keyspace": { + "Name": "unsharded_fk_allow", + "Sharded": false + }, + "TargetTabletType": "PRIMARY", + "BvName": "fkc_vals", + "Cols": [ + 0 + ], + "Query": "update u_tbl3 set col3 = null where (col3) in ::fkc_vals", + "Table": "u_tbl3" + }, + { + "InputName": "Parent", + "OperatorType": "Delete", + "Variant": "Unsharded", + "Keyspace": { + "Name": "unsharded_fk_allow", + "Sharded": false + }, + "TargetTabletType": "PRIMARY", + "Query": "delete from u_tbl2 where id in ::dm_vals", + "Table": "u_tbl2" + } + ] + } + ] + }, + "TablesUsed": [ + "unsharded_fk_allow.u_tbl2", + "unsharded_fk_allow.u_tbl3" + ] + } }, { "comment": "update with fk on cross-shard with a update condition on non-literal value", @@ -3256,5 +3328,117 @@ "unsharded_fk_allow.u_tbl3" ] } + }, + { + "comment": "Delete with limit", + "query": "delete from u_tbl1 order by id limit 1", + "plan": { + "QueryType": "DELETE", + "Original": "delete from u_tbl1 order by id limit 1", + "Instructions": { + "OperatorType": "DeleteWithInput", + "TargetTabletType": "PRIMARY", + "Offset": [ + 0 + ], + "Inputs": [ + { + "OperatorType": "Route", + "Variant": "Unsharded", + "Keyspace": { + "Name": "unsharded_fk_allow", + "Sharded": false + }, + "FieldQuery": "select id from u_tbl1 where 1 != 1", + "Query": "select id from u_tbl1 order by id asc limit 1 for update", + "Table": "u_tbl1" + }, + { + "OperatorType": "FkCascade", + "Inputs": [ + { + "InputName": "Selection", + "OperatorType": "Route", + "Variant": "Unsharded", + "Keyspace": { + "Name": "unsharded_fk_allow", + "Sharded": false + }, + "FieldQuery": "select u_tbl1.col1 from u_tbl1 where 1 != 1", + "Query": "select u_tbl1.col1 from u_tbl1 where id in ::dm_vals for update", + "Table": "u_tbl1" + }, + { + "InputName": "CascadeChild-1", + "OperatorType": "FkCascade", + "BvName": "fkc_vals", + "Cols": [ + 0 + ], + "Inputs": [ + { + "InputName": "Selection", + "OperatorType": "Route", + "Variant": "Unsharded", + "Keyspace": { + "Name": "unsharded_fk_allow", + "Sharded": false + }, + "FieldQuery": "select u_tbl2.col2 from u_tbl2 where 1 != 1", + "Query": "select u_tbl2.col2 from u_tbl2 where (col2) in ::fkc_vals for update", + "Table": "u_tbl2" + }, + { + "InputName": "CascadeChild-1", + "OperatorType": "Update", + "Variant": "Unsharded", + "Keyspace": { + "Name": "unsharded_fk_allow", + "Sharded": false + }, + "TargetTabletType": "PRIMARY", + "BvName": "fkc_vals1", + "Cols": [ + 0 + ], + "Query": "update u_tbl3 set col3 = null where (col3) in ::fkc_vals1", + "Table": "u_tbl3" + }, + { + "InputName": "Parent", + "OperatorType": "Delete", + "Variant": "Unsharded", + "Keyspace": { + "Name": "unsharded_fk_allow", + "Sharded": false + }, + "TargetTabletType": "PRIMARY", + "Query": "delete from u_tbl2 where (col2) in ::fkc_vals", + "Table": "u_tbl2" + } + ] + }, + { + "InputName": "Parent", + "OperatorType": "Delete", + "Variant": "Unsharded", + "Keyspace": { + "Name": "unsharded_fk_allow", + "Sharded": false + }, + "TargetTabletType": "PRIMARY", + "Query": "delete from u_tbl1 where id in ::dm_vals", + "Table": "u_tbl1" + } + ] + } + ] + }, + "TablesUsed": [ + "unsharded_fk_allow.u_tbl1", + "unsharded_fk_allow.u_tbl2", + "unsharded_fk_allow.u_tbl3" + ] + } } ] diff --git a/go/vt/vtgate/planbuilder/testdata/foreignkey_checks_on_cases.json b/go/vt/vtgate/planbuilder/testdata/foreignkey_checks_on_cases.json index 0e22d3ed323..1f961a8c6e6 100644 --- a/go/vt/vtgate/planbuilder/testdata/foreignkey_checks_on_cases.json +++ b/go/vt/vtgate/planbuilder/testdata/foreignkey_checks_on_cases.json @@ -1246,9 +1246,81 @@ "plan": "VT09016: Cannot delete or update a parent row: a foreign key constraint fails" }, { - "comment": "delete in a table with limit - disallowed", + "comment": "delete in a table with limit", "query": "delete from u_tbl2 limit 2", - "plan": "VT12001: unsupported: foreign keys management at vitess with limit" + "plan": { + "QueryType": "DELETE", + "Original": "delete from u_tbl2 limit 2", + "Instructions": { + "OperatorType": "DeleteWithInput", + "TargetTabletType": "PRIMARY", + "Offset": [ + 0 + ], + "Inputs": [ + { + "OperatorType": "Route", + "Variant": "Unsharded", + "Keyspace": { + "Name": "unsharded_fk_allow", + "Sharded": false + }, + "FieldQuery": "select id from u_tbl2 where 1 != 1", + "Query": "select id from u_tbl2 limit 2 for update", + "Table": "u_tbl2" + }, + { + "OperatorType": "FkCascade", + "Inputs": [ + { + "InputName": "Selection", + "OperatorType": "Route", + "Variant": "Unsharded", + "Keyspace": { + "Name": "unsharded_fk_allow", + "Sharded": false + }, + "FieldQuery": "select u_tbl2.col2 from u_tbl2 where 1 != 1", + "Query": "select u_tbl2.col2 from u_tbl2 where id in ::dm_vals for update", + "Table": "u_tbl2" + }, + { + "InputName": "CascadeChild-1", + "OperatorType": "Update", + "Variant": "Unsharded", + "Keyspace": { + "Name": "unsharded_fk_allow", + "Sharded": false + }, + "TargetTabletType": "PRIMARY", + "BvName": "fkc_vals", + "Cols": [ + 0 + ], + "Query": "update /*+ SET_VAR(foreign_key_checks=ON) */ u_tbl3 set col3 = null where (col3) in ::fkc_vals", + "Table": "u_tbl3" + }, + { + "InputName": "Parent", + "OperatorType": "Delete", + "Variant": "Unsharded", + "Keyspace": { + "Name": "unsharded_fk_allow", + "Sharded": false + }, + "TargetTabletType": "PRIMARY", + "Query": "delete /*+ SET_VAR(foreign_key_checks=On) */ from u_tbl2 where id in ::dm_vals", + "Table": "u_tbl2" + } + ] + } + ] + }, + "TablesUsed": [ + "unsharded_fk_allow.u_tbl2", + "unsharded_fk_allow.u_tbl3" + ] + } }, { "comment": "update with fk on cross-shard with a where condition on non-literal value - disallowed",