From 50dad0d8cac934e13538b5294b57e3eb3019d97a Mon Sep 17 00:00:00 2001 From: Drew Kimball Date: Fri, 11 Jun 2021 17:14:17 -0700 Subject: [PATCH] colexec: call SetLength on output batches for window functions 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 --- pkg/sql/colexec/colexecwindow/buffered_window.go | 8 +++++++- pkg/sql/logictest/testdata/logic_test/window | 16 ++++++++++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/pkg/sql/colexec/colexecwindow/buffered_window.go b/pkg/sql/colexec/colexecwindow/buffered_window.go index 248959c921f2..67233a2723e0 100644 --- a/pkg/sql/colexec/colexecwindow/buffered_window.go +++ b/pkg/sql/colexec/colexecwindow/buffered_window.go @@ -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 } diff --git a/pkg/sql/logictest/testdata/logic_test/window b/pkg/sql/logictest/testdata/logic_test/window index 20a57586374b..d8be480e62a5 100644 --- a/pkg/sql/logictest/testdata/logic_test/window +++ b/pkg/sql/logictest/testdata/logic_test/window @@ -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