Repository: vitessio/vitess
Pull request #11786 (2022-11-22 07:31:23)

Push down derived tables under route when possible
## Description

This is a backport of #11379. It fixes the issue described in https://github.com/vitessio/vitess/issues/11763.

## Related Issue(s)

Fix https://github.com/vitessio/vitess/issues/11763

## Checklist

-   [x] "Backport to:" labels have been added if this change should be back-ported
-   [x] Tests were added or are not required
-   [x] Documentation was added or is not required


vitess-bot[bot]: ### Review Checklist
            
Hello reviewers! :wave: Please follow this checklist when reviewing this Pull Request.

#### General
- [ ] Ensure that the Pull Request has a descriptive title.
- [ ] If this is a change that users need to know about, please apply the `release notes (needs details)` label so that merging is blocked unless the summary release notes document is included.

#### If a new flag is being introduced:
- [ ] Is it really necessary to add this flag?
- [ ] Flag names should be clear and intuitive (as far as possible)
- [ ] Help text should be descriptive.
- [ ] Flag names should use dashes (`-`) as word separators rather than underscores (`_`).

#### If a workflow is added or modified:
- [ ] Each item in `Jobs` should be named in order to mark it as `required`.
- [ ] If the workflow should be required, the maintainer team should be notified.

#### Bug fixes
- [ ] There should be at least one unit or end-to-end test.
- [ ] The Pull Request description should include a link to an issue that describes the bug.

#### Non-trivial changes
- [ ] There should be some code comments as to why things are implemented the way they are.

#### New/Existing features
- [ ] Should be documented, either by modifying the existing documentation or creating new documentation.
- [ ] New features should have a link to a feature request issue or an RFC that documents the use cases, corner cases and test cases.

#### Backward compatibility            
- [ ] Protobuf changes should be wire-compatible.
- [ ] Changes to `_vt` tables and RPCs need to be backward compatible.
- [ ] `vtctl` command output order should be stable and `awk`-able.
- [ ] RPC changes should be compatible with vitess-operator
- [ ] If a flag is removed, then it should also be removed from VTop, if used there.


Patch:
diff --git a/go/vt/vtgate/planbuilder/physical/route_planning.go b/go/vt/vtgate/planbuilder/physical/route_planning.go
index ee56cdf4a7..3e5131e79b 100644
--- a/go/vt/vtgate/planbuilder/physical/route_planning.go
+++ b/go/vt/vtgate/planbuilder/physical/route_planning.go
@@ -21,12 +21,8 @@ import (
 	"fmt"
 	"io"
 
-	"vitess.io/vitess/go/vt/key"
-	topodatapb "vitess.io/vitess/go/vt/proto/topodata"
-
-	querypb "vitess.io/vitess/go/vt/proto/query"
-
 	"vitess.io/vitess/go/mysql/collations"
+	"vitess.io/vitess/go/vt/key"
 	"vitess.io/vitess/go/vt/sqlparser"
 	"vitess.io/vitess/go/vt/vterrors"
 	"vitess.io/vitess/go/vt/vtgate/engine"
@@ -36,6 +32,8 @@ import (
 	"vitess.io/vitess/go/vt/vtgate/semantics"
 	"vitess.io/vitess/go/vt/vtgate/vindexes"
 
+	querypb "vitess.io/vitess/go/vt/proto/query"
+	topodatapb "vitess.io/vitess/go/vt/proto/topodata"
 	vtrpcpb "vitess.io/vitess/go/vt/proto/vtrpc"
 )
 
@@ -50,22 +48,9 @@ type (
 func CreatePhysicalOperator(ctx *plancontext.PlanningContext, opTree abstract.LogicalOperator) (abstract.PhysicalOperator, error) {
 	switch op := opTree.(type) {
 	case *abstract.QueryGraph:
-		switch {
-		case ctx.PlannerVersion == querypb.ExecuteOptions_Gen4Left2Right:
-			return leftToRightSolve(ctx, op)
-		default:
-			return greedySolve(ctx, op)
-		}
+		return optimizeQueryGraph(ctx, op)
 	case *abstract.Join:
-		opInner, err := CreatePhysicalOperator(ctx, op.LHS)
-		if err != nil {
-			return nil, err
-		}
-		opOuter, err := CreatePhysicalOperator(ctx, op.RHS)
-		if err != nil {
-			return nil, err
-		}
-		return mergeOrJoin(ctx, opInner, opOuter, sqlparser.SplitAndExpression(nil, op.Predicate), !op.LeftJoin)
+		return optimizeJoin(ctx, op)
 	case *abstract.Derived:
 		return optimizeDerived(ctx, op)
 	case *abstract.SubQuery:
@@ -75,87 +60,161 @@ func CreatePhysicalOperator(ctx *plancontext.PlanningContext, opTree abstract.Lo
 	case *abstract.Concatenate:
 		return optimizeUnion(ctx, op)
 	case *abstract.Filter:
-		src, err := CreatePhysicalOperator(ctx, op.Source)
-		if err != nil {
-			return nil, err
-		}
+		return optimizeFilter(ctx, op)
+	case *abstract.Update:
+		return createPhysicalOperatorFromUpdate(ctx, op)
+	default:
+		return nil, vterrors.Errorf(vtrpcpb.Code_INTERNAL, "invalid operator tree: %T", op)
+	}
+}
 
-		filter := &Filter{
-			Predicates: op.Predicates,
-		}
+func optimizeFilter(ctx *plancontext.PlanningContext, op *abstract.Filter) (abstract.PhysicalOperator, error) {
+	src, err := CreatePhysicalOperator(ctx, op.Source)
+	if err != nil {
+		return nil, err
+	}
 
-		if route, ok := src.(*Route); ok {
-			// let's push the filter into the route
-			filter.Source = route.Source
-			route.Source = filter
-			return route, nil
-		}
+	filter := &Filter{
+		Predicates: op.Predicates,
+	}
 
-		filter.Source = src
+	if route, ok := src.(*Route); ok {
+		// let's push the filter into the route
+		filter.Source = route.Source
+		route.Source = filter
+		return route, nil
+	}
 
-		return filter, nil
-	case *abstract.Update:
-		var typ topodatapb.TabletType
-		var dest key.Destination
+	filter.Source = src
 
-		vindexTable := op.TableInfo.GetVindexTable()
-		opCode := engine.Unsharded
-		if vindexTable.Keyspace.Sharded {
-			opCode = engine.Scatter
-		}
+	return filter, nil
+}
 
-		tblName, ok := op.Table.Alias.Expr.(sqlparser.TableName)
-		if ok {
-			var err error
-			_, _, _, typ, dest, err = ctx.VSchema.FindTableOrVindex(tblName)
-			if err != nil {
-				return nil, err
-			}
-			if dest != nil {
-				if typ != topodatapb.TabletType_PRIMARY {
-					return nil, vterrors.NewErrorf(vtrpcpb.Code_FAILED_PRECONDITION, vterrors.InnodbReadOnly, "unsupported: update statement with a replica target")
-				}
-				// we are dealing with an explicitly targeted UPDATE
-				opCode = engine.ByDestination
-			}
-		}
+func optimizeDerived(ctx *plancontext.PlanningContext, op *abstract.Derived) (abstract.PhysicalOperator, error) {
+	opInner, err := CreatePhysicalOperator(ctx, op.Inner)
+	if err != nil {
+		return nil, err
+	}
+
+	innerRoute, ok := opInner.(*Route)
+	if !ok {
+		return buildDerivedOp(op, opInner), nil
+	}
+
+	derived := &Derived{
+		Source:        innerRoute.Source,
+		Query:         op.Sel,
+		Alias:         op.Alias,
+		ColumnAliases: op.ColumnAliases,
+	}
+
+	if innerRoute.RouteOpCode == engine.EqualUnique {
+		// no need to check anything if we are sure that we will only hit a single shard
+	} else if !derived.IsMergeable(ctx) {
+		return buildDerivedOp(op, opInner), nil
+	}
+
+	innerRoute.Source = derived
+	return innerRoute, nil
+}
+
+func buildDerivedOp(op *abstract.Derived, opInner abstract.PhysicalOperator) *Derived {
+	return &Derived{
+		Source:        opInner,
+		Query:         op.Sel,
+		Alias:         op.Alias,
+		ColumnAliases: op.ColumnAliases,
+	}
+}
+
+func optimizeJoin(ctx *plancontext.PlanningContext, op *abstract.Join) (abstract.PhysicalOperator, error) {
+	lhs, err := CreatePhysicalOperator(ctx, op.LHS)
+	if err != nil {
+		return nil, err
+	}
+	rhs, err := CreatePhysicalOperator(ctx, op.RHS)
+	if err != nil {
+		return nil, err
+	}
+
+	return mergeOrJoin(ctx, lhs, rhs, sqlparser.SplitAndExpression(nil, op.Predicate), !op.LeftJoin)
+}
+
+func optimizeQueryGraph(ctx *plancontext.PlanningContext, op *abstract.QueryGraph) (abstract.PhysicalOperator, error) {
+	switch {
+	case ctx.PlannerVersion == querypb.ExecuteOptions_Gen4Left2Right:
+		return leftToRightSolve(ctx, op)
+	default:
+		return greedySolve(ctx, op)
+	}
+}
+
+func createPhysicalOperatorFromUpdate(ctx *plancontext.PlanningContext, op *abstract.Update) (abstract.PhysicalOperator, error) {
+	vindexTable, opCode, dest, err := buildVindexTableForDML(ctx, op.TableInfo, op.Table, "update")
+	if err != nil {
+		return nil, err
+	}
+
+	vp, cvv, ovq, err := getUpdateVindexInformation(op, vindexTable)
+	if err != nil {
+		return nil, err
+	}
 
-		vp, cvv, ovq, err := getUpdateVindexInformation(op, vindexTable)
+	r := &Route{
+		Source: &Update{
+			QTable:              op.Table,
+			VTable:              vindexTable,
+			Assignments:         op.Assignments,
+			ChangedVindexValues: cvv,
+			OwnedVindexQuery:    ovq,
+			AST:                 op.AST,
+		},
+		RouteOpCode:       opCode,
+		Keyspace:          vindexTable.Keyspace,
+		VindexPreds:       vp,
+		TargetDestination: dest,
+	}
+
+	for _, predicate := range op.Table.Predicates {
+		err := r.UpdateRoutingLogic(ctx, predicate)
 		if err != nil {
 			return nil, err
 		}
+	}
 
-		r := &Route{
-			Source: &Update{
-				QTable:              op.Table,
-				VTable:              vindexTable,
-				Assignments:         op.Assignments,
-				ChangedVindexValues: cvv,
-				OwnedVindexQuery:    ovq,
-				AST:                 op.AST,
-			},
-			RouteOpCode:       opCode,
-			Keyspace:          vindexTable.Keyspace,
-			VindexPreds:       vp,
-			TargetDestination: dest,
-		}
+	if r.RouteOpCode == engine.Scatter && op.AST.Limit != nil {
+		// TODO systay: we should probably check for other op code types - IN could also hit multiple shards (2022-04-07)
+		return nil, vterrors.Errorf(vtrpcpb.Code_UNIMPLEMENTED, "multi shard update with limit is not supported")
+	}
 
-		for _, predicate := range op.Table.Predicates {
-			err := r.UpdateRoutingLogic(ctx, predicate)
-			if err != nil {
-				return nil, err
-			}
-		}
+	return r, nil
+}
 
-		if r.RouteOpCode == engine.Scatter && op.AST.Limit != nil {
-			// TODO systay: we should probably check for other op code types - IN could also hit multiple shards (2022-04-07)
-			return nil, vterrors.Errorf(vtrpcpb.Code_UNIMPLEMENTED, "multi shard update with limit is not supported")
-		}
+func buildVindexTableForDML(ctx *plancontext.PlanningContext, tableInfo semantics.TableInfo, table *abstract.QueryTable, dmlType string) (*vindexes.Table, engine.Opcode, key.Destination, error) {
+	vindexTable := tableInfo.GetVindexTable()
+	opCode := engine.Unsharded
+	if vindexTable.Keyspace.Sharded {
+		opCode = engine.Scatter
+	}
 
-		return r, nil
-	default:
-		return nil, vterrors.Errorf(vtrpcpb.Code_INTERNAL, "invalid operator tree: %T", op)
+	var dest key.Destination
+	var typ topodatapb.TabletType
+	var err error
+	tblName, ok := table.Alias.Expr.(sqlparser.TableName)
+	if ok {
+		_, _, _, typ, dest, err = ctx.VSchema.FindTableOrVindex(tblName)
+		if err != nil {
+			return nil, 0, nil, err
+		}
+		if dest != nil {
+			if typ != topodatapb.TabletType_PRIMARY {
+				return nil, 0, nil, vterrors.NewErrorf(vtrpcpb.Code_FAILED_PRECONDITION, vterrors.InnodbReadOnly, "unsupported: %v statement with a replica target", dmlType)
+			}
+			// we are dealing with an explicitly targeted UPDATE
+			opCode = engine.ByDestination
+		}
 	}
+	return vindexTable, opCode, dest, nil
 }
 
 func getUpdateVindexInformation(op *abstract.Update, vindexTable *vindexes.Table) ([]*VindexPlusPredicates, map[string]*engine.VindexValues, string, error) {
@@ -516,8 +575,25 @@ func getJoinFor(ctx *plancontext.PlanningContext, cm opCacheMap, lhs, rhs abstra
 	return join, nil
 }
 
-func mergeOrJoin(ctx *plancontext.PlanningContext, lhs, rhs abstract.PhysicalOperator, joinPredicates []sqlparser.Expr, inner bool) (abstract.PhysicalOperator, error) {
+func requiresSwitchingSides(ctx *plancontext.PlanningContext, op abstract.PhysicalOperator) bool {
+	required := false
 
+	_ = VisitOperators(op, func(current abstract.PhysicalOperator) (bool, error) {
+		derived, isDerived := current.(*Derived)
+
+		if isDerived && !derived.IsMergeable(ctx) {
+			required = true
+
+			return false, nil
+		}
+
+		return true, nil
+	})
+
+	return required
+}
+
+func mergeOrJoin(ctx *plancontext.PlanningContext, lhs, rhs abstract.PhysicalOperator, joinPredicates []sqlparser.Expr, inner bool) (abstract.PhysicalOperator, error) {
 	merger := func(a, b *Route) (*Route, error) {
 		return createRouteOperatorForJoin(a, b, joinPredicates, inner)
 	}
@@ -527,6 +603,25 @@ func mergeOrJoin(ctx *plancontext.PlanningContext, lhs, rhs abstract.PhysicalOpe
 		return newPlan, nil
 	}
 
+	if len(joinPredicates) > 0 && requiresSwitchingSides(ctx, rhs) {
+		if !inner {
+			return nil, vterrors.Errorf(vtrpcpb.Code_UNIMPLEMENTED, "unsupported: LEFT JOIN not supported for derived tables")
+		}
+
+		if requiresSwitchingSides(ctx, lhs) {
+			return nil, vterrors.Errorf(vtrpcpb.Code_UNIMPLEMENTED, "unsupported: JOIN not supported between derived tables")
+		}
+
+		join := &ApplyJoin{
+			LHS:      rhs.Clone(),
+			RHS:      lhs.Clone(),
+			Vars:     map[string]int{},
+			LeftJoin: !inner,
+		}
+
+		return pushJoinPredicates(ctx, joinPredicates, join)
+	}
+
 	join := &ApplyJoin{
 		LHS:      lhs.Clone(),
 		RHS:      rhs.Clone(),
@@ -553,6 +648,7 @@ func createRouteOperatorForJoin(aRoute, bRoute *Route, joinPredicates []sqlparse
 		Keyspace:            aRoute.Keyspace,
 		VindexPreds:         append(aRoute.VindexPreds, bRoute.VindexPreds...),
 		SysTableTableSchema: append(aRoute.SysTableTableSchema, bRoute.SysTableTableSchema...),
+		SeenPredicates:      append(aRoute.SeenPredicates, bRoute.SeenPredicates...),
 		SysTableTableName:   sysTableName,
 		Source: &ApplyJoin{
 			LHS:       aRoute.Source,
@@ -572,38 +668,13 @@ func createRouteOperatorForJoin(aRoute, bRoute *Route, joinPredicates []sqlparse
 
 type mergeFunc func(a, b *Route) (*Route, error)
 
-// makeRoute return the input as a Route.
-// if the input is a Derived operator and has a Route as its source,
-// we push the Derived inside the Route and return it.
-func makeRoute(j abstract.PhysicalOperator) *Route {
-	route, ok := j.(*Route)
-	if ok {
-		return route
-	}
-
-	derived, ok := j.(*Derived)
-	if !ok {
-		return nil
-	}
-	dp := derived.Clone().(*Derived)
-
-	route = makeRoute(dp.Source)
-	if route == nil {
-		return nil
-	}
-
-	derived.Source = route.Source
-	route.Source = derived
-	return route
-}
-
 func operatorsToRoutes(a, b abstract.PhysicalOperator) (*Route, *Route) {
-	aRoute := makeRoute(a)
-	if aRoute == nil {
+	aRoute, ok := a.(*Route)
+	if !ok {
 		return nil, nil
 	}
-	bRoute := makeRoute(b)
-	if bRoute == nil {
+	bRoute, ok := b.(*Route)
+	if !ok {
 		return nil, nil
 	}
 	return aRoute, bRoute
@@ -650,7 +721,7 @@ func tryMerge(
 		// can merge via join predicates instead.
 		fallthrough
 
-	case engine.Scatter, engine.IN:
+	case engine.Scatter, engine.IN, engine.None:
 		if len(joinPredicates) == 0 {
 			// If we are doing two Scatters, we have to make sure that the
 			// joins are on the correct vindex to allow them to be merged
@@ -669,7 +740,13 @@ func tryMerge(
 		if err != nil {
 			return nil, err
 		}
-		r.PickBestAvailableVindex()
+
+		// If we have a `None` route opcode, we want to keep it -
+		// we only try to find a better Vindex for other route opcodes
+		if aRoute.RouteOpCode != engine.None {
+			r.PickBestAvailableVindex()
+		}
+
 		return r, nil
 	}
 	return nil, nil
@@ -1043,7 +1120,8 @@ func pushJoinPredicateOnJoin(ctx *plancontext.PlanningContext, exprs []sqlparser
 		// rows as early as possible making join cheaper on the vtgate level.
 		depsForExpr := ctx.SemTable.RecursiveDeps(expr)
 		singleSideDeps := false
-		if depsForExpr.IsSolvedBy(node.LHS.TableID()) {
+		lhsTables := node.LHS.TableID()
+		if depsForExpr.IsSolvedBy(lhsTables) {
 			lhsPreds = append(lhsPreds, expr)
 			singleSideDeps = true
 		}
@@ -1056,7 +1134,7 @@ func pushJoinPredicateOnJoin(ctx *plancontext.PlanningContext, exprs []sqlparser
 			continue
 		}
 
-		bvName, cols, predicate, err := BreakExpressionInLHSandRHS(ctx, expr, node.LHS.TableID())
+		bvName, cols, predicate, err := BreakExpressionInLHSandRHS(ctx, expr, lhsTables)
 		if err != nil {
 			return nil, err
 		}
@@ -1119,40 +1197,3 @@ func pushJoinPredicateOnDerived(ctx *plancontext.PlanningContext, exprs []sqlpar
 	node.Source = newInner
 	return node, nil
 }
-
-func optimizeDerived(ctx *plancontext.PlanningContext, op *abstract.Derived) (abstract.PhysicalOperator, error) {
-	opInner, err := CreatePhysicalOperator(ctx, op.Inner)
-	if err != nil {
-		return nil, err
-	}
-
-	innerRoute, ok := opInner.(*Route)
-	if !ok {
-		return buildDerivedOp(op, opInner), nil
-	}
-
-	derived := &Derived{
-		Source:        innerRoute.Source,
-		Query:         op.Sel,
-		Alias:         op.Alias,
-		ColumnAliases: op.ColumnAliases,
-	}
-
-	if innerRoute.RouteOpCode == engine.EqualUnique {
-		// no need to check anything if we are sure that we will only hit a single shard
-	} else if !derived.IsMergeable(ctx) {
-		return buildDerivedOp(op, opInner), nil
-	}
-
-	innerRoute.Source = derived
-	return innerRoute, nil
-}
-
-func buildDerivedOp(op *abstract.Derived, opInner abstract.PhysicalOperator) *Derived {
-	return &Derived{
-		Source:        opInner,
-		Query:         op.Sel,
-		Alias:         op.Alias,
-		ColumnAliases: op.ColumnAliases,
-	}
-}


Test patch:
diff --git a/go/test/endtoend/vtgate/queries/derived/derived_test.go b/go/test/endtoend/vtgate/queries/derived/derived_test.go
index 3ac0716fb1..9a62ce188b 100644
--- a/go/test/endtoend/vtgate/queries/derived/derived_test.go
+++ b/go/test/endtoend/vtgate/queries/derived/derived_test.go
@@ -46,7 +46,6 @@ func start(t *testing.T) (utils.MySQLCompare, func()) {
 }
 
 func TestDerivedTableWithOrderByLimit(t *testing.T) {
-	t.Skip("skipped for now, issue: https://github.com/vitessio/vitess/issues/11763")
 	mcmp, closer := start(t)
 	defer closer()
 
@@ -86,14 +85,6 @@ func TestDerivedTableWithHaving(t *testing.T) {
 	mcmp.Exec("insert into user(id, name) values(1,'toto'), (2,'tata'), (3,'titi'), (4,'tete'), (5,'foo')")
 
 	mcmp.Exec("set sql_mode = ''")
-	mcmp.AssertMatchesAnyNoCompare("select  /*vt+ PLANNER=Gen4 */ * from (select id from user having count(*) >= 1) s", "[[INT64(1)]]", "[[INT64(4)]]")
-}
 
-func TestDerivedTableColumns(t *testing.T) {
-	t.Skip("skipped for now, issue: https://github.com/vitessio/vitess/issues/11763")
-	mcmp, closer := start(t)
-	defer closer()
-
-	mcmp.Exec("insert into user(id, name) values(1,'toto'), (2,'tata'), (3,'titi'), (4,'tete'), (5,'foo')")
-	mcmp.AssertMatches(`SELECT /*vt+ PLANNER=gen4 */ t.id FROM (SELECT id FROM user) AS t(id) ORDER BY t.id DESC`, `[[INT64(5)] [INT64(4)] [INT64(3)] [INT64(2)] [INT64(1)]]`)
+	mcmp.AssertMatchesAnyNoCompare("select /*vt+ PLANNER=Gen4 */ * from (select id from user having count(*) >= 1) s", "[[INT64(1)]]", "[[INT64(4)]]")
 }
diff --git a/go/test/endtoend/vtgate/queries/derived/main_test.go b/go/test/endtoend/vtgate/queries/derived/main_test.go
index 59a3c40e70..3b44811f95 100644
--- a/go/test/endtoend/vtgate/queries/derived/main_test.go
+++ b/go/test/endtoend/vtgate/queries/derived/main_test.go
@@ -63,7 +63,7 @@ func TestMain(m *testing.M) {
 			SchemaSQL: schemaSQL,
 			VSchema:   vschema,
 		}
-		err = clusterInstance.StartKeyspace(*keyspace, []string{"-80", "80-"}, 1, true)
+		err = clusterInstance.StartKeyspace(*keyspace, []string{"-80", "80-"}, 0, false)
 		if err != nil {
 			return 1
 		}
diff --git a/go/vt/vtgate/planbuilder/testdata/select_cases.json b/go/vt/vtgate/planbuilder/testdata/select_cases.json
index 239ff3462f..eee9dda7dc 100644
--- a/go/vt/vtgate/planbuilder/testdata/select_cases.json
+++ b/go/vt/vtgate/planbuilder/testdata/select_cases.json
@@ -5076,22 +5076,26 @@
       "Instructions": {
         "OperatorType": "Join",
         "Variant": "Join",
-        "JoinColumnIndexes": "L:0",
+        "JoinColumnIndexes": "R:0",
         "JoinVars": {
-          "music_id": 0
+          "other_maxt": 0
         },
         "TableName": "music_music",
         "Inputs": [
           {
             "OperatorType": "Route",
-            "Variant": "Scatter",
+            "Variant": "EqualUnique",
             "Keyspace": {
               "Name": "user",
               "Sharded": true
             },
-            "FieldQuery": "select music.id from music where 1 != 1",
-            "Query": "select music.id from music",
-            "Table": "music"
+            "FieldQuery": "select 1 from (select MAX(id) as maxt from music where 1 != 1) as other where 1 != 1",
+            "Query": "select 1 from (select MAX(id) as maxt from music where music.user_id = 5) as other",
+            "Table": "music",
+            "Values": [
+              "INT64(5)"
+            ],
+            "Vindex": "user_index"
           },
           {
             "OperatorType": "Route",
@@ -5100,13 +5104,13 @@
               "Name": "user",
               "Sharded": true
             },
-            "FieldQuery": "select 1 from (select MAX(id) as maxt from music where 1 != 1) as other where 1 != 1",
-            "Query": "select 1 from (select MAX(id) as maxt from music where music.user_id = 5 and MAX(id) = :music_id) as other",
+            "FieldQuery": "select music.id from music where 1 != 1",
+            "Query": "select music.id from music where music.id = :other_maxt",
             "Table": "music",
             "Values": [
-              "INT64(5)"
+              ":other_maxt"
             ],
-            "Vindex": "user_index"
+            "Vindex": "music_user_map"
           }
         ]
       }
@@ -5196,15 +5200,55 @@
       "QueryType": "SELECT",
       "Original": "select id from user join (select user_id from user_extra limit 10) ue on user.id = ue.user_id",
       "Instructions": {
-        "OperatorType": "Route",
-        "Variant": "Scatter",
-        "Keyspace": {
-          "Name": "user",
-          "Sharded": true
+        "OperatorType": "Join",
+        "Variant": "Join",
+        "JoinColumnIndexes": "R:0",
+        "JoinVars": {
+          "ue_user_id": 0
         },
-        "FieldQuery": "select id from `user`, (select user_id from user_extra where 1 != 1) as ue where 1 != 1",
-        "Query": "select id from `user`, (select user_id from user_extra limit 10) as ue where `user`.id = ue.user_id",
-        "Table": "`user`, user_extra"
+        "TableName": "user_extra_`user`",
+        "Inputs": [
+          {
+            "OperatorType": "SimpleProjection",
+            "Columns": [
+              0
+            ],
+            "Inputs": [
+              {
+                "OperatorType": "Limit",
+                "Count": "INT64(10)",
+                "Inputs": [
+                  {
+                    "OperatorType": "Route",
+                    "Variant": "Scatter",
+                    "Keyspace": {
+                      "Name": "user",
+                      "Sharded": true
+                    },
+                    "FieldQuery": "select user_id from user_extra where 1 != 1",
+                    "Query": "select user_id from user_extra limit :__upper_limit",
+                    "Table": "user_extra"
+                  }
+                ]
+              }
+            ]
+          },
+          {
+            "OperatorType": "Route",
+            "Variant": "EqualUnique",
+            "Keyspace": {
+              "Name": "user",
+              "Sharded": true
+            },
+            "FieldQuery": "select id from `user` where 1 != 1",
+            "Query": "select id from `user` where `user`.id = :ue_user_id",
+            "Table": "`user`",
+            "Values": [
+              ":ue_user_id"
+            ],
+            "Vindex": "user_index"
+          }
+        ]
       }
     }
   },
@@ -5217,26 +5261,16 @@
       "Instructions": {
         "OperatorType": "Join",
         "Variant": "Join",
-        "JoinColumnIndexes": "L:1,R:0",
+        "JoinColumnIndexes": "R:0,L:1",
         "JoinVars": {
-          "user_id": 0
+          "t_id": 0
         },
-        "TableName": "`user`_user_extra",
+        "TableName": "user_extra_`user`",
         "Inputs": [
-          {
-            "OperatorType": "Route",
-            "Variant": "Scatter",
-            "Keyspace": {
-              "Name": "user",
-              "Sharded": true
-            },
-            "FieldQuery": "select `user`.id, `user`.a from `user` where 1 != 1",
-            "Query": "select `user`.id, `user`.a from `user`",
-            "Table": "`user`"
-          },
           {
             "OperatorType": "SimpleProjection",
             "Columns": [
+              0,
               1
             ],
             "Inputs": [
@@ -5255,12 +5289,27 @@
                     },
                     "FieldQuery": "select id, count(*) as b, req, weight_string(id), weight_string(req) from user_extra where 1 != 1 group by id, weight_string(id), req, weight_string(req)",
                     "OrderBy": "(0|3) ASC, (2|4) ASC",
-                    "Query": "select id, count(*) as b, req, weight_string(id), weight_string(req) from user_extra where id = :user_id group by id, weight_string(id), req, weight_string(req) order by id asc, req asc",
+                    "Query": "select id, count(*) as b, req, weight_string(id), weight_string(req) from user_extra group by id, weight_string(id), req, weight_string(req) order by id asc, req asc",
                     "Table": "user_extra"
                   }
                 ]
               }
             ]
+          },
+          {
+            "OperatorType": "Route",
+            "Variant": "EqualUnique",
+            "Keyspace": {
+              "Name": "user",
+              "Sharded": true
+            },
+            "FieldQuery": "select `user`.a from `user` where 1 != 1",
+            "Query": "select `user`.a from `user` where `user`.id = :t_id",
+            "Table": "`user`",
+            "Values": [
+              ":t_id"
+            ],
+            "Vindex": "user_index"
           }
         ]
       }
@@ -5269,41 +5318,13 @@
   {
     "comment": "cant switch sides for outer joins",
     "query": "select id from user left join (select user_id from user_extra limit 10) ue on user.id = ue.user_id",
-    "plan": {
-      "QueryType": "SELECT",
-      "Original": "select id from user left join (select user_id from user_extra limit 10) ue on user.id = ue.user_id",
-      "Instructions": {
-        "OperatorType": "Route",
-        "Variant": "Scatter",
-        "Keyspace": {
-          "Name": "user",
-          "Sharded": true
-        },
-        "FieldQuery": "select id from `user` left join (select user_id from user_extra where 1 != 1) as ue on `user`.id = ue.user_id where 1 != 1",
-        "Query": "select id from `user` left join (select user_id from user_extra limit 10) as ue on `user`.id = ue.user_id",
-        "Table": "`user`, user_extra"
-      }
-    }
+    "plan": "unsupported: LEFT JOIN not supported for derived tables"
   },
   {
     "comment": "limit on both sides means that we can't evaluate this at all",
     "query": "select id from (select id from user limit 10) u join (select user_id from user_extra limit 10) ue on u.id = ue.user_id",
     "v3-plan": "unsupported: filtering on results of cross-shard subquery",
-    "gen4-plan": {
-      "QueryType": "SELECT",
-      "Original": "select id from (select id from user limit 10) u join (select user_id from user_extra limit 10) ue on u.id = ue.user_id",
-      "Instructions": {
-        "OperatorType": "Route",
-        "Variant": "Scatter",
-        "Keyspace": {
-          "Name": "user",
-          "Sharded": true
-        },
-        "FieldQuery": "select id from (select id from `user` where 1 != 1) as u, (select user_id from user_extra where 1 != 1) as ue where 1 != 1",
-        "Query": "select id from (select id from `user` limit 10) as u, (select user_id from user_extra limit 10) as ue where u.id = ue.user_id",
-        "Table": "`user`, user_extra"
-      }
-    }
+    "gen4-plan": "unsupported: JOIN not supported between derived tables"
   },
   {
     "query": "SELECT music.id FROM (SELECT MAX(id) as maxt FROM music WHERE music.user_id = 5) other JOIN music ON other.maxt = music.id",