From 534825490d7068ab228e72f9ebca91b14f344583 Mon Sep 17 00:00:00 2001 From: Harshit Gangal Date: Wed, 11 Oct 2023 11:50:55 +0530 Subject: [PATCH 1/3] added failing insert with negative value Signed-off-by: Harshit Gangal --- go/test/endtoend/vtgate/misc_test.go | 8 ++++++++ go/test/endtoend/vtgate/schema.sql | 3 +++ 2 files changed, 11 insertions(+) diff --git a/go/test/endtoend/vtgate/misc_test.go b/go/test/endtoend/vtgate/misc_test.go index 65a6a0525d6..83c41fd7183 100644 --- a/go/test/endtoend/vtgate/misc_test.go +++ b/go/test/endtoend/vtgate/misc_test.go @@ -27,6 +27,14 @@ import ( "github.com/stretchr/testify/require" ) +func TestInsertNeg(t *testing.T) { + conn, closer := start(t) + defer closer() + + utils.Exec(t, conn, "insert ignore into t10(id, sharding_key, col1, col2, col3) values(10, 20, 'a', 1, 2), (20, -20, 'b', 3, 4), (30, -40, 'c', 6, 7), (40, 60, 'd', 4, 10)") + utils.Exec(t, conn, "insert ignore into t10(id, sharding_key, col1, col2, col3) values(1, 2, 'a', 1, 2), (2, -2, 'b', -3, 4), (3, -4, 'c', 6, -7), (4, 6, 'd', 4, -10)") +} + func TestSelectNull(t *testing.T) { conn, closer := start(t) defer closer() diff --git a/go/test/endtoend/vtgate/schema.sql b/go/test/endtoend/vtgate/schema.sql index 536bec397ec..a883a26519f 100644 --- a/go/test/endtoend/vtgate/schema.sql +++ b/go/test/endtoend/vtgate/schema.sql @@ -143,6 +143,9 @@ create table t10 ( id bigint, sharding_key bigint, + col1 varchar(50), + col2 int, + col3 int, primary key (id) ) Engine = InnoDB; From 606ee30f07568c3bfb558c9cf85914927b574e9c Mon Sep 17 00:00:00 2001 From: Harshit Gangal Date: Wed, 11 Oct 2023 12:57:32 +0530 Subject: [PATCH 2/3] fix: walk the expression and add all the arguments to the bindvars Signed-off-by: Harshit Gangal --- go/vt/vtgate/engine/insert.go | 9 ++++--- go/vt/vtgate/executor_dml_test.go | 36 +++++++++++++++++++++++++ go/vt/vtgate/executor_framework_test.go | 4 +-- 3 files changed, 44 insertions(+), 5 deletions(-) diff --git a/go/vt/vtgate/engine/insert.go b/go/vt/vtgate/engine/insert.go index 212bcd6620c..cbe2dfc9eb8 100644 --- a/go/vt/vtgate/engine/insert.go +++ b/go/vt/vtgate/engine/insert.go @@ -766,9 +766,12 @@ func (ins *Insert) getInsertShardedRoute( if keyspaceIDs[index] != nil { mids = append(mids, sqlparser.String(ins.Mid[index])) for _, expr := range ins.Mid[index] { - if arg, ok := expr.(*sqlparser.Argument); ok { - shardBindVars[arg.Name] = bindVars[arg.Name] - } + _ = sqlparser.Walk(func(node sqlparser.SQLNode) (kontinue bool, err error) { + if arg, ok := node.(*sqlparser.Argument); ok { + shardBindVars[arg.Name] = bindVars[arg.Name] + } + return true, nil + }, expr, nil) } } } diff --git a/go/vt/vtgate/executor_dml_test.go b/go/vt/vtgate/executor_dml_test.go index c53d24eb6fb..961e6e32eca 100644 --- a/go/vt/vtgate/executor_dml_test.go +++ b/go/vt/vtgate/executor_dml_test.go @@ -1362,6 +1362,42 @@ func TestInsertSharded(t *testing.T) { testQueryLog(t, executor, logChan, "TestExecute", "INSERT", "insert into `user`(id, v, `name`) values (:vtg1 /* INT64 */, :vtg2 /* INT64 */, _binary :vtg3 /* VARCHAR */)", 1) } +func TestInsertNegativeValue(t *testing.T) { + executor, sbc1, sbc2, sbclookup, ctx := createExecutorEnv(t) + executor.normalize = true + + logChan := executor.queryLogger.Subscribe("Test") + defer executor.queryLogger.Unsubscribe(logChan) + + session := &vtgatepb.Session{ + TargetString: "@primary", + } + _, err := executorExec(ctx, executor, session, "insert into user(id, v, name) values (1, -2, 'myname')", nil) + require.NoError(t, err) + wantQueries := []*querypb.BoundQuery{{ + Sql: "insert into `user`(id, v, `name`) values (:_Id_0, -:vtg2 /* INT64 */, :_name_0)", + BindVariables: map[string]*querypb.BindVariable{ + "_Id_0": sqltypes.Int64BindVariable(1), + "vtg2": sqltypes.Int64BindVariable(2), + "_name_0": sqltypes.StringBindVariable("myname"), + }, + }} + assertQueries(t, sbc1, wantQueries) + assertQueries(t, sbc2, nil) + wantQueries = []*querypb.BoundQuery{{ + Sql: "insert into name_user_map(`name`, user_id) values (:name_0, :user_id_0)", + BindVariables: map[string]*querypb.BindVariable{ + "name_0": sqltypes.StringBindVariable("myname"), + "user_id_0": sqltypes.Uint64BindVariable(1), + }, + }} + assertQueries(t, sbclookup, wantQueries) + + testQueryLog(t, executor, logChan, "MarkSavepoint", "SAVEPOINT", "savepoint x", 0) + testQueryLog(t, executor, logChan, "VindexCreate", "INSERT", "insert into name_user_map(`name`, user_id) values (:name_0, :user_id_0)", 1) + testQueryLog(t, executor, logChan, "TestExecute", "INSERT", "insert into `user`(id, v, `name`) values (:vtg1 /* INT64 */, -:vtg2 /* INT64 */, :vtg3 /* VARCHAR */)", 1) +} + func TestInsertShardedKeyrange(t *testing.T) { executor, _, _, _, ctx := createExecutorEnv(t) diff --git a/go/vt/vtgate/executor_framework_test.go b/go/vt/vtgate/executor_framework_test.go index 1da2cd10154..ceda947e8bb 100644 --- a/go/vt/vtgate/executor_framework_test.go +++ b/go/vt/vtgate/executor_framework_test.go @@ -357,8 +357,8 @@ func assertQueries(t *testing.T, sbc *sandboxconn.SandboxConn, wantQueries []*qu } got := query.Sql expected := wantQueries[idx].Sql - assert.Equal(t, expected, got) - assert.Equal(t, wantQueries[idx].BindVariables, query.BindVariables) + utils.MustMatch(t, expected, got) + utils.MustMatch(t, wantQueries[idx].BindVariables, query.BindVariables) idx++ } } From d0d351664644f3344bc76b52b6919d5774ba643b Mon Sep 17 00:00:00 2001 From: Harshit Gangal Date: Wed, 11 Oct 2023 17:47:26 +0530 Subject: [PATCH 3/3] addressed review comments Signed-off-by: Harshit Gangal --- go/vt/vterrors/code.go | 2 ++ go/vt/vtgate/engine/insert.go | 11 +++++++++-- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/go/vt/vterrors/code.go b/go/vt/vterrors/code.go index e095bb4e885..9bfb7747c09 100644 --- a/go/vt/vterrors/code.go +++ b/go/vt/vterrors/code.go @@ -48,6 +48,7 @@ var ( VT03023 = errorWithoutState("VT03023", vtrpcpb.Code_INVALID_ARGUMENT, "INSERT not supported when targeting a key range: %s", "When targeting a range of shards, Vitess does not know which shard to send the INSERT to.") VT03024 = errorWithoutState("VT03024", vtrpcpb.Code_INVALID_ARGUMENT, "'%s' user defined variable does not exists", "The query cannot be prepared using the user defined variable as it does not exists for this session.") VT03025 = errorWithState("VT03025", vtrpcpb.Code_INVALID_ARGUMENT, WrongArguments, "Incorrect arguments to %s", "The execute statement have wrong number of arguments") + VT03026 = errorWithoutState("VT03024", vtrpcpb.Code_INVALID_ARGUMENT, "'%s' bind variable does not exists", "The query cannot be executed as missing the bind variable.") VT05001 = errorWithState("VT05001", vtrpcpb.Code_NOT_FOUND, DbDropExists, "cannot drop database '%s'; database does not exists", "The given database does not exist; Vitess cannot drop it.") VT05002 = errorWithState("VT05002", vtrpcpb.Code_NOT_FOUND, BadDb, "cannot alter database '%s'; unknown database", "The given database does not exist; Vitess cannot alter it.") @@ -121,6 +122,7 @@ var ( VT03023, VT03024, VT03025, + VT03026, VT05001, VT05002, VT05003, diff --git a/go/vt/vtgate/engine/insert.go b/go/vt/vtgate/engine/insert.go index cbe2dfc9eb8..c3021cb46a0 100644 --- a/go/vt/vtgate/engine/insert.go +++ b/go/vt/vtgate/engine/insert.go @@ -766,12 +766,19 @@ func (ins *Insert) getInsertShardedRoute( if keyspaceIDs[index] != nil { mids = append(mids, sqlparser.String(ins.Mid[index])) for _, expr := range ins.Mid[index] { - _ = sqlparser.Walk(func(node sqlparser.SQLNode) (kontinue bool, err error) { + err = sqlparser.Walk(func(node sqlparser.SQLNode) (kontinue bool, err error) { if arg, ok := node.(*sqlparser.Argument); ok { - shardBindVars[arg.Name] = bindVars[arg.Name] + bv, exists := bindVars[arg.Name] + if !exists { + return false, vterrors.VT03026(arg.Name) + } + shardBindVars[arg.Name] = bv } return true, nil }, expr, nil) + if err != nil { + return nil, nil, err + } } } }