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

delete row using consistent lookup vindex in where clause #6700

Merged
merged 6 commits into from
Sep 14, 2020
Merged
Show file tree
Hide file tree
Changes from 5 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
2 changes: 1 addition & 1 deletion go/test/endtoend/vtgate/lookup_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ func TestConsistentLookup(t *testing.T) {
if got, want := fmt.Sprintf("%v", qr.Rows), "[[INT64(5) VARBINARY(\"\\x16k@\\xb4J\\xbaK\\xd6\")]]"; got != want {
t.Errorf("select:\n%v want\n%v", got, want)
}
exec(t, conn, "delete from t1 where id1=1")
exec(t, conn, "delete from t1 where id2=5")
}

func TestDMLScatter(t *testing.T) {
Expand Down
4 changes: 4 additions & 0 deletions go/vt/vtgate/engine/fake_vcursor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,10 @@ type noopVCursor struct {
ctx context.Context
}

func (t noopVCursor) LookupRowLockShardSession() vtgatepb.CommitOrder {
panic("implement me")
}

func (t noopVCursor) InTransactionAndIsDML() bool {
panic("implement me")
}
Expand Down
2 changes: 2 additions & 0 deletions go/vt/vtgate/engine/primitive.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,8 @@ type (
ExecuteLock(rs *srvtopo.ResolvedShard, query *querypb.BoundQuery) (*sqltypes.Result, error)

InTransactionAndIsDML() bool

LookupRowLockShardSession() vtgatepb.CommitOrder
}

//SessionActions gives primitives ability to interact with the session state
Expand Down
27 changes: 27 additions & 0 deletions go/vt/vtgate/executor_dml_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1709,3 +1709,30 @@ func TestUpdateLastInsertID(t *testing.T) {

require.Equal(t, wantQueries, sbc1.Queries)
}

func TestDeleteLookupOwnedEqual(t *testing.T) {
executor, sbc1, sbc2, _ := createLegacyExecutorEnv()

sbc1.SetResults([]*sqltypes.Result{
sqltypes.MakeTestResult(sqltypes.MakeTestFields("uniq_col|keyspace_id", "int64|varbinary"), "1|N±\u0090ɢú\u0016\u009C"),
})
_, err := executorExec(executor, "delete from t1 where unq_col = 1", nil)
require.NoError(t, err)
tupleBindVar, _ := sqltypes.BuildBindVariable([]int64{1})
sbc1wantQueries := []*querypb.BoundQuery{{
Sql: "select unq_col, keyspace_id from t1_lkp_idx where unq_col in ::__vals for update",
BindVariables: map[string]*querypb.BindVariable{
"__vals": tupleBindVar,
"unq_col": tupleBindVar,
},
}}
sbc2wantQueries := []*querypb.BoundQuery{{
Sql: "select id, unq_col from t1 where unq_col = 1 for update",
BindVariables: map[string]*querypb.BindVariable{},
}, {
Sql: "delete from t1 where unq_col = 1",
BindVariables: map[string]*querypb.BindVariable{},
}}
utils.MustMatch(t, sbc1.Queries, sbc1wantQueries, "")
utils.MustMatch(t, sbc2.Queries, sbc2wantQueries, "")
}
33 changes: 31 additions & 2 deletions go/vt/vtgate/executor_framework_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,16 @@ var executorVSchema = `
},
"krcol_vdx": {
"type": "keyrange_lookuper"
}
},
"t1_lkp_vdx": {
"type": "consistent_lookup_unique",
"params": {
"table": "t1_lkp_idx",
"from": "unq_col",
"to": "keyspace_id"
},
"owner": "t1"
}
},
"tables": {
"user": {
Expand Down Expand Up @@ -231,6 +240,26 @@ var executorVSchema = `
"name": "keyspace_id"
}
]
},
"t1": {
"column_vindexes": [
{
"column": "id",
"name": "hash_index"
},
{
"column": "unq_col",
"name": "t1_lkp_vdx"
}
]
},
"t1_lkp_idx": {
"column_vindexes": [
{
"column": "unq_col",
"name": "hash_index"
}
]
}
}
}
Expand Down Expand Up @@ -349,7 +378,7 @@ func createLegacyExecutorEnv() (executor *Executor, sbc1, sbc2, sbclookup *sandb
sbc2 = hc.AddTestTablet(cell, "40-60", 1, "TestExecutor", "40-60", topodatapb.TabletType_MASTER, true, 1, nil)
// Create these connections so scatter queries don't fail.
_ = hc.AddTestTablet(cell, "20-40", 1, "TestExecutor", "20-40", topodatapb.TabletType_MASTER, true, 1, nil)
_ = hc.AddTestTablet(cell, "60-60", 1, "TestExecutor", "60-80", topodatapb.TabletType_MASTER, true, 1, nil)
_ = hc.AddTestTablet(cell, "60-80", 1, "TestExecutor", "60-80", topodatapb.TabletType_MASTER, true, 1, nil)
_ = hc.AddTestTablet(cell, "80-a0", 1, "TestExecutor", "80-a0", topodatapb.TabletType_MASTER, true, 1, nil)
_ = hc.AddTestTablet(cell, "a0-c0", 1, "TestExecutor", "a0-c0", topodatapb.TabletType_MASTER, true, 1, nil)
_ = hc.AddTestTablet(cell, "c0-e0", 1, "TestExecutor", "c0-e0", topodatapb.TabletType_MASTER, true, 1, nil)
Expand Down
3 changes: 2 additions & 1 deletion go/vt/vtgate/executor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -809,8 +809,9 @@ func TestExecutorShow(t *testing.T) {
buildVarCharRow("TestExecutor", "music_user_map", "lookup_hash_unique", "from=music_id; table=music_user_map; to=user_id", "music"),
buildVarCharRow("TestExecutor", "name_lastname_keyspace_id_map", "lookup", "from=name,lastname; table=name_lastname_keyspace_id_map; to=keyspace_id", "user2"),
buildVarCharRow("TestExecutor", "name_user_map", "lookup_hash", "from=name; table=name_user_map; to=user_id", "user"),
buildVarCharRow("TestExecutor", "t1_lkp_vdx", "consistent_lookup_unique", "from=unq_col; table=t1_lkp_idx; to=keyspace_id", "t1"),
},
RowsAffected: 10,
RowsAffected: 11,
}
if !reflect.DeepEqual(qr, wantqr) {
t.Errorf("show vschema vindexes:\n%+v, want\n%+v", qr, wantqr)
Expand Down
8 changes: 8 additions & 0 deletions go/vt/vtgate/vcursor_impl.go
Original file line number Diff line number Diff line change
Expand Up @@ -332,6 +332,14 @@ func (vc *vcursorImpl) InTransactionAndIsDML() bool {
return false
}

func (vc *vcursorImpl) LookupRowLockShardSession() vtgatepb.CommitOrder {
switch vc.logStats.StmtType {
case "DELETE":
return vtgatepb.CommitOrder_POST
}
return vtgatepb.CommitOrder_PRE
}

func (vc *vcursorImpl) ExecuteLock(rs *srvtopo.ResolvedShard, query *querypb.BoundQuery) (*sqltypes.Result, error) {
query.Sql = vc.marginComments.Leading + query.Sql + vc.marginComments.Trailing
return vc.executor.ExecuteLock(vc.ctx, rs, query, vc.safeSession)
Expand Down
2 changes: 1 addition & 1 deletion go/vt/vtgate/vindexes/consistent_lookup.go
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ func (lu *ConsistentLookupUnique) Map(vcursor VCursor, ids []sqltypes.Value) ([]
return out, nil
}

results, err := lu.lkp.Lookup(vcursor, ids, vtgatepb.CommitOrder_PRE)
results, err := lu.lkp.Lookup(vcursor, ids, vcursor.LookupRowLockShardSession())
if err != nil {
return nil, err
}
Expand Down
4 changes: 4 additions & 0 deletions go/vt/vtgate/vindexes/consistent_lookup_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -506,6 +506,10 @@ type loggingVCursor struct {
log []string
}

func (vc *loggingVCursor) LookupRowLockShardSession() vtgatepb.CommitOrder {
return vtgatepb.CommitOrder_PRE
}

func (vc *loggingVCursor) InTransactionAndIsDML() bool {
return false
}
Expand Down
4 changes: 4 additions & 0 deletions go/vt/vtgate/vindexes/lookup_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,10 @@ type vcursor struct {
keys []sqltypes.Value
}

func (vc *vcursor) LookupRowLockShardSession() vtgatepb.CommitOrder {
panic("implement me")
}

func (vc *vcursor) InTransactionAndIsDML() bool {
return false
}
Expand Down
1 change: 1 addition & 0 deletions go/vt/vtgate/vindexes/vindex.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ type VCursor interface {
Execute(method string, query string, bindvars map[string]*querypb.BindVariable, rollbackOnError bool, co vtgatepb.CommitOrder) (*sqltypes.Result, error)
ExecuteKeyspaceID(keyspace string, ksid []byte, query string, bindVars map[string]*querypb.BindVariable, rollbackOnError, autocommit bool) (*sqltypes.Result, error)
InTransactionAndIsDML() bool
LookupRowLockShardSession() vtgatepb.CommitOrder
}

// Vindex defines the interface required to register a vindex.
Expand Down