physicalplan: preevaluate subqueries on LocalExprs

When the plan is local, we do not serialize expressions. Previously, in such a case we would also not preevaluate the subqueries in the expressions which made `EXPLAIN (VEC)` return unexpected plan (there would `tree.Subquery` in the expression which we don't support in the vectorized, so we would wrap the plan). Now we will preevalute the subqueries before storing in the processor spec. AFAICT it affects only this explain variant and nothing else. Release note: None
cockroachdb · Jun 10, 2020 · 096265d · 096265d
1 parent 3b116e9
commit 096265d
Show file tree

Hide file tree

Showing 2 changed files with 32 additions and 19 deletions.
diff --git a/pkg/sql/logictest/testdata/logic_test/tpch_vec b/pkg/sql/logictest/testdata/logic_test/tpch_vec
@@ -754,13 +754,15 @@ EXPLAIN (VEC) SELECT ps_partkey, sum(ps_supplycost * ps_availqty::float) AS valu
 │
 └ Node 1
   └ *colexec.sortOp
-    └ *rowexec.noopProcessor
-      └ *colexec.hashAggregator
-        └ *rowexec.joinReader
-          └ *rowexec.joinReader
+    └ *colexec.selGTFloat64Float64Op
+      └ *colexec.castOpNullAny
+        └ *colexec.constNullOp
+          └ *colexec.hashAggregator
             └ *rowexec.joinReader
-              └ *colexec.selEQBytesBytesConstOp
-                └ *colexec.colBatchScan
+              └ *rowexec.joinReader
+                └ *rowexec.joinReader
+                  └ *colexec.selEQBytesBytesConstOp
+                    └ *colexec.colBatchScan
 
 # Query 12
 query T
@@ -826,10 +828,12 @@ EXPLAIN (VEC) SELECT s_suppkey, s_name, s_address, s_phone, total_revenue FROM s
   └ *colexec.mergeJoinInnerOp
     ├ *colexec.colBatchScan
     └ *colexec.sortOp
-      └ *rowexec.noopProcessor
-        └ *colexec.hashAggregator
-          └ *rowexec.indexJoiner
-            └ *colexec.colBatchScan
+      └ *colexec.selEQFloat64Float64Op
+        └ *colexec.castOpNullAny
+          └ *colexec.constNullOp
+            └ *colexec.hashAggregator
+              └ *rowexec.indexJoiner
+                └ *colexec.colBatchScan
 
 statement ok
 DROP VIEW revenue0
@@ -1002,4 +1006,11 @@ EXPLAIN (VEC) SELECT cntrycode, count(*) AS numcust, sum(c_acctbal) AS totacctba
   └ *colexec.sortOp
     └ *colexec.hashAggregator
       └ *rowexec.joinReader
-        └ *rowexec.tableReader
+        └ *colexec.selGTFloat64Float64Op
+          └ *colexec.castOpNullAny
+            └ *colexec.constNullOp
+              └ *colexec.selectInOpBytes
+                └ *colexec.substringInt64Int64Operator
+                  └ *colexec.constInt64Op
+                    └ *colexec.constInt64Op
+                      └ *colexec.colBatchScan
diff --git a/pkg/sql/physicalplan/expression.go b/pkg/sql/physicalplan/expression.go
@@ -75,14 +75,6 @@ func MakeExpression(
 		ctx = &fakeExprContext{}
 	}
 
-	if ctx.IsLocal() {
-		if indexVarMap != nil {
-			// Remap our indexed vars.
-			expr = sqlbase.RemapIVarsInTypedExpr(expr, indexVarMap)
-		}
-		return execinfrapb.Expression{LocalExpr: expr}, nil
-	}
-
 	evalCtx := ctx.EvalContext()
 	subqueryVisitor := &evalAndReplaceSubqueryVisitor{
 		evalCtx: evalCtx,
@@ -94,7 +86,17 @@ func MakeExpression(
 		if subqueryVisitor.err != nil {
 			return execinfrapb.Expression{}, subqueryVisitor.err
 		}
+		expr = outExpr.(tree.TypedExpr)
 	}
+
+	if ctx.IsLocal() {
+		if indexVarMap != nil {
+			// Remap our indexed vars.
+			expr = sqlbase.RemapIVarsInTypedExpr(expr, indexVarMap)
+		}
+		return execinfrapb.Expression{LocalExpr: expr}, nil
+	}
+
 	// We format the expression using the IndexedVar and Placeholder formatting interceptors.
 	fmtCtx := execinfrapb.ExprFmtCtxBase(evalCtx)
 	if indexVarMap != nil {