Merge pull request #6700 from planetscale/consistent_lkp_del

delete row using consistent lookup vindex in where clause
vitessio · Sep 14, 2020 · c893acd · c893acd
2 parents 936314c + cd12fe4
commit c893acd
Show file tree

Hide file tree

Showing 11 changed files with 85 additions and 5 deletions.
diff --git a/go/test/endtoend/vtgate/lookup_test.go b/go/test/endtoend/vtgate/lookup_test.go
@@ -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) {

diff --git a/go/vt/vtgate/engine/fake_vcursor_test.go b/go/vt/vtgate/engine/fake_vcursor_test.go
@@ -53,6 +53,10 @@ type noopVCursor struct {
 	ctx context.Context
 }
 
+func (t noopVCursor) LookupRowLockShardSession() vtgatepb.CommitOrder {
+	panic("implement me")
+}
+
 func (t noopVCursor) SetFoundRows(u uint64) {
 	panic("implement me")
 }

diff --git a/go/vt/vtgate/engine/primitive.go b/go/vt/vtgate/engine/primitive.go
@@ -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

diff --git a/go/vt/vtgate/executor_dml_test.go b/go/vt/vtgate/executor_dml_test.go
@@ -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, "")
+}
diff --git a/go/vt/vtgate/executor_framework_test.go b/go/vt/vtgate/executor_framework_test.go
@@ -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": {
@@ -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"
+				}
+			]
 		}
 	}
 }
@@ -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)

diff --git a/go/vt/vtgate/executor_test.go b/go/vt/vtgate/executor_test.go
@@ -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)

diff --git a/go/vt/vtgate/vcursor_impl.go b/go/vt/vtgate/vcursor_impl.go
@@ -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)

diff --git a/go/vt/vtgate/vindexes/consistent_lookup.go b/go/vt/vtgate/vindexes/consistent_lookup.go
@@ -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
 	}

diff --git a/go/vt/vtgate/vindexes/consistent_lookup_test.go b/go/vt/vtgate/vindexes/consistent_lookup_test.go
@@ -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
 }

diff --git a/go/vt/vtgate/vindexes/lookup_test.go b/go/vt/vtgate/vindexes/lookup_test.go
@@ -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
 }

diff --git a/go/vt/vtgate/vindexes/vindex.go b/go/vt/vtgate/vindexes/vindex.go
@@ -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.