public inbox for [email protected]  
help / color / mirror / Atom feed
From: David Rowley <[email protected]>
To: 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 May 2022 10:30:14 +1200
Message-ID: <CAApHDvoR4BXm7oxSrYhpEAFOZ-qMPkkYnn14y6sxtPf=5w5OOw@mail.gmail.com> (raw)
In-Reply-To: <CAGckUK2GLF=d9J5ErEWgK5x8ECqCw4equnq3jEzrqtfJw+iHYw@mail.gmail.com>
References: <[email protected]>
	<CABUevEz7t3OWxaSSVSHOxa5KcYioOk=yRwoL3iMViyZQ8m2wAw@mail.gmail.com>
	<CAGckUK2GLF=d9J5ErEWgK5x8ECqCw4equnq3jEzrqtfJw+iHYw@mail.gmail.com>

On Mon, 30 May 2022 at 06:58, Daniel Farkaš <[email protected]> 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=# explain verbose select a from (select
a,generate_series(1,2) as b from t) t;
                                 QUERY PLAN
----------------------------------------------------------------------------
 Subquery Scan on t  (cost=0.00..131.13 rows=5100 width=4)
   Output: t.a
   ->  ProjectSet  (cost=0.00..80.13 rows=5100 width=8)
         Output: t_1.a, generate_series(1, 2)
         ->  Seq Scan on public.t t_1  (cost=0.00..35.50 rows=2550 width=4)
               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





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]
  Subject: Re: BUG #17502: View based on window functions returns wrong results when queried
  In-Reply-To: <CAApHDvoR4BXm7oxSrYhpEAFOZ-qMPkkYnn14y6sxtPf=5w5OOw@mail.gmail.com>

* 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