Repository: vitessio/vitess
Pull request #7140 (2020-12-08 17:16:10)

Merge SelectDBA routes when possible
It's possible to merge them when the query does
not do any runtime routing by using `table_name`
or `table_schema` predicates in the `WHERE` clause


Patch:
diff --git a/go/vt/vtgate/planbuilder/dml.go b/go/vt/vtgate/planbuilder/dml.go
index 3fe497e7ec..3103b3b42d 100644
--- a/go/vt/vtgate/planbuilder/dml.go
+++ b/go/vt/vtgate/planbuilder/dml.go
@@ -103,7 +103,7 @@ func nameMatch(node sqlparser.Expr, col sqlparser.ColIdent) bool {
 func buildDMLPlan(vschema ContextVSchema, dmlType string, stmt sqlparser.Statement, tableExprs sqlparser.TableExprs, where *sqlparser.Where, orderBy sqlparser.OrderBy, limit *sqlparser.Limit, comments sqlparser.Comments, nodes ...sqlparser.SQLNode) (*engine.DML, vindexes.SingleColumn, string, error) {
 	edml := &engine.DML{}
 	pb := newPrimitiveBuilder(vschema, newJointab(sqlparser.GetBindvars(stmt)))
-	rb, err := pb.processDMLTable(tableExprs)
+	rb, err := pb.processDMLTable(tableExprs, nil)
 	if err != nil {
 		return nil, nil, "", err
 	}
diff --git a/go/vt/vtgate/planbuilder/from.go b/go/vt/vtgate/planbuilder/from.go
index 370e9238b8..0b7bb58437 100644
--- a/go/vt/vtgate/planbuilder/from.go
+++ b/go/vt/vtgate/planbuilder/from.go
@@ -32,8 +32,8 @@ import (
 // This file has functions to analyze the FROM clause.
 
 // processDMLTable analyzes the FROM clause for DMLs and returns a route.
-func (pb *primitiveBuilder) processDMLTable(tableExprs sqlparser.TableExprs) (*route, error) {
-	if err := pb.processTableExprs(tableExprs); err != nil {
+func (pb *primitiveBuilder) processDMLTable(tableExprs sqlparser.TableExprs, where sqlparser.Expr) (*route, error) {
+	if err := pb.processTableExprs(tableExprs, where); err != nil {
 		return nil, err
 	}
 	rb, ok := pb.plan.(*route)
@@ -48,28 +48,28 @@ func (pb *primitiveBuilder) processDMLTable(tableExprs sqlparser.TableExprs) (*r
 
 // processTableExprs analyzes the FROM clause. It produces a logicalPlan
 // with all the routes identified.
-func (pb *primitiveBuilder) processTableExprs(tableExprs sqlparser.TableExprs) error {
+func (pb *primitiveBuilder) processTableExprs(tableExprs sqlparser.TableExprs, where sqlparser.Expr) error {
 	if len(tableExprs) == 1 {
-		return pb.processTableExpr(tableExprs[0])
+		return pb.processTableExpr(tableExprs[0], where)
 	}
 
-	if err := pb.processTableExpr(tableExprs[0]); err != nil {
+	if err := pb.processTableExpr(tableExprs[0], where); err != nil {
 		return err
 	}
 	rpb := newPrimitiveBuilder(pb.vschema, pb.jt)
-	if err := rpb.processTableExprs(tableExprs[1:]); err != nil {
+	if err := rpb.processTableExprs(tableExprs[1:], where); err != nil {
 		return err
 	}
-	return pb.join(rpb, nil)
+	return pb.join(rpb, nil, where)
 }
 
 // processTableExpr produces a logicalPlan subtree for the given TableExpr.
-func (pb *primitiveBuilder) processTableExpr(tableExpr sqlparser.TableExpr) error {
+func (pb *primitiveBuilder) processTableExpr(tableExpr sqlparser.TableExpr, where sqlparser.Expr) error {
 	switch tableExpr := tableExpr.(type) {
 	case *sqlparser.AliasedTableExpr:
 		return pb.processAliasedTable(tableExpr)
 	case *sqlparser.ParenTableExpr:
-		err := pb.processTableExprs(tableExpr.Exprs)
+		err := pb.processTableExprs(tableExpr.Exprs, where)
 		// If it's a route, preserve the parenthesis so things
 		// don't associate differently when more things are pushed
 		// into it. FROM a, (b, c) should not become FROM a, b, c.
@@ -83,7 +83,7 @@ func (pb *primitiveBuilder) processTableExpr(tableExpr sqlparser.TableExpr) erro
 		}
 		return err
 	case *sqlparser.JoinTableExpr:
-		return pb.processJoin(tableExpr)
+		return pb.processJoin(tableExpr, where)
 	}
 	return fmt.Errorf("BUG: unexpected table expression type: %T", tableExpr)
 }
@@ -268,7 +268,7 @@ func (pb *primitiveBuilder) buildTablePrimitive(tableExpr *sqlparser.AliasedTabl
 // processJoin produces a logicalPlan subtree for the given Join.
 // If the left and right nodes can be part of the same route,
 // then it's a route. Otherwise, it's a join.
-func (pb *primitiveBuilder) processJoin(ajoin *sqlparser.JoinTableExpr) error {
+func (pb *primitiveBuilder) processJoin(ajoin *sqlparser.JoinTableExpr, where sqlparser.Expr) error {
 	switch ajoin.Join {
 	case sqlparser.NormalJoinType, sqlparser.StraightJoinType, sqlparser.LeftJoinType:
 	case sqlparser.RightJoinType:
@@ -276,14 +276,14 @@ func (pb *primitiveBuilder) processJoin(ajoin *sqlparser.JoinTableExpr) error {
 	default:
 		return fmt.Errorf("unsupported: %s", ajoin.Join.ToString())
 	}
-	if err := pb.processTableExpr(ajoin.LeftExpr); err != nil {
+	if err := pb.processTableExpr(ajoin.LeftExpr, where); err != nil {
 		return err
 	}
 	rpb := newPrimitiveBuilder(pb.vschema, pb.jt)
-	if err := rpb.processTableExpr(ajoin.RightExpr); err != nil {
+	if err := rpb.processTableExpr(ajoin.RightExpr, where); err != nil {
 		return err
 	}
-	return pb.join(rpb, ajoin)
+	return pb.join(rpb, ajoin, where)
 }
 
 // convertToLeftJoin converts a right join into a left join.
@@ -300,7 +300,7 @@ func convertToLeftJoin(ajoin *sqlparser.JoinTableExpr) {
 	ajoin.Join = sqlparser.LeftJoinType
 }
 
-func (pb *primitiveBuilder) join(rpb *primitiveBuilder, ajoin *sqlparser.JoinTableExpr) error {
+func (pb *primitiveBuilder) join(rpb *primitiveBuilder, ajoin *sqlparser.JoinTableExpr, where sqlparser.Expr) error {
 	// Merge the symbol tables. In the case of a left join, we have to
 	// ideally create new symbols that originate from the join primitive.
 	// However, this is not worth it for now, because the Push functions
@@ -317,7 +317,7 @@ func (pb *primitiveBuilder) join(rpb *primitiveBuilder, ajoin *sqlparser.JoinTab
 	}
 
 	// Try merging the routes.
-	if !lRoute.JoinCanMerge(pb, rRoute, ajoin) {
+	if !lRoute.JoinCanMerge(pb, rRoute, ajoin, where) {
 		return newJoin(pb, rpb, ajoin)
 	}
 
diff --git a/go/vt/vtgate/planbuilder/insert.go b/go/vt/vtgate/planbuilder/insert.go
index ba4e74c543..c27e6c64ed 100644
--- a/go/vt/vtgate/planbuilder/insert.go
+++ b/go/vt/vtgate/planbuilder/insert.go
@@ -34,7 +34,7 @@ func buildInsertPlan(stmt sqlparser.Statement, vschema ContextVSchema) (engine.P
 	ins := stmt.(*sqlparser.Insert)
 	pb := newPrimitiveBuilder(vschema, newJointab(sqlparser.GetBindvars(ins)))
 	exprs := sqlparser.TableExprs{&sqlparser.AliasedTableExpr{Expr: ins.Table}}
-	rb, err := pb.processDMLTable(exprs)
+	rb, err := pb.processDMLTable(exprs, nil)
 	if err != nil {
 		return nil, err
 	}
diff --git a/go/vt/vtgate/planbuilder/route.go b/go/vt/vtgate/planbuilder/route.go
index e6eab2b833..81bffcc92c 100644
--- a/go/vt/vtgate/planbuilder/route.go
+++ b/go/vt/vtgate/planbuilder/route.go
@@ -368,7 +368,7 @@ func (rb *route) isSingleShard() bool {
 // JoinCanMerge, SubqueryCanMerge and unionCanMerge have subtly different behaviors.
 // The difference in behavior is around SelectReference.
 // It's not worth trying to reuse the code between them.
-func (rb *route) JoinCanMerge(pb *primitiveBuilder, rrb *route, ajoin *sqlparser.JoinTableExpr) bool {
+func (rb *route) JoinCanMerge(pb *primitiveBuilder, rrb *route, ajoin *sqlparser.JoinTableExpr, where sqlparser.Expr) bool {
 	if rb.eroute.Keyspace.Name != rrb.eroute.Keyspace.Name {
 		return false
 	}
@@ -386,8 +386,24 @@ func (rb *route) JoinCanMerge(pb *primitiveBuilder, rrb *route, ajoin *sqlparser
 		}
 	case engine.SelectReference:
 		return true
-	case engine.SelectNext, engine.SelectDBA:
+	case engine.SelectNext:
 		return false
+	case engine.SelectDBA:
+		if rrb.eroute.Opcode != engine.SelectDBA {
+			return false
+		}
+		if where == nil {
+			return true
+		}
+		hasRuntimeRoutingPredicates := false
+		sqlparser.Walk(func(node sqlparser.SQLNode) (kontinue bool, err error) {
+			col, ok := node.(*sqlparser.ColName)
+			if ok {
+				hasRuntimeRoutingPredicates = hasRuntimeRoutingPredicates || isTableNameCol(col) || isDbNameCol(col)
+			}
+			return !hasRuntimeRoutingPredicates, nil
+		}, where)
+		return !hasRuntimeRoutingPredicates
 	}
 	if ajoin == nil {
 		return false
diff --git a/go/vt/vtgate/planbuilder/select.go b/go/vt/vtgate/planbuilder/select.go
index 16d117c3b7..87d4831620 100644
--- a/go/vt/vtgate/planbuilder/select.go
+++ b/go/vt/vtgate/planbuilder/select.go
@@ -118,7 +118,11 @@ func (pb *primitiveBuilder) processSelect(sel *sqlparser.Select, outer *symtab,
 		return mysql.NewSQLError(mysql.ERCantUseOptionHere, mysql.SSSyntaxErrorOrAccessViolation, "Incorrect usage/placement of 'INTO'")
 	}
 
-	if err := pb.processTableExprs(sel.From); err != nil {
+	var where sqlparser.Expr
+	if sel.Where != nil {
+		where = sel.Where.Expr
+	}
+	if err := pb.processTableExprs(sel.From, where); err != nil {
 		return err
 	}
 
diff --git a/go/vt/vtgate/planbuilder/system_tables.go b/go/vt/vtgate/planbuilder/system_tables.go
index 3bf7f8ea7d..84ee0d323d 100644
--- a/go/vt/vtgate/planbuilder/system_tables.go
+++ b/go/vt/vtgate/planbuilder/system_tables.go
@@ -70,7 +70,15 @@ func isTableSchemaOrName(e sqlparser.Expr) (isTableSchema bool, isTableName bool
 	if !ok {
 		return false, false
 	}
-	return col.Name.EqualString("table_schema") || col.Name.EqualString("constraint_schema") || col.Name.EqualString("schema_name"), col.Name.EqualString("table_name")
+	return isDbNameCol(col), isTableNameCol(col)
+}
+
+func isDbNameCol(col *sqlparser.ColName) bool {
+	return col.Name.EqualString("table_schema") || col.Name.EqualString("constraint_schema") || col.Name.EqualString("schema_name")
+}
+
+func isTableNameCol(col *sqlparser.ColName) bool {
+	return col.Name.EqualString("table_name")
 }
 
 func extractInfoSchemaRoutingPredicate(in sqlparser.Expr) (bool, evalengine.Expr, error) {


Test patch:
diff --git a/go/vt/vtgate/planbuilder/route_test.go b/go/vt/vtgate/planbuilder/route_test.go
index cf60f09e28..06b83e0984 100644
--- a/go/vt/vtgate/planbuilder/route_test.go
+++ b/go/vt/vtgate/planbuilder/route_test.go
@@ -50,27 +50,30 @@ func TestJoinCanMerge(t *testing.T) {
 		{false, false, false, false, false, false, false, false, true, false},
 		{false, false, false, false, false, false, false, false, true, false},
 		{false, false, false, false, false, false, false, false, true, false},
-		{false, false, false, false, false, false, false, false, true, false},
+		{false, false, false, false, false, false, false, true, true, false},
 		{true, true, true, true, true, true, true, true, true, true},
 		{false, false, false, false, false, false, false, false, true, false},
 	}
 
 	ks := &vindexes.Keyspace{}
-	lRoute := &route{
-		// Setting condition will make SelectEqualUnique match itself.
-		condition: &sqlparser.ColName{},
-	}
-	pb := &primitiveBuilder{
-		plan: lRoute,
-	}
-	rRoute := &route{
-		condition: &sqlparser.ColName{},
-	}
 	for left, vals := range testcases {
-		lRoute.eroute = engine.NewSimpleRoute(engine.RouteOpcode(left), ks)
 		for right, val := range vals {
-			rRoute.eroute = engine.NewSimpleRoute(engine.RouteOpcode(right), ks)
-			assert.Equal(t, val, lRoute.JoinCanMerge(pb, rRoute, nil), fmt.Sprintf("%v:%v", lRoute.eroute.RouteType(), rRoute.eroute.RouteType()))
+			name := fmt.Sprintf("%d:%d", left, right)
+			t.Run(name, func(t *testing.T) {
+				lRoute := &route{
+					// Setting condition will make SelectEqualUnique match itself.
+					condition: &sqlparser.ColName{},
+				}
+				pb := &primitiveBuilder{
+					plan: lRoute,
+				}
+				rRoute := &route{
+					condition: &sqlparser.ColName{},
+				}
+				lRoute.eroute = engine.NewSimpleRoute(engine.RouteOpcode(left), ks)
+				rRoute.eroute = engine.NewSimpleRoute(engine.RouteOpcode(right), ks)
+				assert.Equal(t, val, lRoute.JoinCanMerge(pb, rRoute, nil, nil), fmt.Sprintf("%v:%v", lRoute.eroute.RouteType(), rRoute.eroute.RouteType()))
+			})
 		}
 	}
 }
diff --git a/go/vt/vtgate/planbuilder/testdata/from_cases.txt b/go/vt/vtgate/planbuilder/testdata/from_cases.txt
index 44358a1a30..441085535a 100644
--- a/go/vt/vtgate/planbuilder/testdata/from_cases.txt
+++ b/go/vt/vtgate/planbuilder/testdata/from_cases.txt
@@ -301,32 +301,14 @@
   "QueryType": "SELECT",
   "Original": "select a.id,b.id from information_schema.a as a, information_schema.b as b",
   "Instructions": {
-    "OperatorType": "Join",
-    "Variant": "Join",
-    "JoinColumnIndexes": "-1,1",
-    "TableName": "_",
-    "Inputs": [
-      {
-        "OperatorType": "Route",
-        "Variant": "SelectDBA",
-        "Keyspace": {
-          "Name": "main",
-          "Sharded": false
-        },
-        "FieldQuery": "select a.id from information_schema.a as a where 1 != 1",
-        "Query": "select a.id from information_schema.a as a"
-      },
-      {
-        "OperatorType": "Route",
-        "Variant": "SelectDBA",
-        "Keyspace": {
-          "Name": "main",
-          "Sharded": false
-        },
-        "FieldQuery": "select b.id from information_schema.b as b where 1 != 1",
-        "Query": "select b.id from information_schema.b as b"
-      }
-    ]
+    "OperatorType": "Route",
+    "Variant": "SelectDBA",
+    "Keyspace": {
+      "Name": "main",
+      "Sharded": false
+    },
+    "FieldQuery": "select a.id, b.id from information_schema.a as a, information_schema.b as b where 1 != 1",
+    "Query": "select a.id, b.id from information_schema.a as a, information_schema.b as b"
   }
 }
 
@@ -2188,37 +2170,19 @@
 }
 
 # information schema join
-"select 42 from information_schema.a join information_schema.b"
+"select * from information_schema.a join information_schema.b"
 {
   "QueryType": "SELECT",
-  "Original": "select 42 from information_schema.a join information_schema.b",
+  "Original": "select * from information_schema.a join information_schema.b",
   "Instructions": {
-    "OperatorType": "Join",
-    "Variant": "Join",
-    "JoinColumnIndexes": "-1",
-    "TableName": "_",
-    "Inputs": [
-      {
-        "OperatorType": "Route",
-        "Variant": "SelectDBA",
-        "Keyspace": {
-          "Name": "main",
-          "Sharded": false
-        },
-        "FieldQuery": "select 42 from information_schema.a where 1 != 1",
-        "Query": "select 42 from information_schema.a"
-      },
-      {
-        "OperatorType": "Route",
-        "Variant": "SelectDBA",
-        "Keyspace": {
-          "Name": "main",
-          "Sharded": false
-        },
-        "FieldQuery": "select 1 from information_schema.b where 1 != 1",
-        "Query": "select 1 from information_schema.b"
-      }
-    ]
+    "OperatorType": "Route",
+    "Variant": "SelectDBA",
+    "Keyspace": {
+      "Name": "main",
+      "Sharded": false
+    },
+    "FieldQuery": "select * from information_schema.a join information_schema.b where 1 != 1",
+    "Query": "select * from information_schema.a join information_schema.b"
   }
 }