From b164a6e4b60d8183e546cdcad6d9dd5bba6f9b95 Mon Sep 17 00:00:00 2001 From: Harshit Gangal Date: Wed, 20 Mar 2024 14:43:10 +0530 Subject: [PATCH] fail insert when primary vindex cannot be mapped to a shard (#15500) Signed-off-by: Harshit Gangal --- go/vt/vterrors/code.go | 8 + go/vt/vtgate/engine/insert_common.go | 11 +- go/vt/vtgate/engine/insert_test.go | 33 ++- go/vt/vtgate/executor_dml_test.go | 317 ++++++++++++++++----------- 4 files changed, 217 insertions(+), 152 deletions(-) diff --git a/go/vt/vterrors/code.go b/go/vt/vterrors/code.go index 4ade1b22368..bc4bd9bbe35 100644 --- a/go/vt/vterrors/code.go +++ b/go/vt/vterrors/code.go @@ -92,6 +92,9 @@ var ( VT09019 = errorWithoutState("VT09019", vtrpcpb.Code_FAILED_PRECONDITION, "keyspace '%s' has cyclic foreign keys. Cycle exists between %v", "Vitess doesn't support cyclic foreign keys.") VT09020 = errorWithoutState("VT09020", vtrpcpb.Code_FAILED_PRECONDITION, "can not use multiple vindex hints for table %s", "Vitess does not allow using multiple vindex hints on the same table.") VT09021 = errorWithState("VT09021", vtrpcpb.Code_FAILED_PRECONDITION, KeyDoesNotExist, "Vindex '%s' does not exist in table '%s'", "Vindex hints have to reference an existing vindex, and no such vindex could be found for the given table.") + VT09022 = errorWithoutState("VT09022", vtrpcpb.Code_FAILED_PRECONDITION, "Destination does not have exactly one shard: %v", "Cannot send query to multiple shards.") + VT09023 = errorWithoutState("VT09023", vtrpcpb.Code_FAILED_PRECONDITION, "could not map %v to a keyspace id", "Unable to determine the shard for the given row.") + VT09024 = errorWithoutState("VT09024", vtrpcpb.Code_FAILED_PRECONDITION, "could not map %v to a unique keyspace id: %v", "Unable to determine the shard for the given row.") VT10001 = errorWithoutState("VT10001", vtrpcpb.Code_ABORTED, "foreign key constraints are not allowed", "Foreign key constraints are not allowed, see https://vitess.io/blog/2021-06-15-online-ddl-why-no-fk/.") @@ -171,6 +174,11 @@ var ( VT09017, VT09018, VT09019, + VT09020, + VT09021, + VT09022, + VT09023, + VT09024, VT10001, VT12001, VT12002, diff --git a/go/vt/vtgate/engine/insert_common.go b/go/vt/vtgate/engine/insert_common.go index 8a35732dff4..e29aa7fd792 100644 --- a/go/vt/vtgate/engine/insert_common.go +++ b/go/vt/vtgate/engine/insert_common.go @@ -153,7 +153,7 @@ func (ins *InsertCommon) executeUnshardedTableQuery(ctx context.Context, vcursor return nil, err } if len(rss) != 1 { - return nil, vterrors.Errorf(vtrpcpb.Code_FAILED_PRECONDITION, "Keyspace does not have exactly one shard: %v", rss) + return nil, vterrors.VT09022(rss) } err = allowOnlyPrimary(rss...) if err != nil { @@ -209,12 +209,11 @@ func (ic *InsertCommon) processPrimary(ctx context.Context, vcursor VCursor, vin // This is a single keyspace id, we're good. keyspaceIDs[i] = d case key.DestinationNone: - // No valid keyspace id, we may return an error. - if !ic.Ignore { - return nil, fmt.Errorf("could not map %v to a keyspace id", vindexColumnsKeys[i]) - } + // Not a valid keyspace id, so we cannot determine which shard this row belongs to. + // We have to return an error. + return nil, vterrors.VT09023(vindexColumnsKeys[i]) default: - return nil, fmt.Errorf("could not map %v to a unique keyspace id: %v", vindexColumnsKeys[i], destination) + return nil, vterrors.VT09024(vindexColumnsKeys[i], destination) } } diff --git a/go/vt/vtgate/engine/insert_test.go b/go/vt/vtgate/engine/insert_test.go index e870ffa18c0..762c68a83dc 100644 --- a/go/vt/vtgate/engine/insert_test.go +++ b/go/vt/vtgate/engine/insert_test.go @@ -64,7 +64,7 @@ func TestInsertUnsharded(t *testing.T) { vc = &loggingVCursor{} _, err = ins.TryExecute(context.Background(), vc, map[string]*querypb.BindVariable{}, false) - require.EqualError(t, err, `Keyspace does not have exactly one shard: []`) + require.EqualError(t, err, `VT09022: Destination does not have exactly one shard: []`) } func TestInsertUnshardedGenerate(t *testing.T) { @@ -515,7 +515,7 @@ func TestInsertShardedFail(t *testing.T) { // The lookup will fail to map to a keyspace id. _, err := ins.TryExecute(context.Background(), vc, map[string]*querypb.BindVariable{}, false) - require.EqualError(t, err, `could not map [INT64(1)] to a keyspace id`) + require.EqualError(t, err, `VT09023: could not map [INT64(1)] to a keyspace id`) } func TestInsertShardedGenerate(t *testing.T) { @@ -964,7 +964,6 @@ func TestInsertShardedIgnoreOwned(t *testing.T) { // rows for id evalengine.NewLiteralInt(1), - evalengine.NewLiteralInt(2), evalengine.NewLiteralInt(3), evalengine.NewLiteralInt(4), }, @@ -973,14 +972,12 @@ func TestInsertShardedIgnoreOwned(t *testing.T) { { // rows for c1 evalengine.NewLiteralInt(5), - evalengine.NewLiteralInt(6), evalengine.NewLiteralInt(7), evalengine.NewLiteralInt(8), }, { // rows for c2 evalengine.NewLiteralInt(9), - evalengine.NewLiteralInt(10), evalengine.NewLiteralInt(11), evalengine.NewLiteralInt(12), }, @@ -989,7 +986,6 @@ func TestInsertShardedIgnoreOwned(t *testing.T) { { // rows for c3 evalengine.NewLiteralInt(13), - evalengine.NewLiteralInt(14), evalengine.NewLiteralInt(15), evalengine.NewLiteralInt(16), }, @@ -1000,7 +996,6 @@ func TestInsertShardedIgnoreOwned(t *testing.T) { {&sqlparser.Argument{Name: "_id_0", Type: sqltypes.Int64}, &sqlparser.Argument{Name: "_c1_0", Type: sqltypes.Int64}, &sqlparser.Argument{Name: "_c2_0", Type: sqltypes.Int64}, &sqlparser.Argument{Name: "_c3_0", Type: sqltypes.Int64}}, {&sqlparser.Argument{Name: "_id_1", Type: sqltypes.Int64}, &sqlparser.Argument{Name: "_c1_1", Type: sqltypes.Int64}, &sqlparser.Argument{Name: "_c2_1", Type: sqltypes.Int64}, &sqlparser.Argument{Name: "_c3_1", Type: sqltypes.Int64}}, {&sqlparser.Argument{Name: "_id_2", Type: sqltypes.Int64}, &sqlparser.Argument{Name: "_c1_2", Type: sqltypes.Int64}, &sqlparser.Argument{Name: "_c2_2", Type: sqltypes.Int64}, &sqlparser.Argument{Name: "_c3_2", Type: sqltypes.Int64}}, - {&sqlparser.Argument{Name: "_id_3", Type: sqltypes.Int64}, &sqlparser.Argument{Name: "_c1_3", Type: sqltypes.Int64}, &sqlparser.Argument{Name: "_c2_3", Type: sqltypes.Int64}, &sqlparser.Argument{Name: "_c3_3", Type: sqltypes.Int64}}, }, nil, ) @@ -1045,31 +1040,29 @@ func TestInsertShardedIgnoreOwned(t *testing.T) { t.Fatal(err) } vc.ExpectLog(t, []string{ - `Execute select from1, toc from prim where from1 in ::from1 from1: type:TUPLE values:{type:INT64 value:"1"} values:{type:INT64 value:"2"} values:{type:INT64 value:"3"} values:{type:INT64 value:"4"} false`, - `Execute insert ignore into lkp2(from1, from2, toc) values(:from1_0, :from2_0, :toc_0), (:from1_1, :from2_1, :toc_1), (:from1_2, :from2_2, :toc_2) ` + + `Execute select from1, toc from prim where from1 in ::from1 ` + + `from1: type:TUPLE values:{type:INT64 value:"1"} values:{type:INT64 value:"3"} values:{type:INT64 value:"4"} false`, + `Execute insert ignore into lkp2(from1, from2, toc) values` + + `(:from1_0, :from2_0, :toc_0), (:from1_1, :from2_1, :toc_1), (:from1_2, :from2_2, :toc_2) ` + `from1_0: type:INT64 value:"5" from1_1: type:INT64 value:"7" from1_2: type:INT64 value:"8" ` + `from2_0: type:INT64 value:"9" from2_1: type:INT64 value:"11" from2_2: type:INT64 value:"12" ` + - `toc_0: type:VARBINARY value:"\x00" toc_1: type:VARBINARY value:"\x00" toc_2: type:VARBINARY value:"\x00" ` + - `true`, - // row 2 is out because it didn't map to a ksid. + `toc_0: type:VARBINARY value:"\x00" toc_1: type:VARBINARY value:"\x00" toc_2: type:VARBINARY value:"\x00" true`, `Execute select from1 from lkp2 where from1 = :from1 and toc = :toc from1: type:INT64 value:"5" toc: type:VARBINARY value:"\x00" false`, `Execute select from1 from lkp2 where from1 = :from1 and toc = :toc from1: type:INT64 value:"7" toc: type:VARBINARY value:"\x00" false`, `Execute select from1 from lkp2 where from1 = :from1 and toc = :toc from1: type:INT64 value:"8" toc: type:VARBINARY value:"\x00" false`, `Execute insert ignore into lkp1(from, toc) values(:from_0, :toc_0), (:from_1, :toc_1) ` + `from_0: type:INT64 value:"13" from_1: type:INT64 value:"16" ` + - `toc_0: type:VARBINARY value:"\x00" toc_1: type:VARBINARY value:"\x00" ` + - `true`, - // row 3 is out because it failed Verify. Only two verifications from lkp1. + `toc_0: type:VARBINARY value:"\x00" toc_1: type:VARBINARY value:"\x00" true`, + // row 2 is out because it failed Verify. Only two verifications from lkp1. `Execute select from from lkp1 where from = :from and toc = :toc from: type:INT64 value:"13" toc: type:VARBINARY value:"\x00" false`, `Execute select from from lkp1 where from = :from and toc = :toc from: type:INT64 value:"16" toc: type:VARBINARY value:"\x00" false`, - `ResolveDestinations sharded [value:"0" value:"3"] Destinations:DestinationKeyspaceID(00),DestinationKeyspaceID(00)`, - // Bind vars for rows 2 & 3 may be missing because they were not sent. + `ResolveDestinations sharded [value:"0" value:"2"] Destinations:DestinationKeyspaceID(00),DestinationKeyspaceID(00)`, + // Bind vars for rows 2 may be missing because they were not sent. `ExecuteMultiShard ` + `sharded.20-: prefix(:_id_0 /* INT64 */, :_c1_0 /* INT64 */, :_c2_0 /* INT64 */, :_c3_0 /* INT64 */) ` + `{_c1_0: type:INT64 value:"5" _c2_0: type:INT64 value:"9" _c3_0: type:INT64 value:"13" _id_0: type:INT64 value:"1"} ` + - `sharded.-20: prefix(:_id_3 /* INT64 */, :_c1_3 /* INT64 */, :_c2_3 /* INT64 */, :_c3_3 /* INT64 */) ` + - `{_c1_3: type:INT64 value:"8" _c2_3: type:INT64 value:"12" _c3_3: type:INT64 value:"16" _id_3: type:INT64 value:"4"} ` + - `true false`, + `sharded.-20: prefix(:_id_2 /* INT64 */, :_c1_2 /* INT64 */, :_c2_2 /* INT64 */, :_c3_2 /* INT64 */) ` + + `{_c1_2: type:INT64 value:"8" _c2_2: type:INT64 value:"12" _c3_2: type:INT64 value:"16" _id_2: type:INT64 value:"4"} true false`, }) } diff --git a/go/vt/vtgate/executor_dml_test.go b/go/vt/vtgate/executor_dml_test.go index d0efcfe765a..b1d62966a43 100644 --- a/go/vt/vtgate/executor_dml_test.go +++ b/go/vt/vtgate/executor_dml_test.go @@ -1406,7 +1406,7 @@ func TestInsertShardedKeyrange(t *testing.T) { TargetString: "@primary", } _, err := executorExec(ctx, executor, session, "insert into keyrange_table(krcol_unique, krcol) values(1, 1)", nil) - require.EqualError(t, err, "could not map [INT64(1)] to a unique keyspace id: DestinationKeyRange(-10)") + require.EqualError(t, err, "VT09024: could not map [INT64(1)] to a unique keyspace id: DestinationKeyRange(-10)") } func TestInsertShardedAutocommitLookup(t *testing.T) { @@ -1503,145 +1503,210 @@ func TestInsertShardedAutocommitLookup(t *testing.T) { func TestInsertShardedIgnore(t *testing.T) { executor, sbc1, sbc2, sbclookup, ctx := createExecutorEnv(t) - // Build the sequence of responses for sbclookup. This should - // match the sequence of queries we validate below. + int1 := sqltypes.Int64BindVariable(1) + int2 := sqltypes.Int64BindVariable(2) + int3 := sqltypes.Int64BindVariable(3) + int4 := sqltypes.Int64BindVariable(4) + int5 := sqltypes.Int64BindVariable(5) + int6 := sqltypes.Int64BindVariable(6) + uint1 := sqltypes.Uint64BindVariable(1) + uint3 := sqltypes.Uint64BindVariable(3) + + var1 := &querypb.BindVariable{Type: querypb.Type_TUPLE, + Values: []*querypb.Value{{Type: int1.Type, Value: int1.Value}}, + } + var2 := &querypb.BindVariable{Type: querypb.Type_TUPLE, + Values: []*querypb.Value{{Type: int2.Type, Value: int2.Value}}, + } + var3 := &querypb.BindVariable{Type: querypb.Type_TUPLE, + Values: []*querypb.Value{{Type: int3.Type, Value: int3.Value}}, + } + var4 := &querypb.BindVariable{Type: querypb.Type_TUPLE, + Values: []*querypb.Value{{Type: int4.Type, Value: int4.Value}}, + } + var5 := &querypb.BindVariable{Type: querypb.Type_TUPLE, + Values: []*querypb.Value{{Type: int5.Type, Value: int5.Value}}, + } + var6 := &querypb.BindVariable{Type: querypb.Type_TUPLE, + Values: []*querypb.Value{{Type: int6.Type, Value: int6.Value}}, + } fields := sqltypes.MakeTestFields("b|a", "int64|int64") field := sqltypes.MakeTestFields("a", "int64") - sbclookup.SetResults([]*sqltypes.Result{ - // select music_id - sqltypes.MakeTestResult(fields, "1|1", "3|1", "4|1", "5|1", "6|3"), - // insert ins_lookup - {}, - // select ins_lookup 1 - sqltypes.MakeTestResult(field, "1"), - // select ins_lookup 3 - {}, - // select ins_lookup 4 - sqltypes.MakeTestResult(field, "4"), - // select ins_lookup 5 - sqltypes.MakeTestResult(field, "5"), - // select ins_lookup 6 - sqltypes.MakeTestResult(field, "6"), - }) - // First row: first shard. - // Second row: will fail because primary vindex will fail to map. - // Third row: will fail because verification will fail on owned vindex after Create. - // Fourth row: will fail because verification will fail on unowned hash vindex. - // Fifth row: first shard. - // Sixth row: second shard (because 3 hash maps to 40-60). - query := "insert ignore into insert_ignore_test(pv, owned, verify) values (1, 1, 1), (2, 2, 2), (3, 3, 1), (4, 4, 4), (5, 5, 1), (6, 6, 3)" - session := &vtgatepb.Session{ - TargetString: "@primary", - } - _, err := executorExec(ctx, executor, session, query, nil) - require.NoError(t, err) - wantQueries := []*querypb.BoundQuery{{ - Sql: "insert ignore into insert_ignore_test(pv, owned, verify) values (:_pv_0, :_owned_0, :_verify_0),(:_pv_4, :_owned_4, :_verify_4)", - BindVariables: map[string]*querypb.BindVariable{ - "_pv_0": sqltypes.Int64BindVariable(1), - "_pv_4": sqltypes.Int64BindVariable(5), - "_owned_0": sqltypes.Int64BindVariable(1), - "_owned_4": sqltypes.Int64BindVariable(5), - "_verify_0": sqltypes.Int64BindVariable(1), - "_verify_4": sqltypes.Int64BindVariable(1), - }, - }} - assertQueries(t, sbc1, wantQueries) - wantQueries = []*querypb.BoundQuery{{ - Sql: "insert ignore into insert_ignore_test(pv, owned, verify) values (:_pv_5, :_owned_5, :_verify_5)", - BindVariables: map[string]*querypb.BindVariable{ - "_pv_5": sqltypes.Int64BindVariable(6), - "_owned_5": sqltypes.Int64BindVariable(6), - "_verify_5": sqltypes.Int64BindVariable(3), - }, - }} - assertQueries(t, sbc2, wantQueries) + tcases := []struct { + query string + input []*sqltypes.Result - vars, err := sqltypes.BuildBindVariable([]any{ - sqltypes.NewInt64(1), - sqltypes.NewInt64(2), - sqltypes.NewInt64(3), - sqltypes.NewInt64(4), - sqltypes.NewInt64(5), - sqltypes.NewInt64(6), - }) - require.NoError(t, err) - wantQueries = []*querypb.BoundQuery{{ - Sql: "select music_id, user_id from music_user_map where music_id in ::music_id for update", - BindVariables: map[string]*querypb.BindVariable{ - "music_id": vars, + expectedQueries [3][]*querypb.BoundQuery + errString string + }{{ + // First row: first shard. + query: "insert ignore into insert_ignore_test(pv, owned, verify) values (1, 1, 1)", + input: []*sqltypes.Result{ + // select music_id + sqltypes.MakeTestResult(fields, "1|1"), + // insert ins_lookup 1 + sqltypes.MakeTestResult(nil), + // select ins_lookup 1 + sqltypes.MakeTestResult(field, "1"), + }, + expectedQueries: [3][]*querypb.BoundQuery{ + {{ + Sql: "insert ignore into insert_ignore_test(pv, owned, verify) values (:_pv_0, :_owned_0, :_verify_0)", + BindVariables: map[string]*querypb.BindVariable{"_pv_0": int1, "_owned_0": int1, "_verify_0": int1}, + }}, + nil, + {{ + Sql: "select music_id, user_id from music_user_map where music_id in ::music_id for update", + BindVariables: map[string]*querypb.BindVariable{"music_id": var1}, + }, { + Sql: "insert ignore into ins_lookup(fromcol, tocol) values (:fromcol_0, :tocol_0)", + BindVariables: map[string]*querypb.BindVariable{"fromcol_0": int1, "tocol_0": uint1}, + }, { + Sql: "select fromcol from ins_lookup where fromcol = :fromcol and tocol = :tocol", + BindVariables: map[string]*querypb.BindVariable{"fromcol": int1, "tocol": uint1}, + }}, }, }, { - Sql: "insert ignore into ins_lookup(fromcol, tocol) values (:fromcol_0, :tocol_0), (:fromcol_1, :tocol_1), (:fromcol_2, :tocol_2), (:fromcol_3, :tocol_3), (:fromcol_4, :tocol_4)", - BindVariables: map[string]*querypb.BindVariable{ - "fromcol_0": sqltypes.Int64BindVariable(1), - "tocol_0": sqltypes.Uint64BindVariable(1), - "fromcol_1": sqltypes.Int64BindVariable(3), - "tocol_1": sqltypes.Uint64BindVariable(1), - "fromcol_2": sqltypes.Int64BindVariable(4), - "tocol_2": sqltypes.Uint64BindVariable(1), - "fromcol_3": sqltypes.Int64BindVariable(5), - "tocol_3": sqltypes.Uint64BindVariable(1), - "fromcol_4": sqltypes.Int64BindVariable(6), - "tocol_4": sqltypes.Uint64BindVariable(3), - }, + // Second row: will fail because primary vindex will fail to map. + query: "insert ignore into insert_ignore_test(pv, owned, verify) values (2, 2, 2)", + input: []*sqltypes.Result{ + // select music_id + sqltypes.MakeTestResult(fields), + }, + expectedQueries: [3][]*querypb.BoundQuery{ + nil, + nil, + {{ + Sql: "select music_id, user_id from music_user_map where music_id in ::music_id for update", + BindVariables: map[string]*querypb.BindVariable{"music_id": var2}, + }}, + }, + errString: "could not map [INT64(2)] to a keyspace id", }, { - Sql: "select fromcol from ins_lookup where fromcol = :fromcol and tocol = :tocol", - BindVariables: map[string]*querypb.BindVariable{ - "fromcol": sqltypes.Int64BindVariable(1), - "tocol": sqltypes.Uint64BindVariable(1), - }, - }, { - Sql: "select fromcol from ins_lookup where fromcol = :fromcol and tocol = :tocol", - BindVariables: map[string]*querypb.BindVariable{ - "fromcol": sqltypes.Int64BindVariable(3), - "tocol": sqltypes.Uint64BindVariable(1), + // Third row: will fail because verification will fail on owned vindex after Create. + query: "insert ignore into insert_ignore_test(pv, owned, verify) values (3, 3, 1)", + input: []*sqltypes.Result{ + // select music_id + sqltypes.MakeTestResult(fields, "3|1"), + // insert ins_lookup 3 + sqltypes.MakeTestResult(nil), + // select ins_lookup 3 + sqltypes.MakeTestResult(field), + }, + expectedQueries: [3][]*querypb.BoundQuery{ + nil, + nil, + {{ + Sql: "select music_id, user_id from music_user_map where music_id in ::music_id for update", + BindVariables: map[string]*querypb.BindVariable{"music_id": var3}, + }, { + Sql: "insert ignore into ins_lookup(fromcol, tocol) values (:fromcol_0, :tocol_0)", + BindVariables: map[string]*querypb.BindVariable{"fromcol_0": int3, "tocol_0": uint1}, + }, { + Sql: "select fromcol from ins_lookup where fromcol = :fromcol and tocol = :tocol", + BindVariables: map[string]*querypb.BindVariable{"fromcol": int3, "tocol": uint1}, + }}, }, }, { - Sql: "select fromcol from ins_lookup where fromcol = :fromcol and tocol = :tocol", - BindVariables: map[string]*querypb.BindVariable{ - "fromcol": sqltypes.Int64BindVariable(4), - "tocol": sqltypes.Uint64BindVariable(1), + // Fourth row: will fail because verification will fail on unowned hash vindex. + query: "insert ignore into insert_ignore_test(pv, owned, verify) values (4, 4, 4)", + input: []*sqltypes.Result{ + // select music_id + sqltypes.MakeTestResult(fields, "4|1"), + // insert ins_lookup 4 + sqltypes.MakeTestResult(nil), + // select ins_lookup 4 + sqltypes.MakeTestResult(field, "4"), + sqltypes.MakeTestResult(nil), + }, + expectedQueries: [3][]*querypb.BoundQuery{ + nil, + nil, + {{ + Sql: "select music_id, user_id from music_user_map where music_id in ::music_id for update", + BindVariables: map[string]*querypb.BindVariable{"music_id": var4}, + }, { + Sql: "insert ignore into ins_lookup(fromcol, tocol) values (:fromcol_0, :tocol_0)", + BindVariables: map[string]*querypb.BindVariable{"fromcol_0": int4, "tocol_0": uint1}, + }, { + Sql: "select fromcol from ins_lookup where fromcol = :fromcol and tocol = :tocol", + BindVariables: map[string]*querypb.BindVariable{"fromcol": int4, "tocol": uint1}, + }}, }, }, { - Sql: "select fromcol from ins_lookup where fromcol = :fromcol and tocol = :tocol", - BindVariables: map[string]*querypb.BindVariable{ - "fromcol": sqltypes.Int64BindVariable(5), - "tocol": sqltypes.Uint64BindVariable(1), + // Fifth row: first shard. + query: "insert ignore into insert_ignore_test(pv, owned, verify) values (5, 5, 1)", + input: []*sqltypes.Result{ + // select music_id + sqltypes.MakeTestResult(fields, "5|1"), + // select ins_lookup 5 + sqltypes.MakeTestResult(field, "5"), + }, + expectedQueries: [3][]*querypb.BoundQuery{ + {{ + Sql: "insert ignore into insert_ignore_test(pv, owned, verify) values (:_pv_0, :_owned_0, :_verify_0)", + BindVariables: map[string]*querypb.BindVariable{"_pv_0": int5, "_owned_0": int5, "_verify_0": int1}, + }}, + nil, + {{ + Sql: "select music_id, user_id from music_user_map where music_id in ::music_id for update", + BindVariables: map[string]*querypb.BindVariable{"music_id": var5}, + }, { + Sql: "insert ignore into ins_lookup(fromcol, tocol) values (:fromcol_0, :tocol_0)", + BindVariables: map[string]*querypb.BindVariable{"fromcol_0": int5, "tocol_0": uint1}, + }, { + Sql: "select fromcol from ins_lookup where fromcol = :fromcol and tocol = :tocol", + BindVariables: map[string]*querypb.BindVariable{"fromcol": int5, "tocol": uint1}, + }}, }, }, { - Sql: "select fromcol from ins_lookup where fromcol = :fromcol and tocol = :tocol", - BindVariables: map[string]*querypb.BindVariable{ - "fromcol": sqltypes.Int64BindVariable(6), - "tocol": sqltypes.Uint64BindVariable(3), - }, - }} - assertQueries(t, sbclookup, wantQueries) + // Sixth row: second shard (because 3 hash maps to 40-60). + query: "insert ignore into insert_ignore_test(pv, owned, verify) values (6, 6, 3)", + input: []*sqltypes.Result{ + // select music_id + sqltypes.MakeTestResult(fields, "6|3"), + // select ins_lookup 6 + sqltypes.MakeTestResult(field, "6"), + }, + expectedQueries: [3][]*querypb.BoundQuery{ + nil, + {{ + Sql: "insert ignore into insert_ignore_test(pv, owned, verify) values (:_pv_0, :_owned_0, :_verify_0)", + BindVariables: map[string]*querypb.BindVariable{"_pv_0": int6, "_owned_0": int6, "_verify_0": int3}, + }}, + {{ + Sql: "select music_id, user_id from music_user_map where music_id in ::music_id for update", + BindVariables: map[string]*querypb.BindVariable{"music_id": var6}, + }, { + Sql: "insert ignore into ins_lookup(fromcol, tocol) values (:fromcol_0, :tocol_0)", + BindVariables: map[string]*querypb.BindVariable{"fromcol_0": int6, "tocol_0": uint3}, + }, { + Sql: "select fromcol from ins_lookup where fromcol = :fromcol and tocol = :tocol", + BindVariables: map[string]*querypb.BindVariable{"fromcol": int6, "tocol": uint3}, + }}, + }, + }} + + session := &vtgatepb.Session{Autocommit: true} + for _, tcase := range tcases { + t.Run(tcase.query, func(t *testing.T) { + // reset + sbc1.Queries = nil + sbc2.Queries = nil + sbclookup.Queries = nil - // Test the 0 rows case, - sbc1.Queries = nil - sbc2.Queries = nil - sbclookup.Queries = nil - sbclookup.SetResults([]*sqltypes.Result{ - {}, - }) - query = "insert ignore into insert_ignore_test(pv, owned, verify) values (1, 1, 1)" - qr, err := executorExec(ctx, executor, session, query, nil) - require.NoError(t, err) - if !qr.Equal(&sqltypes.Result{}) { - t.Errorf("qr: %v, want empty result", qr) + // Build the sequence of responses for sbclookup. This should + // match the sequence of queries we validate below. + sbclookup.SetResults(tcase.input) + _, err := executorExec(ctx, executor, session, tcase.query, nil) + if tcase.errString != "" { + require.ErrorContains(t, err, tcase.errString) + } + utils.MustMatch(t, tcase.expectedQueries[0], sbc1.Queries, "sbc1 queries do not match") + utils.MustMatch(t, tcase.expectedQueries[1], sbc2.Queries, "sbc2 queries do not match") + utils.MustMatch(t, tcase.expectedQueries[2], sbclookup.Queries, "sbclookup queries do not match") + }) } - assertQueries(t, sbc1, nil) - assertQueries(t, sbc2, nil) - vars, err = sqltypes.BuildBindVariable([]any{sqltypes.NewInt64(1)}) - require.NoError(t, err) - wantQueries = []*querypb.BoundQuery{{ - Sql: "select music_id, user_id from music_user_map where music_id in ::music_id for update", - BindVariables: map[string]*querypb.BindVariable{ - "music_id": vars, - }, - }} - assertQueries(t, sbclookup, wantQueries) } func TestInsertOnDupKey(t *testing.T) {