v3: revamp: simplify some relationships

This is a fundamental change in the relationships
of some core data structures.

Previously, every primitive was required to point at
the symatb it was part of. At the same time,
a symtab pointed at various primitives through
the symbols it was holding. This was problematic when
symtabs merged, which required the old symtabs to 'redirect'
to the new ones. There was no clarity on when those redirects
were applicable. There were places where it was not used.

In this new code, the primitive->symtab relationship has
been removed. A new top level planBuilder object now holds
the symtab, along with other 'global' things like vschema,
and the jointab. Most functions that needed the symtab
have been made members of planBuilder. There are a few
stray cases (members of the builder primitives) that still
need it. In those cases, the symtab is passed in. But these
are far and few.

This has led to an overall simplification because this has
caused most function signatures to shrink. There is also
no ambiguity about the exact symtab being referenced.

The route merge functionality had to be refactored. Some
of those functions have moved to planBuilder. Also,
newJoin was affected. Also, the code in newJoin was
conceptually incorrect when handling left join. This
has now been fixed to better represent the meaning
of the functionality.

In builder, LeftMost has been renamed to First, which
is a more appropriate name.

Also, while handling group by expressions, there were
some risky type-casts. That has now been cleaned. If
a group by is possible, then the analyzer returns
an explicit object that can handle them.

Signed-off-by: Sugu Sougoumarane <ssougou@gmail.com>

Author: sougou

go/vt/vtgate/engine/route.go

@@ -106,7 +106,8 @@ func (route *Route) MarshalJSON() ([]byte, error) {
 }
 
 // RouteOpcode is a number representing the opcode
-// for the Route primitve.
+// for the Route primitve. Adding new opcodes here
+// will require review of the join code in planbuilder.
 type RouteOpcode int
 
 // This is the list of RouteOpcode values.

go/vt/vtgate/planbuilder/builder.go

@@ -31,10 +31,6 @@ import (
 // builder defines the interface that a primitive must
 // satisfy.
 type builder interface {
-	// Symtab returns the associated symtab.
-	// Please copy code from an existing primitive to define this function.
-	Symtab() *symtab
-
 	// Order is the execution order of the primitve. If there are subprimitves,
 	// the order is one above the order of the subprimitives.
 	// This is because the primitive executes its subprimitives first and
@@ -55,12 +51,13 @@ type builder interface {
 	// Primitve returns the underlying primitive.
 	Primitive() engine.Primitive
 
-	// Leftmost returns the leftmost builder.
-	Leftmost() builder
+	// First returns the first builder of the tree,
+	// which is usually the left most.
+	First() builder
 
 	// PushFilter pushes a WHERE or HAVING clause expression
 	// to the specified origin.
-	PushFilter(filter sqlparser.Expr, whereType string, origin builder) error
+	PushFilter(pb *planBuilder, filter sqlparser.Expr, whereType string, origin builder) error
 
 	// PushSelect pushes the select expression to the specified
 	// originator. If successful, the originator must create

go/vt/vtgate/planbuilder/delete.go

@@ -33,31 +33,31 @@ func buildDeletePlan(del *sqlparser.Delete, vschema ContextVSchema) (*engine.Del
 	edel := &engine.Delete{
 		Query: generateQuery(del),
 	}
-	bldr, err := processTableExprs(del.TableExprs, vschema)
-	if err != nil {
+	pb := newPlanBuilder(vschema, newJointab(sqlparser.GetBindvars(del)))
+	if err := pb.processTableExprs(del.TableExprs); err != nil {
 		return nil, err
 	}
-	rb, ok := bldr.(*route)
+	rb, ok := pb.bldr.(*route)
 	if !ok {
 		return nil, errors.New("unsupported: multi-table delete statement in sharded keyspace")
 	}
 	edel.Keyspace = rb.ERoute.Keyspace
 	if !edel.Keyspace.Sharded {
 		// We only validate non-table subexpressions because the previous analysis has already validated them.
-		if !validateSubquerySamePlan(rb.ERoute.Keyspace.Name, rb, vschema, del.Targets, del.Where, del.OrderBy, del.Limit) {
+		if !pb.validateSubquerySamePlan(del.Targets, del.Where, del.OrderBy, del.Limit) {
 			return nil, errors.New("unsupported: sharded subqueries in DML")
 		}
 		edel.Opcode = engine.DeleteUnsharded
 		return edel, nil
 	}
-	if del.Targets != nil || len(rb.Symtab().tables) != 1 {
+	if del.Targets != nil || len(pb.st.tables) != 1 {
 		return nil, errors.New("unsupported: multi-table delete statement in sharded keyspace")
 	}
 	if hasSubquery(del) {
 		return nil, errors.New("unsupported: subqueries in sharded DML")
 	}
 	var tableName sqlparser.TableName
-	for t := range rb.Symtab().tables {
+	for t := range pb.st.tables {
 		tableName = t
 	}
 	table, _, destTabletType, destTarget, err := vschema.FindTable(tableName)

go/vt/vtgate/planbuilder/expr.go

@@ -79,39 +79,40 @@ func skipParenthesis(node sqlparser.Expr) sqlparser.Expr {
 //
 // If an expression has no references to the current query, then the left-most
 // origin is chosen as the default.
-func findOrigin(expr sqlparser.Expr, bldr builder) (origin builder, err error) {
-	highestOrigin := bldr.Leftmost()
+func (pb *planBuilder) findOrigin(expr sqlparser.Expr) (origin builder, err error) {
+	highestOrigin := pb.bldr.First()
 	var subroutes []*route
 	err = sqlparser.Walk(func(node sqlparser.SQLNode) (kontinue bool, err error) {
 		switch node := node.(type) {
 		case *sqlparser.ColName:
-			newOrigin, isLocal, err := bldr.Symtab().Find(node)
+			newOrigin, isLocal, err := pb.st.Find(node)
 			if err != nil {
 				return false, err
 			}
 			if isLocal && newOrigin.Order() > highestOrigin.Order() {
 				highestOrigin = newOrigin
 			}
 		case *sqlparser.Subquery:
-			var subplan builder
+			spb := newPlanBuilder(pb.vschema, pb.jt)
 			switch stmt := node.Select.(type) {
 			case *sqlparser.Select:
-				subplan, err = processSelect(stmt, bldr.Symtab().VSchema, bldr)
+				if err := spb.processSelect(stmt, pb.st); err != nil {
+					return false, err
+				}
 			case *sqlparser.Union:
-				subplan, err = processUnion(stmt, bldr.Symtab().VSchema, bldr)
+				if err := spb.processUnion(stmt, pb.st); err != nil {
+					return false, err
+				}
 			default:
 				panic(fmt.Sprintf("BUG: unexpected SELECT type: %T", node))
 			}
-			if err != nil {
-				return false, err
-			}
-			subroute, isRoute := subplan.(*route)
+			subroute, isRoute := spb.bldr.(*route)
 			if !isRoute {
 				return false, errors.New("unsupported: cross-shard query in subqueries")
 			}
-			for _, extern := range subroute.Symtab().Externs {
+			for _, extern := range spb.st.Externs {
 				// No error expected. These are resolved externs.
-				newOrigin, isLocal, _ := bldr.Symtab().Find(extern)
+				newOrigin, isLocal, _ := pb.st.Find(extern)
 				if isLocal && newOrigin.Order() > highestOrigin.Order() {
 					highestOrigin = newOrigin
 				}
@@ -123,7 +124,7 @@ func findOrigin(expr sqlparser.Expr, bldr builder) (origin builder, err error) {
 			if !node.Name.EqualString("last_insert_id") {
 				return true, nil
 			}
-			if rb, isRoute := bldr.(*route); !isRoute || rb.ERoute.Keyspace.Sharded {
+			if rb, isRoute := pb.bldr.(*route); !isRoute || rb.ERoute.Keyspace.Sharded {
 				return false, errors.New("unsupported: LAST_INSERT_ID is only allowed for unsharded keyspaces")
 			}
 		}
@@ -137,7 +138,7 @@ func findOrigin(expr sqlparser.Expr, bldr builder) (origin builder, err error) {
 		return nil, errors.New("unsupported: subquery cannot be merged with cross-shard subquery")
 	}
 	for _, subroute := range subroutes {
-		if err := highestRoute.SubqueryCanMerge(subroute); err != nil {
+		if err := highestRoute.SubqueryCanMerge(pb, subroute); err != nil {
 			return nil, err
 		}
 		subroute.Redirect = highestRoute
@@ -157,7 +158,11 @@ func hasSubquery(node sqlparser.SQLNode) bool {
 	return has
 }
 
-func validateSubquerySamePlan(keyspace string, bldr builder, vschema ContextVSchema, nodes ...sqlparser.SQLNode) bool {
+func (pb *planBuilder) validateSubquerySamePlan(nodes ...sqlparser.SQLNode) bool {
+	var keyspace string
+	if rb, ok := pb.bldr.(*route); ok {
+		keyspace = rb.ERoute.Keyspace.Name
+	}
 	samePlan := true
 
 	for _, node := range nodes {
@@ -171,12 +176,12 @@ func validateSubquerySamePlan(keyspace string, bldr builder, vschema ContextVSch
 				if !inSubQuery {
 					return true, nil
 				}
-				bldr, err := processSelect(nodeType, vschema, bldr)
-				if err != nil {
+				spb := newPlanBuilder(pb.vschema, pb.jt)
+				if err := spb.processSelect(nodeType, pb.st); err != nil {
 					samePlan = false
 					return false, err
 				}
-				innerRoute, ok := bldr.(*route)
+				innerRoute, ok := spb.bldr.(*route)
 				if !ok {
 					samePlan = false
 					return false, errors.New("dummy")
@@ -189,12 +194,12 @@ func validateSubquerySamePlan(keyspace string, bldr builder, vschema ContextVSch
 				if !inSubQuery {
 					return true, nil
 				}
-				bldr, err := processUnion(nodeType, vschema, nil)
-				if err != nil {
+				spb := newPlanBuilder(pb.vschema, pb.jt)
+				if err := spb.processUnion(nodeType, pb.st); err != nil {
 					samePlan = false
 					return false, err
 				}
-				innerRoute, ok := bldr.(*route)
+				innerRoute, ok := spb.bldr.(*route)
 				if !ok {
 					samePlan = false
 					return false, errors.New("dummy")