MIME-Version: 1.0
References: <17502-281a7aaacfaa872a@postgresql.org>
 <CABUevEz7t3OWxaSSVSHOxa5KcYioOk=yRwoL3iMViyZQ8m2wAw@mail.gmail.com>
 <CAGckUK2GLF=d9J5ErEWgK5x8ECqCw4equnq3jEzrqtfJw+iHYw@mail.gmail.com>
In-Reply-To: 
 <CAGckUK2GLF=d9J5ErEWgK5x8ECqCw4equnq3jEzrqtfJw+iHYw@mail.gmail.com>
From: David Rowley <dgrowleyml@gmail.com>
Date: Mon, 30 May 2022 10:30:14 +1200
Message-ID: 
 <CAApHDvoR4BXm7oxSrYhpEAFOZ-qMPkkYnn14y6sxtPf=5w5OOw@mail.gmail.com>
Subject: Re: BUG #17502: View based on window functions returns wrong results
 when queried
To: =?UTF-8?Q?Daniel_Farka=C5=A1?= <daniel.farkas@datoris.com>
Cc: Magnus Hagander <magnus@hagander.net>,
	PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Archived-At: 
 <https://www.postgresql.org/message-id/CAApHDvoR4BXm7oxSrYhpEAFOZ-qMPkkYnn14y6sxtPf%3D5w5OOw%40mail.gmail.com>
Precedence: bulk

On Mon, 30 May 2022 at 06:58, Daniel Farka=C5=A1 <daniel.farkas@datoris.com=
> wrote:
> SELECT metric_name FROM analytics_view;
> metric_name|
> -----------+
> metric_1   |
> metric_1   |
> metric_1   |
> metric_1   |

This is certainly a bug. Thanks for reporting it.

The problem seems to be down to the fact that
remove_unused_subquery_outputs() does not check if the to-be-removed
target entry references WindowClauses which contain set-returning
functions.

We only seem to check if the target entry itself is an SRF, per:

/*
* If it contains a set-returning function, we can't remove it since
* that could change the number of rows returned by the subquery.
*/
if (subquery->hasTargetSRFs &&
expression_returns_set(texpr))
continue;

This ensures queries such as the following don't have SRF columns removed:

postgres=3D# explain verbose select a from (select
a,generate_series(1,2) as b from t) t;
                                 QUERY PLAN
---------------------------------------------------------------------------=
-
 Subquery Scan on t  (cost=3D0.00..131.13 rows=3D5100 width=3D4)
   Output: t.a
   ->  ProjectSet  (cost=3D0.00..80.13 rows=3D5100 width=3D8)
         Output: t_1.a, generate_series(1, 2)
         ->  Seq Scan on public.t t_1  (cost=3D0.00..35.50 rows=3D2550 widt=
h=3D4)
               Output: t_1.a
(6 rows)

I'm a little bit uncertain if the correct fix is to have
expression_returns_set() look deeper into WindowFuncs to check if the
WindowClause that the function belongs to has any SRFs in the
PARTITION BY / ORDER BY clause.  Unfortunately, doing that means
having to pass the PlannerInfo to expression_returns_set(). I don't
quite see how that could be made to work in the back branches.

The other fix would be to make remove_unused_subquery_outputs() pull
out all WindowFuncs from the texpr and check if any of the
WindowClauses have SRFs in the PARTITION BY / ORDER BY clause.

I'll need to look a bit deeper into the usages of
expression_returns_set() to know which of the fixes is correct.  There
might be some other bugs lurking due to expression_returns_set() not
checking WindowClauses for SRFs.

David