Skip to content

Commit

Permalink
sql: avoid calling planNode.Close() during execution
Browse files Browse the repository at this point in the history
We want that a planNode does not get closed during
execution. This was already mostly true, except for
`unionNode`. This patch modifies `unionNode` to make it compliant
with the new protocol, and clarifies the constraint in the
interface documentation for `planNode.Close()`.

Release note: None
  • Loading branch information
knz committed Jan 28, 2019
1 parent 9f084ad commit 67af366
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 20 deletions.
4 changes: 4 additions & 0 deletions pkg/sql/plan.go
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,10 @@ type planNode interface {
// This method should be called if the node has been used in any way (any
// methods on it have been called) after it was constructed. Note that this
// doesn't imply that startPlan() has been necessarily called.
//
// This method must not be called during execution - the planNode
// tree must remain "live" and readable via walk() even after
// execution completes.
Close(ctx context.Context)
}

Expand Down
3 changes: 0 additions & 3 deletions pkg/sql/subquery.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,9 +86,6 @@ func (p *planTop) evalSubqueries(params runParams) error {
}

func (s *subquery) doEval(params runParams) (result tree.Datum, err error) {
// After evaluation, there is no plan remaining.
defer func() { s.plan.Close(params.ctx); s.plan = nil }()

switch s.execMode {
case distsqlrun.SubqueryExecModeExists:
// For EXISTS expressions, all we want to know is if there is at least one
Expand Down
29 changes: 12 additions & 17 deletions pkg/sql/union.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,9 @@ import (
type unionNode struct {
// right and left are the data source operands.
// right is read first, to populate the `emit` field.
right, left planNode
right, left planNode
rightClosed, leftClosed bool

// columns contains the metadata for the results of this node.
columns sqlbase.ResultColumns
// inverted, when true, indicates that the right plan corresponds to
Expand Down Expand Up @@ -198,34 +200,28 @@ func (n *unionNode) Next(params runParams) (bool, error) {
if err := params.p.cancelChecker.Check(); err != nil {
return false, err
}
if n.right != nil {
if !n.rightClosed {
return n.readRight(params)
}
if n.left != nil {
if !n.leftClosed {
return n.readLeft(params)
}
return false, nil
}

func (n *unionNode) Values() tree.Datums {
if n.right != nil {
if !n.rightClosed {
return n.right.Values()
}
if n.left != nil {
if !n.leftClosed {
return n.left.Values()
}
return nil
}

func (n *unionNode) Close(ctx context.Context) {
if n.right != nil {
n.right.Close(ctx)
n.right = nil
}
if n.left != nil {
n.left.Close(ctx)
n.left = nil
}
n.right.Close(ctx)
n.left.Close(ctx)
}

func (n *unionNode) readRight(params runParams) (bool, error) {
Expand All @@ -250,8 +246,7 @@ func (n *unionNode) readRight(params runParams) (bool, error) {
return false, err
}

n.right.Close(params.ctx)
n.right = nil
n.rightClosed = true
return n.readLeft(params)
}

Expand All @@ -273,8 +268,8 @@ func (n *unionNode) readLeft(params runParams) (bool, error) {
if err != nil {
return false, err
}
n.left.Close(params.ctx)
n.left = nil

n.leftClosed = true
return false, nil
}

Expand Down

0 comments on commit 67af366

Please sign in to comment.