Merge #51585

51585: sql: remove unnecessary stmtType from explain code r=RaduBerinde a=RaduBerinde The `tree.StatementType` is not needed for `EXPLAIN (PLAN)` or `(VEC)`. I believe it is only necessary for `EXPLAIN ANALYZE` (when we actually run the query). This commit removes the plumbing of `stmtType` from the paths where it is not necessary. Release note: None Co-authored-by: Radu Berinde <[email protected]>
cockroachdb · Jul 19, 2020 · a0123f1 · a0123f1
2 parents 6de0863 + 8f7661d
commit a0123f1
Show file tree

Hide file tree

Showing 3 changed files with 58 additions and 60 deletions.
diff --git a/pkg/sql/exec_factory_util.go b/pkg/sql/exec_factory_util.go
@@ -131,7 +131,6 @@ func constructExplainPlanNode(
 			options:       options,
 			plan:          p.main,
 			subqueryPlans: p.subqueryPlans,
-			stmtType:      stmtType,
 		}, nil
 
 	case tree.ExplainPlan:
@@ -142,7 +141,6 @@ func constructExplainPlanNode(
 			context.TODO(),
 			options,
 			&p.planComponents,
-			stmtType,
 		)
 
 	default:

diff --git a/pkg/sql/explain_plan.go b/pkg/sql/explain_plan.go
@@ -17,6 +17,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/roachpb"
 	"github.com/cockroachdb/cockroach/pkg/sql/colflow"
 	"github.com/cockroachdb/cockroach/pkg/sql/opt/exec/explain"
+	"github.com/cockroachdb/cockroach/pkg/sql/physicalplan"
 	"github.com/cockroachdb/cockroach/pkg/sql/rowexec"
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
 	"github.com/cockroachdb/cockroach/pkg/sql/sessiondata"
@@ -40,15 +41,13 @@ type explainPlanNode struct {
 
 	plan planComponents
 
-	stmtType tree.StatementType
-
 	run explainPlanRun
 }
 
 // makeExplainPlanNodeWithPlan instantiates a planNode that EXPLAINs an
 // underlying plan.
 func (p *planner) makeExplainPlanNodeWithPlan(
-	ctx context.Context, opts *tree.ExplainOptions, plan *planComponents, stmtType tree.StatementType,
+	ctx context.Context, opts *tree.ExplainOptions, plan *planComponents,
 ) (planNode, error) {
 	flags := explain.MakeFlags(opts)
 
@@ -60,8 +59,7 @@ func (p *planner) makeExplainPlanNodeWithPlan(
 	columns = append(sqlbase.ResultColumns(nil), columns...)
 
 	node := &explainPlanNode{
-		plan:     *plan,
-		stmtType: stmtType,
+		plan: *plan,
 		run: explainPlanRun{
 			results: p.newContainerValuesNode(columns, 0),
 		},
@@ -99,7 +97,7 @@ type explainPlanRun struct {
 }
 
 func (e *explainPlanNode) startExec(params runParams) error {
-	return populateExplain(params, &e.explainer, e.run.results, &e.plan, e.stmtType)
+	return populateExplain(params, &e.explainer, e.run.results, &e.plan)
 }
 
 func (e *explainPlanNode) Next(params runParams) (bool, error) { return e.run.results.Next(params) }
@@ -140,54 +138,10 @@ func (e *explainer) init(flags explain.Flags) {
 // populateExplain walks the plan and generates rows in a valuesNode.
 // The subquery plans, if any are known to the planner, are printed
 // at the bottom.
-func populateExplain(
-	params runParams, e *explainer, v *valuesNode, plan *planComponents, stmtType tree.StatementType,
-) error {
-	// Determine the "distributed" and "vectorized" values, which we will emit as
+func populateExplain(params runParams, e *explainer, v *valuesNode, plan *planComponents) error {
+	// Determine the "distributed" and "vectorized" values and emit them as
 	// special rows.
-	var willVectorize bool
-	distSQLPlanner := params.extendedEvalCtx.DistSQLPlanner
-	distribution := getPlanDistributionForExplainPurposes(
-		params.ctx, params.extendedEvalCtx.ExecCfg.NodeID,
-		params.extendedEvalCtx.SessionData.DistSQLMode, plan.main,
-	)
-	willDistribute := distribution.WillDistribute()
-	outerSubqueries := params.p.curPlan.subqueryPlans
-	planCtx := newPlanningCtxForExplainPurposes(distSQLPlanner, params, stmtType, plan.subqueryPlans, distribution)
-	defer func() {
-		planCtx.planner.curPlan.subqueryPlans = outerSubqueries
-	}()
-	physicalPlan, err := newPhysPlanForExplainPurposes(planCtx, distSQLPlanner, plan.main)
-	if err == nil {
-		// There might be an issue making the physical plan, but that should not
-		// cause an error or panic, so swallow the error. See #40677 for example.
-		distSQLPlanner.FinalizePlan(planCtx, physicalPlan)
-		flows := physicalPlan.GenerateFlowSpecs()
-		flowCtx := newFlowCtxForExplainPurposes(planCtx, params)
-		flowCtx.Cfg.ClusterID = &distSQLPlanner.rpcCtx.ClusterID
-
-		ctxSessionData := flowCtx.EvalCtx.SessionData
-		vectorizedThresholdMet := physicalPlan.MaxEstimatedRowCount >= ctxSessionData.VectorizeRowCountThreshold
-		if ctxSessionData.VectorizeMode == sessiondata.VectorizeOff {
-			willVectorize = false
-		} else if !vectorizedThresholdMet && (ctxSessionData.VectorizeMode == sessiondata.Vectorize201Auto || ctxSessionData.VectorizeMode == sessiondata.VectorizeOn) {
-			willVectorize = false
-		} else {
-			willVectorize = true
-			thisNodeID, _ := params.extendedEvalCtx.NodeID.OptionalNodeID()
-			for scheduledOnNodeID, flow := range flows {
-				scheduledOnRemoteNode := scheduledOnNodeID != thisNodeID
-				if _, err := colflow.SupportsVectorized(
-					params.ctx, flowCtx, flow.Processors, !willDistribute, nil /* output */, scheduledOnRemoteNode,
-				); err != nil {
-					willVectorize = false
-					break
-				}
-			}
-		}
-	}
-
-	// First, emit the "distribution" and "vectorized" information rows.
+	distribution, willVectorize := explainGetDistributedAndVectorized(params, plan)
 	e.ob.AddField("distribution", distribution.String())
 	e.ob.AddField("vectorized", fmt.Sprintf("%t", willVectorize))
 
@@ -381,3 +335,53 @@ func (e *explainer) leaveNode(name string, _ planNode) error {
 	e.ob.LeaveNode()
 	return nil
 }
+
+// explainGetDistributedAndVectorized determines the "distributed" and
+// "vectorized" properties for EXPLAIN.
+func explainGetDistributedAndVectorized(
+	params runParams, plan *planComponents,
+) (distribution physicalplan.PlanDistribution, willVectorize bool) {
+	// Determine the "distributed" and "vectorized" values, which we will emit as
+	// special rows.
+	distSQLPlanner := params.extendedEvalCtx.DistSQLPlanner
+	distribution = getPlanDistributionForExplainPurposes(
+		params.ctx, params.extendedEvalCtx.ExecCfg.NodeID,
+		params.extendedEvalCtx.SessionData.DistSQLMode, plan.main,
+	)
+	willDistribute := distribution.WillDistribute()
+	outerSubqueries := params.p.curPlan.subqueryPlans
+	planCtx := newPlanningCtxForExplainPurposes(distSQLPlanner, params, plan.subqueryPlans, distribution)
+	defer func() {
+		planCtx.planner.curPlan.subqueryPlans = outerSubqueries
+	}()
+	physicalPlan, err := newPhysPlanForExplainPurposes(planCtx, distSQLPlanner, plan.main)
+	if err == nil {
+		// There might be an issue making the physical plan, but that should not
+		// cause an error or panic, so swallow the error. See #40677 for example.
+		distSQLPlanner.FinalizePlan(planCtx, physicalPlan)
+		flows := physicalPlan.GenerateFlowSpecs()
+		flowCtx := newFlowCtxForExplainPurposes(planCtx, params)
+		flowCtx.Cfg.ClusterID = &distSQLPlanner.rpcCtx.ClusterID
+
+		ctxSessionData := flowCtx.EvalCtx.SessionData
+		vectorizedThresholdMet := physicalPlan.MaxEstimatedRowCount >= ctxSessionData.VectorizeRowCountThreshold
+		if ctxSessionData.VectorizeMode == sessiondata.VectorizeOff {
+			willVectorize = false
+		} else if !vectorizedThresholdMet && (ctxSessionData.VectorizeMode == sessiondata.Vectorize201Auto || ctxSessionData.VectorizeMode == sessiondata.VectorizeOn) {
+			willVectorize = false
+		} else {
+			willVectorize = true
+			thisNodeID, _ := params.extendedEvalCtx.NodeID.OptionalNodeID()
+			for scheduledOnNodeID, flow := range flows {
+				scheduledOnRemoteNode := scheduledOnNodeID != thisNodeID
+				if _, err := colflow.SupportsVectorized(
+					params.ctx, flowCtx, flow.Processors, !willDistribute, nil /* output */, scheduledOnRemoteNode,
+				); err != nil {
+					willVectorize = false
+					break
+				}
+			}
+		}
+	}
+	return distribution, willVectorize
+}
diff --git a/pkg/sql/explain_vec.go b/pkg/sql/explain_vec.go
@@ -37,8 +37,6 @@ type explainVecNode struct {
 	options *tree.ExplainOptions
 	plan    planMaybePhysical
 
-	stmtType tree.StatementType
-
 	run struct {
 		lines []string
 		// The current row returned by the node.
@@ -61,7 +59,7 @@ func (n *explainVecNode) startExec(params runParams) error {
 	)
 	willDistribute := distribution.WillDistribute()
 	outerSubqueries := params.p.curPlan.subqueryPlans
-	planCtx := newPlanningCtxForExplainPurposes(distSQLPlanner, params, n.stmtType, n.subqueryPlans, distribution)
+	planCtx := newPlanningCtxForExplainPurposes(distSQLPlanner, params, n.subqueryPlans, distribution)
 	defer func() {
 		planCtx.planner.curPlan.subqueryPlans = outerSubqueries
 	}()
@@ -138,13 +136,11 @@ func newFlowCtxForExplainPurposes(planCtx *PlanningCtx, params runParams) *execi
 func newPlanningCtxForExplainPurposes(
 	distSQLPlanner *DistSQLPlanner,
 	params runParams,
-	stmtType tree.StatementType,
 	subqueryPlans []subquery,
 	distribution physicalplan.PlanDistribution,
 ) *PlanningCtx {
 	planCtx := distSQLPlanner.NewPlanningCtx(params.ctx, params.extendedEvalCtx, params.p, params.p.txn, distribution.WillDistribute())
 	planCtx.ignoreClose = true
-	planCtx.stmtType = stmtType
 	planCtx.planner.curPlan.subqueryPlans = subqueryPlans
 	for i := range planCtx.planner.curPlan.subqueryPlans {
 		p := &planCtx.planner.curPlan.subqueryPlans[i]