From 0f41eda9c5018a3f2603e8a8c1431e2a7b73db44 Mon Sep 17 00:00:00 2001 From: Manan Gupta Date: Tue, 27 Feb 2024 15:59:29 +0530 Subject: [PATCH 1/6] refactor: move the check earlier Signed-off-by: Manan Gupta --- go/vt/vtgate/planbuilder/operators/update.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/go/vt/vtgate/planbuilder/operators/update.go b/go/vt/vtgate/planbuilder/operators/update.go index cf55f91fddd..224ca6cfa09 100644 --- a/go/vt/vtgate/planbuilder/operators/update.go +++ b/go/vt/vtgate/planbuilder/operators/update.go @@ -135,12 +135,6 @@ func createOperatorFromUpdate(ctx *plancontext.PlanningContext, updStmt *sqlpars panic(vterrors.VT12001("update with limit with foreign key constraints")) } - // Now we check if any of the foreign key columns that are being udpated have dependencies on other updated columns. - // This is unsafe, and we currently don't support this in Vitess. - if err := ctx.SemTable.ErrIfFkDependentColumnUpdated(updStmt.Exprs); err != nil { - panic(err) - } - return buildFkOperator(ctx, op, updClone, parentFks, childFks, vTbl) } @@ -167,6 +161,12 @@ func errIfUpdateNotSupported(ctx *plancontext.PlanningContext, stmt *sqlparser.U panic(vterrors.VT12001("multi-table UPDATE statement with multi-target column update")) } } + + // Now we check if any of the foreign key columns that are being udpated have dependencies on other updated columns. + // This is unsafe, and we currently don't support this in Vitess. + if err := ctx.SemTable.ErrIfFkDependentColumnUpdated(stmt.Exprs); err != nil { + panic(err) + } } func createUpdateOperator(ctx *plancontext.PlanningContext, updStmt *sqlparser.Update) (Operator, *vindexes.Table, *sqlparser.Update) { From bfaedaec453c92943178f452110d9a149c5b1d4d Mon Sep 17 00:00:00 2001 From: Manan Gupta Date: Tue, 27 Feb 2024 17:51:42 +0530 Subject: [PATCH 2/6] feat: add limit planning for updates with foreign keys Signed-off-by: Manan Gupta --- go/vt/vtgate/planbuilder/operators/update.go | 73 ++++++- .../testdata/foreignkey_cases.json | 185 +++++++++++++++++- .../testdata/foreignkey_checks_on_cases.json | 76 ++++++- 3 files changed, 321 insertions(+), 13 deletions(-) diff --git a/go/vt/vtgate/planbuilder/operators/update.go b/go/vt/vtgate/planbuilder/operators/update.go index 224ca6cfa09..4f5ae20e90d 100644 --- a/go/vt/vtgate/planbuilder/operators/update.go +++ b/go/vt/vtgate/planbuilder/operators/update.go @@ -112,6 +112,15 @@ func (u *Update) ShortDescription() string { func createOperatorFromUpdate(ctx *plancontext.PlanningContext, updStmt *sqlparser.Update) (op Operator) { errIfUpdateNotSupported(ctx, updStmt) + parentFks := ctx.SemTable.GetParentForeignKeysList() + childFks := ctx.SemTable.GetChildForeignKeysList() + + // We check if dml with input plan is required. DML with input planning is generally + // slower, because it does a selection and then creates a update statement wherein we have to + // list all the primary key values. + if updateWithInputPlanningRequired(childFks, parentFks, updStmt) { + return updateWithInputPlanningForFk(ctx, updStmt) + } var updClone *sqlparser.Update var vTbl *vindexes.Table @@ -124,18 +133,70 @@ func createOperatorFromUpdate(ctx *plancontext.PlanningContext, updStmt *sqlpars Lock: sqlparser.ShareModeLock, } - parentFks := ctx.SemTable.GetParentForeignKeysList() - childFks := ctx.SemTable.GetChildForeignKeysList() if len(childFks) == 0 && len(parentFks) == 0 { return op } - // If the delete statement has a limit, we don't support it yet. - if updStmt.Limit != nil { - panic(vterrors.VT12001("update with limit with foreign key constraints")) + return buildFkOperator(ctx, op, updClone, parentFks, childFks, vTbl) +} + +func updateWithInputPlanningForFk(ctx *plancontext.PlanningContext, upd *sqlparser.Update) Operator { + updClone := ctx.SemTable.Clone(upd).(*sqlparser.Update) + upd.Limit = nil + + selectStmt := &sqlparser.Select{ + From: updClone.TableExprs, + Where: updClone.Where, + OrderBy: updClone.OrderBy, + Limit: updClone.Limit, + Lock: sqlparser.ForUpdateLock, } - return buildFkOperator(ctx, op, updClone, parentFks, childFks, vTbl) + ate, isAliasTableExpr := upd.TableExprs[0].(*sqlparser.AliasedTableExpr) + if !isAliasTableExpr { + panic(vterrors.VT12001("update with limit with foreign key constraints using a complex table")) + } + ts := ctx.SemTable.TableSetFor(ate) + 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.NewColNameWithQualifier(col.String(), vTbl.GetTableName()) + 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.DmlVals), nil) + + upd.Where = sqlparser.NewWhere(sqlparser.WhereClause, compExpr) + return &DMLWithInput{ + DML: createOperatorFromUpdate(ctx, upd), + Source: createOperatorFromSelect(ctx, selectStmt), + cols: cols, + } +} + +func updateWithInputPlanningRequired(childFks []vindexes.ChildFKInfo, parentFks []vindexes.ParentFKInfo, updateStmt *sqlparser.Update) bool { + // If there are no foreign keys, we don't need to use delete with input. + if len(childFks) == 0 && len(parentFks) == 0 { + return false + } + // Limit requires dml with input. + if updateStmt.Limit != nil { + return true + } + return false } func errIfUpdateNotSupported(ctx *plancontext.PlanningContext, stmt *sqlparser.Update) { diff --git a/go/vt/vtgate/planbuilder/testdata/foreignkey_cases.json b/go/vt/vtgate/planbuilder/testdata/foreignkey_cases.json index c8f059b7b88..ed357b1c677 100644 --- a/go/vt/vtgate/planbuilder/testdata/foreignkey_cases.json +++ b/go/vt/vtgate/planbuilder/testdata/foreignkey_cases.json @@ -793,11 +793,6 @@ ] } }, - { - "comment": "update in a table with limit - disallowed", - "query": "update u_tbl2 set col2 = 'bar' limit 2", - "plan": "VT12001: unsupported: update with limit with foreign key constraints" - }, { "comment": "update in a table with non-literal value - set null", "query": "update u_tbl2 set m = 2, col2 = col1 + 'bar' where id = 1", @@ -3806,5 +3801,185 @@ "unsharded_fk_allow.u_tbl8" ] } + }, + { + "comment": "update with limit with foreign keys", + "query": "update u_tbl2 set col2 = 'bar' limit 2", + "plan": { + "QueryType": "UPDATE", + "Original": "update u_tbl2 set col2 = 'bar' limit 2", + "Instructions": { + "OperatorType": "DMLWithInput", + "TargetTabletType": "PRIMARY", + "Offset": [ + 0 + ], + "Inputs": [ + { + "OperatorType": "Route", + "Variant": "Unsharded", + "Keyspace": { + "Name": "unsharded_fk_allow", + "Sharded": false + }, + "FieldQuery": "select u_tbl2.id from u_tbl2 where 1 != 1", + "Query": "select u_tbl2.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 u_tbl2.id in ::dml_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 and (col3) not in ((cast('bar' as CHAR)))", + "Table": "u_tbl3" + }, + { + "InputName": "Parent", + "OperatorType": "Update", + "Variant": "Unsharded", + "Keyspace": { + "Name": "unsharded_fk_allow", + "Sharded": false + }, + "TargetTabletType": "PRIMARY", + "Query": "update u_tbl2 set col2 = 'bar' where u_tbl2.id in ::dml_vals", + "Table": "u_tbl2" + } + ] + } + ] + }, + "TablesUsed": [ + "unsharded_fk_allow.u_tbl2", + "unsharded_fk_allow.u_tbl3" + ] + } + }, + { + "comment": "non literal update with order by and limit", + "query": "update u_tbl2 set col2 = id + 1 order by id limit 2", + "plan": { + "QueryType": "UPDATE", + "Original": "update u_tbl2 set col2 = id + 1 order by id limit 2", + "Instructions": { + "OperatorType": "DMLWithInput", + "TargetTabletType": "PRIMARY", + "Offset": [ + 0 + ], + "Inputs": [ + { + "OperatorType": "Route", + "Variant": "Unsharded", + "Keyspace": { + "Name": "unsharded_fk_allow", + "Sharded": false + }, + "FieldQuery": "select u_tbl2.id from u_tbl2 where 1 != 1", + "Query": "select u_tbl2.id from u_tbl2 order by id asc limit 2 for update", + "Table": "u_tbl2" + }, + { + "OperatorType": "FKVerify", + "Inputs": [ + { + "InputName": "VerifyParent-1", + "OperatorType": "Route", + "Variant": "Unsharded", + "Keyspace": { + "Name": "unsharded_fk_allow", + "Sharded": false + }, + "FieldQuery": "select 1 from u_tbl2 left join u_tbl1 on u_tbl1.col1 = cast(u_tbl2.id + 1 as CHAR) where 1 != 1", + "Query": "select 1 from u_tbl2 left join u_tbl1 on u_tbl1.col1 = cast(u_tbl2.id + 1 as CHAR) where u_tbl1.col1 is null and cast(u_tbl2.id + 1 as CHAR) is not null and not (u_tbl2.col2) <=> (cast(u_tbl2.id + 1 as CHAR)) and u_tbl2.id in ::dml_vals limit 1 for share", + "Table": "u_tbl1, u_tbl2" + }, + { + "InputName": "PostVerify", + "OperatorType": "FkCascade", + "Inputs": [ + { + "InputName": "Selection", + "OperatorType": "Route", + "Variant": "Unsharded", + "Keyspace": { + "Name": "unsharded_fk_allow", + "Sharded": false + }, + "FieldQuery": "select u_tbl2.col2, col2 <=> cast(id + 1 as CHAR), cast(id + 1 as CHAR) from u_tbl2 where 1 != 1", + "Query": "select u_tbl2.col2, col2 <=> cast(id + 1 as CHAR), cast(id + 1 as CHAR) from u_tbl2 where u_tbl2.id in ::dml_vals order by id asc 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 + ], + "NonLiteralUpdateInfo": [ + { + "CompExprCol": 1, + "UpdateExprCol": 2, + "UpdateExprBvName": "fkc_upd" + } + ], + "Query": "update u_tbl3 set col3 = null where (col3) in ::fkc_vals and (:fkc_upd is null or (col3) not in ((:fkc_upd)))", + "Table": "u_tbl3" + }, + { + "InputName": "Parent", + "OperatorType": "Update", + "Variant": "Unsharded", + "Keyspace": { + "Name": "unsharded_fk_allow", + "Sharded": false + }, + "TargetTabletType": "PRIMARY", + "Query": "update /*+ SET_VAR(foreign_key_checks=OFF) */ u_tbl2 set col2 = id + 1 where u_tbl2.id in ::dml_vals order by id asc", + "Table": "u_tbl2" + } + ] + } + ] + } + ] + }, + "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 311ff6adbff..4d31e6820b5 100644 --- a/go/vt/vtgate/planbuilder/testdata/foreignkey_checks_on_cases.json +++ b/go/vt/vtgate/planbuilder/testdata/foreignkey_checks_on_cases.json @@ -794,9 +794,81 @@ } }, { - "comment": "update in a table with limit - disallowed", + "comment": "update with limit with foreign keys", "query": "update u_tbl2 set col2 = 'bar' limit 2", - "plan": "VT12001: unsupported: update with limit with foreign key constraints" + "plan": { + "QueryType": "UPDATE", + "Original": "update u_tbl2 set col2 = 'bar' limit 2", + "Instructions": { + "OperatorType": "DMLWithInput", + "TargetTabletType": "PRIMARY", + "Offset": [ + 0 + ], + "Inputs": [ + { + "OperatorType": "Route", + "Variant": "Unsharded", + "Keyspace": { + "Name": "unsharded_fk_allow", + "Sharded": false + }, + "FieldQuery": "select u_tbl2.id from u_tbl2 where 1 != 1", + "Query": "select u_tbl2.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 u_tbl2.id in ::dml_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 and (col3) not in ((cast('bar' as CHAR)))", + "Table": "u_tbl3" + }, + { + "InputName": "Parent", + "OperatorType": "Update", + "Variant": "Unsharded", + "Keyspace": { + "Name": "unsharded_fk_allow", + "Sharded": false + }, + "TargetTabletType": "PRIMARY", + "Query": "update /*+ SET_VAR(foreign_key_checks=On) */ u_tbl2 set col2 = 'bar' where u_tbl2.id in ::dml_vals", + "Table": "u_tbl2" + } + ] + } + ] + }, + "TablesUsed": [ + "unsharded_fk_allow.u_tbl2", + "unsharded_fk_allow.u_tbl3" + ] + } }, { "comment": "update in a table with non-literal value - set null fail due to child update where condition", From 40473394448b69bf6ab3225f580d308bed37e6a8 Mon Sep 17 00:00:00 2001 From: Manan Gupta Date: Tue, 27 Feb 2024 18:06:05 +0530 Subject: [PATCH 3/6] test: add e2e testing for update with limits Signed-off-by: Manan Gupta --- go/test/endtoend/vtgate/foreignkey/fk_test.go | 45 +++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/go/test/endtoend/vtgate/foreignkey/fk_test.go b/go/test/endtoend/vtgate/foreignkey/fk_test.go index b16da5bfabd..c47020d2561 100644 --- a/go/test/endtoend/vtgate/foreignkey/fk_test.go +++ b/go/test/endtoend/vtgate/foreignkey/fk_test.go @@ -725,6 +725,51 @@ func TestFkScenarios(t *testing.T) { "select * from fk_t17 order by id", "select * from fk_t19 order by id", }, + }, { + name: "Update 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: "update fk_t15 set col = '2' 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: "Update 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: "update fk_t15 set col = '8' 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", + }, + }, { + name: "Update with non-literal update and 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: "update fk_t15 set col = id + 3 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", + }, }, } From d416ea804c6bf408823e9419fa7e49f166f46497 Mon Sep 17 00:00:00 2001 From: Manan Gupta Date: Tue, 27 Feb 2024 18:22:48 +0530 Subject: [PATCH 4/6] test: add fuzzer tests Signed-off-by: Manan Gupta --- .../vtgate/foreignkey/fk_fuzz_test.go | 22 +++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/go/test/endtoend/vtgate/foreignkey/fk_fuzz_test.go b/go/test/endtoend/vtgate/foreignkey/fk_fuzz_test.go index 8ff1d660537..d430c31e35a 100644 --- a/go/test/endtoend/vtgate/foreignkey/fk_fuzz_test.go +++ b/go/test/endtoend/vtgate/foreignkey/fk_fuzz_test.go @@ -157,10 +157,15 @@ func (fz *fuzzer) generateUpdateDMLQuery() string { idValue := 1 + rand.Intn(fz.maxValForId) tableName := fkTables[tableId] setVarFkChecksVal := fz.getSetVarFkChecksVal() + updWithLimit := rand.Intn(2) + limitCount := rand.Intn(3) if tableName == "fk_t20" { colValue := convertIntValueToString(rand.Intn(1 + fz.maxValForCol)) col2Value := convertIntValueToString(rand.Intn(1 + fz.maxValForCol)) - return fmt.Sprintf("update %v%v set col = %v, col2 = %v where id = %v", setVarFkChecksVal, tableName, colValue, col2Value, idValue) + if updWithLimit == 0 { + return fmt.Sprintf("update %v%v set col = %v, col2 = %v where id = %v", setVarFkChecksVal, tableName, colValue, col2Value, idValue) + } + return fmt.Sprintf("update %v%v set col = %v, col2 = %v order by id limit %v", setVarFkChecksVal, tableName, colValue, col2Value, limitCount) } else if isMultiColFkTable(tableName) { if rand.Intn(2) == 0 { colaValue := convertIntValueToString(rand.Intn(1 + fz.maxValForCol)) @@ -169,15 +174,24 @@ func (fz *fuzzer) generateUpdateDMLQuery() string { colaValue = fz.generateExpression(rand.Intn(4)+1, "cola", "colb", "id") colbValue = fz.generateExpression(rand.Intn(4)+1, "cola", "colb", "id") } - return fmt.Sprintf("update %v%v set cola = %v, colb = %v where id = %v", setVarFkChecksVal, tableName, colaValue, colbValue, idValue) + if updWithLimit == 0 { + return fmt.Sprintf("update %v%v set cola = %v, colb = %v where id = %v", setVarFkChecksVal, tableName, colaValue, colbValue, idValue) + } + return fmt.Sprintf("update %v%v set cola = %v, colb = %v order by id limit %v", setVarFkChecksVal, tableName, colaValue, colbValue, limitCount) } else { colValue := fz.generateExpression(rand.Intn(4)+1, "cola", "colb", "id") colToUpdate := []string{"cola", "colb"}[rand.Intn(2)] - return fmt.Sprintf("update %v set %v = %v where id = %v", tableName, colToUpdate, colValue, idValue) + if updWithLimit == 0 { + return fmt.Sprintf("update %v set %v = %v where id = %v", tableName, colToUpdate, colValue, idValue) + } + return fmt.Sprintf("update %v set %v = %v order by id limit %v", tableName, colToUpdate, colValue, limitCount) } } else { colValue := fz.generateExpression(rand.Intn(4)+1, "col", "id") - return fmt.Sprintf("update %v%v set col = %v where id = %v", setVarFkChecksVal, tableName, colValue, idValue) + if updWithLimit == 0 { + return fmt.Sprintf("update %v%v set col = %v where id = %v", setVarFkChecksVal, tableName, colValue, idValue) + } + return fmt.Sprintf("update %v%v set col = %v order by id limit %v", setVarFkChecksVal, tableName, colValue, limitCount) } } From f7d942049d4e8247418e0b718078ae33465a6dd5 Mon Sep 17 00:00:00 2001 From: Manan Gupta Date: Wed, 28 Feb 2024 11:26:55 +0530 Subject: [PATCH 5/6] feat: fix build issues post merge Signed-off-by: Manan Gupta --- go/vt/vtgate/planbuilder/operators/update.go | 4 ++-- go/vt/vtgate/planbuilder/testdata/foreignkey_cases.json | 4 ++-- .../planbuilder/testdata/foreignkey_checks_on_cases.json | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/go/vt/vtgate/planbuilder/operators/update.go b/go/vt/vtgate/planbuilder/operators/update.go index c93983badd1..7f394634f6c 100644 --- a/go/vt/vtgate/planbuilder/operators/update.go +++ b/go/vt/vtgate/planbuilder/operators/update.go @@ -164,9 +164,9 @@ func updateWithInputPlanningForFk(ctx *plancontext.PlanningContext, upd *sqlpars upd.Where = sqlparser.NewWhere(sqlparser.WhereClause, compExpr) return &DMLWithInput{ - DML: createOperatorFromUpdate(ctx, upd), + DML: []Operator{createOperatorFromUpdate(ctx, upd)}, Source: createOperatorFromSelect(ctx, selectStmt), - cols: cols, + cols: [][]*sqlparser.ColName{cols}, } } diff --git a/go/vt/vtgate/planbuilder/testdata/foreignkey_cases.json b/go/vt/vtgate/planbuilder/testdata/foreignkey_cases.json index 8857e25fbbf..bf95af52f1e 100644 --- a/go/vt/vtgate/planbuilder/testdata/foreignkey_cases.json +++ b/go/vt/vtgate/planbuilder/testdata/foreignkey_cases.json @@ -3817,7 +3817,7 @@ "OperatorType": "DMLWithInput", "TargetTabletType": "PRIMARY", "Offset": [ - 0 + "0:[0]" ], "Inputs": [ { @@ -3894,7 +3894,7 @@ "OperatorType": "DMLWithInput", "TargetTabletType": "PRIMARY", "Offset": [ - 0 + "0:[0]" ], "Inputs": [ { 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 4176d6686ed..7ade2be3954 100644 --- a/go/vt/vtgate/planbuilder/testdata/foreignkey_checks_on_cases.json +++ b/go/vt/vtgate/planbuilder/testdata/foreignkey_checks_on_cases.json @@ -803,7 +803,7 @@ "OperatorType": "DMLWithInput", "TargetTabletType": "PRIMARY", "Offset": [ - 0 + "0:[0]" ], "Inputs": [ { From dd69bb34492f607ffaaddc9ce66ffd3734dc0e8c Mon Sep 17 00:00:00 2001 From: Manan Gupta Date: Mon, 4 Mar 2024 15:51:15 +0530 Subject: [PATCH 6/6] test: add testing where we update with limit and ordering is importnat Signed-off-by: Manan Gupta --- go/test/endtoend/vtgate/foreignkey/fk_test.go | 226 +++++++++++------- 1 file changed, 135 insertions(+), 91 deletions(-) diff --git a/go/test/endtoend/vtgate/foreignkey/fk_test.go b/go/test/endtoend/vtgate/foreignkey/fk_test.go index c47020d2561..d95477f1f5d 100644 --- a/go/test/endtoend/vtgate/foreignkey/fk_test.go +++ b/go/test/endtoend/vtgate/foreignkey/fk_test.go @@ -379,6 +379,7 @@ func TestFkScenarios(t *testing.T) { name string dataQueries []string dmlQuery string + dmlShouldErr bool assertionQueries []string }{ { @@ -386,7 +387,8 @@ func TestFkScenarios(t *testing.T) { dataQueries: []string{ "insert into fk_t1(id, col) values (1, 5)", }, - dmlQuery: "insert into t2(id, col) values (1, 7)", + dmlQuery: "insert into t2(id, col) values (1, 7)", + dmlShouldErr: true, assertionQueries: []string{ "select * from fk_t1 order by id", "select * from fk_t2 order by id", @@ -407,7 +409,8 @@ func TestFkScenarios(t *testing.T) { "insert into fk_t1(id, col) values (1, 7)", "insert into fk_t2(id, col) values (1, 7)", }, - dmlQuery: "update fk_t1 set col = 5 where id = 1", + dmlShouldErr: true, + dmlQuery: "update fk_t1 set col = 5 where id = 1", assertionQueries: []string{ "select * from fk_t1 order by id", "select * from fk_t2 order by id", @@ -429,7 +432,8 @@ func TestFkScenarios(t *testing.T) { "insert into fk_t1(id, col) values (1, 7)", "insert into fk_t2(id, col) values (1, 7)", }, - dmlQuery: "delete from fk_t1 where id = 1", + dmlQuery: "delete from fk_t1 where id = 1", + dmlShouldErr: true, assertionQueries: []string{ "select * from fk_t1 order by id", "select * from fk_t2 order by id", @@ -450,7 +454,7 @@ func TestFkScenarios(t *testing.T) { dataQueries: []string{ "insert into fk_t1(id, col) values (1, 7), (2, 9)", "insert into fk_t2(id, col) values (1, 7), (2, 9)", - "insert into fk_t3(id, col) values (1, 7), (2, 9)", + "insert into fk_t3(id, col) values (1, 7)", "insert into fk_t6(id, col) values (1, 7)", }, dmlQuery: "update fk_t3 set col = 9 where id = 1", @@ -469,7 +473,8 @@ func TestFkScenarios(t *testing.T) { "insert into fk_t4(id, col) values (1, 7)", "insert into fk_t5(id, col) values (1, 7)", }, - dmlQuery: "update fk_t3 set col = 9 where id = 1", + dmlQuery: "update fk_t3 set col = 9 where id = 1", + dmlShouldErr: true, assertionQueries: []string{ "select * from fk_t1 order by id", "select * from fk_t2 order by id", @@ -518,7 +523,8 @@ func TestFkScenarios(t *testing.T) { "insert into fk_t4(id, col) values (1, 7)", "insert into fk_t5(id, col) values (1, 7)", }, - dmlQuery: "delete from fk_t3 where id = 1", + dmlQuery: "delete from fk_t3 where id = 1", + dmlShouldErr: true, assertionQueries: []string{ "select * from fk_t1 order by id", "select * from fk_t2 order by id", @@ -561,7 +567,8 @@ func TestFkScenarios(t *testing.T) { "insert into fk_t11(id, col) values (1, 7)", "insert into fk_t13(id, col) values (1, 7)", }, - dmlQuery: "update fk_t10 set col = 5 where id = 1", + dmlQuery: "update fk_t10 set col = 5 where id = 1", + dmlShouldErr: true, assertionQueries: []string{ "select * from fk_t10 order by id", "select * from fk_t11 order by id", @@ -598,7 +605,8 @@ func TestFkScenarios(t *testing.T) { "insert into fk_t11(id, col) values (1, 7)", "insert into fk_t13(id, col) values (1, 7)", }, - dmlQuery: "delete from fk_t10 where id = 1", + dmlQuery: "delete from fk_t10 where id = 1", + dmlShouldErr: true, assertionQueries: []string{ "select * from fk_t10 order by id", "select * from fk_t11 order by id", @@ -620,47 +628,47 @@ func TestFkScenarios(t *testing.T) { }, { name: "Delete success with set null to an update cascade foreign key", 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_t18(id, col) values (1, 7)", + "insert into fk_multicol_t15(id, cola, colb) values (1, 7, 1), (2, 9, 1)", + "insert into fk_multicol_t16(id, cola, colb) values (1, 7, 1), (2, 9, 1)", + "insert into fk_multicol_t17(id, cola, colb) values (1, 7, 1)", + "insert into fk_multicol_t18(id, cola, colb) values (1, 7, 1)", }, - dmlQuery: "delete from fk_t16 where id = 1", + dmlQuery: "delete from fk_multicol_t16 where id = 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_t18 order by id", + "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_t18 order by id", }, }, { name: "Delete success with cascade to delete with set null to an update set null foreign key", 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)", + "insert into fk_multicol_t15(id, cola, colb) values (1, 7, 1), (2, 9, 1)", + "insert into fk_multicol_t16(id, cola, colb) values (1, 7, 1), (2, 9, 1)", + "insert into fk_multicol_t17(id, cola, colb) values (1, 7, 1)", + "insert into fk_multicol_t18(id, cola, colb) values (1, 7, 1)", }, - dmlQuery: "delete from fk_t15 where id = 1", + dmlQuery: "delete from fk_multicol_t15 where id = 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", + "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_t18 order by id", }, }, { name: "Update success with cascade to an update set null to an update cascade foreign key", 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_t18(id, col) values (1, 7)", + "insert into fk_multicol_t15(id, cola, colb) values (1, 7, 1), (2, 9, 1)", + "insert into fk_multicol_t16(id, cola, colb) values (1, 7, 1), (2, 9, 1)", + "insert into fk_multicol_t17(id, cola, colb) values (1, 7, 1)", + "insert into fk_multicol_t18(id, cola, colb) values (1, 7, 1)", }, - dmlQuery: "update fk_t15 set col = 3 where id = 1", + dmlQuery: "update fk_multicol_t15 set cola = 3 where id = 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_t18 order by id", + "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_t18 order by id", }, }, { name: "Insert success for self-referenced foreign key", @@ -676,99 +684,130 @@ func TestFkScenarios(t *testing.T) { dataQueries: []string{ "insert into fk_t20(id, col, col2) values (5, 7, NULL)", }, - dmlQuery: "insert into fk_t20(id, col, col2) values (6, 9, 6)", + dmlQuery: "insert into fk_t20(id, col, col2) values (6, 9, 6)", + dmlShouldErr: true, assertionQueries: []string{ "select * from fk_t20 order by id", }, }, { name: "Multi Table Delete 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)", + "insert into fk_multicol_t15(id, cola, colb) values (1, 7, 1), (2, 9, 1)", + "insert into fk_multicol_t16(id, cola, colb) values (1, 7, 1), (2, 9, 1)", + "insert into fk_multicol_t17(id, cola, colb) values (1, 7, 1)", + "insert into fk_multicol_t19(id, cola, colb) values (1, 7, 1)", }, - dmlQuery: "delete fk_t15 from fk_t15 join fk_t17 using id", + dmlQuery: "delete fk_multicol_t15 from fk_multicol_t15 join fk_multicol_t17 where fk_multicol_t15.id = fk_multicol_t17.id", 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", + "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{ - "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)", + "insert into fk_multicol_t15(id, cola, colb) values (1, 7, 1), (2, 9, 1)", + "insert into fk_multicol_t16(id, cola, colb) values (1, 7, 1), (2, 9, 1)", + "insert into fk_multicol_t17(id, cola, colb) values (1, 7, 1)", + "insert into fk_multicol_t19(id, cola, colb) values (1, 7, 1)", }, - dmlQuery: "delete from fk_t15 order by id limit 1", + dmlQuery: "delete from fk_multicol_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", + "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 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)", + "insert into fk_multicol_t15(id, cola, colb) values (1, 7, 1), (2, 9, 1)", + "insert into fk_multicol_t16(id, cola, colb) values (1, 7, 1), (2, 9, 1)", + "insert into fk_multicol_t17(id, cola, colb) values (1, 7, 1)", + "insert into fk_multicol_t19(id, cola, colb) values (1, 7, 1)", }, - dmlQuery: "delete from fk_t15 order by id limit 0", + dmlQuery: "delete from fk_multicol_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", + "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: "Update 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)", + "insert into fk_multicol_t15(id, cola, colb) values (1, 7, 1), (2, 9, 1)", + "insert into fk_multicol_t16(id, cola, colb) values (1, 7, 1), (2, 9, 1)", + "insert into fk_multicol_t17(id, cola, colb) values (1, 7, 1)", + "insert into fk_multicol_t19(id, cola, colb) values (1, 7, 1)", }, - dmlQuery: "update fk_t15 set col = '2' order by id limit 1", + dmlQuery: "update fk_multicol_t15 set cola = '2' 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", + "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: "Update 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)", + "insert into fk_multicol_t15(id, cola, colb) values (1, 7, 1), (2, 9, 1)", + "insert into fk_multicol_t16(id, cola, colb) values (1, 7, 1), (2, 9, 1)", + "insert into fk_multicol_t17(id, cola, colb) values (1, 7, 1)", + "insert into fk_multicol_t19(id, cola, colb) values (1, 7, 1)", }, - dmlQuery: "update fk_t15 set col = '8' order by id limit 0", + dmlQuery: "update fk_multicol_t15 set cola = '8' 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", + "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: "Update with non-literal update and 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)", + "insert into fk_multicol_t15(id, cola, colb) values (1, 7, 1), (2, 9, 1)", + "insert into fk_multicol_t16(id, cola, colb) values (1, 7, 1), (2, 9, 1)", + "insert into fk_multicol_t17(id, cola, colb) values (1, 7, 1)", + "insert into fk_multicol_t19(id, cola, colb) values (1, 7, 1)", }, - dmlQuery: "update fk_t15 set col = id + 3 order by id limit 1", + dmlQuery: "update fk_multicol_t15 set cola = id + 3 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", + "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: "Update with non-literal update order by and limit - multiple update", + dataQueries: []string{ + "insert into fk_multicol_t15(id, cola, colb) values (1, 7, 1), (2, 9, 1), (3, 12, 1)", + "insert into fk_multicol_t16(id, cola, colb) values (1, 7, 1), (2, 9, 1), (3, 12, 1)", + "insert into fk_multicol_t17(id, cola, colb) values (1, 7, 1)", + "insert into fk_multicol_t19(id, cola, colb) values (1, 7, 1)", + }, + dmlQuery: "update fk_multicol_t15 set cola = id + 8 order by id asc limit 2", + 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: "Update with non-literal update order by and limit - single update", + dataQueries: []string{ + "insert into fk_multicol_t15(id, cola, colb) values (1, 7, 1), (2, 9, 1), (3, 12, 1)", + "insert into fk_multicol_t16(id, cola, colb) values (1, 7, 1), (2, 9, 1), (3, 12, 1)", + "insert into fk_multicol_t17(id, cola, colb) values (1, 7, 1)", + "insert into fk_multicol_t19(id, cola, colb) values (1, 7, 1)", + }, + dmlQuery: "update fk_multicol_t15 set cola = id + 8 where id < 3 order by id desc limit 2", + 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", }, }, } @@ -792,7 +831,12 @@ func TestFkScenarios(t *testing.T) { } // Run the DML query that needs to be tested and verify output with MySQL. - _, _ = mcmp.ExecAllowAndCompareError(tt.dmlQuery) + _, err := mcmp.ExecAllowAndCompareError(tt.dmlQuery) + if tt.dmlShouldErr { + assert.Error(t, err) + } else { + assert.NoError(t, err) + } // Run the assertion queries and verify we get the expected outputs. for _, query := range tt.assertionQueries {