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.

Sign up for GitHub

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

Pass on vindex errors with wrap than overriding them #14737

Merged
merged 7 commits into from
Dec 12, 2023
Merged

Pass on vindex errors with wrap than overriding them #14737

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
38e8f90
pass on the error from lookup execute and wrap if needed
harshit-gangal Dec 8, 2023
54a60f8
more vindexes to use error code and wrap errors
harshit-gangal Dec 11, 2023
0c51f98
fix test setup
harshit-gangal Dec 11, 2023
a77b0bf
fix test error message
harshit-gangal Dec 11, 2023
2f4bb5c
addressed review comments
harshit-gangal Dec 12, 2023
e5543c9
Merge remote-tracking branch 'upstream/main' into lkp-errors-fix
harshit-gangal Dec 12, 2023
b7f07d9
add missing error code to the list
frouioui Dec 12, 2023
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
56 changes: 51 additions & 5 deletions go/test/endtoend/vtgate/misc_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,13 @@ import (
"testing"
"time"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"vitess.io/vitess/go/mysql"
"vitess.io/vitess/go/mysql/sqlerror"
"vitess.io/vitess/go/test/endtoend/cluster"
"vitess.io/vitess/go/test/endtoend/utils"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

func TestInsertOnDuplicateKey(t *testing.T) {
Expand Down Expand Up @@ -790,12 +791,57 @@ func TestJoinWithMergedRouteWithPredicate(t *testing.T) {
}

func TestRowCountExceed(t *testing.T) {
conn, closer := start(t)
defer closer()
conn, _ := start(t)
defer func() {
cluster.PanicHandler(t)
// needs special delete logic as it exceeds row count.
for i := 50; i <= 300; i += 50 {
utils.Exec(t, conn, fmt.Sprintf("delete from t1 where id1 < %d", i))
}
conn.Close()
}()

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

utils.AssertContainsError(t, conn, "select id1 from t1 where id1 < 1000", `Row count exceeded 100`)
}

func TestLookupErrorMetric(t *testing.T) {
conn, closer := start(t)
defer closer()

oldErrCount := getVtgateApiErrorCounts(t)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so nice :)


utils.Exec(t, conn, `insert into t1 values (1,1)`)
_, err := utils.ExecAllowError(t, conn, `insert into t1 values (2,1)`)
require.ErrorContains(t, err, `(errno 1062) (sqlstate 23000)`)

newErrCount := getVtgateApiErrorCounts(t)
require.EqualValues(t, oldErrCount+1, newErrCount)
}

func getVtgateApiErrorCounts(t *testing.T) float64 {
apiErr := getVar(t, "VtgateApiErrorCounts")
if apiErr == nil {
return 0
}
mapErrors := apiErr.(map[string]interface{})
val, exists := mapErrors["Execute.ks.primary.ALREADY_EXISTS"]
if exists {
return val.(float64)
}
return 0
}

func getVar(t *testing.T, key string) interface{} {
harshit-gangal marked this conversation as resolved.
Show resolved Hide resolved
vars, err := clusterInstance.VtgateProcess.GetVars()
require.NoError(t, err)

val, exists := vars[key]
if !exists {
return nil
}
return val
}
12 changes: 6 additions & 6 deletions go/test/endtoend/vtgate/queries/dml/insert_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,11 +68,11 @@ func TestFailureInsertSelect(t *testing.T) {

// primary key same
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`)
// lookup key same (does not fail on MySQL as there is no lookup, and we have not put unique contrains on num column)
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`)
// lookup key same (does not fail on MySQL as there is no lookup, and we have not put unique constraint on num column)
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)`)
// mismatch column count
mcmp.AssertContainsError("insert into s_tbl(id, num) select 100,200,300", `column count does not match value count at row 1`)
mcmp.AssertContainsError("insert into s_tbl(id, num) select 100", `column count does not match value count at row 1`)
mcmp.AssertContainsError("insert into s_tbl(id, num) select 100,200,300", `column count does not match value count with the row`)
mcmp.AssertContainsError("insert into s_tbl(id, num) select 100", `column count does not match value count with the row`)
})
}
}
Expand Down Expand Up @@ -298,7 +298,7 @@ func TestIgnoreInsertSelect(t *testing.T) {
mcmp.Exec("insert into order_tbl(region_id, oid, cust_no) values (1,1,100),(1,2,200),(1,3,300)")

// inserting same rows, throws error.
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`)
mcmp.AssertContainsError("insert into order_tbl(region_id, oid, cust_no) select region_id, oid, cust_no from order_tbl", `(errno 1062) (sqlstate 23000)`)
// inserting same rows with ignore
qr := mcmp.Exec("insert ignore into order_tbl(region_id, oid, cust_no) select region_id, oid, cust_no from order_tbl")
assert.EqualValues(t, 0, qr.RowsAffected)
Expand Down Expand Up @@ -336,7 +336,7 @@ func TestIgnoreInsertSelectOlapMode(t *testing.T) {
mcmp.Exec("insert into order_tbl(region_id, oid, cust_no) values (1,1,100),(1,2,200),(1,3,300)")

// inserting same rows, throws error.
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`)
mcmp.AssertContainsError("insert into order_tbl(region_id, oid, cust_no) select region_id, oid, cust_no from order_tbl", `(errno 1062) (sqlstate 23000)`)
// inserting same rows with ignore
qr := mcmp.Exec("insert ignore into order_tbl(region_id, oid, cust_no) select region_id, oid, cust_no from order_tbl")
assert.EqualValues(t, 0, qr.RowsAffected)
Expand Down
8 changes: 7 additions & 1 deletion go/vt/vterrors/code.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ var (
VT03003 = errorWithState("VT03003", vtrpcpb.Code_INVALID_ARGUMENT, UnknownTable, "unknown table '%s' in MULTI DELETE", "The specified table in this DELETE statement is unknown.")
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.")
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.")
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.")
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.")
VT03007 = errorWithoutState("VT03007", vtrpcpb.Code_INVALID_ARGUMENT, "keyspace not specified", "You need to add a keyspace qualifier.")
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.")
VT03009 = errorWithState("VT03009", vtrpcpb.Code_INVALID_ARGUMENT, WrongValueForVar, "unexpected value type for '%s': %v", "You cannot assign this type to the given variable.")
Expand All @@ -53,6 +53,9 @@ var (
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.")
VT03027 = errorWithState("VT03027", vtrpcpb.Code_INVALID_ARGUMENT, BadNullError, "Column '%s' cannot be null", "The column cannot have null value.")
VT03028 = errorWithState("VT03028", vtrpcpb.Code_INVALID_ARGUMENT, BadNullError, "Column '%s' cannot be null on row %d, col %d", "The column cannot have null value.")
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.")
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.")

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.")
Expand Down Expand Up @@ -132,6 +135,9 @@ var (
VT03025,
VT03026,
VT03027,
VT03028,
VT03029,
Comment on lines +138 to +139
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

VT03030 is missing from this list

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed via b7f07d9

VT03030,
VT05001,
VT05002,
VT05003,
Expand Down
12 changes: 6 additions & 6 deletions go/vt/vtgate/planbuilder/testdata/dml_cases.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1207,7 +1207,7 @@
{
"comment": "insert with mimatched column list",
"query": "insert into user(id) values (1, 2)",
"plan": "VT03006: column count does not match value count at row 1"
"plan": "VT03006: column count does not match value count with the row"
},
{
"comment": "insert no column list for sharded authoritative table",
Expand Down Expand Up @@ -3759,17 +3759,17 @@
{
"comment": "insert using select with more columns in insert",
"query": "insert into music(id, user_id) select 1",
"plan": "VT03006: column count does not match value count at row 1"
"plan": "VT03006: column count does not match value count with the row"
},
{
"comment": "insert using select with more columns in select",
"query": "insert into music(id, user_id) select id, count(user_id), sum(user_id) from user group by id",
"plan": "VT03006: column count does not match value count at row 1"
"plan": "VT03006: column count does not match value count with the row"
},
{
"comment": "insert using select with more columns in select after accounting for star column",
"query": "insert into music(id, user_id) select id, *, 2 from user",
"plan": "VT03006: column count does not match value count at row 1"
"plan": "VT03006: column count does not match value count with the row"
},
{
"comment": "insert using select with auto-inc column using vitess sequence, sequence column not present",
Expand Down Expand Up @@ -4893,12 +4893,12 @@
{
"comment": "insert row values smaller than number of columns",
"query": "insert into user(one, two, three, four) values (1, 2, 3)",
"plan": "VT03006: column count does not match value count at row 1"
"plan": "VT03006: column count does not match value count with the row"
},
{
"comment": "insert row values greater than number of columns",
"query": "insert into user(one, two, three) values (1, 2, 3, 4)",
"plan": "VT03006: column count does not match value count at row 1"
"plan": "VT03006: column count does not match value count with the row"
},
{
"comment": "insert on duplicate key update with database qualifier",
Expand Down
2 changes: 1 addition & 1 deletion go/vt/vtgate/planbuilder/testdata/unsupported_cases.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@
{
"comment": "unsharded insert, col list does not match values",
"query": "insert into unsharded_auto(id, val) values(1)",
"plan": "VT03006: column count does not match value count at row 1"
"plan": "VT03006: column count does not match value count with the row"
},
{
"comment": "sharded upsert can't change vindex",
Expand Down
7 changes: 4 additions & 3 deletions go/vt/vtgate/vindexes/consistent_lookup.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import (
"vitess.io/vitess/go/sqltypes"
"vitess.io/vitess/go/vt/key"
"vitess.io/vitess/go/vt/sqlparser"
"vitess.io/vitess/go/vt/vterrors"
"vitess.io/vitess/go/vt/vtgate/evalengine"

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

//====================================================================
// ====================================================================

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

//====================================================================
// ====================================================================

// clCommon defines a vindex that uses a lookup table.
// The table is expected to define the id column as unique. It's
Expand Down Expand Up @@ -309,7 +310,7 @@ func (lu *clCommon) SetOwnerInfo(keyspace, table string, cols []sqlparser.Identi
lu.keyspace = keyspace
lu.ownerTable = sqlparser.String(sqlparser.NewIdentifierCS(table))
if len(cols) != len(lu.lkp.FromColumns) {
return fmt.Errorf("owner table column count does not match vindex %s", lu.name)
return vterrors.VT03029(lu.name)
}
lu.ownerColumns = make([]string, len(cols))
for i, col := range cols {
Expand Down
21 changes: 11 additions & 10 deletions go/vt/vtgate/vindexes/lookup_hash.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (
"vitess.io/vitess/go/vt/key"
topodatapb "vitess.io/vitess/go/vt/proto/topodata"
vtgatepb "vitess.io/vitess/go/vt/proto/vtgate"
"vitess.io/vitess/go/vt/vterrors"
)

const (
Expand Down Expand Up @@ -52,7 +53,7 @@ func init() {
Register("lookup_hash_unique", newLookupHashUnique)
}

//====================================================================
// ====================================================================

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

values, err := unhashList(ksids)
if err != nil {
return nil, fmt.Errorf("lookup.Verify.vunhash: %v", err)
return nil, vterrors.Wrap(err, "lookup.Verify.vunhash")
}
return lh.lkp.Verify(ctx, vcursor, ids, values)
}
Expand All @@ -214,7 +215,7 @@ func (lh *LookupHash) Verify(ctx context.Context, vcursor VCursor, ids []sqltype
func (lh *LookupHash) Create(ctx context.Context, vcursor VCursor, rowsColValues [][]sqltypes.Value, ksids [][]byte, ignoreMode bool) error {
values, err := unhashList(ksids)
if err != nil {
return fmt.Errorf("lookup.Create.vunhash: %v", err)
return vterrors.Wrap(err, "lookup.Create.vunhash")
}
return lh.lkp.Create(ctx, vcursor, rowsColValues, values, ignoreMode)
}
Expand All @@ -223,7 +224,7 @@ func (lh *LookupHash) Create(ctx context.Context, vcursor VCursor, rowsColValues
func (lh *LookupHash) Update(ctx context.Context, vcursor VCursor, oldValues []sqltypes.Value, ksid []byte, newValues []sqltypes.Value) error {
v, err := vunhash(ksid)
if err != nil {
return fmt.Errorf("lookup.Update.vunhash: %v", err)
return vterrors.Wrap(err, "lookup.Update.vunhash")
}
return lh.lkp.Update(ctx, vcursor, oldValues, ksid, sqltypes.NewUint64(v), newValues)
}
Expand All @@ -232,7 +233,7 @@ func (lh *LookupHash) Update(ctx context.Context, vcursor VCursor, oldValues []s
func (lh *LookupHash) Delete(ctx context.Context, vcursor VCursor, rowsColValues [][]sqltypes.Value, ksid []byte) error {
v, err := vunhash(ksid)
if err != nil {
return fmt.Errorf("lookup.Delete.vunhash: %v", err)
return vterrors.Wrap(err, "lookup.Delete.vunhash")
}
return lh.lkp.Delete(ctx, vcursor, rowsColValues, sqltypes.NewUint64(v), vtgatepb.CommitOrder_NORMAL)
}
Expand Down Expand Up @@ -260,7 +261,7 @@ func unhashList(ksids [][]byte) ([]sqltypes.Value, error) {
return values, nil
}

//====================================================================
// ====================================================================

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

values, err := unhashList(ksids)
if err != nil {
return nil, fmt.Errorf("lookup.Verify.vunhash: %v", err)
return nil, vterrors.Wrap(err, "lookup.Verify.vunhash")
}
return lhu.lkp.Verify(ctx, vcursor, ids, values)
}
Expand All @@ -392,7 +393,7 @@ func (lhu *LookupHashUnique) Verify(ctx context.Context, vcursor VCursor, ids []
func (lhu *LookupHashUnique) Create(ctx context.Context, vcursor VCursor, rowsColValues [][]sqltypes.Value, ksids [][]byte, ignoreMode bool) error {
values, err := unhashList(ksids)
if err != nil {
return fmt.Errorf("lookup.Create.vunhash: %v", err)
return vterrors.Wrap(err, "lookup.Create.vunhash")
}
return lhu.lkp.Create(ctx, vcursor, rowsColValues, values, ignoreMode)
}
Expand All @@ -401,7 +402,7 @@ func (lhu *LookupHashUnique) Create(ctx context.Context, vcursor VCursor, rowsCo
func (lhu *LookupHashUnique) Delete(ctx context.Context, vcursor VCursor, rowsColValues [][]sqltypes.Value, ksid []byte) error {
v, err := vunhash(ksid)
if err != nil {
return fmt.Errorf("lookup.Delete.vunhash: %v", err)
return vterrors.Wrap(err, "lookup.Delete.vunhash")
}
return lhu.lkp.Delete(ctx, vcursor, rowsColValues, sqltypes.NewUint64(v), vtgatepb.CommitOrder_NORMAL)
}
Expand All @@ -410,7 +411,7 @@ func (lhu *LookupHashUnique) Delete(ctx context.Context, vcursor VCursor, rowsCo
func (lhu *LookupHashUnique) Update(ctx context.Context, vcursor VCursor, oldValues []sqltypes.Value, ksid []byte, newValues []sqltypes.Value) error {
v, err := vunhash(ksid)
if err != nil {
return fmt.Errorf("lookup.Update.vunhash: %v", err)
return vterrors.Wrap(err, "lookup.Update.vunhash")
}
return lhu.lkp.Update(ctx, vcursor, oldValues, ksid, sqltypes.NewUint64(v), newValues)
}
Expand Down
10 changes: 2 additions & 8 deletions go/vt/vtgate/vindexes/lookup_hash_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -236,10 +236,7 @@ func TestLookupHashCreate(t *testing.T) {
}

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

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

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

func TestLookupHashDelete(t *testing.T) {
Expand Down
Loading
Loading