From f828764e48eb8a4f30d6d6f74b60952d684a0cdf Mon Sep 17 00:00:00 2001 From: Manan Gupta Date: Thu, 21 Sep 2023 15:34:29 +0530 Subject: [PATCH 1/4] feat: fix queries to take care of bindvariables which can be NULL values on runtime Signed-off-by: Manan Gupta --- go/vt/vtgate/planbuilder/operators/update.go | 24 ++- .../testdata/foreignkey_cases.json | 185 +++++++++++++++++- 2 files changed, 206 insertions(+), 3 deletions(-) diff --git a/go/vt/vtgate/planbuilder/operators/update.go b/go/vt/vtgate/planbuilder/operators/update.go index 8a16b97117e..89216c6a93d 100644 --- a/go/vt/vtgate/planbuilder/operators/update.go +++ b/go/vt/vtgate/planbuilder/operators/update.go @@ -426,9 +426,19 @@ func buildChildUpdOpForSetNull(ctx *plancontext.PlanningContext, fk vindexes.Chi } } if !colSetToNull { + var finalExpr sqlparser.Expr = sqlparser.NewComparisonExpr(sqlparser.NotInOp, valTuple, sqlparser.ValTuple{updateValues}, nil) + for _, value := range updateValues { + finalExpr = &sqlparser.OrExpr{ + Left: &sqlparser.IsExpr{ + Left: value, + Right: sqlparser.IsNullOp, + }, + Right: finalExpr, + } + } childWhereExpr = &sqlparser.AndExpr{ Left: childWhereExpr, - Right: sqlparser.NewComparisonExpr(sqlparser.NotInOp, valTuple, sqlparser.ValTuple{updateValues}, nil), + Right: finalExpr, } } childUpdStmt := &sqlparser.Update{ @@ -618,7 +628,17 @@ func createFkVerifyOpForChildFKForUpdate(ctx *plancontext.PlanningContext, updSt for _, column := range cFk.ParentColumns { valTuple = append(valTuple, sqlparser.NewColNameWithQualifier(column.String(), parentTbl)) } - whereCond = sqlparser.AndExpressions(whereCond, sqlparser.NewComparisonExpr(sqlparser.NotInOp, valTuple, sqlparser.ValTuple{updateValues}, nil)) + var finalExpr sqlparser.Expr = sqlparser.NewComparisonExpr(sqlparser.NotInOp, valTuple, sqlparser.ValTuple{updateValues}, nil) + for _, value := range updateValues { + finalExpr = &sqlparser.OrExpr{ + Left: &sqlparser.IsExpr{ + Left: value, + Right: sqlparser.IsNullOp, + }, + Right: finalExpr, + } + } + whereCond = sqlparser.AndExpressions(whereCond, finalExpr) } return createSelectionOp(ctx, diff --git a/go/vt/vtgate/planbuilder/testdata/foreignkey_cases.json b/go/vt/vtgate/planbuilder/testdata/foreignkey_cases.json index c9d9097e9a4..55ec36adc86 100644 --- a/go/vt/vtgate/planbuilder/testdata/foreignkey_cases.json +++ b/go/vt/vtgate/planbuilder/testdata/foreignkey_cases.json @@ -1259,6 +1259,95 @@ ] } }, + { + "comment": "Update that cascades and requires parent fk and restrict child fk verification - bindVariable", + "query": "update u_tbl7 set col7 = :v1", + "plan": { + "QueryType": "UPDATE", + "Original": "update u_tbl7 set col7 = :v1", + "Instructions": { + "OperatorType": "FkCascade", + "Inputs": [ + { + "InputName": "Selection", + "OperatorType": "Route", + "Variant": "Unsharded", + "Keyspace": { + "Name": "unsharded_fk_allow", + "Sharded": false + }, + "FieldQuery": "select col7 from u_tbl7 where 1 != 1", + "Query": "select col7 from u_tbl7 for update", + "Table": "u_tbl7" + }, + { + "InputName": "CascadeChild-1", + "OperatorType": "FKVerify", + "BvName": "fkc_vals", + "Cols": [ + 0 + ], + "Inputs": [ + { + "InputName": "VerifyParent-1", + "OperatorType": "Route", + "Variant": "Unsharded", + "Keyspace": { + "Name": "unsharded_fk_allow", + "Sharded": false + }, + "FieldQuery": "select 1 from u_tbl4 left join u_tbl3 on u_tbl3.col3 = :v1 where 1 != 1", + "Query": "select 1 from u_tbl4 left join u_tbl3 on u_tbl3.col3 = :v1 where (u_tbl4.col4) in ::fkc_vals and u_tbl3.col3 is null limit 1 lock in share mode", + "Table": "u_tbl3, u_tbl4" + }, + { + "InputName": "VerifyChild-2", + "OperatorType": "Route", + "Variant": "Unsharded", + "Keyspace": { + "Name": "unsharded_fk_allow", + "Sharded": false + }, + "FieldQuery": "select 1 from u_tbl4, u_tbl9 where 1 != 1", + "Query": "select 1 from u_tbl4, u_tbl9 where (u_tbl4.col4) in ::fkc_vals and (:v1 is null or (u_tbl4.col4) not in ((:v1))) and u_tbl4.col4 = u_tbl9.col9 limit 1 lock in share mode", + "Table": "u_tbl4, u_tbl9" + }, + { + "InputName": "PostVerify", + "OperatorType": "Update", + "Variant": "Unsharded", + "Keyspace": { + "Name": "unsharded_fk_allow", + "Sharded": false + }, + "TargetTabletType": "PRIMARY", + "Query": "update /*+ SET_VAR(foreign_key_checks=OFF) */ u_tbl4 set col4 = :v1 where (u_tbl4.col4) in ::fkc_vals", + "Table": "u_tbl4" + } + ] + }, + { + "InputName": "Parent", + "OperatorType": "Update", + "Variant": "Unsharded", + "Keyspace": { + "Name": "unsharded_fk_allow", + "Sharded": false + }, + "TargetTabletType": "PRIMARY", + "Query": "update u_tbl7 set col7 = :v1", + "Table": "u_tbl7" + } + ] + }, + "TablesUsed": [ + "unsharded_fk_allow.u_tbl3", + "unsharded_fk_allow.u_tbl4", + "unsharded_fk_allow.u_tbl7", + "unsharded_fk_allow.u_tbl9" + ] + } + }, { "comment": "Insert with on duplicate key update - foreign keys disallowed", "query": "insert into u_tbl1 (id, col1) values (1, 3) on duplicate key update col1 = 5", @@ -1364,7 +1453,7 @@ "Sharded": false }, "TargetTabletType": "PRIMARY", - "Query": "update u_multicol_tbl2 set cola = null, colb = null where (cola, colb) in ::fkc_vals and (cola, colb) not in ((1, 2))", + "Query": "update u_multicol_tbl2 set cola = null, colb = null where (cola, colb) in ::fkc_vals and (1 is null or (cola, colb) not in ((1, 2)))", "Table": "u_multicol_tbl2" } ] @@ -1389,5 +1478,99 @@ "unsharded_fk_allow.u_multicol_tbl3" ] } + }, + { + "comment": "update on a multicol foreign key that set nulls and then cascades - bindVariables", + "query": "update u_multicol_tbl1 set cola = :v1, colb = :v2 where id = :v3", + "plan": { + "QueryType": "UPDATE", + "Original": "update u_multicol_tbl1 set cola = :v1, colb = :v2 where id = :v3", + "Instructions": { + "OperatorType": "FkCascade", + "Inputs": [ + { + "InputName": "Selection", + "OperatorType": "Route", + "Variant": "Unsharded", + "Keyspace": { + "Name": "unsharded_fk_allow", + "Sharded": false + }, + "FieldQuery": "select cola, colb from u_multicol_tbl1 where 1 != 1", + "Query": "select cola, colb from u_multicol_tbl1 where id = :v3 for update", + "Table": "u_multicol_tbl1" + }, + { + "InputName": "CascadeChild-1", + "OperatorType": "FkCascade", + "BvName": "fkc_vals", + "Cols": [ + 0, + 1 + ], + "Inputs": [ + { + "InputName": "Selection", + "OperatorType": "Route", + "Variant": "Unsharded", + "Keyspace": { + "Name": "unsharded_fk_allow", + "Sharded": false + }, + "FieldQuery": "select cola, colb from u_multicol_tbl2 where 1 != 1", + "Query": "select cola, colb from u_multicol_tbl2 where (cola, colb) in ::fkc_vals and (:v2 is null or (:v1 is null or (cola, colb) not in ((:v1, :v2)))) for update", + "Table": "u_multicol_tbl2" + }, + { + "InputName": "CascadeChild-1", + "OperatorType": "Update", + "Variant": "Unsharded", + "Keyspace": { + "Name": "unsharded_fk_allow", + "Sharded": false + }, + "TargetTabletType": "PRIMARY", + "BvName": "fkc_vals1", + "Cols": [ + 0, + 1 + ], + "Query": "update /*+ SET_VAR(foreign_key_checks=OFF) */ u_multicol_tbl3 set cola = null, colb = null where (cola, colb) in ::fkc_vals1", + "Table": "u_multicol_tbl3" + }, + { + "InputName": "Parent", + "OperatorType": "Update", + "Variant": "Unsharded", + "Keyspace": { + "Name": "unsharded_fk_allow", + "Sharded": false + }, + "TargetTabletType": "PRIMARY", + "Query": "update u_multicol_tbl2 set cola = null, colb = null where (cola, colb) in ::fkc_vals and (:v2 is null or (:v1 is null or (cola, colb) not in ((:v1, :v2))))", + "Table": "u_multicol_tbl2" + } + ] + }, + { + "InputName": "Parent", + "OperatorType": "Update", + "Variant": "Unsharded", + "Keyspace": { + "Name": "unsharded_fk_allow", + "Sharded": false + }, + "TargetTabletType": "PRIMARY", + "Query": "update u_multicol_tbl1 set cola = :v1, colb = :v2 where id = :v3", + "Table": "u_multicol_tbl1" + } + ] + }, + "TablesUsed": [ + "unsharded_fk_allow.u_multicol_tbl1", + "unsharded_fk_allow.u_multicol_tbl2", + "unsharded_fk_allow.u_multicol_tbl3" + ] + } } ] From 7d8518dd5982885265a8e6dcc9fdda2082aad3ad Mon Sep 17 00:00:00 2001 From: Manan Gupta Date: Fri, 22 Sep 2023 11:04:44 +0530 Subject: [PATCH 2/4] test: update test expectation Signed-off-by: Manan Gupta --- go/vt/vtgate/planbuilder/testdata/foreignkey_cases.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go/vt/vtgate/planbuilder/testdata/foreignkey_cases.json b/go/vt/vtgate/planbuilder/testdata/foreignkey_cases.json index 56c94c3053b..c41fcdf868f 100644 --- a/go/vt/vtgate/planbuilder/testdata/foreignkey_cases.json +++ b/go/vt/vtgate/planbuilder/testdata/foreignkey_cases.json @@ -1453,7 +1453,7 @@ "Sharded": false }, "TargetTabletType": "PRIMARY", - "Query": "update u_multicol_tbl2 set cola = null, colb = null where (cola, colb) in ::fkc_vals and (1 is null or (cola, colb) not in ((1, 2)))", + "Query": "update u_multicol_tbl2 set cola = null, colb = null where (cola, colb) in ::fkc_vals and (cola, colb) not in ((1, 2))", "Table": "u_multicol_tbl2" } ] From abeac2de06437cf910f7cbff2b237a3cdfc52f9a Mon Sep 17 00:00:00 2001 From: Manan Gupta Date: Fri, 22 Sep 2023 12:15:07 +0530 Subject: [PATCH 3/4] refactor: add code comments to explain it better Signed-off-by: Manan Gupta --- go/vt/vtgate/planbuilder/operators/update.go | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/go/vt/vtgate/planbuilder/operators/update.go b/go/vt/vtgate/planbuilder/operators/update.go index 89216c6a93d..88662b31d70 100644 --- a/go/vt/vtgate/planbuilder/operators/update.go +++ b/go/vt/vtgate/planbuilder/operators/update.go @@ -398,7 +398,7 @@ func buildChildUpdOpForCascade(ctx *plancontext.PlanningContext, fk vindexes.Chi // // `UPDATE SET // WHERE IN () -// [AND NOT IN ()]` +// [AND ({ IS NULL OR}... NOT IN ())]` func buildChildUpdOpForSetNull(ctx *plancontext.PlanningContext, fk vindexes.ChildFKInfo, updStmt *sqlparser.Update, childWhereExpr sqlparser.Expr, valTuple sqlparser.ValTuple) (ops.Operator, error) { // For the SET NULL type constraint, we need to set all the child columns to NULL. var childUpdateExprs sqlparser.UpdateExprs @@ -411,8 +411,13 @@ func buildChildUpdOpForSetNull(ctx *plancontext.PlanningContext, fk vindexes.Chi // SET NULL cascade should be avoided for the case where the parent columns remains unchanged on the update. // We need to add a condition to the where clause to handle this case. - // The additional condition looks like [AND NOT IN ()]. + // The additional condition looks like [AND ({ IS NULL OR}... NOT IN ())]. // If any of the parent columns is being set to NULL, then we don't need this condition. + // However, we don't necessarily know on Plan time if the Expr being updated to is NULL or not. Specifically, bindVariables in Prepared statements can be NULL on runtime. + // Therefore, in the condition we create, we also need to make it resilient to NULL values. Therefore we check if each individual value is NULL or not and OR it with the main condition. + // For example, if we are setting `update parent cola = :v1 and colb = :v2`, then on the child, the where condition would look something like this - + // `:v1 IS NULL OR :v2 IS NULL OR (child_cola, child_colb) NOT IN ((:v1,:v2))` + // So, if either of :v1 or :v2 is NULL, then the entire condition is true (which is the same as not having the condition when :v1 or :v2 is NULL). var updateValues sqlparser.ValTuple colSetToNull := false for _, updateExpr := range updStmt.Exprs { @@ -566,13 +571,13 @@ func createFkVerifyOpForParentFKForUpdate(ctx *plancontext.PlanningContext, updS } // Each child foreign key constraint is verified by a join query of the form: -// select 1 from child_tbl join parent_tbl on where [AND NOT IN ()] limit 1 +// select 1 from child_tbl join parent_tbl on where [AND ({ IS NULL OR}... NOT IN ())] limit 1 // E.g: // Child (c1, c2) references Parent (p1, p2) // update Parent set p1 = 1 where id = 1 // verify query: // select 1 from Child join Parent on Parent.p1 = Child.c1 and Parent.p2 = Child.c2 -// where Parent.id = 1 and (parent.p1) NOT IN ((1)) limit 1 +// where Parent.id = 1 and (1 IS NULL OR (parent.p1) NOT IN ((1))) limit 1 func createFkVerifyOpForChildFKForUpdate(ctx *plancontext.PlanningContext, updStmt *sqlparser.Update, cFk vindexes.ChildFKInfo) (ops.Operator, error) { // ON UPDATE RESTRICT foreign keys that require validation, should only be allowed in the case where we // are verifying all the FKs on vtgate level. @@ -608,8 +613,13 @@ func createFkVerifyOpForChildFKForUpdate(ctx *plancontext.PlanningContext, updSt // We don't want to fail the RESTRICT for the case where the parent columns remains unchanged on the update. // We need to add a condition to the where clause to handle this case. - // The additional condition looks like [AND NOT IN ()]. + // The additional condition looks like [AND ({ IS NULL OR}... NOT IN ())]. // If any of the parent columns is being set to NULL, then we don't need this condition. + // However, we don't necessarily know on Plan time if the Expr being updated to is NULL or not. Specifically, bindVariables in Prepared statements can be NULL on runtime. + // Therefore, in the condition we create, we also need to make it resilient to NULL values. Therefore we check if each individual value is NULL or not and OR it with the main condition. + // For example, if we are setting `update child cola = :v1 and colb = :v2`, then on the parent, the where condition would look something like this - + // `:v1 IS NULL OR :v2 IS NULL OR (parent_cola, parent_colb) NOT IN ((:v1,:v2))` + // So, if either of :v1 or :v2 is NULL, then the entire condition is true (which is the same as not having the condition when :v1 or :v2 is NULL). var updateValues sqlparser.ValTuple colSetToNull := false for _, updateExpr := range updStmt.Exprs { From c14abfe633bd74abd4256a2fcc731237fed5e4fe Mon Sep 17 00:00:00 2001 From: Manan Gupta Date: Tue, 26 Sep 2023 11:28:31 +0530 Subject: [PATCH 4/4] feat: refactor code to extract out common code Signed-off-by: Manan Gupta --- go/vt/vtgate/planbuilder/operators/update.go | 106 ++++++++---------- .../testdata/foreignkey_cases.json | 30 ++--- go/vt/vtgate/semantics/semantic_state.go | 4 + 3 files changed, 66 insertions(+), 74 deletions(-) diff --git a/go/vt/vtgate/planbuilder/operators/update.go b/go/vt/vtgate/planbuilder/operators/update.go index 88662b31d70..2a101438cb2 100644 --- a/go/vt/vtgate/planbuilder/operators/update.go +++ b/go/vt/vtgate/planbuilder/operators/update.go @@ -338,7 +338,7 @@ func createFkChildForUpdate(ctx *plancontext.PlanningContext, fk vindexes.ChildF case sqlparser.Cascade: childOp, err = buildChildUpdOpForCascade(ctx, fk, updStmt, childWhereExpr, updatedTable) case sqlparser.SetNull: - childOp, err = buildChildUpdOpForSetNull(ctx, fk, updStmt, childWhereExpr, valTuple) + childOp, err = buildChildUpdOpForSetNull(ctx, fk, updStmt, childWhereExpr) case sqlparser.SetDefault: return nil, vterrors.VT09016() } @@ -399,7 +399,7 @@ func buildChildUpdOpForCascade(ctx *plancontext.PlanningContext, fk vindexes.Chi // `UPDATE SET // WHERE IN () // [AND ({ IS NULL OR}... NOT IN ())]` -func buildChildUpdOpForSetNull(ctx *plancontext.PlanningContext, fk vindexes.ChildFKInfo, updStmt *sqlparser.Update, childWhereExpr sqlparser.Expr, valTuple sqlparser.ValTuple) (ops.Operator, error) { +func buildChildUpdOpForSetNull(ctx *plancontext.PlanningContext, fk vindexes.ChildFKInfo, updStmt *sqlparser.Update, childWhereExpr sqlparser.Expr) (ops.Operator, error) { // For the SET NULL type constraint, we need to set all the child columns to NULL. var childUpdateExprs sqlparser.UpdateExprs for _, column := range fk.ChildColumns { @@ -418,32 +418,11 @@ func buildChildUpdOpForSetNull(ctx *plancontext.PlanningContext, fk vindexes.Chi // For example, if we are setting `update parent cola = :v1 and colb = :v2`, then on the child, the where condition would look something like this - // `:v1 IS NULL OR :v2 IS NULL OR (child_cola, child_colb) NOT IN ((:v1,:v2))` // So, if either of :v1 or :v2 is NULL, then the entire condition is true (which is the same as not having the condition when :v1 or :v2 is NULL). - var updateValues sqlparser.ValTuple - colSetToNull := false - for _, updateExpr := range updStmt.Exprs { - colIdx := fk.ParentColumns.FindColumn(updateExpr.Name.Name) - if colIdx >= 0 { - if sqlparser.IsNull(updateExpr.Expr) { - colSetToNull = true - break - } - updateValues = append(updateValues, updateExpr.Expr) - } - } - if !colSetToNull { - var finalExpr sqlparser.Expr = sqlparser.NewComparisonExpr(sqlparser.NotInOp, valTuple, sqlparser.ValTuple{updateValues}, nil) - for _, value := range updateValues { - finalExpr = &sqlparser.OrExpr{ - Left: &sqlparser.IsExpr{ - Left: value, - Right: sqlparser.IsNullOp, - }, - Right: finalExpr, - } - } + compExpr := nullSafeNotInComparison(updStmt.Exprs, fk) + if compExpr != nil { childWhereExpr = &sqlparser.AndExpr{ Left: childWhereExpr, - Right: finalExpr, + Right: compExpr, } } childUpdStmt := &sqlparser.Update{ @@ -571,13 +550,13 @@ func createFkVerifyOpForParentFKForUpdate(ctx *plancontext.PlanningContext, updS } // Each child foreign key constraint is verified by a join query of the form: -// select 1 from child_tbl join parent_tbl on where [AND ({ IS NULL OR}... NOT IN ())] limit 1 +// select 1 from child_tbl join parent_tbl on where [AND ({ IS NULL OR}... NOT IN ())] limit 1 // E.g: // Child (c1, c2) references Parent (p1, p2) // update Parent set p1 = 1 where id = 1 // verify query: // select 1 from Child join Parent on Parent.p1 = Child.c1 and Parent.p2 = Child.c2 -// where Parent.id = 1 and (1 IS NULL OR (parent.p1) NOT IN ((1))) limit 1 +// where Parent.id = 1 and (1 IS NULL OR (child.c1) NOT IN ((1))) limit 1 func createFkVerifyOpForChildFKForUpdate(ctx *plancontext.PlanningContext, updStmt *sqlparser.Update, cFk vindexes.ChildFKInfo) (ops.Operator, error) { // ON UPDATE RESTRICT foreign keys that require validation, should only be allowed in the case where we // are verifying all the FKs on vtgate level. @@ -613,42 +592,16 @@ func createFkVerifyOpForChildFKForUpdate(ctx *plancontext.PlanningContext, updSt // We don't want to fail the RESTRICT for the case where the parent columns remains unchanged on the update. // We need to add a condition to the where clause to handle this case. - // The additional condition looks like [AND ({ IS NULL OR}... NOT IN ())]. + // The additional condition looks like [AND ({ IS NULL OR}... NOT IN ())]. // If any of the parent columns is being set to NULL, then we don't need this condition. // However, we don't necessarily know on Plan time if the Expr being updated to is NULL or not. Specifically, bindVariables in Prepared statements can be NULL on runtime. // Therefore, in the condition we create, we also need to make it resilient to NULL values. Therefore we check if each individual value is NULL or not and OR it with the main condition. // For example, if we are setting `update child cola = :v1 and colb = :v2`, then on the parent, the where condition would look something like this - - // `:v1 IS NULL OR :v2 IS NULL OR (parent_cola, parent_colb) NOT IN ((:v1,:v2))` + // `:v1 IS NULL OR :v2 IS NULL OR (cola, colb) NOT IN ((:v1,:v2))` // So, if either of :v1 or :v2 is NULL, then the entire condition is true (which is the same as not having the condition when :v1 or :v2 is NULL). - var updateValues sqlparser.ValTuple - colSetToNull := false - for _, updateExpr := range updStmt.Exprs { - colIdx := cFk.ParentColumns.FindColumn(updateExpr.Name.Name) - if colIdx >= 0 { - if sqlparser.IsNull(updateExpr.Expr) { - colSetToNull = true - break - } - updateValues = append(updateValues, updateExpr.Expr) - } - } - if !colSetToNull { - // Create a ValTuple of child column names - var valTuple sqlparser.ValTuple - for _, column := range cFk.ParentColumns { - valTuple = append(valTuple, sqlparser.NewColNameWithQualifier(column.String(), parentTbl)) - } - var finalExpr sqlparser.Expr = sqlparser.NewComparisonExpr(sqlparser.NotInOp, valTuple, sqlparser.ValTuple{updateValues}, nil) - for _, value := range updateValues { - finalExpr = &sqlparser.OrExpr{ - Left: &sqlparser.IsExpr{ - Left: value, - Right: sqlparser.IsNullOp, - }, - Right: finalExpr, - } - } - whereCond = sqlparser.AndExpressions(whereCond, finalExpr) + compExpr := nullSafeNotInComparison(updStmt.Exprs, cFk) + if compExpr != nil { + whereCond = sqlparser.AndExpressions(whereCond, compExpr) } return createSelectionOp(ctx, @@ -664,3 +617,38 @@ func createFkVerifyOpForChildFKForUpdate(ctx *plancontext.PlanningContext, updSt sqlparser.NewLimitWithoutOffset(1), sqlparser.ShareModeLock) } + +// nullSafeNotInComparison is used to compare the child columns in the foreign key constraint aren't the same as the updateExpressions exactly. +// This comparison has to be null safe so we create an expression which looks like the following for a query like `update child cola = :v1 and colb = :v2` - +// `:v1 IS NULL OR :v2 IS NULL OR (cola, colb) NOT IN ((:v1,:v2))` +// So, if either of :v1 or :v2 is NULL, then the entire condition is true (which is the same as not having the condition when :v1 or :v2 is NULL) +// This expression is used in cascading SET NULLs and in verifying whether an update should be restricted. +func nullSafeNotInComparison(updateExprs sqlparser.UpdateExprs, cFk vindexes.ChildFKInfo) sqlparser.Expr { + var updateValues sqlparser.ValTuple + for _, updateExpr := range updateExprs { + colIdx := cFk.ParentColumns.FindColumn(updateExpr.Name.Name) + if colIdx >= 0 { + if sqlparser.IsNull(updateExpr.Expr) { + return nil + } + updateValues = append(updateValues, updateExpr.Expr) + } + } + // Create a ValTuple of child column names + var valTuple sqlparser.ValTuple + for _, column := range cFk.ChildColumns { + valTuple = append(valTuple, sqlparser.NewColNameWithQualifier(column.String(), cFk.Table.GetTableName())) + } + var finalExpr sqlparser.Expr = sqlparser.NewComparisonExpr(sqlparser.NotInOp, valTuple, sqlparser.ValTuple{updateValues}, nil) + for _, value := range updateValues { + finalExpr = &sqlparser.OrExpr{ + Left: &sqlparser.IsExpr{ + Left: value, + Right: sqlparser.IsNullOp, + }, + Right: finalExpr, + } + } + + return finalExpr +} diff --git a/go/vt/vtgate/planbuilder/testdata/foreignkey_cases.json b/go/vt/vtgate/planbuilder/testdata/foreignkey_cases.json index c41fcdf868f..3c03a4740f6 100644 --- a/go/vt/vtgate/planbuilder/testdata/foreignkey_cases.json +++ b/go/vt/vtgate/planbuilder/testdata/foreignkey_cases.json @@ -328,7 +328,7 @@ "Cols": [ 0 ], - "Query": "update u_tbl3 set col3 = null where (col3) in ::fkc_vals and (col3) not in (('bar'))", + "Query": "update u_tbl3 set col3 = null where (col3) in ::fkc_vals and (u_tbl3.col3) not in (('bar'))", "Table": "u_tbl3" }, { @@ -439,7 +439,7 @@ "Cols": [ 0 ], - "Query": "update tbl4 set t4col4 = null where (t4col4) in ::fkc_vals and (t4col4) not in (('foo'))", + "Query": "update tbl4 set t4col4 = null where (t4col4) in ::fkc_vals and (tbl4.t4col4) not in (('foo'))", "Table": "tbl4" }, { @@ -693,7 +693,7 @@ "Cols": [ 0 ], - "Query": "update u_tbl3 set col3 = null where (col3) in ::fkc_vals1 and (col3) not in (('foo'))", + "Query": "update u_tbl3 set col3 = null where (col3) in ::fkc_vals1 and (u_tbl3.col3) not in (('foo'))", "Table": "u_tbl3" }, { @@ -727,7 +727,7 @@ "Sharded": false }, "FieldQuery": "select col9 from u_tbl9 where 1 != 1", - "Query": "select col9 from u_tbl9 where (col9) in ::fkc_vals2 and (col9) not in (('foo')) for update", + "Query": "select col9 from u_tbl9 where (col9) in ::fkc_vals2 and (u_tbl9.col9) not in (('foo')) for update", "Table": "u_tbl9" }, { @@ -755,7 +755,7 @@ "Sharded": false }, "TargetTabletType": "PRIMARY", - "Query": "update u_tbl9 set col9 = null where (col9) in ::fkc_vals2 and (col9) not in (('foo'))", + "Query": "update u_tbl9 set col9 = null where (col9) in ::fkc_vals2 and (u_tbl9.col9) not in (('foo'))", "Table": "u_tbl9" } ] @@ -832,7 +832,7 @@ "Cols": [ 0 ], - "Query": "update u_tbl3 set col3 = null where (col3) in ::fkc_vals and (col3) not in ((2))", + "Query": "update u_tbl3 set col3 = null where (col3) in ::fkc_vals and (u_tbl3.col3) not in ((2))", "Table": "u_tbl3" }, { @@ -909,7 +909,7 @@ "Cols": [ 0 ], - "Query": "update u_tbl3 set col3 = null where (col3) in ::fkc_vals1 and (col3) not in ((2))", + "Query": "update u_tbl3 set col3 = null where (col3) in ::fkc_vals1 and (u_tbl3.col3) not in ((2))", "Table": "u_tbl3" }, { @@ -943,7 +943,7 @@ "Sharded": false }, "FieldQuery": "select col9 from u_tbl9 where 1 != 1", - "Query": "select col9 from u_tbl9 where (col9) in ::fkc_vals2 and (col9) not in ((2)) for update", + "Query": "select col9 from u_tbl9 where (col9) in ::fkc_vals2 and (u_tbl9.col9) not in ((2)) for update", "Table": "u_tbl9" }, { @@ -971,7 +971,7 @@ "Sharded": false }, "TargetTabletType": "PRIMARY", - "Query": "update u_tbl9 set col9 = null where (col9) in ::fkc_vals2 and (col9) not in ((2))", + "Query": "update u_tbl9 set col9 = null where (col9) in ::fkc_vals2 and (u_tbl9.col9) not in ((2))", "Table": "u_tbl9" } ] @@ -1220,7 +1220,7 @@ "Sharded": false }, "FieldQuery": "select 1 from u_tbl4, u_tbl9 where 1 != 1", - "Query": "select 1 from u_tbl4, u_tbl9 where (u_tbl4.col4) in ::fkc_vals and (u_tbl4.col4) not in (('foo')) and u_tbl4.col4 = u_tbl9.col9 limit 1 lock in share mode", + "Query": "select 1 from u_tbl4, u_tbl9 where (u_tbl4.col4) in ::fkc_vals and (u_tbl9.col9) not in (('foo')) and u_tbl4.col4 = u_tbl9.col9 limit 1 lock in share mode", "Table": "u_tbl4, u_tbl9" }, { @@ -1309,7 +1309,7 @@ "Sharded": false }, "FieldQuery": "select 1 from u_tbl4, u_tbl9 where 1 != 1", - "Query": "select 1 from u_tbl4, u_tbl9 where (u_tbl4.col4) in ::fkc_vals and (:v1 is null or (u_tbl4.col4) not in ((:v1))) and u_tbl4.col4 = u_tbl9.col9 limit 1 lock in share mode", + "Query": "select 1 from u_tbl4, u_tbl9 where (u_tbl4.col4) in ::fkc_vals and (:v1 is null or (u_tbl9.col9) not in ((:v1))) and u_tbl4.col4 = u_tbl9.col9 limit 1 lock in share mode", "Table": "u_tbl4, u_tbl9" }, { @@ -1424,7 +1424,7 @@ "Sharded": false }, "FieldQuery": "select cola, colb from u_multicol_tbl2 where 1 != 1", - "Query": "select cola, colb from u_multicol_tbl2 where (cola, colb) in ::fkc_vals and (cola, colb) not in ((1, 2)) for update", + "Query": "select cola, colb from u_multicol_tbl2 where (cola, colb) in ::fkc_vals and (u_multicol_tbl2.cola, u_multicol_tbl2.colb) not in ((1, 2)) for update", "Table": "u_multicol_tbl2" }, { @@ -1453,7 +1453,7 @@ "Sharded": false }, "TargetTabletType": "PRIMARY", - "Query": "update u_multicol_tbl2 set cola = null, colb = null where (cola, colb) in ::fkc_vals and (cola, colb) not in ((1, 2))", + "Query": "update u_multicol_tbl2 set cola = null, colb = null where (cola, colb) in ::fkc_vals and (u_multicol_tbl2.cola, u_multicol_tbl2.colb) not in ((1, 2))", "Table": "u_multicol_tbl2" } ] @@ -1518,7 +1518,7 @@ "Sharded": false }, "FieldQuery": "select cola, colb from u_multicol_tbl2 where 1 != 1", - "Query": "select cola, colb from u_multicol_tbl2 where (cola, colb) in ::fkc_vals and (:v2 is null or (:v1 is null or (cola, colb) not in ((:v1, :v2)))) for update", + "Query": "select cola, colb from u_multicol_tbl2 where (cola, colb) in ::fkc_vals and (:v2 is null or (:v1 is null or (u_multicol_tbl2.cola, u_multicol_tbl2.colb) not in ((:v1, :v2)))) for update", "Table": "u_multicol_tbl2" }, { @@ -1547,7 +1547,7 @@ "Sharded": false }, "TargetTabletType": "PRIMARY", - "Query": "update u_multicol_tbl2 set cola = null, colb = null where (cola, colb) in ::fkc_vals and (:v2 is null or (:v1 is null or (cola, colb) not in ((:v1, :v2))))", + "Query": "update u_multicol_tbl2 set cola = null, colb = null where (cola, colb) in ::fkc_vals and (:v2 is null or (:v1 is null or (u_multicol_tbl2.cola, u_multicol_tbl2.colb) not in ((:v1, :v2))))", "Table": "u_multicol_tbl2" } ] diff --git a/go/vt/vtgate/semantics/semantic_state.go b/go/vt/vtgate/semantics/semantic_state.go index 6f3a4962961..61bd8c372f0 100644 --- a/go/vt/vtgate/semantics/semantic_state.go +++ b/go/vt/vtgate/semantics/semantic_state.go @@ -488,6 +488,10 @@ func (st *SemTable) SingleUnshardedKeyspace() (*vindexes.Keyspace, []*vindexes.T // The expression in the select list is not equal to the one in the ORDER BY, // but they point to the same column and would be considered equal by this method func (st *SemTable) EqualsExpr(a, b sqlparser.Expr) bool { + // If there is no SemTable, then we cannot compare the expressions. + if st == nil { + return false + } return st.ASTEquals().Expr(a, b) }