Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Fk verification and update queries to accommodate for bindVariables being NULL #14061

Merged
merged 6 commits into from
Sep 26, 2023
Merged

Fix Fk verification and update queries to accommodate for bindVariables being NULL #14061

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
f828764
feat: fix queries to take care of bindvariables which can be NULL val…
GuptaManan100 Sep 21, 2023
a14cd2f
Merge remote-tracking branch 'upstream/main' into not-null-fk-fix
GuptaManan100 Sep 21, 2023
7db4f56
Merge remote-tracking branch 'upstream/main' into not-null-fk-fix
GuptaManan100 Sep 22, 2023
7d8518d
test: update test expectation
GuptaManan100 Sep 22, 2023
abeac2d
refactor: add code comments to explain it better
GuptaManan100 Sep 22, 2023
c14abfe
feat: refactor code to extract out common code
GuptaManan100 Sep 26, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 37 additions & 7 deletions go/vt/vtgate/planbuilder/operators/update.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -398,7 +398,7 @@ func buildChildUpdOpForCascade(ctx *plancontext.PlanningContext, fk vindexes.Chi
//
// `UPDATE <child_table> SET <child_column_updated_using_update_exprs_from_parent_update_query>
// WHERE <child_columns_in_fk> IN (<bind variable for the output from SELECT>)
// [AND <child_columns_in_fk> NOT IN (<bind variables in the SET clause of the original update>)]`
// [AND ({<bind variables in the SET clause of the original update> IS NULL OR}... <child_columns_in_fk> NOT IN (<bind variables in the SET clause of the original update>))]`
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
Expand All @@ -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 <child_columns_in_fk> NOT IN (<bind variables in the SET clause of the original update>)].
// The additional condition looks like [AND ({<bind variables in the SET clause of the original update> IS NULL OR}... <child_columns_in_fk> NOT IN (<bind variables in the SET clause of the original update>))].
// 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 {
Expand All @@ -426,9 +431,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{
Expand Down Expand Up @@ -556,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 <columns in fk> where <clause same as original update> [AND <parent_columns_in_fk> NOT IN (<bind variables in the SET clause of the original update>)] limit 1
// select 1 from child_tbl join parent_tbl on <columns in fk> where <clause same as original update> [AND ({<bind variables in the SET clause of the original update> IS NULL OR}... <parent_columns_in_fk> NOT IN (<bind variables in the SET clause of the original update>))] 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.
Expand Down Expand Up @@ -598,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 <parent_columns_in_fk> NOT IN (<bind variables in the SET clause of the original update>)].
// The additional condition looks like [AND ({<bind variables in the SET clause of the original update> IS NULL OR}... <parent_columns_in_fk> NOT IN (<bind variables in the SET clause of the original update>))].
// 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 {
Expand All @@ -618,7 +638,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)
GuptaManan100 marked this conversation as resolved.
Show resolved Hide resolved
}

return createSelectionOp(ctx,
Expand Down
183 changes: 183 additions & 0 deletions go/vt/vtgate/planbuilder/testdata/foreignkey_cases.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -1390,6 +1479,100 @@
]
}
},
{
"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",
GuptaManan100 marked this conversation as resolved.
Show resolved Hide resolved
"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"
]
}
},
{
"comment": "Cascaded delete run from prepared statement",
"query": "execute prep_delete using @foo",
Expand Down
Loading