From 79ddc2f3a488fad617c23431d00f3859cbfcfef4 Mon Sep 17 00:00:00 2001 From: Harshit Gangal Date: Thu, 19 Dec 2024 22:53:03 +0530 Subject: [PATCH] wip Signed-off-by: Harshit Gangal --- .../planbuilder/operators/cte_merging.go | 2 +- go/vt/vtgate/planbuilder/operators/delete.go | 2 +- .../planbuilder/operators/join_merging.go | 14 ++++---- .../operators/subquery_planning.go | 6 ++-- .../planbuilder/operators/union_merging.go | 2 +- go/vt/vtgate/planbuilder/operators/update.go | 4 +-- go/vt/vtgate/planbuilder/plan_test.go | 1 + .../vtgate/planbuilder/testdata/onecase.json | 2 +- .../planbuilder/testdata/vschemas/schema.json | 35 +++++++++++++++++++ go/vt/vtgate/semantics/analyzer.go | 2 +- go/vt/vtgate/semantics/semantic_table.go | 14 ++++---- 11 files changed, 61 insertions(+), 23 deletions(-) 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..9cc7ffc2381 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) } diff --git a/go/vt/vtgate/planbuilder/operators/join_merging.go b/go/vt/vtgate/planbuilder/operators/join_merging.go index c035b7d11ed..707f41d6f51 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,15 @@ func getRoutesOrAlternates(lhsRoute, rhsRoute *Route) (*Route, *Route, Routing, return lhsRoute, rhsRoute, routingA, routingB, sameKeyspace } - if refA, ok := routingA.(*AnyShardRouting); ok { + 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..4d871b077ef 100644 --- a/go/vt/vtgate/planbuilder/operators/subquery_planning.go +++ b/go/vt/vtgate/planbuilder/operators/subquery_planning.go @@ -377,8 +377,8 @@ func rewriteOriginalPushedToRHS(ctx *plancontext.PlanningContext, expression sql // this is used when we push an operator from above the subquery into the outer side of the subquery func rewriteColNameToArgument( ctx *plancontext.PlanningContext, - in sqlparser.Expr, // the expression to rewrite - se SubQueryExpression, // the subquery expression we are rewriting + in sqlparser.Expr, // the expression to rewrite + se SubQueryExpression, // the subquery expression we are rewriting subqueries ...*SubQuery, // the inner subquery operators ) sqlparser.Expr { // the visitor function that will rewrite the expression tree @@ -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 7135f4dff29..33e58d1334e 100644 --- a/go/vt/vtgate/planbuilder/plan_test.go +++ b/go/vt/vtgate/planbuilder/plan_test.go @@ -305,6 +305,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, "legacy", []string{"users"}, []string{"idUser"}) s.testFile("onecase.json", vw, false) } diff --git a/go/vt/vtgate/planbuilder/testdata/onecase.json b/go/vt/vtgate/planbuilder/testdata/onecase.json index 9d653b2f6e9..defc2bcad33 100644 --- a/go/vt/vtgate/planbuilder/testdata/onecase.json +++ b/go/vt/vtgate/planbuilder/testdata/onecase.json @@ -1,7 +1,7 @@ [ { "comment": "Add your test case here for debugging and run go test -run=One.", - "query": "", + "query": "update legacy.users as u join legacy.users2sites as u2s on u2s.idUser = u.idUser inner join sites2024.sites as s on s.idSite = u2s.idSite set u.email = 'asdf@example.com' where s.idSite = 234234234;", "plan": { } } diff --git a/go/vt/vtgate/planbuilder/testdata/vschemas/schema.json b/go/vt/vtgate/planbuilder/testdata/vschemas/schema.json index a5de9d3697e..280d204a59a 100644 --- a/go/vt/vtgate/planbuilder/testdata/vschemas/schema.json +++ b/go/vt/vtgate/planbuilder/testdata/vschemas/schema.json @@ -1007,6 +1007,41 @@ ] } } + }, + "sites2024": { + "sharded": true, + "require_explicit_routing": false, + "vindexes": { + "a_standard_hash": { + "type": "xxhash" + } + }, + "tables": { + "users": { + "type": "reference", + "source": "legacy.users" + }, + "users2sites": { + "type": "reference", + "source": "legacy.users2sites" + }, + "sites": { + "column_vindexes": [ + { + "column": "idSite", + "name": "a_standard_hash" + } + ] + } + } + }, + "legacy": { + "require_explicit_routing": false, + "sharded": false, + "tables": { + "users": {}, + "users2sites": {} + } } } } diff --git a/go/vt/vtgate/semantics/analyzer.go b/go/vt/vtgate/semantics/analyzer.go index 0a9d2480d9b..25ab19a9947 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 f9856a901a6..9e2a3703669 100644 --- a/go/vt/vtgate/semantics/semantic_table.go +++ b/go/vt/vtgate/semantics/semantic_table.go @@ -129,8 +129,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 @@ -202,7 +202,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 @@ -210,7 +210,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]...) } @@ -238,7 +238,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 @@ -246,7 +246,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]...) } @@ -970,7 +970,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