Skip to content

Commit

Permalink
colbuilder: fix planning with no aggregate funcs in non-scalar context
Browse files Browse the repository at this point in the history
Previously, in a case when the aggregator doesn't have any aggregate
functions to evaluate, we would replace it with a special "fixed num
tuples" operator, with the number of tuples always set to 1. But that is
only correct if the aggregator is in scalar context - in the non-scalar
context we should return no tuples. The bug has been present since
forever, and I couldn't come up with a repro without disabling some of
the optimizer rules, so it seems unlikely to occur in production, thus,
there is no release note.

Release justification: bug fix.

Release note: None
  • Loading branch information
yuzefovich committed Sep 6, 2022
1 parent 87f44e9 commit 6543c8f
Showing 1 changed file with 9 additions and 3 deletions.
12 changes: 9 additions & 3 deletions pkg/sql/colexec/colbuilder/execplan.go
Original file line number Diff line number Diff line change
Expand Up @@ -811,14 +811,20 @@ func NewColOperator(
// present if HAVING clause is present, for example, with a
// query as follows: SELECT 1 FROM t HAVING true. In this case,
// we plan a special operator that outputs a batch of length 1
// without actual columns once and then zero-length batches. The
// actual "data" will be added by projections below.
// or 0 (depending on whether the aggregate is in scalar context
// or not) without actual columns once and then zero-length
// batches. The actual "data" will be added by projections
// below.
// TODO(solon): The distsql plan for this case includes a
// TableReader, so we end up creating an orphaned colBatchScan.
// We should avoid that. Ideally the optimizer would not plan a
// scan in this unusual case.
numTuples := 0
if aggSpec.IsScalar() {
numTuples = 1
}
result.Root, err = colexecutils.NewFixedNumTuplesNoInputOp(
getStreamingAllocator(ctx, args), 1 /* numTuples */, inputs[0].Root,
getStreamingAllocator(ctx, args), numTuples, inputs[0].Root,
), nil
// We make ColumnTypes non-nil so that sanity check doesn't
// panic.
Expand Down

0 comments on commit 6543c8f

Please sign in to comment.