diff --git a/go/test/endtoend/utils/cmp.go b/go/test/endtoend/utils/cmp.go index dd9614e79fa..3a05c75c33b 100644 --- a/go/test/endtoend/utils/cmp.go +++ b/go/test/endtoend/utils/cmp.go @@ -215,6 +215,18 @@ func (mcmp *MySQLCompare) Exec(query string) *sqltypes.Result { return vtQr } +// ExecVitessAndMySQL executes Vitess and MySQL with the queries provided. +func (mcmp *MySQLCompare) ExecVitessAndMySQL(vtQ, mQ string) *sqltypes.Result { + mcmp.t.Helper() + vtQr, err := mcmp.VtConn.ExecuteFetch(vtQ, 1000, true) + require.NoError(mcmp.t, err, "[Vitess Error] for query: "+vtQ) + + mysqlQr, err := mcmp.MySQLConn.ExecuteFetch(mQ, 1000, true) + require.NoError(mcmp.t, err, "[MySQL Error] for query: "+mQ) + compareVitessAndMySQLResults(mcmp.t, vtQ, mcmp.VtConn, vtQr, mysqlQr, CompareOptions{}) + return vtQr +} + // ExecAssert is the same as Exec, but it only does assertions, it won't FailNow func (mcmp *MySQLCompare) ExecAssert(query string) *sqltypes.Result { mcmp.t.Helper() diff --git a/go/test/endtoend/vtgate/plan_tests/main_test.go b/go/test/endtoend/vtgate/plan_tests/main_test.go index 504ec3ffb26..2dc2e70120b 100644 --- a/go/test/endtoend/vtgate/plan_tests/main_test.go +++ b/go/test/endtoend/vtgate/plan_tests/main_test.go @@ -22,6 +22,7 @@ import ( "os" "testing" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "vitess.io/vitess/go/mysql" @@ -86,7 +87,7 @@ func TestMain(m *testing.M) { // TODO: (@GuptaManan100/@systay): Also run the tests with normalizer on. clusterInstance.VtGateExtraArgs = append(clusterInstance.VtGateExtraArgs, "--normalize_queries=false", - "--schema_change_signal=false", + "--schema_change_signal=true", ) // Start vtgate @@ -178,7 +179,7 @@ func verifyTestExpectations(t *testing.T, pd engine.PrimitiveDescription, test p // 1. Verify that the Join primitive sees atleast 1 row on the left side. engine.WalkPrimitiveDescription(pd, func(description engine.PrimitiveDescription) { if description.OperatorType == "Join" { - require.NotZero(t, description.Inputs[0].RowsReceived[0]) + assert.NotZero(t, description.Inputs[0].RowsReceived[0]) } }) diff --git a/go/test/endtoend/vtgate/plan_tests/plan_e2e_test.go b/go/test/endtoend/vtgate/plan_tests/plan_e2e_test.go index 5c5447fe6b6..ffe712c141c 100644 --- a/go/test/endtoend/vtgate/plan_tests/plan_e2e_test.go +++ b/go/test/endtoend/vtgate/plan_tests/plan_e2e_test.go @@ -19,11 +19,22 @@ package plan_tests import ( "testing" + "github.com/stretchr/testify/require" + "vitess.io/vitess/go/test/endtoend/utils" + "vitess.io/vitess/go/vt/sqlparser" ) func TestE2ECases(t *testing.T) { - e2eTestCaseFiles := []string{"select_cases.json", "filter_cases.json", "dml_cases.json"} + err := utils.WaitForAuthoritative(t, "main", "source_of_ref", clusterInstance.VtgateProcess.ReadVSchema) + require.NoError(t, err) + + e2eTestCaseFiles := []string{ + "select_cases.json", + "filter_cases.json", + "dml_cases.json", + "reference_cases.json", + } mcmp, closer := start(t) defer closer() loadSampleData(t, mcmp) @@ -34,7 +45,11 @@ func TestE2ECases(t *testing.T) { if test.SkipE2E { mcmp.AsT().Skip(test.Query) } - mcmp.Exec(test.Query) + stmt, err := sqlparser.NewTestParser().Parse(test.Query) + require.NoError(mcmp.AsT(), err) + sqlparser.RemoveKeyspaceIgnoreSysSchema(stmt) + + mcmp.ExecVitessAndMySQL(test.Query, sqlparser.String(stmt)) pd := utils.ExecTrace(mcmp.AsT(), mcmp.VtConn, test.Query) verifyTestExpectations(mcmp.AsT(), pd, test) if mcmp.VtConn.IsClosed() { diff --git a/go/vt/sqlparser/ast_funcs.go b/go/vt/sqlparser/ast_funcs.go index 836c824010d..2891d532d16 100644 --- a/go/vt/sqlparser/ast_funcs.go +++ b/go/vt/sqlparser/ast_funcs.go @@ -2440,6 +2440,25 @@ func RemoveKeyspace(in SQLNode) { }) } +// RemoveKeyspaceIgnoreSysSchema removes the Qualifier.Qualifier on all ColNames and Qualifier on all TableNames in the AST +// except for the system schema. +func RemoveKeyspaceIgnoreSysSchema(in SQLNode) { + Rewrite(in, nil, func(cursor *Cursor) bool { + switch expr := cursor.Node().(type) { + case *ColName: + if expr.Qualifier.Qualifier.NotEmpty() && !SystemSchema(expr.Qualifier.Qualifier.String()) { + expr.Qualifier.Qualifier = NewIdentifierCS("") + } + case TableName: + if expr.Qualifier.NotEmpty() && !SystemSchema(expr.Qualifier.String()) { + expr.Qualifier = NewIdentifierCS("") + cursor.Replace(expr) + } + } + return true + }) +} + func convertStringToInt(integer string) int { val, _ := strconv.Atoi(integer) return val diff --git a/go/vt/vtgate/planbuilder/operators/cte_merging.go b/go/vt/vtgate/planbuilder/operators/cte_merging.go index cb19e06b2a7..0c1556c81e4 100644 --- a/go/vt/vtgate/planbuilder/operators/cte_merging.go +++ b/go/vt/vtgate/planbuilder/operators/cte_merging.go @@ -31,7 +31,7 @@ func tryMergeRecurse(ctx *plancontext.PlanningContext, in *RecurseCTE) (Operator } func tryMergeCTE(ctx *plancontext.PlanningContext, seed, term Operator, in *RecurseCTE) *Route { - seedRoute, termRoute, routingA, routingB, a, b, sameKeyspace := prepareInputRoutes(seed, term) + seedRoute, termRoute, routingA, routingB, a, b, sameKeyspace := prepareInputRoutes(ctx, seed, term) if seedRoute == nil { return nil } diff --git a/go/vt/vtgate/planbuilder/operators/delete.go b/go/vt/vtgate/planbuilder/operators/delete.go index 81e36d54315..015220470e0 100644 --- a/go/vt/vtgate/planbuilder/operators/delete.go +++ b/go/vt/vtgate/planbuilder/operators/delete.go @@ -124,7 +124,7 @@ func createDeleteWithInputOp(ctx *plancontext.PlanningContext, del *sqlparser.De } var delOps []dmlOp - for _, target := range ctx.SemTable.Targets.Constituents() { + for _, target := range ctx.SemTable.DMLTargets.Constituents() { op := createDeleteOpWithTarget(ctx, target, del.Ignore) delOps = append(delOps, op) } @@ -322,7 +322,7 @@ func updateQueryGraphWithSource(ctx *plancontext.PlanningContext, input Operator return op, NoRewrite } if len(qg.Tables) > 1 { - panic(vterrors.VT12001("DELETE on reference table with join")) + panic(vterrors.VT12001("DML on reference table with join")) } for _, tbl := range qg.Tables { if tbl.ID != tblID { diff --git a/go/vt/vtgate/planbuilder/operators/join_merging.go b/go/vt/vtgate/planbuilder/operators/join_merging.go index c035b7d11ed..cb3569cf79e 100644 --- a/go/vt/vtgate/planbuilder/operators/join_merging.go +++ b/go/vt/vtgate/planbuilder/operators/join_merging.go @@ -28,7 +28,7 @@ import ( // If they can be merged, a new operator with the merged routing is returned // If they cannot be merged, nil is returned. func (jm *joinMerger) mergeJoinInputs(ctx *plancontext.PlanningContext, lhs, rhs Operator, joinPredicates []sqlparser.Expr) *Route { - lhsRoute, rhsRoute, routingA, routingB, a, b, sameKeyspace := prepareInputRoutes(lhs, rhs) + lhsRoute, rhsRoute, routingA, routingB, a, b, sameKeyspace := prepareInputRoutes(ctx, lhs, rhs) if lhsRoute == nil { return nil } @@ -102,13 +102,13 @@ func mergeAnyShardRoutings(ctx *plancontext.PlanningContext, a, b *AnyShardRouti } } -func prepareInputRoutes(lhs Operator, rhs Operator) (*Route, *Route, Routing, Routing, routingType, routingType, bool) { +func prepareInputRoutes(ctx *plancontext.PlanningContext, lhs Operator, rhs Operator) (*Route, *Route, Routing, Routing, routingType, routingType, bool) { lhsRoute, rhsRoute := operatorsToRoutes(lhs, rhs) if lhsRoute == nil || rhsRoute == nil { return nil, nil, nil, nil, 0, 0, false } - lhsRoute, rhsRoute, routingA, routingB, sameKeyspace := getRoutesOrAlternates(lhsRoute, rhsRoute) + lhsRoute, rhsRoute, routingA, routingB, sameKeyspace := getRoutesOrAlternates(ctx, lhsRoute, rhsRoute) a, b := getRoutingType(routingA), getRoutingType(routingB) return lhsRoute, rhsRoute, routingA, routingB, a, b, sameKeyspace @@ -159,7 +159,7 @@ func (rt routingType) String() string { // getRoutesOrAlternates gets the Routings from each Route. If they are from different keyspaces, // we check if this is a table with alternates in other keyspaces that we can use -func getRoutesOrAlternates(lhsRoute, rhsRoute *Route) (*Route, *Route, Routing, Routing, bool) { +func getRoutesOrAlternates(ctx *plancontext.PlanningContext, lhsRoute, rhsRoute *Route) (*Route, *Route, Routing, Routing, bool) { routingA := lhsRoute.Routing routingB := rhsRoute.Routing sameKeyspace := routingA.Keyspace() == routingB.Keyspace() @@ -171,13 +171,17 @@ func getRoutesOrAlternates(lhsRoute, rhsRoute *Route) (*Route, *Route, Routing, return lhsRoute, rhsRoute, routingA, routingB, sameKeyspace } - if refA, ok := routingA.(*AnyShardRouting); ok { + // If we have a reference route, we will try to find an alternate route in same keyspace as other routing keyspace. + // If the reference route is part of DML table update target, alternate keyspace route cannot be considered. + if refA, ok := routingA.(*AnyShardRouting); ok && + !TableID(lhsRoute).IsOverlapping(ctx.SemTable.DMLTargets) { if altARoute := refA.AlternateInKeyspace(routingB.Keyspace()); altARoute != nil { return altARoute, rhsRoute, altARoute.Routing, routingB, true } } - if refB, ok := routingB.(*AnyShardRouting); ok { + if refB, ok := routingB.(*AnyShardRouting); ok && + !TableID(rhsRoute).IsOverlapping(ctx.SemTable.DMLTargets) { if altBRoute := refB.AlternateInKeyspace(routingA.Keyspace()); altBRoute != nil { return lhsRoute, altBRoute, routingA, altBRoute.Routing, true } diff --git a/go/vt/vtgate/planbuilder/operators/subquery_planning.go b/go/vt/vtgate/planbuilder/operators/subquery_planning.go index a2aca74fb6e..e222ae0f343 100644 --- a/go/vt/vtgate/planbuilder/operators/subquery_planning.go +++ b/go/vt/vtgate/planbuilder/operators/subquery_planning.go @@ -730,7 +730,7 @@ func mergeSubqueryInputs(ctx *plancontext.PlanningContext, in, out Operator, joi return nil } - inRoute, outRoute, inRouting, outRouting, sameKeyspace := getRoutesOrAlternates(inRoute, outRoute) + inRoute, outRoute, inRouting, outRouting, sameKeyspace := getRoutesOrAlternates(ctx, inRoute, outRoute) inner, outer := getRoutingType(inRouting), getRoutingType(outRouting) switch { diff --git a/go/vt/vtgate/planbuilder/operators/union_merging.go b/go/vt/vtgate/planbuilder/operators/union_merging.go index 000d176b61a..6173b59e0dc 100644 --- a/go/vt/vtgate/planbuilder/operators/union_merging.go +++ b/go/vt/vtgate/planbuilder/operators/union_merging.go @@ -108,7 +108,7 @@ func mergeUnionInputs( lhsExprs, rhsExprs sqlparser.SelectExprs, distinct bool, ) (Operator, sqlparser.SelectExprs) { - lhsRoute, rhsRoute, routingA, routingB, a, b, sameKeyspace := prepareInputRoutes(lhs, rhs) + lhsRoute, rhsRoute, routingA, routingB, a, b, sameKeyspace := prepareInputRoutes(ctx, lhs, rhs) if lhsRoute == nil { return nil, nil } diff --git a/go/vt/vtgate/planbuilder/operators/update.go b/go/vt/vtgate/planbuilder/operators/update.go index dd0a86c2de2..18a81175f7b 100644 --- a/go/vt/vtgate/planbuilder/operators/update.go +++ b/go/vt/vtgate/planbuilder/operators/update.go @@ -164,7 +164,7 @@ func createUpdateWithInputOp(ctx *plancontext.PlanningContext, upd *sqlparser.Up ueMap := prepareUpdateExpressionList(ctx, upd) var updOps []dmlOp - for _, target := range ctx.SemTable.Targets.Constituents() { + for _, target := range ctx.SemTable.DMLTargets.Constituents() { op := createUpdateOpWithTarget(ctx, upd, target, ueMap[target]) updOps = append(updOps, op) } @@ -308,7 +308,7 @@ func errIfUpdateNotSupported(ctx *plancontext.PlanningContext, stmt *sqlparser.U } } - // Now we check if any of the foreign key columns that are being udpated have dependencies on other updated columns. + // Now we check if any of the foreign key columns that are being updated have dependencies on other updated columns. // This is unsafe, and we currently don't support this in Vitess. if err := ctx.SemTable.ErrIfFkDependentColumnUpdated(stmt.Exprs); err != nil { panic(err) diff --git a/go/vt/vtgate/planbuilder/plan_test.go b/go/vt/vtgate/planbuilder/plan_test.go index df813b04dea..f3bed93e3c8 100644 --- a/go/vt/vtgate/planbuilder/plan_test.go +++ b/go/vt/vtgate/planbuilder/plan_test.go @@ -84,6 +84,7 @@ func (s *planTestSuite) TestPlan() { s.addPKsProvided(vschema, "user", []string{"user_extra"}, []string{"id", "user_id"}) s.addPKsProvided(vschema, "ordering", []string{"order"}, []string{"oid", "region_id"}) s.addPKsProvided(vschema, "ordering", []string{"order_event"}, []string{"oid", "ename"}) + s.addPKsProvided(vschema, "main", []string{"source_of_ref"}, []string{"id"}) // You will notice that some tests expect user.Id instead of user.id. // This is because we now pre-create vindex columns in the symbol @@ -305,6 +306,7 @@ func (s *planTestSuite) TestOne() { s.addPKsProvided(vschema, "user", []string{"user_extra"}, []string{"id", "user_id"}) s.addPKsProvided(vschema, "ordering", []string{"order"}, []string{"oid", "region_id"}) s.addPKsProvided(vschema, "ordering", []string{"order_event"}, []string{"oid", "ename"}) + s.addPKsProvided(vschema, "main", []string{"source_of_ref"}, []string{"id"}) s.testFile("onecase.json", vw, false) } @@ -666,7 +668,7 @@ func (s *planTestSuite) testFile(filename string, vschema *vschemawrapper.VSchem current := PlanTest{ Comment: tcase.Comment, Query: tcase.Query, - SkipE2E: true, + SkipE2E: tcase.SkipE2E, } vschema.Version = Gen4 out := getPlanOutput(tcase, vschema, render) diff --git a/go/vt/vtgate/planbuilder/testdata/reference_cases.json b/go/vt/vtgate/planbuilder/testdata/reference_cases.json index 6aa01355934..1bf893beeef 100644 --- a/go/vt/vtgate/planbuilder/testdata/reference_cases.json +++ b/go/vt/vtgate/planbuilder/testdata/reference_cases.json @@ -2,6 +2,7 @@ { "comment": "select from unqualified ambiguous reference routes to reference source", "query": "select * from ambiguous_ref_with_source", + "skip_e2e": true, "plan": { "QueryType": "SELECT", "Original": "select * from ambiguous_ref_with_source", @@ -24,6 +25,7 @@ { "comment": "join with unqualified ambiguous reference table routes to optimal keyspace", "query": "select user.col from user join ambiguous_ref_with_source", + "skip_e2e": true, "plan": { "QueryType": "SELECT", "Original": "select user.col from user join ambiguous_ref_with_source", @@ -47,6 +49,7 @@ { "comment": "ambiguous unqualified reference table self-join routes to reference source", "query": "select r1.col from ambiguous_ref_with_source r1 join ambiguous_ref_with_source", + "skip_e2e": true, "plan": { "QueryType": "SELECT", "Original": "select r1.col from ambiguous_ref_with_source r1 join ambiguous_ref_with_source", @@ -69,6 +72,7 @@ { "comment": "ambiguous unqualified reference table can merge with other opcodes left to right.", "query": "select ambiguous_ref_with_source.col from ambiguous_ref_with_source join user", + "skip_e2e": true, "plan": { "QueryType": "SELECT", "Original": "select ambiguous_ref_with_source.col from ambiguous_ref_with_source join user", @@ -92,6 +96,7 @@ { "comment": "ambiguous unqualified reference table can merge with other opcodes left to right and vindex value is in the plan", "query": "select ambiguous_ref_with_source.col from ambiguous_ref_with_source join (select aa from user where user.id=1) user", + "skip_e2e": true, "plan": { "QueryType": "SELECT", "Original": "select ambiguous_ref_with_source.col from ambiguous_ref_with_source join (select aa from user where user.id=1) user", @@ -119,6 +124,7 @@ { "comment": "qualified join to reference table routes to optimal keyspace", "query": "select user.col from user join main.ambiguous_ref_with_source", + "skip_e2e": true, "plan": { "QueryType": "SELECT", "Original": "select user.col from user join main.ambiguous_ref_with_source", @@ -142,6 +148,7 @@ { "comment": "insert into ambiguous unqualified reference table routes to source", "query": "insert into ambiguous_ref_with_source(col) values(1)", + "skip_e2e": true, "plan": { "QueryType": "INSERT", "Original": "insert into ambiguous_ref_with_source(col) values(1)", @@ -164,6 +171,7 @@ { "comment": "Reference tables using left join with a derived table having a limit clause", "query": "SELECT u.id FROM ( SELECT a.id, a.u_id FROM user.ref_with_source AS a WHERE a.id IN (3) ORDER BY a.d_at LIMIT 1) as u LEFT JOIN user.ref_with_source AS u0 ON u.u_id = u0.u_uid ORDER BY u.id", + "skip_e2e": true, "plan": { "QueryType": "SELECT", "Original": "SELECT u.id FROM ( SELECT a.id, a.u_id FROM user.ref_with_source AS a WHERE a.id IN (3) ORDER BY a.d_at LIMIT 1) as u LEFT JOIN user.ref_with_source AS u0 ON u.u_id = u0.u_uid ORDER BY u.id", @@ -208,6 +216,7 @@ { "comment": "insert into qualified ambiguous reference table routes to source", "query": "insert into user.ambiguous_ref_with_source(col) values(1)", + "skip_e2e": true, "plan": { "QueryType": "INSERT", "Original": "insert into user.ambiguous_ref_with_source(col) values(1)", @@ -230,6 +239,7 @@ { "comment": "update unqualified ambiguous reference table routes to source", "query": "update ambiguous_ref_with_source set col = 1", + "skip_e2e": true, "plan": { "QueryType": "UPDATE", "Original": "update ambiguous_ref_with_source set col = 1", @@ -252,6 +262,7 @@ { "comment": "update qualified ambiguous reference table route to source", "query": "update user.ambiguous_ref_with_source set col = 1", + "skip_e2e": true, "plan": { "QueryType": "UPDATE", "Original": "update user.ambiguous_ref_with_source set col = 1", @@ -274,6 +285,7 @@ { "comment": "delete from unqualified ambiguous reference table routes to source", "query": "delete from ambiguous_ref_with_source where col = 1", + "skip_e2e": true, "plan": { "QueryType": "DELETE", "Original": "delete from ambiguous_ref_with_source where col = 1", @@ -296,6 +308,7 @@ { "comment": "delete from qualified ambiguous reference table route to source", "query": "delete from user.ambiguous_ref_with_source where col = 1", + "skip_e2e": true, "plan": { "QueryType": "DELETE", "Original": "delete from user.ambiguous_ref_with_source where col = 1", @@ -318,6 +331,7 @@ { "comment": "join with unqualified unambiguous ref with source routes to requested table", "query": "select user.col from user join ref_with_source", + "skip_e2e": true, "plan": { "QueryType": "SELECT", "Original": "select user.col from user join ref_with_source", @@ -341,6 +355,7 @@ { "comment": "join with unqualified reference optimize routes when source & reference have different names", "query": "select user.col from user join source_of_ref", + "skip_e2e": true, "plan": { "QueryType": "SELECT", "Original": "select user.col from user join source_of_ref", @@ -364,6 +379,7 @@ { "comment": "join with unqualified reference respects routing rules", "query": "select user.col from user join rerouted_ref", + "skip_e2e": true, "plan": { "QueryType": "SELECT", "Original": "select user.col from user join rerouted_ref", @@ -387,6 +403,7 @@ { "comment": "join with reference to unqualified source routes to optimal keyspace", "query": "select user.col from user join global_ref", + "skip_e2e": true, "plan": { "QueryType": "SELECT", "Original": "select user.col from user join global_ref", @@ -410,6 +427,7 @@ { "comment": "insert into qualified reference with unqualified source routes to source", "query": "insert into user.global_ref(col) values(1)", + "skip_e2e": true, "plan": { "QueryType": "INSERT", "Original": "insert into user.global_ref(col) values(1)", @@ -432,6 +450,7 @@ { "comment": "delete from reference table with another name - query send to source table", "query": "delete from user.ref_with_source where col = 1", + "skip_e2e": true, "plan": { "QueryType": "DELETE", "Original": "delete from user.ref_with_source where col = 1", @@ -454,6 +473,7 @@ { "comment": "update from reference table with another name - query send to source table", "query": "update user.ref_with_source set x = 4 where col = 1", + "skip_e2e": true, "plan": { "QueryType": "UPDATE", "Original": "update user.ref_with_source set x = 4 where col = 1", @@ -476,6 +496,7 @@ { "comment": "insert from reference table with another name - query send to source table", "query": "insert into user.ref_with_source(x) values(4)", + "skip_e2e": true, "plan": { "QueryType": "INSERT", "Original": "insert into user.ref_with_source(x) values(4)", @@ -498,6 +519,7 @@ { "comment": "delete from reference table - query send to source table", "query": "delete from source_of_ref where col = 1", + "skip_e2e": true, "plan": { "QueryType": "DELETE", "Original": "delete from source_of_ref where col = 1", @@ -520,6 +542,7 @@ { "comment": "update from reference table - query send to source table", "query": "update source_of_ref set x = 4 where col = 1", + "skip_e2e": true, "plan": { "QueryType": "UPDATE", "Original": "update source_of_ref set x = 4 where col = 1", @@ -542,6 +565,7 @@ { "comment": "insert from reference table - query send to source table", "query": "insert into source_of_ref(x) values(4)", + "skip_e2e": true, "plan": { "QueryType": "INSERT", "Original": "insert into source_of_ref(x) values(4)", @@ -564,6 +588,7 @@ { "comment": "delete from reference table qualified with unsharded - query send to source table", "query": "delete from main.source_of_ref where col = 1", + "skip_e2e": true, "plan": { "QueryType": "DELETE", "Original": "delete from main.source_of_ref where col = 1", @@ -586,6 +611,7 @@ { "comment": "update from reference table qualified with unsharded - query send to source table", "query": "update main.source_of_ref set x = 4 where col = 1", + "skip_e2e": true, "plan": { "QueryType": "UPDATE", "Original": "update main.source_of_ref set x = 4 where col = 1", @@ -608,6 +634,7 @@ { "comment": "insert from reference table qualified with unsharded - query send to source table", "query": "insert into main.source_of_ref(x) values(4)", + "skip_e2e": true, "plan": { "QueryType": "INSERT", "Original": "insert into main.source_of_ref(x) values(4)", @@ -630,6 +657,7 @@ { "comment": "delete from reference table with another name - query send to source table", "query": "delete from user.ref_with_source where col = 1", + "skip_e2e": true, "plan": { "QueryType": "DELETE", "Original": "delete from user.ref_with_source where col = 1", @@ -652,6 +680,7 @@ { "comment": "update from reference table with another name - query send to source table", "query": "update user.ref_with_source set x = 4 where col = 1", + "skip_e2e": true, "plan": { "QueryType": "UPDATE", "Original": "update user.ref_with_source set x = 4 where col = 1", @@ -674,6 +703,7 @@ { "comment": "insert from reference table with another name - query send to source table", "query": "insert into user.ref_with_source(x) values(4)", + "skip_e2e": true, "plan": { "QueryType": "INSERT", "Original": "insert into user.ref_with_source(x) values(4)", @@ -696,6 +726,7 @@ { "comment": "select with join to reference table in sharded keyspace: should route shard-scoped", "query": "select * from user.ref_with_source ref, `user`.`user` u where ref.id = u.ref_id and u.id = 2", + "skip_e2e": true, "plan": { "QueryType": "SELECT", "Original": "select * from user.ref_with_source ref, `user`.`user` u where ref.id = u.ref_id and u.id = 2", @@ -723,6 +754,7 @@ { "comment": "select with join to reference table in unsharded keyspace: should route shard-scoped", "query": "select * from source_of_ref ref, `user`.`user` u where ref.id = u.ref_id and u.id = 2", + "skip_e2e": true, "plan": { "QueryType": "SELECT", "Original": "select * from source_of_ref ref, `user`.`user` u where ref.id = u.ref_id and u.id = 2", @@ -750,6 +782,7 @@ { "comment": "two sharded and two unsharded reference table join - all should be merged into one route", "query": "select 1 from user u join user_extra ue on u.id = ue.user_id join main.source_of_ref sr on sr.foo = ue.foo join main.rerouted_ref rr on rr.bar = sr.bar", + "skip_e2e": true, "plan": { "QueryType": "SELECT", "Original": "select 1 from user u join user_extra ue on u.id = ue.user_id join main.source_of_ref sr on sr.foo = ue.foo join main.rerouted_ref rr on rr.bar = sr.bar", @@ -771,5 +804,145 @@ "user.user_extra" ] } + }, + { + "comment": "update reference table with join on sharded table", + "query": "update main.source_of_ref as sr join main.rerouted_ref as rr on sr.id = rr.id inner join user.music as m on sr.col = m.col set sr.tt = 5 where m.user_id = 1", + "plan": { + "QueryType": "UPDATE", + "Original": "update main.source_of_ref as sr join main.rerouted_ref as rr on sr.id = rr.id inner join user.music as m on sr.col = m.col set sr.tt = 5 where m.user_id = 1", + "Instructions": { + "OperatorType": "DMLWithInput", + "TargetTabletType": "PRIMARY", + "Offset": [ + "0:[0]" + ], + "Inputs": [ + { + "OperatorType": "Join", + "Variant": "Join", + "JoinColumnIndexes": "R:0", + "JoinVars": { + "m_col": 0 + }, + "TableName": "music_rerouted_ref, source_of_ref", + "Inputs": [ + { + "OperatorType": "Route", + "Variant": "EqualUnique", + "Keyspace": { + "Name": "user", + "Sharded": true + }, + "FieldQuery": "select m.col from music as m where 1 != 1", + "Query": "select m.col from music as m where m.user_id = 1 lock in share mode", + "Table": "music", + "Values": [ + "1" + ], + "Vindex": "user_index" + }, + { + "OperatorType": "Route", + "Variant": "Unsharded", + "Keyspace": { + "Name": "main", + "Sharded": false + }, + "FieldQuery": "select sr.id from source_of_ref as sr, rerouted_ref as rr where 1 != 1", + "Query": "select sr.id from source_of_ref as sr, rerouted_ref as rr where sr.col = :m_col and sr.id = rr.id lock in share mode", + "Table": "rerouted_ref, source_of_ref" + } + ] + }, + { + "OperatorType": "Update", + "Variant": "Unsharded", + "Keyspace": { + "Name": "main", + "Sharded": false + }, + "TargetTabletType": "PRIMARY", + "Query": "update source_of_ref as sr set sr.tt = 5 where sr.id in ::dml_vals", + "Table": "source_of_ref" + } + ] + }, + "TablesUsed": [ + "main.rerouted_ref", + "main.source_of_ref", + "user.music" + ] + } + }, + { + "comment": "delete from reference table with join on sharded table", + "query": "delete sr from main.source_of_ref as sr join main.rerouted_ref as rr on sr.id = rr.id inner join user.music as m on sr.col = m.col where m.user_id = 1", + "plan": { + "QueryType": "DELETE", + "Original": "delete sr from main.source_of_ref as sr join main.rerouted_ref as rr on sr.id = rr.id inner join user.music as m on sr.col = m.col where m.user_id = 1", + "Instructions": { + "OperatorType": "DMLWithInput", + "TargetTabletType": "PRIMARY", + "Offset": [ + "0:[0]" + ], + "Inputs": [ + { + "OperatorType": "Join", + "Variant": "Join", + "JoinColumnIndexes": "R:0", + "JoinVars": { + "m_col": 0 + }, + "TableName": "music_rerouted_ref, source_of_ref", + "Inputs": [ + { + "OperatorType": "Route", + "Variant": "EqualUnique", + "Keyspace": { + "Name": "user", + "Sharded": true + }, + "FieldQuery": "select m.col from music as m where 1 != 1", + "Query": "select m.col from music as m where m.user_id = 1", + "Table": "music", + "Values": [ + "1" + ], + "Vindex": "user_index" + }, + { + "OperatorType": "Route", + "Variant": "Unsharded", + "Keyspace": { + "Name": "main", + "Sharded": false + }, + "FieldQuery": "select sr.id from source_of_ref as sr, rerouted_ref as rr where 1 != 1", + "Query": "select sr.id from source_of_ref as sr, rerouted_ref as rr where sr.col = :m_col and sr.id = rr.id", + "Table": "rerouted_ref, source_of_ref" + } + ] + }, + { + "OperatorType": "Delete", + "Variant": "Unsharded", + "Keyspace": { + "Name": "main", + "Sharded": false + }, + "TargetTabletType": "PRIMARY", + "Query": "delete from source_of_ref as sr where sr.id in ::dml_vals", + "Table": "source_of_ref" + } + ] + }, + "TablesUsed": [ + "main.rerouted_ref", + "main.source_of_ref", + "user.music" + ] + } } ] diff --git a/go/vt/vtgate/planbuilder/testdata/sampledata/user.sql b/go/vt/vtgate/planbuilder/testdata/sampledata/user.sql index 044a1ee140d..ff1afd68fca 100644 --- a/go/vt/vtgate/planbuilder/testdata/sampledata/user.sql +++ b/go/vt/vtgate/planbuilder/testdata/sampledata/user.sql @@ -11,4 +11,13 @@ INSERT INTO sales_extra(colx, cola, colb, start, end) VALUES (13, 'a_3', 'b_3',1000, 1500); INSERT INTO sales_extra(colx, cola, colb, start, end) -VALUES (14, 'a_4', 'b_4',1500, 2000); \ No newline at end of file +VALUES (14, 'a_4', 'b_4',1500, 2000); + +INSERT INTO music (id, user_id, col) +VALUES (100, 1, 'foo'); + +INSERT INTO source_of_ref (id, col, tt) +VALUES (200, 'foo', 2); + +INSERT INTO rerouted_ref (id, ref_col, name) +VALUES (200, 'bar', 'baz'); \ No newline at end of file diff --git a/go/vt/vtgate/planbuilder/testdata/schemas/main.sql b/go/vt/vtgate/planbuilder/testdata/schemas/main.sql index fb03b69419b..e615871cf2b 100644 --- a/go/vt/vtgate/planbuilder/testdata/schemas/main.sql +++ b/go/vt/vtgate/planbuilder/testdata/schemas/main.sql @@ -1,26 +1,44 @@ -CREATE TABLE `unsharded` ( - `id` INT NOT NULL PRIMARY KEY, - `col` VARCHAR(255) DEFAULT NULL, - `col1` VARCHAR(255) DEFAULT NULL, - `col2` VARCHAR(255) DEFAULT NULL, - `name` VARCHAR(255) DEFAULT NULL, - `baz` INT +CREATE TABLE `unsharded` +( + `id` INT NOT NULL PRIMARY KEY, + `col` VARCHAR(255) DEFAULT NULL, + `col1` VARCHAR(255) DEFAULT NULL, + `col2` VARCHAR(255) DEFAULT NULL, + `name` VARCHAR(255) DEFAULT NULL, + `baz` INT ); -CREATE TABLE `unsharded_auto` ( +CREATE TABLE `unsharded_auto` +( `id` INT NOT NULL PRIMARY KEY, `col1` VARCHAR(255) DEFAULT NULL, `col2` VARCHAR(255) DEFAULT NULL ); -CREATE TABLE `unsharded_a` ( +CREATE TABLE `unsharded_a` +( `id` INT NOT NULL PRIMARY KEY, `col` VARCHAR(255) DEFAULT NULL, `name` VARCHAR(255) DEFAULT NULL ); -CREATE TABLE `unsharded_b` ( +CREATE TABLE `unsharded_b` +( `id` INT NOT NULL PRIMARY KEY, `col` VARCHAR(255) DEFAULT NULL, `name` VARCHAR(255) DEFAULT NULL +); + +CREATE TABLE `source_of_ref` +( + `id` INT NOT NULL PRIMARY KEY, + `col` VARCHAR(255) DEFAULT NULL, + `tt` BIGINT DEFAULT NULL +); + +CREATE TABLE `rerouted_ref` +( + `id` INT NOT NULL PRIMARY KEY, + `ref_col` VARCHAR(255) DEFAULT NULL, + `name` VARCHAR(255) DEFAULT NULL ); \ No newline at end of file diff --git a/go/vt/vtgate/planbuilder/testdata/schemas/user.sql b/go/vt/vtgate/planbuilder/testdata/schemas/user.sql index 818d2508069..10c1886a992 100644 --- a/go/vt/vtgate/planbuilder/testdata/schemas/user.sql +++ b/go/vt/vtgate/planbuilder/testdata/schemas/user.sql @@ -1,25 +1,25 @@ CREATE TABLE user ( - id INT PRIMARY KEY, - col BIGINT, - intcol BIGINT, - user_id INT, - id1 INT, - id2 INT, - id3 INT, - m INT, - bar INT, - a INT, - name VARCHAR(255), - col1 VARCHAR(255), - col2 VARCHAR(255), - costly VARCHAR(255), - predef1 VARCHAR(255), - predef2 VARCHAR(255), - textcol1 VARCHAR(255), - textcol2 VARCHAR(255), - someColumn VARCHAR(255), - foo VARCHAR(255) + id INT PRIMARY KEY, + col BIGINT, + intcol BIGINT, + user_id INT, + id1 INT, + id2 INT, + id3 INT, + m INT, + bar INT, + a INT, + name VARCHAR(255), + col1 VARCHAR(255), + col2 VARCHAR(255), + costly VARCHAR(255), + predef1 VARCHAR(255), + predef2 VARCHAR(255), + textcol1 VARCHAR(255), + textcol2 VARCHAR(255), + someColumn VARCHAR(255), + foo VARCHAR(255) ); CREATE TABLE user_metadata @@ -34,15 +34,23 @@ CREATE TABLE user_metadata CREATE TABLE music ( - user_id INT, - id INT, - col1 VARCHAR(255), - col2 VARCHAR(255), - genre VARCHAR(255), + user_id INT, + id INT, + col VARCHAR(255), + col1 VARCHAR(255), + col2 VARCHAR(255), + genre VARCHAR(255), componist VARCHAR(255), PRIMARY KEY (user_id) ); +CREATE TABLE name_user_vdx +( + name INT, + keyspace_id VARBINARY(10), + primary key (name) +); + CREATE TABLE samecolvin ( col VARCHAR(255), @@ -118,69 +126,63 @@ CREATE TABLE authoritative CREATE TABLE colb_colc_map ( - colb INT PRIMARY KEY, - colc INT, + colb INT PRIMARY KEY, + colc INT, keyspace_id VARCHAR(255) ); CREATE TABLE seq ( - id INT, - next_id BIGINT, - cache BIGINT, + id INT, + next_id BIGINT, + cache BIGINT, PRIMARY KEY (id) ) COMMENT 'vitess_sequence'; CREATE TABLE user_extra ( - id INT, - user_id INT, - extra_id INT, - col INT, - m2 INT, + id INT, + user_id INT, + extra_id INT, + col INT, + m2 INT, PRIMARY KEY (id, extra_id) ); CREATE TABLE name_user_map ( - name VARCHAR(255), - keyspace_id VARCHAR(255) -); - -CREATE TABLE name_user_vdx -( - name VARCHAR(255), - keyspace_id VARCHAR(255) + name VARCHAR(255), + keyspace_id VARCHAR(255) ); CREATE TABLE costly_map ( - costly VARCHAR(255), - keyspace_id VARCHAR(255) + costly VARCHAR(255), + keyspace_id VARCHAR(255) ); CREATE TABLE unq_binary_idx ( - id INT PRIMARY KEY, - col1 INT + id INT PRIMARY KEY, + col1 INT ); CREATE TABLE sales ( - oid INT PRIMARY KEY, - col1 VARCHAR(255) + oid INT PRIMARY KEY, + col1 VARCHAR(255) ); CREATE TABLE sales_extra ( - colx INT PRIMARY KEY, - cola VARCHAR(255), - colb VARCHAR(255), - start INT, - end INT + colx INT PRIMARY KEY, + cola VARCHAR(255), + colb VARCHAR(255), + start INT, + end INT ); CREATE TABLE ref ( - col INT PRIMARY KEY + col INT PRIMARY KEY ); \ No newline at end of file diff --git a/go/vt/vtgate/planbuilder/testdata/unsupported_cases.json b/go/vt/vtgate/planbuilder/testdata/unsupported_cases.json index 9241cec595c..55329586b0e 100644 --- a/go/vt/vtgate/planbuilder/testdata/unsupported_cases.json +++ b/go/vt/vtgate/planbuilder/testdata/unsupported_cases.json @@ -342,7 +342,12 @@ { "comment": "reference table delete with join", "query": "delete r from user u join ref_with_source r on u.col = r.col", - "plan": "VT12001: unsupported: DELETE on reference table with join" + "plan": "VT12001: unsupported: DML on reference table with join" + }, + { + "comment": "reference table update with join", + "query": "update user u join ref_with_source r on u.col = r.col set r.col = 5", + "plan": "VT12001: unsupported: DML on reference table with join" }, { "comment": "group_concat unsupported when needs full evaluation at vtgate with more than 1 column", diff --git a/go/vt/vtgate/semantics/analyzer.go b/go/vt/vtgate/semantics/analyzer.go index 988932f4414..62cdc019ddf 100644 --- a/go/vt/vtgate/semantics/analyzer.go +++ b/go/vt/vtgate/semantics/analyzer.go @@ -174,7 +174,7 @@ func (a *analyzer) newSemTable( Direct: a.binder.direct, ExprTypes: a.typer.m, Tables: a.tables.Tables, - Targets: a.binder.targets, + DMLTargets: a.binder.targets, NotSingleRouteErr: a.notSingleRouteErr, NotUnshardedErr: a.unshardedErr, Warning: a.warning, diff --git a/go/vt/vtgate/semantics/semantic_table.go b/go/vt/vtgate/semantics/semantic_table.go index 492259427c5..30a41ba5f12 100644 --- a/go/vt/vtgate/semantics/semantic_table.go +++ b/go/vt/vtgate/semantics/semantic_table.go @@ -130,8 +130,8 @@ type ( // It doesn't recurse inside derived tables to find the original dependencies. Direct ExprDependencies - // Targets contains the TableSet of each table getting modified by the update/delete statement. - Targets TableSet + // DMLTargets contains the TableSet of each table getting modified by the update/delete statement. + DMLTargets TableSet // ColumnEqualities is used for transitive closures (e.g., if a == b and b == c, then a == c). ColumnEqualities map[columnName][]sqlparser.Expr @@ -203,7 +203,7 @@ func (st *SemTable) CopyDependencies(from, to sqlparser.Expr) { // GetChildForeignKeysForTargets gets the child foreign keys as a list for all the target tables. func (st *SemTable) GetChildForeignKeysForTargets() (fks []vindexes.ChildFKInfo) { - for _, ts := range st.Targets.Constituents() { + for _, ts := range st.DMLTargets.Constituents() { fks = append(fks, st.childForeignKeysInvolved[ts]...) } return fks @@ -211,7 +211,7 @@ func (st *SemTable) GetChildForeignKeysForTargets() (fks []vindexes.ChildFKInfo) // GetChildForeignKeysForTableSet gets the child foreign keys as a listfor the TableSet. func (st *SemTable) GetChildForeignKeysForTableSet(target TableSet) (fks []vindexes.ChildFKInfo) { - for _, ts := range st.Targets.Constituents() { + for _, ts := range st.DMLTargets.Constituents() { if target.IsSolvedBy(ts) { fks = append(fks, st.childForeignKeysInvolved[ts]...) } @@ -239,7 +239,7 @@ func (st *SemTable) GetChildForeignKeysList() []vindexes.ChildFKInfo { // GetParentForeignKeysForTargets gets the parent foreign keys as a list for all the target tables. func (st *SemTable) GetParentForeignKeysForTargets() (fks []vindexes.ParentFKInfo) { - for _, ts := range st.Targets.Constituents() { + for _, ts := range st.DMLTargets.Constituents() { fks = append(fks, st.parentForeignKeysInvolved[ts]...) } return fks @@ -247,7 +247,7 @@ func (st *SemTable) GetParentForeignKeysForTargets() (fks []vindexes.ParentFKInf // GetParentForeignKeysForTableSet gets the parent foreign keys as a list for the TableSet. func (st *SemTable) GetParentForeignKeysForTableSet(target TableSet) (fks []vindexes.ParentFKInfo) { - for _, ts := range st.Targets.Constituents() { + for _, ts := range st.DMLTargets.Constituents() { if target.IsSolvedBy(ts) { fks = append(fks, st.parentForeignKeysInvolved[ts]...) } @@ -971,7 +971,7 @@ func (st *SemTable) UpdateChildFKExpr(origUpdExpr *sqlparser.UpdateExpr, newExpr // GetTargetTableSetForTableName returns the TableSet for the given table name from the target tables. func (st *SemTable) GetTargetTableSetForTableName(name sqlparser.TableName) (TableSet, error) { - for _, target := range st.Targets.Constituents() { + for _, target := range st.DMLTargets.Constituents() { tbl, err := st.Tables[target.TableOffset()].Name() if err != nil { return "", err