Skip to content

Commit

Permalink
feat: augment verification queries to work with non-literal values in…
Browse files Browse the repository at this point in the history 
… updates

Signed-off-by: Manan Gupta <[email protected]>
  • Loading branch information
@GuptaManan100
GuptaManan100 committed Oct 20, 2023
1 parent 465df5d commit a1b04e4
Show file tree
Hide file tree
Showing 3 changed files with 196 additions and 17 deletions.
26 changes: 26 additions & 0 deletions go/test/endtoend/vtgate/foreignkey/fk_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -808,6 +808,32 @@ func TestFkQueries(t *testing.T) {
"insert into fk_t11 (id, col) values (1,1),(2,2),(3,3),(4,4),(5,5)",
"update fk_t10 set col = id + 3 order by id desc",
},
}, {
name: "Non-literal update with order by that require parent and child foreign keys verification - success",
queries: []string{
"insert into fk_t10 (id, col) values (1,1),(2,2),(3,3),(4,4),(5,5),(6,6),(7,7),(8,8)",
"insert into fk_t11 (id, col) values (1,1),(2,2),(3,3),(4,4),(5,5)",
"insert into fk_t12 (id, col) values (1,1),(2,2),(3,3),(4,4),(5,5)",
"insert into fk_t13 (id, col) values (1,1),(2,2)",
"update fk_t11 set col = id + 3 where id >= 3",
},
}, {
name: "Non-literal update with order by that require parent and child foreign keys verification - parent fails",
queries: []string{
"insert into fk_t10 (id, col) values (1,1),(2,2),(3,3),(4,4),(5,5)",
"insert into fk_t11 (id, col) values (1,1),(2,2),(3,3),(4,4),(5,5)",
"insert into fk_t12 (id, col) values (1,1),(2,2),(3,3),(4,4),(5,5)",
"update fk_t11 set col = id + 3",
},
}, {
name: "Non-literal update with order by that require parent and child foreign keys verification - child fails",
queries: []string{
"insert into fk_t10 (id, col) values (1,1),(2,2),(3,3),(4,4),(5,5),(6,6),(7,7),(8,8)",
"insert into fk_t11 (id, col) values (1,1),(2,2),(3,3),(4,4),(5,5)",
"insert into fk_t12 (id, col) values (1,1),(2,2),(3,3),(4,4),(5,5)",
"insert into fk_t13 (id, col) values (1,1),(2,2)",
"update fk_t11 set col = id + 3",
},
},
}

Expand Down
17 changes: 6 additions & 11 deletions go/vt/vtgate/planbuilder/operators/update.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -470,7 +470,7 @@ 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).
compExpr := nullSafeNotInComparison(ctx.SemTable.ChildFkToUpdExprs[fk.String(updatedTable)], fk, updateExprBvNames)
compExpr := nullSafeNotInComparison(ctx.SemTable.ChildFkToUpdExprs[fk.String(updatedTable)], fk, updatedTable.GetTableName(), updateExprBvNames)
if compExpr != nil {
childWhereExpr = &sqlparser.AndExpr{
Left: childWhereExpr,
Expand All @@ -491,11 +491,6 @@ func createFKVerifyOp(ctx *plancontext.PlanningContext, childOp ops.Operator, up
return childOp, nil
}

// We only support simple expressions in update queries for foreign key verification.
if semantics.HasNonLiteral(updStmt.Exprs, nil, restrictChildFks) {
return nil, vterrors.VT12001("update expression with non-literal values with foreign key constraints")
}

var Verify []*VerifyOp
// This validates that new values exists on the parent table.
for _, fk := range parentFks {
Expand Down Expand Up @@ -619,10 +614,10 @@ func createFkVerifyOpForParentFKForUpdate(ctx *plancontext.PlanningContext, updS
// 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}... <child_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
// update Parent set p1 = col + 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 (child.c1) NOT IN ((1))) limit 1
// where Parent.id = 1 and ((Parent.col + 1) IS NULL OR (child.c1) NOT IN ((Parent.col + 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 @@ -665,7 +660,7 @@ func createFkVerifyOpForChildFKForUpdate(ctx *plancontext.PlanningContext, updSt
// 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 (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).
compExpr := nullSafeNotInComparison(updStmt.Exprs, cFk, nil)
compExpr := nullSafeNotInComparison(updStmt.Exprs, cFk, parentTbl, nil)
if compExpr != nil {
whereCond = sqlparser.AndExpressions(whereCond, compExpr)
}
Expand All @@ -690,15 +685,15 @@ func createFkVerifyOpForChildFKForUpdate(ctx *plancontext.PlanningContext, updSt
// `: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, updatedExprBvNames []string) sqlparser.Expr {
func nullSafeNotInComparison(updateExprs sqlparser.UpdateExprs, cFk vindexes.ChildFKInfo, parentTbl sqlparser.TableName, updatedExprBvNames []string) sqlparser.Expr {
var updateValues sqlparser.ValTuple
for idx, updateExpr := range updateExprs {
colIdx := cFk.ParentColumns.FindColumn(updateExpr.Name.Name)
if colIdx >= 0 {
if sqlparser.IsNull(updateExpr.Expr) {
return nil
}
childUpdateExpr := updateExpr.Expr
childUpdateExpr := prefixColNames(parentTbl, updateExpr.Expr)
if len(updatedExprBvNames) > 0 && updatedExprBvNames[idx] != "" {
childUpdateExpr = sqlparser.NewArgument(updatedExprBvNames[idx])
}
Expand Down
170 changes: 164 additions & 6 deletions go/vt/vtgate/planbuilder/testdata/foreignkey_cases.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -791,7 +791,87 @@
{
"comment": "update in a table with non-literal value - set null fail due to child update where condition",
"query": "update u_tbl2 set m = 2, col2 = col1 + 'bar' where id = 1",
"plan": "VT12001: unsupported: update expression with non-literal values with foreign key constraints"
"plan": {
"QueryType": "UPDATE",
"Original": "update u_tbl2 set m = 2, col2 = col1 + 'bar' where id = 1",
"Instructions": {
"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 = u_tbl2.col1 + 'bar' where 1 != 1",
"Query": "select 1 from u_tbl2 left join u_tbl1 on u_tbl1.col1 = u_tbl2.col1 + 'bar' where u_tbl2.col1 + 'bar' is not null and u_tbl2.id = 1 and u_tbl1.col1 is null limit 1 lock in share mode",
"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 col2, u_tbl2.col1 + 'bar', col2 != u_tbl2.col1 + 'bar' from u_tbl2 where 1 != 1",
"Query": "select col2, u_tbl2.col1 + 'bar', col2 != u_tbl2.col1 + 'bar' from u_tbl2 where u_tbl2.id = 1 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
],
"CompExprCols": [
2
],
"Query": "update u_tbl3 set col3 = null where (col3) in ::fkc_vals and (:fkc_upd is null or (u_tbl3.col3) not in ((:fkc_upd)))",
"Table": "u_tbl3",
"UpdateExprBvNames": [
"fkc_upd"
],
"UpdateExprCols": [
1
]
},
{
"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 m = 2, col2 = u_tbl2.col1 + 'bar' where u_tbl2.id = 1",
"Table": "u_tbl2"
}
]
}
]
},
"TablesUsed": [
"unsharded_fk_allow.u_tbl1",
"unsharded_fk_allow.u_tbl2",
"unsharded_fk_allow.u_tbl3"
]
}
},
{
"comment": "update in a table with non-literal value - with cascade fail as the cascade value is not known",
Expand All @@ -810,8 +890,8 @@
"Name": "unsharded_fk_allow",
"Sharded": false
},
"FieldQuery": "select col1, x + 'bar', col1 != x + 'bar' from u_tbl1 where 1 != 1",
"Query": "select col1, x + 'bar', col1 != x + 'bar' from u_tbl1 where id = 1 for update",
"FieldQuery": "select col1, u_tbl1.x + 'bar', col1 != u_tbl1.x + 'bar' from u_tbl1 where 1 != 1",
"Query": "select col1, u_tbl1.x + 'bar', col1 != u_tbl1.x + 'bar' from u_tbl1 where id = 1 for update",
"Table": "u_tbl1"
},
{
Expand Down Expand Up @@ -941,7 +1021,7 @@
"Sharded": false
},
"TargetTabletType": "PRIMARY",
"Query": "update /*+ SET_VAR(foreign_key_checks=OFF) */ u_tbl1 set m = 2, col1 = x + 'bar' where id = 1",
"Query": "update /*+ SET_VAR(foreign_key_checks=OFF) */ u_tbl1 set m = 2, col1 = u_tbl1.x + 'bar' where id = 1",
"Table": "u_tbl1"
}
]
Expand Down Expand Up @@ -1169,7 +1249,85 @@
{
"comment": "update with fk on cross-shard with a where condition on non-literal value - disallowed",
"query": "update tbl3 set coly = colx + 10 where coly = 10",
"plan": "VT12001: unsupported: update expression with non-literal values with foreign key constraints"
"plan": {
"QueryType": "UPDATE",
"Original": "update tbl3 set coly = colx + 10 where coly = 10",
"Instructions": {
"OperatorType": "FKVerify",
"Inputs": [
{
"InputName": "VerifyParent-1",
"OperatorType": "Limit",
"Count": "INT64(1)",
"Inputs": [
{
"OperatorType": "Projection",
"Expressions": [
"INT64(1) as 1"
],
"Inputs": [
{
"OperatorType": "Filter",
"Predicate": "tbl1.t1col1 is null",
"Inputs": [
{
"OperatorType": "Join",
"Variant": "LeftJoin",
"JoinColumnIndexes": "R:0,R:0",
"JoinVars": {
"tbl3_colx": 0
},
"TableName": "tbl3_tbl1",
"Inputs": [
{
"OperatorType": "Route",
"Variant": "Scatter",
"Keyspace": {
"Name": "sharded_fk_allow",
"Sharded": true
},
"FieldQuery": "select tbl3.colx from tbl3 where 1 != 1",
"Query": "select tbl3.colx from tbl3 where tbl3.colx + 10 is not null and tbl3.coly = 10 lock in share mode",
"Table": "tbl3"
},
{
"OperatorType": "Route",
"Variant": "Scatter",
"Keyspace": {
"Name": "sharded_fk_allow",
"Sharded": true
},
"FieldQuery": "select tbl1.t1col1 from tbl1 where 1 != 1",
"Query": "select tbl1.t1col1 from tbl1 where tbl1.t1col1 = :tbl3_colx + 10 lock in share mode",
"Table": "tbl1"
}
]
}
]
}
]
}
]
},
{
"InputName": "PostVerify",
"OperatorType": "Update",
"Variant": "Scatter",
"Keyspace": {
"Name": "sharded_fk_allow",
"Sharded": true
},
"TargetTabletType": "PRIMARY",
"Query": "update /*+ SET_VAR(foreign_key_checks=OFF) */ tbl3 set coly = tbl3.colx + 10 where tbl3.coly = 10",
"Table": "tbl3"
}
]
},
"TablesUsed": [
"sharded_fk_allow.tbl1",
"sharded_fk_allow.tbl3"
]
}
},
{
"comment": "update with fk on cross-shard with a where condition",
Expand Down Expand Up @@ -1454,7 +1612,7 @@
"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",
"Query": "select 1 from u_tbl4 left join u_tbl3 on u_tbl3.col3 = :v1 where (u_tbl4.col4) in ::fkc_vals and :v1 is not null and u_tbl3.col3 is null limit 1 lock in share mode",
"Table": "u_tbl3, u_tbl4"
},
{
Expand Down

0 comments on commit a1b04e4

Please sign in to comment.