public inbox for [email protected]
help / color / mirror / Atom feedFrom: Tom Lane <[email protected]>
To: Richard Guo <[email protected]>
Cc: David Rowley <[email protected]>
Cc: Daniel Farkaš <[email protected]>
Cc: Magnus Hagander <[email protected]>
Cc: PostgreSQL mailing lists <[email protected]>
Subject: Re: BUG #17502: View based on window functions returns wrong results when queried
Date: Mon, 30 Jan 2023 17:53:33 -0500
Message-ID: <[email protected]> (raw)
In-Reply-To: <CAMbWs48yS=f8tj6zstiMd6yfV3U4YpssyB55PXqjcLUyUA5bww@mail.gmail.com>
References: <[email protected]>
<CABUevEz7t3OWxaSSVSHOxa5KcYioOk=yRwoL3iMViyZQ8m2wAw@mail.gmail.com>
<CAGckUK2GLF=d9J5ErEWgK5x8ECqCw4equnq3jEzrqtfJw+iHYw@mail.gmail.com>
<CAApHDvoR4BXm7oxSrYhpEAFOZ-qMPkkYnn14y6sxtPf=5w5OOw@mail.gmail.com>
<CAApHDvqKO00nNQtchBs65VTfjEMUsYiB5r2P4VUreTdXE9RY1g@mail.gmail.com>
<CAMbWs48yS=f8tj6zstiMd6yfV3U4YpssyB55PXqjcLUyUA5bww@mail.gmail.com>
Richard Guo <[email protected]> writes:
> On Mon, May 30, 2022 at 9:12 AM David Rowley <[email protected]> wrote:
>> The following is also pretty strange. Why should adding the SRF column
>> to the ORDER BY change the number of output rows?
> Is this a bug we should fix?
This bug seems to have slipped off the radar screen, but it's still
a bug. I continue to believe that the best fix is to disallow SRFs
in window definitions, and present the trivial patch to do so.
As discussed, I'm comfortable with leaving this alone in the back
branches.
regards, tom lane
Attachments:
[text/x-diff] disallow-SRFs-in-window-definitions-altogether.patch (3.7K, ../[email protected]/2-disallow-SRFs-in-window-definitions-altogether.patch)
download | inline diff:
diff --git a/src/backend/parser/parse_func.c b/src/backend/parser/parse_func.c
index ca14f06308..a13f28615b 100644
--- a/src/backend/parser/parse_func.c
+++ b/src/backend/parser/parse_func.c
@@ -2570,9 +2570,6 @@ check_srf_call_placement(ParseState *pstate, Node *last_srf, int location)
break;
case EXPR_KIND_WINDOW_PARTITION:
case EXPR_KIND_WINDOW_ORDER:
- /* okay, these are effectively GROUP BY/ORDER BY */
- pstate->p_hasTargetSRFs = true;
- break;
case EXPR_KIND_WINDOW_FRAME_RANGE:
case EXPR_KIND_WINDOW_FRAME_ROWS:
case EXPR_KIND_WINDOW_FRAME_GROUPS:
diff --git a/src/test/regress/expected/tsrf.out b/src/test/regress/expected/tsrf.out
index d47b5f6ec5..600652581e 100644
--- a/src/test/regress/expected/tsrf.out
+++ b/src/test/regress/expected/tsrf.out
@@ -265,7 +265,21 @@ ERROR: window function calls cannot contain set-returning function calls
LINE 1: SELECT min(generate_series(1, 3)) OVER() FROM few;
^
HINT: You might be able to move the set-returning function into a LATERAL FROM item.
--- SRFs are normally computed after window functions
+--- ... nor in window definitions
+SELECT sum(id) OVER (PARTITION BY generate_series(1, 3)) FROM few;
+ERROR: set-returning functions are not allowed in window definitions
+LINE 1: SELECT sum(id) OVER (PARTITION BY generate_series(1, 3)) FRO...
+ ^
+SELECT sum(id) OVER (ORDER BY generate_series(1, 3)) FROM few;
+ERROR: set-returning functions are not allowed in window definitions
+LINE 1: SELECT sum(id) OVER (ORDER BY generate_series(1, 3)) FROM fe...
+ ^
+SELECT sum(id) OVER (ROWS BETWEEN UNBOUNDED PRECEDING
+ AND generate_series(1, 3) FOLLOWING) FROM few;
+ERROR: set-returning functions are not allowed in window definitions
+LINE 2: AND generate_series(1, 3) FOLLOWING) FR...
+ ^
+-- SRFs are computed after window functions
SELECT id,lag(id) OVER(), count(*) OVER(), generate_series(1,3) FROM few;
id | lag | count | generate_series
----+-----+-------+-----------------
@@ -280,15 +294,6 @@ SELECT id,lag(id) OVER(), count(*) OVER(), generate_series(1,3) FROM few;
3 | 2 | 3 | 3
(9 rows)
--- unless referencing SRFs
-SELECT SUM(count(*)) OVER(PARTITION BY generate_series(1,3) ORDER BY generate_series(1,3)), generate_series(1,3) g FROM few GROUP BY g;
- sum | g
------+---
- 3 | 1
- 3 | 2
- 3 | 3
-(3 rows)
-
-- sorting + grouping
SELECT few.dataa, count(*), min(id), max(id), generate_series(1,3) FROM few GROUP BY few.dataa ORDER BY 5, 1;
dataa | count | min | max | generate_series
diff --git a/src/test/regress/sql/tsrf.sql b/src/test/regress/sql/tsrf.sql
index 7c22529a0d..8442ba9e74 100644
--- a/src/test/regress/sql/tsrf.sql
+++ b/src/test/regress/sql/tsrf.sql
@@ -82,10 +82,14 @@ SELECT sum((3 = ANY(SELECT lag(x) over(order by x)
-- SRFs are not allowed in window function arguments, either
SELECT min(generate_series(1, 3)) OVER() FROM few;
--- SRFs are normally computed after window functions
+--- ... nor in window definitions
+SELECT sum(id) OVER (PARTITION BY generate_series(1, 3)) FROM few;
+SELECT sum(id) OVER (ORDER BY generate_series(1, 3)) FROM few;
+SELECT sum(id) OVER (ROWS BETWEEN UNBOUNDED PRECEDING
+ AND generate_series(1, 3) FOLLOWING) FROM few;
+
+-- SRFs are computed after window functions
SELECT id,lag(id) OVER(), count(*) OVER(), generate_series(1,3) FROM few;
--- unless referencing SRFs
-SELECT SUM(count(*)) OVER(PARTITION BY generate_series(1,3) ORDER BY generate_series(1,3)), generate_series(1,3) g FROM few GROUP BY g;
-- sorting + grouping
SELECT few.dataa, count(*), min(id), max(id), generate_series(1,3) FROM few GROUP BY few.dataa ORDER BY 5, 1;
view thread (18+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: BUG #17502: View based on window functions returns wrong results when queried
In-Reply-To: <[email protected]>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox