From: Tom Lane <tgl@sss.pgh.pa.us>
To: Richard Guo <guofenglinux@gmail.com>
cc: sully@msully.net,
        PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>
Subject: Re: BUG #17479: "plan should not reference subplan's variable" when
 calling `grouping` on result of subquery
In-reply-to: 
 <CAMbWs49peoSCckCr4ZtWxJ3-gTOL7kPof+492aDMocGEwGtJ4g@mail.gmail.com>
References: <17479-6260deceaf0ad304@postgresql.org>
 <CAMbWs4-AmKNFMKwpc1AsJWPUskuaJ3EAJ9-ymV=qe=JQx_e3qA@mail.gmail.com>
 <CAMbWs48m-p46J7-mWn3xZwstbo9HCq9oEom1usbKYb7EeGnTog@mail.gmail.com>
 <218738.1652310685@sss.pgh.pa.us>
 <CAMbWs49peoSCckCr4ZtWxJ3-gTOL7kPof+492aDMocGEwGtJ4g@mail.gmail.com>
Comments: In-reply-to Richard Guo <guofenglinux@gmail.com>
	message dated "Thu, 12 May 2022 15:51:01 +0800"
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <327115.1652367082.1@sss.pgh.pa.us>
Date: Thu, 12 May 2022 10:51:22 -0400
Message-ID: <327116.1652367082@sss.pgh.pa.us>
Archived-At: 
 <https://www.postgresql.org/message-id/327116.1652367082%40sss.pgh.pa.us>
Precedence: bulk

Richard Guo <guofenglinux@gmail.com> writes:
> On Thu, May 12, 2022 at 7:11 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> What became of SubPlan 1?  Maybe this is fine but it looks a little
>> shaky.  We should at least run down why that's happening and make
>> sure we're not leaving dangling pointers anywhere.

> I did some debug on this. The 'SubPlan 1' is not explained because its
> SubPlanState is not created and added to AggState->subPlan.

Yeah, concur: there are two subplans in the plan tree, so no dangling
pointers.

> Not sure if this is fine or not.

It seems cosmetic.  We could somehow force initialization of the
GroupingFunc's argument tree, but that would just be wasted cycles
in non-EXPLAIN cases, and it seems semantically wrong anyway given
that evaluation of the argument is not supposed to happen.  (Right
now that's hypothetical, but what if ExecInitExpr started to
pre-evaluate some things?)

Given the history of this thing, maybe we will find something that
depends on such initialization happening.  But without evidence
for that I'm not inclined to add code for it.

I'll run with the patch as you suggested.

			regards, tom lane