From: Tom Lane <tgl@sss.pgh.pa.us>
To: David Rowley <dgrowleyml@gmail.com>
cc: Richard Guo <guofenglinux@gmail.com>, zuming.jiang@inf.ethz.ch,
        pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #18305: Unexpected error: "WindowFunc not found in subplan
 target lists" triggered by subqueries
In-reply-to: 
 <CAApHDvrF+N4tYJLPRvvHirsLo=oiv4VztB3xCLvMLWRuj86OOA@mail.gmail.com>
References: <18305-33c49b4c830b37b3@postgresql.org>
 <3738975.1705938286@sss.pgh.pa.us>
 <CAMbWs4-fxoO+DoDxNC6AcqOcaNgdVVCKJCsq33Sm3KW8Vt2rNQ@mail.gmail.com>
 <4071133.1706038452@sss.pgh.pa.us>
 <CAApHDvozLB5=2jCj=uymnTBRcNowKrSeptiFvcKVvX9=BK7ErA@mail.gmail.com>
 <CAMbWs49rajSFwxkHgQmVGxka+58kMEZe=puy7mNMnS02cMrNBg@mail.gmail.com>
 <CAApHDvrF+N4tYJLPRvvHirsLo=oiv4VztB3xCLvMLWRuj86OOA@mail.gmail.com>
Comments: In-reply-to David Rowley <dgrowleyml@gmail.com>
	message dated "Wed, 24 Apr 2024 14:05:04 +1200"
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <3941550.1714059560.1@sss.pgh.pa.us>
Date: Thu, 25 Apr 2024 11:39:20 -0400
Message-ID: <3941551.1714059560@sss.pgh.pa.us>
Archived-At: 
 <https://www.postgresql.org/message-id/3941551.1714059560%40sss.pgh.pa.us>
Precedence: bulk

David Rowley <dgrowleyml@gmail.com> writes:
> I've attached a more complete version of the patch (0002) and another
> patch which is what I'd proposed as a fix for the backbranches (0001).
> Note quite a few tests needed to be adjusted because of disabling this
> optimisation.

IIUC, 0002 is meant to be applied on top of 0001, but it reverses
quite a lot of 0001?  Please don't commit it like that, it'll just
add a lot of useless thrashing to "git blame" results.

It'd be easier to review this if you presented it as two independent
patches, one for HEAD and one for the back branches.

The fact that you had to use a cheesy "eval_const_expressions(NULL,
..." call in 0001 demonstrates that it was a mistake to not include
PlannerInfo in SupportRequestWFuncMonotonic, as every other planner-
invoked support request has.  I realize that we can't change that
in back branches, and that it's no longer immediately necessary
in HEAD either after 0002.  But let's learn from experience and
add it to the struct while we're here.

> The 0002 patch will require a cat version bump as it adds a field to
> WindowFunc.  Ideally, I'd be applying this fix to master, but I
> imagine some people might feel we should delay applying a fix like
> this until after we branch for v18. Happy to hear people's views on
> that.

catversion bumps are not a problem at this stage --- we will surely
do at least one more for assigned-OID renumbering.  I'd rather avoid
the git history thrashing implicit in treating 0002 as a follow-on
patch.

			regards, tom lane