From 2539df54fd5e4879ae451a274d8c81b8d0ab0175 Mon Sep 17 00:00:00 2001 From: Yahor Yuzefovich Date: Tue, 30 Apr 2019 13:32:09 -0700 Subject: [PATCH] sql: fix window functions with GROUPS offset FOLLOWING Previously, boundaries of peer groups were incorrectly determined with GROUPS BETWEEN offset FOLLOWING AND UNBOUNDED FOLLOWING mode of framing due to an incorrect special case (I don't know what I was thinking when I wrote that) that didn't confirm to the contract of the methods. Release note: None --- pkg/sql/logictest/testdata/logic_test/window | 8 ++++++++ pkg/sql/sem/builtins/window_builtins.go | 2 +- pkg/sql/sem/tree/window_funcs_util.go | 15 +-------------- 3 files changed, 10 insertions(+), 15 deletions(-) diff --git a/pkg/sql/logictest/testdata/logic_test/window b/pkg/sql/logictest/testdata/logic_test/window index 3e3c0301991b..f08ae971c44e 100644 --- a/pkg/sql/logictest/testdata/logic_test/window +++ b/pkg/sql/logictest/testdata/logic_test/window @@ -3049,3 +3049,11 @@ query T SELECT string_agg('foo', s) OVER () FROM (SELECT * FROM kv LIMIT 1) ---- foo + +# Regression test for #37201. +query I +SELECT jsonb_agg(a) OVER (ORDER BY a GROUPS BETWEEN 5 FOLLOWING AND UNBOUNDED FOLLOWING) FROM x +---- +NULL +NULL +NULL diff --git a/pkg/sql/sem/builtins/window_builtins.go b/pkg/sql/sem/builtins/window_builtins.go index 14e29c73f952..d35d118adccb 100644 --- a/pkg/sql/sem/builtins/window_builtins.go +++ b/pkg/sql/sem/builtins/window_builtins.go @@ -268,7 +268,7 @@ func newFramableAggregateWindow( agg tree.AggregateFunc, aggConstructor func(*tree.EvalContext, tree.Datums) tree.AggregateFunc, ) tree.WindowFunc { return &framableAggregateWindowFunc{ - agg: &aggregateWindowFunc{agg: agg}, + agg: &aggregateWindowFunc{agg: agg, peerRes: tree.DNull}, aggConstructor: aggConstructor, } } diff --git a/pkg/sql/sem/tree/window_funcs_util.go b/pkg/sql/sem/tree/window_funcs_util.go index 14bbac3c1f7c..ce49306cd325 100644 --- a/pkg/sql/sem/tree/window_funcs_util.go +++ b/pkg/sql/sem/tree/window_funcs_util.go @@ -62,7 +62,7 @@ func (p *PeerGroupsIndicesHelper) Init(wfr *WindowFrameRun, peerGrouper PeerGrou p.peerGrouper = peerGrouper startIdxOfFirstPeerGroupWithinFrame := 0 if wfr.Frame != nil && wfr.Frame.Mode == GROUPS && wfr.Frame.Bounds.StartBound.BoundType == OffsetFollowing { - // In GROUPS mode with OFFSET_PRECEDING as a start bound, 'peerGroupOffset' + // In GROUPS mode with OFFSET_FOLLOWING as a start bound, 'peerGroupOffset' // number of peer groups needs to be processed upfront before we get to // peer groups that will be within a frame of the first row. // If start bound is of type: @@ -207,13 +207,6 @@ func (p *PeerGroupsIndicesHelper) Update(wfr *WindowFrameRun) error { // GetFirstPeerIdx returns index of the first peer within peer group of number // peerGroupNum (counting from 0). func (p *PeerGroupsIndicesHelper) GetFirstPeerIdx(peerGroupNum int) int { - if p.allPeerGroupsSkipped { - // Special case: we have skipped all peer groups in Init, so the frame is - // always empty. It happens only with frames like GROUPS 100 FOLLOWING - // which (if we have less than 100 peer groups total) behaves exactly like - // GROUPS UNBOUNDED FOLLOWING (if it were allowed). - return p.unboundedFollowing - } posInBuffer := peerGroupNum - p.headPeerGroupNum if posInBuffer < 0 || p.groups.Len() < posInBuffer { panic("peerGroupNum out of bounds") @@ -224,12 +217,6 @@ func (p *PeerGroupsIndicesHelper) GetFirstPeerIdx(peerGroupNum int) int { // GetRowCount returns the number of rows within peer group of number // peerGroupNum (counting from 0). func (p *PeerGroupsIndicesHelper) GetRowCount(peerGroupNum int) int { - if p.allPeerGroupsSkipped { - // Special case: we have skipped all peer groups in Init, so the frame is - // always empty. It happens only with frames like GROUPS 100 FOLLOWING - // if we have less than 100 peer groups total. - return 0 - } posInBuffer := peerGroupNum - p.headPeerGroupNum if posInBuffer < 0 || p.groups.Len() < posInBuffer { panic("peerGroupNum out of bounds")