From 8f7661d66dffc531fa992c421f6987f7819dc40c Mon Sep 17 00:00:00 2001 From: Radu Berinde Date: Sat, 18 Jul 2020 20:12:34 -0700 Subject: [PATCH] sql: remove unnecessary stmtType from explain code 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 --- pkg/sql/exec_factory_util.go | 2 - pkg/sql/explain_plan.go | 110 ++++++++++++++++++----------------- pkg/sql/explain_vec.go | 6 +- 3 files changed, 58 insertions(+), 60 deletions(-) diff --git a/pkg/sql/exec_factory_util.go b/pkg/sql/exec_factory_util.go index 9b61f7dec4e0..41b269f075bf 100644 --- 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 index be543bc2805c..4d852f85aa01 100644 --- 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 index b348471afb3c..b869bffd1b2f 100644 --- 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]