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

Pass on vindex errors with wrap than overriding them #14737

Merged
merged 7 commits into from
Dec 12, 2023
Merged
Show file tree
Hide file tree
Changes from 4 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
55 changes: 50 additions & 5 deletions go/test/endtoend/vtgate/misc_test.go
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 TestInsertNeg(t *testing.T) {
Expand Down Expand Up @@ -780,12 +781,56 @@ 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()

var errCount float64
apiErr := getVar(t, "VtgateApiErrorCounts")
if apiErr != nil {
mapErrors := apiErr.(map[string]interface{})
val, exists := mapErrors["Execute.ks.primary.ALREADY_EXISTS"]
if exists {
errCount = val.(float64)
}
}

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)`)

apiErr = getVar(t, "VtgateApiErrorCounts")
require.NotNil(t, apiErr)
mapErrors := apiErr.(map[string]interface{})
val, exists := mapErrors["Execute.ks.primary.ALREADY_EXISTS"]
require.True(t, exists)
require.EqualValues(t, errCount+1, val)
}

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
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
2 changes: 1 addition & 1 deletion go/vt/vterrors/code.go
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 Down
12 changes: 6 additions & 6 deletions go/vt/vtgate/planbuilder/testdata/dml_cases.json
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,11 +4893,11 @@
{
"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"
}
]
2 changes: 1 addition & 1 deletion go/vt/vtgate/planbuilder/testdata/unsupported_cases.json
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
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.Wrapf(vterrors.VT03006(), "owner table column count does not match vindex %s", lu.name)
harshit-gangal marked this conversation as resolved.
Show resolved Hide resolved
}
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
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
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, "VT03027: Column 'fromc' cannot be null")

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