Skip to content

Commit 5ee1b9b

Browse files
harshit-gangalfrouioui
harshit-gangal
and
frouioui
authored
Pass on vindex errors with wrap than overriding them (#14737)
Co-authored-by: Florent Poinsard <florent.poinsard@outlook.fr>
1 parent 5d05612 commit 5ee1b9b

File tree

11 files changed

+107
-66
lines changed

11 files changed

+107
-66
lines changed

‎go/test/endtoend/vtgate/misc_test.go

Lines changed: 51 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -23,12 +23,13 @@ import (
2323
"testing"
2424
"time"
2525

26+
"github.com/stretchr/testify/assert"
27+
"github.com/stretchr/testify/require"
28+
2629
"vitess.io/vitess/go/mysql"
2730
"vitess.io/vitess/go/mysql/sqlerror"
31+
"vitess.io/vitess/go/test/endtoend/cluster"
2832
"vitess.io/vitess/go/test/endtoend/utils"
29-
30-
"github.com/stretchr/testify/assert"
31-
"github.com/stretchr/testify/require"
3233
)
3334

3435
func TestInsertOnDuplicateKey(t *testing.T) {
@@ -790,12 +791,57 @@ func TestJoinWithMergedRouteWithPredicate(t *testing.T) {
790791
}
791792

792793
func TestRowCountExceed(t *testing.T) {
793-
conn, closer := start(t)
794-
defer closer()
794+
conn, _ := start(t)
795+
defer func() {
796+
cluster.PanicHandler(t)
797+
// needs special delete logic as it exceeds row count.
798+
for i := 50; i <= 300; i += 50 {
799+
utils.Exec(t, conn, fmt.Sprintf("delete from t1 where id1 < %d", i))
800+
}
801+
conn.Close()
802+
}()
795803

796804
for i := 0; i < 250; i++ {
797805
utils.Exec(t, conn, fmt.Sprintf("insert into t1 (id1, id2) values (%d, %d)", i, i+1))
798806
}
799807

800808
utils.AssertContainsError(t, conn, "select id1 from t1 where id1 < 1000", `Row count exceeded 100`)
801809
}
810+
811+
func TestLookupErrorMetric(t *testing.T) {
812+
conn, closer := start(t)
813+
defer closer()
814+
815+
oldErrCount := getVtgateApiErrorCounts(t)
816+
817+
utils.Exec(t, conn, `insert into t1 values (1,1)`)
818+
_, err := utils.ExecAllowError(t, conn, `insert into t1 values (2,1)`)
819+
require.ErrorContains(t, err, `(errno 1062) (sqlstate 23000)`)
820+
821+
newErrCount := getVtgateApiErrorCounts(t)
822+
require.EqualValues(t, oldErrCount+1, newErrCount)
823+
}
824+
825+
func getVtgateApiErrorCounts(t *testing.T) float64 {
826+
apiErr := getVar(t, "VtgateApiErrorCounts")
827+
if apiErr == nil {
828+
return 0
829+
}
830+
mapErrors := apiErr.(map[string]interface{})
831+
val, exists := mapErrors["Execute.ks.primary.ALREADY_EXISTS"]
832+
if exists {
833+
return val.(float64)
834+
}
835+
return 0
836+
}
837+
838+
func getVar(t *testing.T, key string) interface{} {
839+
vars, err := clusterInstance.VtgateProcess.GetVars()
840+
require.NoError(t, err)
841+
842+
val, exists := vars[key]
843+
if !exists {
844+
return nil
845+
}
846+
return val
847+
}

‎go/test/endtoend/vtgate/queries/dml/insert_test.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -68,11 +68,11 @@ func TestFailureInsertSelect(t *testing.T) {
6868

6969
// primary key same
7070
mcmp.AssertContainsError("insert into s_tbl(id, num) select id, num*20 from s_tbl where id = 1", `AlreadyExists desc = Duplicate entry '1' for key`)
71-
// lookup key same (does not fail on MySQL as there is no lookup, and we have not put unique contrains on num column)
72-
utils.AssertContainsError(t, mcmp.VtConn, "insert into s_tbl(id, num) select id*20, num from s_tbl where id = 1", `lookup.Create: Code: ALREADY_EXISTS`)
71+
// lookup key same (does not fail on MySQL as there is no lookup, and we have not put unique constraint on num column)
72+
utils.AssertContainsError(t, mcmp.VtConn, "insert into s_tbl(id, num) select id*20, num from s_tbl where id = 1", `(errno 1062) (sqlstate 23000)`)
7373
// mismatch column count
74-
mcmp.AssertContainsError("insert into s_tbl(id, num) select 100,200,300", `column count does not match value count at row 1`)
75-
mcmp.AssertContainsError("insert into s_tbl(id, num) select 100", `column count does not match value count at row 1`)
74+
mcmp.AssertContainsError("insert into s_tbl(id, num) select 100,200,300", `column count does not match value count with the row`)
75+
mcmp.AssertContainsError("insert into s_tbl(id, num) select 100", `column count does not match value count with the row`)
7676
})
7777
}
7878
}
@@ -298,7 +298,7 @@ func TestIgnoreInsertSelect(t *testing.T) {
298298
mcmp.Exec("insert into order_tbl(region_id, oid, cust_no) values (1,1,100),(1,2,200),(1,3,300)")
299299

300300
// inserting same rows, throws error.
301-
mcmp.AssertContainsError("insert into order_tbl(region_id, oid, cust_no) select region_id, oid, cust_no from order_tbl", `lookup.Create: Code: ALREADY_EXISTS`)
301+
mcmp.AssertContainsError("insert into order_tbl(region_id, oid, cust_no) select region_id, oid, cust_no from order_tbl", `(errno 1062) (sqlstate 23000)`)
302302
// inserting same rows with ignore
303303
qr := mcmp.Exec("insert ignore into order_tbl(region_id, oid, cust_no) select region_id, oid, cust_no from order_tbl")
304304
assert.EqualValues(t, 0, qr.RowsAffected)
@@ -336,7 +336,7 @@ func TestIgnoreInsertSelectOlapMode(t *testing.T) {
336336
mcmp.Exec("insert into order_tbl(region_id, oid, cust_no) values (1,1,100),(1,2,200),(1,3,300)")
337337

338338
// inserting same rows, throws error.
339-
mcmp.AssertContainsError("insert into order_tbl(region_id, oid, cust_no) select region_id, oid, cust_no from order_tbl", `lookup.Create: Code: ALREADY_EXISTS`)
339+
mcmp.AssertContainsError("insert into order_tbl(region_id, oid, cust_no) select region_id, oid, cust_no from order_tbl", `(errno 1062) (sqlstate 23000)`)
340340
// inserting same rows with ignore
341341
qr := mcmp.Exec("insert ignore into order_tbl(region_id, oid, cust_no) select region_id, oid, cust_no from order_tbl")
342342
assert.EqualValues(t, 0, qr.RowsAffected)

‎go/vt/vterrors/code.go

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ var (
3131
VT03003 = errorWithState("VT03003", vtrpcpb.Code_INVALID_ARGUMENT, UnknownTable, "unknown table '%s' in MULTI DELETE", "The specified table in this DELETE statement is unknown.")
3232
VT03004 = errorWithState("VT03004", vtrpcpb.Code_INVALID_ARGUMENT, NonUpdateableTable, "the target table %s of the DELETE is not updatable", "You cannot delete something that is not a real MySQL table.")
3333
VT03005 = errorWithState("VT03005", vtrpcpb.Code_INVALID_ARGUMENT, WrongGroupField, "cannot group on '%s'", "The planner does not allow grouping on certain field. For instance, aggregation function.")
34-
VT03006 = errorWithState("VT03006", vtrpcpb.Code_INVALID_ARGUMENT, WrongValueCountOnRow, "column count does not match value count at row 1", "The number of columns you want to insert do not match the number of columns of your SELECT query.")
34+
VT03006 = errorWithState("VT03006", vtrpcpb.Code_INVALID_ARGUMENT, WrongValueCountOnRow, "column count does not match value count with the row", "The number of columns you want to insert do not match the number of columns of your SELECT query.")
3535
VT03007 = errorWithoutState("VT03007", vtrpcpb.Code_INVALID_ARGUMENT, "keyspace not specified", "You need to add a keyspace qualifier.")
3636
VT03008 = errorWithState("VT03008", vtrpcpb.Code_INVALID_ARGUMENT, CantUseOptionHere, "incorrect usage/placement of '%s'", "The given token is not usable in this situation. Please refer to the MySQL documentation to learn more about your token's syntax.")
3737
VT03009 = errorWithState("VT03009", vtrpcpb.Code_INVALID_ARGUMENT, WrongValueForVar, "unexpected value type for '%s': %v", "You cannot assign this type to the given variable.")
@@ -53,6 +53,9 @@ var (
5353
VT03025 = errorWithState("VT03025", vtrpcpb.Code_INVALID_ARGUMENT, WrongArguments, "Incorrect arguments to %s", "The execute statement have wrong number of arguments")
5454
VT03026 = errorWithoutState("VT03024", vtrpcpb.Code_INVALID_ARGUMENT, "'%s' bind variable does not exists", "The query cannot be executed as missing the bind variable.")
5555
VT03027 = errorWithState("VT03027", vtrpcpb.Code_INVALID_ARGUMENT, BadNullError, "Column '%s' cannot be null", "The column cannot have null value.")
56+
VT03028 = errorWithState("VT03028", vtrpcpb.Code_INVALID_ARGUMENT, BadNullError, "Column '%s' cannot be null on row %d, col %d", "The column cannot have null value.")
57+
VT03029 = errorWithState("VT03029", vtrpcpb.Code_INVALID_ARGUMENT, WrongValueCountOnRow, "column count does not match value count with the row for vindex '%s'", "The number of columns you want to insert do not match the number of columns of your SELECT query.")
58+
VT03030 = errorWithState("VT03030", vtrpcpb.Code_INVALID_ARGUMENT, WrongValueCountOnRow, "lookup column count does not match value count with the row (columns, count): (%v, %d)", "The number of columns you want to insert do not match the number of columns of your SELECT query.")
5659

5760
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.")
5861
VT05002 = errorWithState("VT05002", vtrpcpb.Code_NOT_FOUND, BadDb, "cannot alter database '%s'; unknown database", "The given database does not exist; Vitess cannot alter it.")
@@ -132,6 +135,9 @@ var (
132135
VT03025,
133136
VT03026,
134137
VT03027,
138+
VT03028,
139+
VT03029,
140+
VT03030,
135141
VT05001,
136142
VT05002,
137143
VT05003,

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

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1207,7 +1207,7 @@
12071207
{
12081208
"comment": "insert with mimatched column list",
12091209
"query": "insert into user(id) values (1, 2)",
1210-
"plan": "VT03006: column count does not match value count at row 1"
1210+
"plan": "VT03006: column count does not match value count with the row"
12111211
},
12121212
{
12131213
"comment": "insert no column list for sharded authoritative table",
@@ -3759,17 +3759,17 @@
37593759
{
37603760
"comment": "insert using select with more columns in insert",
37613761
"query": "insert into music(id, user_id) select 1",
3762-
"plan": "VT03006: column count does not match value count at row 1"
3762+
"plan": "VT03006: column count does not match value count with the row"
37633763
},
37643764
{
37653765
"comment": "insert using select with more columns in select",
37663766
"query": "insert into music(id, user_id) select id, count(user_id), sum(user_id) from user group by id",
3767-
"plan": "VT03006: column count does not match value count at row 1"
3767+
"plan": "VT03006: column count does not match value count with the row"
37683768
},
37693769
{
37703770
"comment": "insert using select with more columns in select after accounting for star column",
37713771
"query": "insert into music(id, user_id) select id, *, 2 from user",
3772-
"plan": "VT03006: column count does not match value count at row 1"
3772+
"plan": "VT03006: column count does not match value count with the row"
37733773
},
37743774
{
37753775
"comment": "insert using select with auto-inc column using vitess sequence, sequence column not present",
@@ -4893,12 +4893,12 @@
48934893
{
48944894
"comment": "insert row values smaller than number of columns",
48954895
"query": "insert into user(one, two, three, four) values (1, 2, 3)",
4896-
"plan": "VT03006: column count does not match value count at row 1"
4896+
"plan": "VT03006: column count does not match value count with the row"
48974897
},
48984898
{
48994899
"comment": "insert row values greater than number of columns",
49004900
"query": "insert into user(one, two, three) values (1, 2, 3, 4)",
4901-
"plan": "VT03006: column count does not match value count at row 1"
4901+
"plan": "VT03006: column count does not match value count with the row"
49024902
},
49034903
{
49044904
"comment": "insert on duplicate key update with database qualifier",

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,7 @@
102102
{
103103
"comment": "unsharded insert, col list does not match values",
104104
"query": "insert into unsharded_auto(id, val) values(1)",
105-
"plan": "VT03006: column count does not match value count at row 1"
105+
"plan": "VT03006: column count does not match value count with the row"
106106
},
107107
{
108108
"comment": "sharded upsert can't change vindex",

‎go/vt/vtgate/vindexes/consistent_lookup.go

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ import (
2727
"vitess.io/vitess/go/sqltypes"
2828
"vitess.io/vitess/go/vt/key"
2929
"vitess.io/vitess/go/vt/sqlparser"
30+
"vitess.io/vitess/go/vt/vterrors"
3031
"vitess.io/vitess/go/vt/vtgate/evalengine"
3132

3233
querypb "vitess.io/vitess/go/vt/proto/query"
@@ -171,7 +172,7 @@ func (lu *ConsistentLookup) UnknownParams() []string {
171172
return lu.unknownParams
172173
}
173174

174-
//====================================================================
175+
// ====================================================================
175176

176177
// ConsistentLookupUnique defines a vindex that uses a lookup table.
177178
// The table is expected to define the id column as unique. It's
@@ -271,7 +272,7 @@ func (lu *ConsistentLookupUnique) AutoCommitEnabled() bool {
271272
return lu.lkp.Autocommit
272273
}
273274

274-
//====================================================================
275+
// ====================================================================
275276

276277
// clCommon defines a vindex that uses a lookup table.
277278
// The table is expected to define the id column as unique. It's
@@ -309,7 +310,7 @@ func (lu *clCommon) SetOwnerInfo(keyspace, table string, cols []sqlparser.Identi
309310
lu.keyspace = keyspace
310311
lu.ownerTable = sqlparser.String(sqlparser.NewIdentifierCS(table))
311312
if len(cols) != len(lu.lkp.FromColumns) {
312-
return fmt.Errorf("owner table column count does not match vindex %s", lu.name)
313+
return vterrors.VT03029(lu.name)
313314
}
314315
lu.ownerColumns = make([]string, len(cols))
315316
for i, col := range cols {

‎go/vt/vtgate/vindexes/lookup_hash.go

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import (
2525
"vitess.io/vitess/go/vt/key"
2626
topodatapb "vitess.io/vitess/go/vt/proto/topodata"
2727
vtgatepb "vitess.io/vitess/go/vt/proto/vtgate"
28+
"vitess.io/vitess/go/vt/vterrors"
2829
)
2930

3031
const (
@@ -52,7 +53,7 @@ func init() {
5253
Register("lookup_hash_unique", newLookupHashUnique)
5354
}
5455

55-
//====================================================================
56+
// ====================================================================
5657

5758
// LookupHash defines a vindex that uses a lookup table.
5859
// The table is expected to define the id column as unique. It's
@@ -205,7 +206,7 @@ func (lh *LookupHash) Verify(ctx context.Context, vcursor VCursor, ids []sqltype
205206

206207
values, err := unhashList(ksids)
207208
if err != nil {
208-
return nil, fmt.Errorf("lookup.Verify.vunhash: %v", err)
209+
return nil, vterrors.Wrap(err, "lookup.Verify.vunhash")
209210
}
210211
return lh.lkp.Verify(ctx, vcursor, ids, values)
211212
}
@@ -214,7 +215,7 @@ func (lh *LookupHash) Verify(ctx context.Context, vcursor VCursor, ids []sqltype
214215
func (lh *LookupHash) Create(ctx context.Context, vcursor VCursor, rowsColValues [][]sqltypes.Value, ksids [][]byte, ignoreMode bool) error {
215216
values, err := unhashList(ksids)
216217
if err != nil {
217-
return fmt.Errorf("lookup.Create.vunhash: %v", err)
218+
return vterrors.Wrap(err, "lookup.Create.vunhash")
218219
}
219220
return lh.lkp.Create(ctx, vcursor, rowsColValues, values, ignoreMode)
220221
}
@@ -223,7 +224,7 @@ func (lh *LookupHash) Create(ctx context.Context, vcursor VCursor, rowsColValues
223224
func (lh *LookupHash) Update(ctx context.Context, vcursor VCursor, oldValues []sqltypes.Value, ksid []byte, newValues []sqltypes.Value) error {
224225
v, err := vunhash(ksid)
225226
if err != nil {
226-
return fmt.Errorf("lookup.Update.vunhash: %v", err)
227+
return vterrors.Wrap(err, "lookup.Update.vunhash")
227228
}
228229
return lh.lkp.Update(ctx, vcursor, oldValues, ksid, sqltypes.NewUint64(v), newValues)
229230
}
@@ -232,7 +233,7 @@ func (lh *LookupHash) Update(ctx context.Context, vcursor VCursor, oldValues []s
232233
func (lh *LookupHash) Delete(ctx context.Context, vcursor VCursor, rowsColValues [][]sqltypes.Value, ksid []byte) error {
233234
v, err := vunhash(ksid)
234235
if err != nil {
235-
return fmt.Errorf("lookup.Delete.vunhash: %v", err)
236+
return vterrors.Wrap(err, "lookup.Delete.vunhash")
236237
}
237238
return lh.lkp.Delete(ctx, vcursor, rowsColValues, sqltypes.NewUint64(v), vtgatepb.CommitOrder_NORMAL)
238239
}
@@ -260,7 +261,7 @@ func unhashList(ksids [][]byte) ([]sqltypes.Value, error) {
260261
return values, nil
261262
}
262263

263-
//====================================================================
264+
// ====================================================================
264265

265266
// LookupHashUnique defines a vindex that uses a lookup table.
266267
// The table is expected to define the id column as unique. It's
@@ -383,7 +384,7 @@ func (lhu *LookupHashUnique) Verify(ctx context.Context, vcursor VCursor, ids []
383384

384385
values, err := unhashList(ksids)
385386
if err != nil {
386-
return nil, fmt.Errorf("lookup.Verify.vunhash: %v", err)
387+
return nil, vterrors.Wrap(err, "lookup.Verify.vunhash")
387388
}
388389
return lhu.lkp.Verify(ctx, vcursor, ids, values)
389390
}
@@ -392,7 +393,7 @@ func (lhu *LookupHashUnique) Verify(ctx context.Context, vcursor VCursor, ids []
392393
func (lhu *LookupHashUnique) Create(ctx context.Context, vcursor VCursor, rowsColValues [][]sqltypes.Value, ksids [][]byte, ignoreMode bool) error {
393394
values, err := unhashList(ksids)
394395
if err != nil {
395-
return fmt.Errorf("lookup.Create.vunhash: %v", err)
396+
return vterrors.Wrap(err, "lookup.Create.vunhash")
396397
}
397398
return lhu.lkp.Create(ctx, vcursor, rowsColValues, values, ignoreMode)
398399
}
@@ -401,7 +402,7 @@ func (lhu *LookupHashUnique) Create(ctx context.Context, vcursor VCursor, rowsCo
401402
func (lhu *LookupHashUnique) Delete(ctx context.Context, vcursor VCursor, rowsColValues [][]sqltypes.Value, ksid []byte) error {
402403
v, err := vunhash(ksid)
403404
if err != nil {
404-
return fmt.Errorf("lookup.Delete.vunhash: %v", err)
405+
return vterrors.Wrap(err, "lookup.Delete.vunhash")
405406
}
406407
return lhu.lkp.Delete(ctx, vcursor, rowsColValues, sqltypes.NewUint64(v), vtgatepb.CommitOrder_NORMAL)
407408
}
@@ -410,7 +411,7 @@ func (lhu *LookupHashUnique) Delete(ctx context.Context, vcursor VCursor, rowsCo
410411
func (lhu *LookupHashUnique) Update(ctx context.Context, vcursor VCursor, oldValues []sqltypes.Value, ksid []byte, newValues []sqltypes.Value) error {
411412
v, err := vunhash(ksid)
412413
if err != nil {
413-
return fmt.Errorf("lookup.Update.vunhash: %v", err)
414+
return vterrors.Wrap(err, "lookup.Update.vunhash")
414415
}
415416
return lhu.lkp.Update(ctx, vcursor, oldValues, ksid, sqltypes.NewUint64(v), newValues)
416417
}

‎go/vt/vtgate/vindexes/lookup_hash_test.go

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -236,10 +236,7 @@ func TestLookupHashCreate(t *testing.T) {
236236
}
237237

238238
err = lookuphash.(Lookup).Create(context.Background(), vc, [][]sqltypes.Value{{sqltypes.NULL}}, [][]byte{[]byte("\x16k@\xb4J\xbaK\xd6")}, false /* ignoreMode */)
239-
want := "lookup.Create: input has null values: row: 0, col: 0"
240-
if err == nil || err.Error() != want {
241-
t.Errorf("lookuphash.Create(NULL) err: %v, want %s", err, want)
242-
}
239+
require.ErrorContains(t, err, "VT03028: Column 'fromc' cannot be null on row 0, col 0")
243240

244241
vc.queries = nil
245242
lookuphash.(*LookupHash).lkp.IgnoreNulls = true
@@ -250,10 +247,7 @@ func TestLookupHashCreate(t *testing.T) {
250247
}
251248

252249
err = lookuphash.(Lookup).Create(context.Background(), vc, [][]sqltypes.Value{{sqltypes.NewInt64(1)}}, [][]byte{[]byte("bogus")}, false /* ignoreMode */)
253-
want = "lookup.Create.vunhash: invalid keyspace id: 626f677573"
254-
if err == nil || err.Error() != want {
255-
t.Errorf("lookuphash.Create(bogus) err: %v, want %s", err, want)
256-
}
250+
require.ErrorContains(t, err, "lookup.Create.vunhash: invalid keyspace id: 626f677573")
257251
}
258252

259253
func TestLookupHashDelete(t *testing.T) {

0 commit comments

Comments
 (0)