MIME-Version: 1.0
References: <17502-281a7aaacfaa872a@postgresql.org>
 <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>
 <965990.1675119213@sss.pgh.pa.us>
In-Reply-To: <965990.1675119213@sss.pgh.pa.us>
From: David Rowley <dgrowleyml@gmail.com>
Date: Wed, 1 Feb 2023 00:03:47 +1300
Message-ID: 
 <CAApHDvqZkGVd=7DzTsnsNwq-Q+HmEyMi5zrKh50s1tgpchQ_aw@mail.gmail.com>
Subject: Re: BUG #17502: View based on window functions returns wrong results
 when queried
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Richard Guo <guofenglinux@gmail.com>,
 =?UTF-8?Q?Daniel_Farka=C5=A1?= <daniel.farkas@datoris.com>,
	Magnus Hagander <magnus@hagander.net>,
	PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>
Content-Type: text/plain; charset="UTF-8"
Archived-At: 
 <https://www.postgresql.org/message-id/CAApHDvqZkGVd%3D7DzTsnsNwq-Q%2BHmEyMi5zrKh50s1tgpchQ_aw%40mail.gmail.com>
Precedence: bulk

On Tue, 31 Jan 2023 at 11:53, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> 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.

I don't really object to doing this, but I'm not convinced that
someone else won't eventually come along complaining about us changing
this. As far as I'm aware, the behaviour is not wrong today provided
you always use the column so that remove_unused_subquery_outputs()
does not get rid of it.

If we disallow this, then maybe it's worth also giving ourselves a bit
more flexibility to make it work correctly in the future should
someone come along and convince us we shouldn't have disallowed this.
Adjusting  expression_returns_set() to pass it the relevant
PlannerInfo for the Node might be a good idea. If we change our minds
in the future then we could easily then modify
expression_returns_set_walker() to have it check if the given
WindowFunc's WindowClause has SRFs so that we don't remove the unused
WindowFunc subquery columns in remove_unused_subquery_outputs(). I'd
have probably gone with this as a master-only fix, but is quite likely
you're thinking of something that I'm not.  Certainly, the consistency
of when SRFs are evaluated is pretty terrible so I understand the
desire to disallow the WindowClause SRFs so we have one less thing to
get wrong.

David