Skip to content

Commit

Permalink
Merge #66398
Browse files Browse the repository at this point in the history
66398: colexec: call SetLength on output batches for window functions r=DrewKimball a=DrewKimball

Previously, `bufferedWindowOp` would return batches immediately
after their output columns were entirely filled. However, this caused
the non-decreasing invariant to be violated for bytes output columns
when there were trailing nulls.

This patch modifies `bufferedWindowOp` to call `SetLength` on each
batch before it is returned. This ensures that the bytes invariant is
maintained when there are trailing nulls.

See #64793

Release note: None

Co-authored-by: Drew Kimball <[email protected]>
  • Loading branch information
craig[bot] and DrewKimball committed Jun 12, 2021
2 parents 2e3fca3 + 50dad0d commit 9335ade
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 1 deletion.
8 changes: 7 additions & 1 deletion pkg/sql/colexec/colexecwindow/buffered_window.go
Original file line number Diff line number Diff line change
Expand Up @@ -310,12 +310,18 @@ func (b *bufferedWindowOp) Next() coldata.Batch {
// first of the tuples yet to be processed was somewhere in the batch.
b.processingIdx = 0
}
// Although we didn't change the length of the batch, it is necessary to
// set the length anyway (to maintain the invariant of flat bytes).
output.SetLength(output.Length())
return output
}
if b.currentBatch.Length() > 0 {
b.windower.processBatch(b.currentBatch, b.processingIdx, b.nextPartitionIdx)
if b.nextPartitionIdx >= b.currentBatch.Length() {
// This was the last batch and it has been entirely filled.
// This was the last batch and it has been entirely filled. Although
// we didn't change the length of the batch, it is necessary to set
// the length anyway (to maintain the invariant of flat bytes).
b.currentBatch.SetLength(b.currentBatch.Length())
b.state = windowFinished
return b.currentBatch
}
Expand Down
16 changes: 16 additions & 0 deletions pkg/sql/logictest/testdata/logic_test/window
Original file line number Diff line number Diff line change
Expand Up @@ -3988,3 +3988,19 @@ ORDER BY x
88.0 7527.842222222222222222222 836.42691358024691358 940.98027777777777778 28.921046204801217626 30.675401835636607970
88.0 7527.842222222222222222222 836.42691358024691358 940.98027777777777778 28.921046204801217626 30.675401835636607970
89.0 8885.844 888.5844 987.316 29.809132828715430446 31.421584937746218024

# Regression test for #64793. The output bytes column should not have decreasing
# offsets.
statement ok
CREATE TABLE t64793 (b TEXT)

statement ok
INSERT INTO t64793 VALUES
('alpha'),
(NULL::TEXT)

query T
SELECT lag(b, 0) OVER (ORDER BY b DESC) FROM t64793 ORDER BY b
----
NULL
alpha

0 comments on commit 9335ade

Please sign in to comment.