Repository: vitessio/vitess
Pull request #6573 (2020-08-14 21:09:16)

Forward Port 6554 to master from 7.0
Forward port from 
6.0: #6554
7.0: #6572


Patch:
diff --git a/go/vt/vtgate/planbuilder/dml.go b/go/vt/vtgate/planbuilder/dml.go
index 67da3070e3..7fcd57983e 100644
--- a/go/vt/vtgate/planbuilder/dml.go
+++ b/go/vt/vtgate/planbuilder/dml.go
@@ -146,19 +146,20 @@ func buildDMLPlan(vschema ContextVSchema, dmlType string, stmt sqlparser.Stateme
 		return nil, nil, "", vterrors.New(vtrpcpb.Code_INTERNAL, "internal error: table.vindexTable is mysteriously nil")
 	}
 
+	routingType, ksidVindex, ksidCol, vindex, values, err := getDMLRouting(where, eupd.Table)
+	if err != nil {
+		return nil, nil, "", err
+	}
+
 	if ro.eroute.TargetDestination != nil {
 		if ro.eroute.TargetTabletType != topodatapb.TabletType_MASTER {
 			return nil, nil, "", vterrors.Errorf(vtrpcpb.Code_INVALID_ARGUMENT, "unsupported: %s statement with a replica target", dmlType)
 		}
 		eupd.Opcode = engine.ByDestination
 		eupd.TargetDestination = ro.eroute.TargetDestination
-		return eupd, nil, "", nil
+		return eupd, ksidVindex, ksidCol, nil
 	}
 
-	routingType, ksidVindex, ksidCol, vindex, values, err := getDMLRouting(where, eupd.Table)
-	if err != nil {
-		return nil, nil, "", err
-	}
 	eupd.Opcode = routingType
 	if routingType == engine.Scatter {
 		if limit != nil {


Test patch:
diff --git a/go/test/endtoend/vtgate/lookup_test.go b/go/test/endtoend/vtgate/lookup_test.go
index 90ac5e2a1f..29829410d3 100644
--- a/go/test/endtoend/vtgate/lookup_test.go
+++ b/go/test/endtoend/vtgate/lookup_test.go
@@ -164,6 +164,186 @@ func TestConsistentLookup(t *testing.T) {
 	exec(t, conn, "delete from t1 where id1=1")
 }
 
+func TestDMLScatter(t *testing.T) {
+	ctx := context.Background()
+	conn, err := mysql.Connect(ctx, &vtParams)
+	require.NoError(t, err)
+	defer conn.Close()
+
+	/* Simple insert. after this dml, the tables will contain the following:
+	t3 (id5, id6, id7):
+	1 2 3
+	2 2 3
+	3 4 3
+	4 5 4
+
+	t3_id7_idx (id7, keyspace_id:id6):
+	3 2
+	3 2
+	3 4
+	4 5
+	*/
+	exec(t, conn, "begin")
+	exec(t, conn, "insert into t3(id5, id6, id7) values(1, 2, 3), (2, 2, 3), (3, 4, 3), (4, 5, 4)")
+	exec(t, conn, "commit")
+	qr := exec(t, conn, "select id5, id6, id7 from t3 order by id5")
+	if got, want := fmt.Sprintf("%v", qr.Rows), "[[INT64(1) INT64(2) INT64(3)] [INT64(2) INT64(2) INT64(3)] [INT64(3) INT64(4) INT64(3)] [INT64(4) INT64(5) INT64(4)]]"; got != want {
+		t.Errorf("select:\n%v want\n%v", got, want)
+	}
+
+	/* Updating a non lookup column. after this dml, the tables will contain the following:
+	t3 (id5, id6, id7):
+	42 2 3
+	2 2 3
+	3 4 3
+	4 5 4
+
+	t3_id7_idx (id7, keyspace_id:id6):
+	3 2
+	3 2
+	3 4
+	4 5
+	*/
+	exec(t, conn, "update `ks[-]`.t3 set id5 = 42 where id5 = 1")
+	qr = exec(t, conn, "select id5, id6, id7 from t3 order by id5")
+	if got, want := fmt.Sprintf("%v", qr.Rows), "[[INT64(2) INT64(2) INT64(3)] [INT64(3) INT64(4) INT64(3)] [INT64(4) INT64(5) INT64(4)] [INT64(42) INT64(2) INT64(3)]]"; got != want {
+		t.Errorf("select:\n%v want\n%v", got, want)
+	}
+
+	/* Updating a lookup column. after this dml, the tables will contain the following:
+	t3 (id5, id6, id7):
+	42 2 42
+	2 2 42
+	3 4 3
+	4 5 4
+
+	t3_id7_idx (id7, keyspace_id:id6):
+	42 2
+	42 2
+	3 4
+	4 5
+	*/
+	exec(t, conn, "begin")
+	exec(t, conn, "update t3 set id7 = 42 where id6 = 2")
+	exec(t, conn, "commit")
+	qr = exec(t, conn, "select id5, id6, id7 from t3 order by id5")
+	if got, want := fmt.Sprintf("%v", qr.Rows), "[[INT64(2) INT64(2) INT64(42)] [INT64(3) INT64(4) INT64(3)] [INT64(4) INT64(5) INT64(4)] [INT64(42) INT64(2) INT64(42)]]"; got != want {
+		t.Errorf("select:\n%v want\n%v", got, want)
+	}
+
+	/* delete one specific keyspace id. after this dml, the tables will contain the following:
+	t3 (id5, id6, id7):
+	3 4 3
+	4 5 4
+
+	t3_id7_idx (id7, keyspace_id:id6):
+	3 4
+	4 5
+	*/
+	exec(t, conn, "delete from t3 where id6 = 2")
+	qr = exec(t, conn, "select * from t3 where id6 = 2")
+	require.Empty(t, qr.Rows)
+	qr = exec(t, conn, "select * from t3_id7_idx where id6 = 2")
+	require.Empty(t, qr.Rows)
+
+	// delete all the rows.
+	exec(t, conn, "delete from `ks[-]`.t3")
+	qr = exec(t, conn, "select * from t3")
+	require.Empty(t, qr.Rows)
+	qr = exec(t, conn, "select * from t3_id7_idx")
+	require.Empty(t, qr.Rows)
+}
+
+func TestDMLIn(t *testing.T) {
+	ctx := context.Background()
+	conn, err := mysql.Connect(ctx, &vtParams)
+	require.NoError(t, err)
+	defer conn.Close()
+
+	/* Simple insert. after this dml, the tables will contain the following:
+	t3 (id5, id6, id7):
+	1 2 3
+	2 2 3
+	3 4 3
+	4 5 4
+
+	t3_id7_idx (id7, keyspace_id:id6):
+	3 2
+	3 2
+	3 4
+	4 5
+	*/
+	exec(t, conn, "begin")
+	exec(t, conn, "insert into t3(id5, id6, id7) values(1, 2, 3), (2, 2, 3), (3, 4, 3), (4, 5, 4)")
+	exec(t, conn, "commit")
+	qr := exec(t, conn, "select id5, id6, id7 from t3 order by id5, id6")
+	if got, want := fmt.Sprintf("%v", qr.Rows), "[[INT64(1) INT64(2) INT64(3)] [INT64(2) INT64(2) INT64(3)] [INT64(3) INT64(4) INT64(3)] [INT64(4) INT64(5) INT64(4)]]"; got != want {
+		t.Errorf("select:\n%v want\n%v", got, want)
+	}
+
+	/* Updating a non lookup column. after this dml, the tables will contain the following:
+	t3 (id5, id6, id7):
+	1 2 3
+	2 2 3
+	42 4 3
+	42 5 4
+
+	t3_id7_idx (id7, keyspace_id:id6):
+	3 2
+	3 2
+	3 4
+	4 5
+	*/
+	exec(t, conn, "update t3 set id5 = 42 where id6 in (4, 5)")
+	qr = exec(t, conn, "select id5, id6, id7 from t3 order by id5, id6")
+	if got, want := fmt.Sprintf("%v", qr.Rows), "[[INT64(1) INT64(2) INT64(3)] [INT64(2) INT64(2) INT64(3)] [INT64(42) INT64(4) INT64(3)] [INT64(42) INT64(5) INT64(4)]]"; got != want {
+		t.Errorf("select:\n%v want\n%v", got, want)
+	}
+
+	/* Updating a non lookup column. after this dml, the tables will contain the following:
+	t3 (id5, id6, id7):
+	1 2 42
+	2 2 42
+	42 4 3
+	42 5 4
+
+	t3_id7_idx (id7, keyspace_id:id6):
+	42 2
+	42 2
+	3 4
+	42 5
+	*/
+	exec(t, conn, "begin")
+	exec(t, conn, "update t3 set id7 = 42 where id6 in (2, 5)")
+	exec(t, conn, "commit")
+	qr = exec(t, conn, "select id5, id6, id7 from t3 order by id5, id6")
+	if got, want := fmt.Sprintf("%v", qr.Rows), "[[INT64(1) INT64(2) INT64(42)] [INT64(2) INT64(2) INT64(42)] [INT64(42) INT64(4) INT64(3)] [INT64(42) INT64(5) INT64(42)]]"; got != want {
+		t.Errorf("select:\n%v want\n%v", got, want)
+	}
+
+	/* Updating a non lookup column. after this dml, the tables will contain the following:
+	t3 (id5, id6, id7):
+	42 4 3
+	42 5 4
+
+	t3_id7_idx (id7, keyspace_id:id6):
+	3 4
+	42 5
+	*/
+	exec(t, conn, "delete from t3 where id6 in (2)")
+	qr = exec(t, conn, "select * from t3 where id6 = 2")
+	require.Empty(t, qr.Rows)
+	qr = exec(t, conn, "select * from t3_id7_idx where id6 = 2")
+	require.Empty(t, qr.Rows)
+
+	// delete all the rows.
+	exec(t, conn, "delete from t3 where id6 in (4, 5)")
+	qr = exec(t, conn, "select * from t3")
+	require.Empty(t, qr.Rows)
+	qr = exec(t, conn, "select * from t3_id7_idx")
+	require.Empty(t, qr.Rows)
+}
+
 func TestConsistentLookupMultiInsert(t *testing.T) {
 	defer cluster.PanicHandler(t)
 	ctx := context.Background()
diff --git a/go/test/endtoend/vtgate/misc_test.go b/go/test/endtoend/vtgate/misc_test.go
index aec30cd7b6..81faeb428c 100644
--- a/go/test/endtoend/vtgate/misc_test.go
+++ b/go/test/endtoend/vtgate/misc_test.go
@@ -34,186 +34,6 @@ import (
 	"vitess.io/vitess/go/test/endtoend/cluster"
 )
 
-func TestDMLScatter(t *testing.T) {
-	ctx := context.Background()
-	conn, err := mysql.Connect(ctx, &vtParams)
-	require.NoError(t, err)
-	defer conn.Close()
-
-	/* Simple insert. after this dml, the tables will contain the following:
-	t3 (id5, id6, id7):
-	1 2 3
-	2 2 3
-	3 4 3
-	4 5 4
-
-	t3_id7_idx (id7, keyspace_id:id6):
-	3 2
-	3 2
-	3 4
-	4 5
-	*/
-	exec(t, conn, "begin")
-	exec(t, conn, "insert into t3(id5, id6, id7) values(1, 2, 3), (2, 2, 3), (3, 4, 3), (4, 5, 4)")
-	exec(t, conn, "commit")
-	qr := exec(t, conn, "select id5, id6, id7 from t3 order by id5")
-	if got, want := fmt.Sprintf("%v", qr.Rows), "[[INT64(1) INT64(2) INT64(3)] [INT64(2) INT64(2) INT64(3)] [INT64(3) INT64(4) INT64(3)] [INT64(4) INT64(5) INT64(4)]]"; got != want {
-		t.Errorf("select:\n%v want\n%v", got, want)
-	}
-
-	/* Updating a non lookup column. after this dml, the tables will contain the following:
-	t3 (id5, id6, id7):
-	42 2 3
-	2 2 3
-	3 4 3
-	4 5 4
-
-	t3_id7_idx (id7, keyspace_id:id6):
-	3 2
-	3 2
-	3 4
-	4 5
-	*/
-	exec(t, conn, "update t3 set id5 = 42 where id5 = 1")
-	qr = exec(t, conn, "select id5, id6, id7 from t3 order by id5")
-	if got, want := fmt.Sprintf("%v", qr.Rows), "[[INT64(2) INT64(2) INT64(3)] [INT64(3) INT64(4) INT64(3)] [INT64(4) INT64(5) INT64(4)] [INT64(42) INT64(2) INT64(3)]]"; got != want {
-		t.Errorf("select:\n%v want\n%v", got, want)
-	}
-
-	/* Updating a lookup column. after this dml, the tables will contain the following:
-	t3 (id5, id6, id7):
-	42 2 42
-	2 2 42
-	3 4 3
-	4 5 4
-
-	t3_id7_idx (id7, keyspace_id:id6):
-	42 2
-	42 2
-	3 4
-	4 5
-	*/
-	exec(t, conn, "begin")
-	exec(t, conn, "update t3 set id7 = 42 where id6 = 2")
-	exec(t, conn, "commit")
-	qr = exec(t, conn, "select id5, id6, id7 from t3 order by id5")
-	if got, want := fmt.Sprintf("%v", qr.Rows), "[[INT64(2) INT64(2) INT64(42)] [INT64(3) INT64(4) INT64(3)] [INT64(4) INT64(5) INT64(4)] [INT64(42) INT64(2) INT64(42)]]"; got != want {
-		t.Errorf("select:\n%v want\n%v", got, want)
-	}
-
-	/* delete one specific keyspace id. after this dml, the tables will contain the following:
-	t3 (id5, id6, id7):
-	3 4 3
-	4 5 4
-
-	t3_id7_idx (id7, keyspace_id:id6):
-	3 4
-	4 5
-	*/
-	exec(t, conn, "delete from t3 where id6 = 2")
-	qr = exec(t, conn, "select * from t3 where id6 = 2")
-	require.Empty(t, qr.Rows)
-	qr = exec(t, conn, "select * from t3_id7_idx where id6 = 2")
-	require.Empty(t, qr.Rows)
-
-	// delete all the rows.
-	exec(t, conn, "delete from t3")
-	qr = exec(t, conn, "select * from t3")
-	require.Empty(t, qr.Rows)
-	qr = exec(t, conn, "select * from t3_id7_idx")
-	require.Empty(t, qr.Rows)
-}
-
-func TestDMLIn(t *testing.T) {
-	ctx := context.Background()
-	conn, err := mysql.Connect(ctx, &vtParams)
-	require.NoError(t, err)
-	defer conn.Close()
-
-	/* Simple insert. after this dml, the tables will contain the following:
-	t3 (id5, id6, id7):
-	1 2 3
-	2 2 3
-	3 4 3
-	4 5 4
-
-	t3_id7_idx (id7, keyspace_id:id6):
-	3 2
-	3 2
-	3 4
-	4 5
-	*/
-	exec(t, conn, "begin")
-	exec(t, conn, "insert into t3(id5, id6, id7) values(1, 2, 3), (2, 2, 3), (3, 4, 3), (4, 5, 4)")
-	exec(t, conn, "commit")
-	qr := exec(t, conn, "select id5, id6, id7 from t3 order by id5, id6")
-	if got, want := fmt.Sprintf("%v", qr.Rows), "[[INT64(1) INT64(2) INT64(3)] [INT64(2) INT64(2) INT64(3)] [INT64(3) INT64(4) INT64(3)] [INT64(4) INT64(5) INT64(4)]]"; got != want {
-		t.Errorf("select:\n%v want\n%v", got, want)
-	}
-
-	/* Updating a non lookup column. after this dml, the tables will contain the following:
-	t3 (id5, id6, id7):
-	1 2 3
-	2 2 3
-	42 4 3
-	42 5 4
-
-	t3_id7_idx (id7, keyspace_id:id6):
-	3 2
-	3 2
-	3 4
-	4 5
-	*/
-	exec(t, conn, "update t3 set id5 = 42 where id6 in (4, 5)")
-	qr = exec(t, conn, "select id5, id6, id7 from t3 order by id5, id6")
-	if got, want := fmt.Sprintf("%v", qr.Rows), "[[INT64(1) INT64(2) INT64(3)] [INT64(2) INT64(2) INT64(3)] [INT64(42) INT64(4) INT64(3)] [INT64(42) INT64(5) INT64(4)]]"; got != want {
-		t.Errorf("select:\n%v want\n%v", got, want)
-	}
-
-	/* Updating a non lookup column. after this dml, the tables will contain the following:
-	t3 (id5, id6, id7):
-	1 2 42
-	2 2 42
-	42 4 3
-	42 5 4
-
-	t3_id7_idx (id7, keyspace_id:id6):
-	42 2
-	42 2
-	3 4
-	42 5
-	*/
-	exec(t, conn, "begin")
-	exec(t, conn, "update t3 set id7 = 42 where id6 in (2, 5)")
-	exec(t, conn, "commit")
-	qr = exec(t, conn, "select id5, id6, id7 from t3 order by id5, id6")
-	if got, want := fmt.Sprintf("%v", qr.Rows), "[[INT64(1) INT64(2) INT64(42)] [INT64(2) INT64(2) INT64(42)] [INT64(42) INT64(4) INT64(3)] [INT64(42) INT64(5) INT64(42)]]"; got != want {
-		t.Errorf("select:\n%v want\n%v", got, want)
-	}
-
-	/* Updating a non lookup column. after this dml, the tables will contain the following:
-	t3 (id5, id6, id7):
-	42 4 3
-	42 5 4
-
-	t3_id7_idx (id7, keyspace_id:id6):
-	3 4
-	42 5
-	*/
-	exec(t, conn, "delete from t3 where id6 in (2)")
-	qr = exec(t, conn, "select * from t3 where id6 = 2")
-	require.Empty(t, qr.Rows)
-	qr = exec(t, conn, "select * from t3_id7_idx where id6 = 2")
-	require.Empty(t, qr.Rows)
-
-	// delete all the rows.
-	exec(t, conn, "delete from t3 where id6 in (4, 5)")
-	qr = exec(t, conn, "select * from t3")
-	require.Empty(t, qr.Rows)
-	qr = exec(t, conn, "select * from t3_id7_idx")
-	require.Empty(t, qr.Rows)
-}
-
 func TestSelectNull(t *testing.T) {
 	ctx := context.Background()
 	conn, err := mysql.Connect(ctx, &vtParams)
diff --git a/go/vt/vtgate/planbuilder/testdata/dml_cases.txt b/go/vt/vtgate/planbuilder/testdata/dml_cases.txt
index d2b6d94659..6dfb273ed3 100644
--- a/go/vt/vtgate/planbuilder/testdata/dml_cases.txt
+++ b/go/vt/vtgate/planbuilder/testdata/dml_cases.txt
@@ -2150,3 +2150,67 @@
     "Table": "user"
   }
 }
+
+# delete with shard targeting
+"delete from `user[-]`.user"
+{
+  "QueryType": "DELETE",
+  "Original": "delete from `user[-]`.user",
+  "Instructions": {
+    "OperatorType": "Delete",
+    "Variant": "ByDestination",
+    "Keyspace": {
+      "Name": "user",
+      "Sharded": true
+    },
+    "TargetTabletType": "MASTER",
+    "KsidVindex": "user_index",
+    "MultiShardAutocommit": false,
+    "OwnedVindexQuery": "select Id, Name, Costly from user for update",
+    "Query": "delete from user",
+    "Table": "user"
+  }
+}
+
+# update with shard targeting
+"update `user[-]`.user set name = 'myname'"
+{
+  "QueryType": "UPDATE",
+  "Original": "update `user[-]`.user set name = 'myname'",
+  "Instructions": {
+    "OperatorType": "Update",
+    "Variant": "ByDestination",
+    "Keyspace": {
+      "Name": "user",
+      "Sharded": true
+    },
+    "TargetTabletType": "MASTER",
+    "ChangedVindexValues": [
+      "name_user_map"
+    ],
+    "KsidVindex": "user_index",
+    "MultiShardAutocommit": false,
+    "OwnedVindexQuery": "select Id, Name, Costly from user for update",
+    "Query": "update user set name = 'myname'",
+    "Table": "user"
+  }
+}
+
+# update with shard targeting without vindex
+"update `user[-]`.user_extra set val = 1"
+{
+  "QueryType": "UPDATE",
+  "Original": "update `user[-]`.user_extra set val = 1",
+  "Instructions": {
+    "OperatorType": "Update",
+    "Variant": "ByDestination",
+    "Keyspace": {
+      "Name": "user",
+      "Sharded": true
+    },
+    "TargetTabletType": "MASTER",
+    "MultiShardAutocommit": false,
+    "Query": "update user_extra set val = 1",
+    "Table": "user_extra"
+  }
+}