Skip to content

Commit fe14d97

Browse files
GuptaManan100GuptaManan100
authored
Fix type coercion in cascading non-literal updates (#14524)
Signed-off-by: Manan Gupta <[email protected]>
1 parent ae5d2de commit fe14d97

File tree

5 files changed

+59
-22
lines changed

5 files changed

+59
-22
lines changed

‎go/test/endtoend/vtgate/foreignkey/fk_test.go

+7
Original file line numberDiff line numberDiff line change
@@ -880,6 +880,13 @@ func TestFkQueries(t *testing.T) {
880880
"insert into fk_multicol_t16 (id, cola, colb) values (1,1,1),(2,2,2)",
881881
"update fk_multicol_t15 set cola = 3, colb = (id * 2) - 2",
882882
},
883+
}, {
884+
name: "Update that sets to 0 and -0 values",
885+
queries: []string{
886+
"insert into fk_t15 (id, col) values (1,'-0'), (2, '0'), (3, '5'), (4, '-5')",
887+
"insert into fk_t16 (id, col) values (1,'-0'), (2, '0'), (3, '5'), (4, '-5')",
888+
"update fk_t15 set col = col * (col - (col))",
889+
},
883890
},
884891
}
885892

‎go/vt/vtgate/engine/cached_size.go

+2-2
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎go/vt/vtgate/engine/fk_cascade.go

+21-6
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import (
2525
querypb "vitess.io/vitess/go/vt/proto/query"
2626
vtrpcpb "vitess.io/vitess/go/vt/proto/vtrpc"
2727
"vitess.io/vitess/go/vt/vterrors"
28+
"vitess.io/vitess/go/vt/vtgate/evalengine"
2829
)
2930

3031
// FkChild contains the Child Primitive to be executed collecting the values from the Selection Primitive using the column indexes.
@@ -40,14 +41,16 @@ type FkChild struct {
4041
}
4142

4243
// NonLiteralUpdateInfo stores the information required to process non-literal update queries.
43-
// It stores 3 information-
44-
// 1. UpdateExprCol- The index of the updated expression in the select query.
45-
// 2. UpdateExprBvName- The bind variable name to store the updated expression into.
46-
// 3. CompExprCol- The index of the comparison expression in the select query to know if the row value is actually being changed or not.
44+
// It stores 4 information-
45+
// 1. ExprCol- The index of the column being updated in the select query.
46+
// 2. CompExprCol- The index of the comparison expression in the select query to know if the row value is actually being changed or not.
47+
// 3. UpdateExprCol- The index of the updated expression in the select query.
48+
// 4. UpdateExprBvName- The bind variable name to store the updated expression into.
4749
type NonLiteralUpdateInfo struct {
50+
ExprCol int
51+
CompExprCol int
4852
UpdateExprCol int
4953
UpdateExprBvName string
50-
CompExprCol int
5154
}
5255

5356
// FkCascade is a primitive that implements foreign key cascading using Selection as values required to execute the FkChild Primitives.
@@ -185,7 +188,19 @@ func (fkc *FkCascade) executeNonLiteralExprFkChild(ctx context.Context, vcursor
185188

186189
// Next, we need to copy the updated expressions value into the bind variables map.
187190
for _, info := range child.NonLiteralInfo {
188-
bindVars[info.UpdateExprBvName] = sqltypes.ValueBindVariable(row[info.UpdateExprCol])
191+
// Type case the value to that of the column that we are updating.
192+
// This is required for example when we receive an updated float value of -0, but
193+
// the column being updated is a varchar column, then if we don't coerce the value of -0 to
194+
// varchar, MySQL ends up setting it to '0' instead of '-0'.
195+
finalVal := row[info.UpdateExprCol]
196+
if !finalVal.IsNull() {
197+
var err error
198+
finalVal, err = evalengine.CoerceTo(finalVal, selectionRes.Fields[info.ExprCol].Type)
199+
if err != nil {
200+
return err
201+
}
202+
}
203+
bindVars[info.UpdateExprBvName] = sqltypes.ValueBindVariable(finalVal)
189204
}
190205
_, err := vcursor.ExecutePrimitive(ctx, child.Exec, bindVars, wantfields)
191206
if err != nil {

‎go/vt/vtgate/planbuilder/operators/update.go

+8
Original file line numberDiff line numberDiff line change
@@ -334,6 +334,7 @@ func addNonLiteralUpdExprToSelect(ctx *plancontext.PlanningContext, updExpr *sql
334334
info := engine.NonLiteralUpdateInfo{
335335
CompExprCol: -1,
336336
UpdateExprCol: -1,
337+
ExprCol: -1,
337338
}
338339
// Add the expressions to the select expressions. We make sure to reuse the offset if it has already been added once.
339340
for idx, selectExpr := range exprs {
@@ -343,6 +344,9 @@ func addNonLiteralUpdExprToSelect(ctx *plancontext.PlanningContext, updExpr *sql
343344
if ctx.SemTable.EqualsExpr(selectExpr.(*sqlparser.AliasedExpr).Expr, updExpr.Expr) {
344345
info.UpdateExprCol = idx
345346
}
347+
if ctx.SemTable.EqualsExpr(selectExpr.(*sqlparser.AliasedExpr).Expr, updExpr.Name) {
348+
info.ExprCol = idx
349+
}
346350
}
347351
// If the expression doesn't exist, then we add the expression and store the offset.
348352
if info.CompExprCol == -1 {
@@ -353,6 +357,10 @@ func addNonLiteralUpdExprToSelect(ctx *plancontext.PlanningContext, updExpr *sql
353357
info.UpdateExprCol = len(exprs)
354358
exprs = append(exprs, aeWrap(updExpr.Expr))
355359
}
360+
if info.ExprCol == -1 {
361+
info.ExprCol = len(exprs)
362+
exprs = append(exprs, aeWrap(updExpr.Name))
363+
}
356364
return info, exprs
357365
}
358366

‎go/vt/vtgate/planbuilder/testdata/foreignkey_cases.json

+21-14
Original file line numberDiff line numberDiff line change
@@ -840,9 +840,10 @@
840840
],
841841
"NonLiteralUpdateInfo": [
842842
{
843+
"ExprCol": 0,
844+
"CompExprCol": 1,
843845
"UpdateExprCol": 2,
844-
"UpdateExprBvName": "fkc_upd",
845-
"CompExprCol": 1
846+
"UpdateExprBvName": "fkc_upd"
846847
}
847848
],
848849
"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)))",
@@ -901,9 +902,10 @@
901902
],
902903
"NonLiteralUpdateInfo": [
903904
{
905+
"ExprCol": 0,
906+
"CompExprCol": 1,
904907
"UpdateExprCol": 2,
905-
"UpdateExprBvName": "fkc_upd",
906-
"CompExprCol": 1
908+
"UpdateExprBvName": "fkc_upd"
907909
}
908910
],
909911
"Inputs": [
@@ -958,9 +960,10 @@
958960
],
959961
"NonLiteralUpdateInfo": [
960962
{
963+
"ExprCol": 0,
964+
"CompExprCol": 1,
961965
"UpdateExprCol": 2,
962-
"UpdateExprBvName": "fkc_upd1",
963-
"CompExprCol": 1
966+
"UpdateExprBvName": "fkc_upd1"
964967
}
965968
],
966969
"Inputs": [
@@ -2102,9 +2105,10 @@
21022105
],
21032106
"NonLiteralUpdateInfo": [
21042107
{
2108+
"ExprCol": 0,
2109+
"CompExprCol": 1,
21052110
"UpdateExprCol": 2,
2106-
"UpdateExprBvName": "fkc_upd",
2107-
"CompExprCol": 1
2111+
"UpdateExprBvName": "fkc_upd"
21082112
}
21092113
],
21102114
"Inputs": [
@@ -2199,9 +2203,10 @@
21992203
],
22002204
"NonLiteralUpdateInfo": [
22012205
{
2206+
"ExprCol": 0,
2207+
"CompExprCol": 2,
22022208
"UpdateExprCol": 3,
2203-
"UpdateExprBvName": "fkc_upd",
2204-
"CompExprCol": 2
2209+
"UpdateExprBvName": "fkc_upd"
22052210
}
22062211
],
22072212
"Inputs": [
@@ -2327,14 +2332,16 @@
23272332
],
23282333
"NonLiteralUpdateInfo": [
23292334
{
2335+
"ExprCol": 0,
2336+
"CompExprCol": 2,
23302337
"UpdateExprCol": 3,
2331-
"UpdateExprBvName": "fkc_upd",
2332-
"CompExprCol": 2
2338+
"UpdateExprBvName": "fkc_upd"
23332339
},
23342340
{
2341+
"ExprCol": 1,
2342+
"CompExprCol": 4,
23352343
"UpdateExprCol": 5,
2336-
"UpdateExprBvName": "fkc_upd1",
2337-
"CompExprCol": 4
2344+
"UpdateExprBvName": "fkc_upd1"
23382345
}
23392346
],
23402347
"Query": "update /*+ SET_VAR(foreign_key_checks=OFF) */ u_multicol_tbl3 set cola = :fkc_upd, colb = :fkc_upd1 where (cola, colb) in ::fkc_vals",

0 commit comments

Comments
 (0)