Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fail insert when primary vindex cannot be mapped to a shard #15500

Merged
merged 1 commit into from
Mar 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions go/vt/vterrors/code.go
Original file line number Diff line number Diff line change
Expand Up @@ -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/.")

Expand Down Expand Up @@ -171,6 +174,11 @@ var (
VT09017,
VT09018,
VT09019,
VT09020,
VT09021,
VT09022,
VT09023,
VT09024,
VT10001,
VT12001,
VT12002,
Expand Down
11 changes: 5 additions & 6 deletions go/vt/vtgate/engine/insert_common.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
}
}

Expand Down
33 changes: 13 additions & 20 deletions go/vt/vtgate/engine/insert_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -964,7 +964,6 @@ func TestInsertShardedIgnoreOwned(t *testing.T) {
// rows for id

evalengine.NewLiteralInt(1),
evalengine.NewLiteralInt(2),
evalengine.NewLiteralInt(3),
evalengine.NewLiteralInt(4),
},
Expand All @@ -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),
},
Expand All @@ -989,7 +986,6 @@ func TestInsertShardedIgnoreOwned(t *testing.T) {
{
// rows for c3
evalengine.NewLiteralInt(13),
evalengine.NewLiteralInt(14),
evalengine.NewLiteralInt(15),
evalengine.NewLiteralInt(16),
},
Expand All @@ -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,
)
Expand Down Expand Up @@ -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`,
})
}

Expand Down
Loading
Loading