Skip to content

Commit

Permalink
fix: fail for dependent column update on same statement
Browse files Browse the repository at this point in the history
Signed-off-by: Harshit Gangal <[email protected]>
  • Loading branch information
harshit-gangal committed May 21, 2024
1 parent f863651 commit be4fd87
Show file tree
Hide file tree
Showing 2 changed files with 42 additions and 10 deletions.
47 changes: 37 additions & 10 deletions go/vt/vtgate/planbuilder/operators/update.go
Original file line number Diff line number Diff line change
Expand Up @@ -168,16 +168,7 @@ func createUpdateWithInputOp(ctx *plancontext.PlanningContext, upd *sqlparser.Up
upd.Limit = nil

// Prepare the update expressions list
// Any update expression requiring column value from any other table is rewritten to take it as bindvar column.
// E.g. UPDATE t1 join t2 on t1.col = t2.col SET t1.col = t2.col + 1 where t2.col = 10;
// SET t1.col = t2.col + 1 -> SET t1.col = :t2_col + 1 (t2_col is the bindvar column which will be provided from the input)
ueMap := make(map[semantics.TableSet]updList)
for _, ue := range upd.Exprs {
target := ctx.SemTable.DirectDeps(ue.Name)
exprDeps := ctx.SemTable.RecursiveDeps(ue.Expr)
jc := breakExpressionInLHSandRHS(ctx, ue.Expr, exprDeps.Remove(target))
ueMap[target] = append(ueMap[target], updColumn{ue.Name, jc})
}
ueMap := prepareUpdateExpressionList(ctx, upd)

var updOps []dmlOp
for _, target := range ctx.SemTable.Targets.Constituents() {
Expand Down Expand Up @@ -223,6 +214,42 @@ func createUpdateWithInputOp(ctx *plancontext.PlanningContext, upd *sqlparser.Up
return op
}

func prepareUpdateExpressionList(ctx *plancontext.PlanningContext, upd *sqlparser.Update) map[semantics.TableSet]updList {
// Any update expression requiring column value from any other table is rewritten to take it as bindvar column.
// E.g. UPDATE t1 join t2 on t1.col = t2.col SET t1.col = t2.col + 1 where t2.col = 10;
// SET t1.col = t2.col + 1 -> SET t1.col = :t2_col + 1 (t2_col is the bindvar column which will be provided from the input)
ueMap := make(map[semantics.TableSet]updList)
var dependentCols updList
for _, ue := range upd.Exprs {
target := ctx.SemTable.DirectDeps(ue.Name)
exprDeps := ctx.SemTable.RecursiveDeps(ue.Expr)
jc := breakExpressionInLHSandRHS(ctx, ue.Expr, exprDeps.Remove(target))
updCol := updColumn{ue.Name, jc}
ueMap[target] = append(ueMap[target], updCol)
dependentCols = append(dependentCols, updCol)
}

// Check if any of the dependent columns are updated in the same query.
// This can result in a mismatch of rows on how MySQL interprets it and how Vitess would have updated those rows.
// It is safe to fail for those cases.
errIfDependentColumnUpdated(ctx, upd, dependentCols)

return ueMap
}

func errIfDependentColumnUpdated(ctx *plancontext.PlanningContext, upd *sqlparser.Update, dependentCols updList) {
for _, ue := range upd.Exprs {
for _, dc := range dependentCols {
for _, bvExpr := range dc.jc.LHSExprs {
if ctx.SemTable.EqualsExprWithDeps(ue.Name, bvExpr.Expr) {
panic(vterrors.VT12001(
fmt.Sprintf("'%s' column referenced in update expression '%s' is itself updated", sqlparser.String(ue.Name), sqlparser.String(dc.jc.Original))))
}
}
}
}
}

func createUpdateOpWithTarget(ctx *plancontext.PlanningContext, updStmt *sqlparser.Update, target semantics.TableSet, uList updList) dmlOp {
if len(uList) == 0 {
panic(vterrors.VT13001("no update expression for the target"))
Expand Down
5 changes: 5 additions & 0 deletions go/vt/vtgate/planbuilder/testdata/unsupported_cases.json
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,11 @@
"query": "update user_metadata set email = '[email protected]' where user_id = 1 limit 10",
"plan": "VT12001: unsupported: Vindex update should have ORDER BY clause when using LIMIT"
},
{
"comment": "multi table update with dependent column getting updated",
"query": "update user u, user_extra ue set u.name = 'test' + ue.col, ue.col = 5 where u.id = ue.id and u.id = 1;",
"plan": "VT12001: unsupported: 'ue.col' column referenced in update expression ''test' + ue.col' is itself updated"
},
{
"comment": "unsharded insert, col list does not match values",
"query": "insert into unsharded_auto(id, val) values(1)",
Expand Down

0 comments on commit be4fd87

Please sign in to comment.