Skip to content

Commit

Permalink
sql: remove planDataSource field in renderNode
Browse files Browse the repository at this point in the history
The `planDataSource` field of `renderNode` had a `columns` sub-field
that was only set some of the time, causing confusion with the
`renderNode`'s `columns` field. `planDataSource` has been replaced with
`planNode` to avoid confusion and misuse.

Release note: None
  • Loading branch information
mgartner committed Dec 20, 2024
1 parent cf65619 commit a67c92f
Show file tree
Hide file tree
Showing 5 changed files with 15 additions and 14 deletions.
8 changes: 4 additions & 4 deletions pkg/sql/distsql_physical_planner.go
Original file line number Diff line number Diff line change
Expand Up @@ -693,7 +693,7 @@ func checkSupportForPlanNode(
return cannotDistribute, err
}
}
return checkSupportForPlanNode(ctx, n.source.plan, distSQLVisitor, sd)
return checkSupportForPlanNode(ctx, n.source, distSQLVisitor, sd)

case *scanNode:
if n.lockingStrength != descpb.ScanLockingStrength_FOR_NONE {
Expand Down Expand Up @@ -4024,7 +4024,7 @@ func (dsp *DistSQLPlanner) createPhysPlanForPlanNode(
plan, err = dsp.createPlanForProjectSet(ctx, planCtx, n)

case *renderNode:
plan, err = dsp.createPhysPlanForPlanNode(ctx, planCtx, n.source.plan)
plan, err = dsp.createPhysPlanForPlanNode(ctx, planCtx, n.source)
if err != nil {
return nil, err
}
Expand All @@ -4037,8 +4037,8 @@ func (dsp *DistSQLPlanner) createPhysPlanForPlanNode(
if in, ok := n.source.(*insertNode); ok {
// Skip over any renderNodes.
nod := in.source
for r, ok := nod.(*renderNode); ok; r, ok = r.source.plan.(*renderNode) {
nod = r.source.plan
for r, ok := nod.(*renderNode); ok; r, ok = r.source.(*renderNode) {
nod = r.source
}
if v, ok := nod.(*valuesNode); ok {
if v.coldataBatch != nil {
Expand Down
4 changes: 2 additions & 2 deletions pkg/sql/distsql_physical_planner_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2011,14 +2011,14 @@ func TestCheckScanParallelizationIfLocal(t *testing.T) {
},
{
plan: planComponents{main: planMaybePhysical{planNode: &renderNode{
source: planDataSource{plan: scanToParallelize},
source: scanToParallelize,
render: []tree.TypedExpr{&tree.IndexedVar{Idx: 0}},
}}},
hasScanNodeToParallelize: true,
},
{
plan: planComponents{main: planMaybePhysical{planNode: &renderNode{
source: planDataSource{plan: scanToParallelize},
source: scanToParallelize,
render: []tree.TypedExpr{&tree.IsNullExpr{}},
}}},
// Not a simple projection (some expressions might be handled by
Expand Down
9 changes: 5 additions & 4 deletions pkg/sql/opt_exec_factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -2262,15 +2262,16 @@ type renderBuilder struct {

// init initializes the renderNode with render expressions.
func (rb *renderBuilder) init(n exec.Node, reqOrdering exec.OutputOrdering) {
src := asDataSource(n)
p := n.(planNode)
rb.r = &renderNode{
source: src,
source: p,
columns: planColumns(p),
}
rb.r.reqOrdering = ReqOrdering(reqOrdering)

// If there's a spool, pull it up.
if spool, ok := rb.r.source.plan.(*spoolNode); ok {
rb.r.source.plan = spool.source
if spool, ok := rb.r.source.(*spoolNode); ok {
rb.r.source = spool.source
spool.source = rb.r
rb.res = spool
} else {
Expand Down
6 changes: 3 additions & 3 deletions pkg/sql/render.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ type renderNode struct {
// source describes where the data is coming from.
// populated initially by initFrom().
// potentially modified by index selection.
source planDataSource
source planNode

// Rendering expressions for rows and corresponding output columns.
render []tree.TypedExpr
Expand All @@ -51,7 +51,7 @@ var _ tree.IndexedVarContainer = &renderNode{}

// IndexedVarResolvedType implements the tree.IndexedVarContainer interface.
func (r *renderNode) IndexedVarResolvedType(idx int) *types.T {
return r.source.columns[idx].Typ
return r.columns[idx].Typ
}

func (r *renderNode) startExec(runParams) error {
Expand All @@ -66,7 +66,7 @@ func (r *renderNode) Values() tree.Datums {
panic("renderNode can't be run in local mode")
}

func (r *renderNode) Close(ctx context.Context) { r.source.plan.Close(ctx) }
func (r *renderNode) Close(ctx context.Context) { r.source.Close(ctx) }

// getTimestamp will get the timestamp for an AS OF clause. It will also
// verify the timestamp against the transaction. If AS OF SYSTEM TIME is
Expand Down
2 changes: 1 addition & 1 deletion pkg/sql/walk.go
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ func (v *planVisitor) visitInternal(plan planNode, name string) {
n.source.plan = v.visit(n.source.plan)

case *renderNode:
n.source.plan = v.visit(n.source.plan)
n.source = v.visit(n.source)

case *indexJoinNode:
n.input = v.visit(n.input)
Expand Down

0 comments on commit a67c92f

Please sign in to comment.