From: Tom Lane <tgl@sss.pgh.pa.us>
To: Richard Guo <guofenglinux@gmail.com>
cc: PostgreSQL-development <pgsql-hackers@postgresql.org>
Subject: Re: Oversight in reparameterize_path_by_child leading to executor crash
In-reply-to: <CAMbWs496+N=UAjOc=rcD3P7B6oJe4rZw08e_TZRUsWbPxZW3Tw@mail.gmail.com>
References: <CAMbWs496+N=UAjOc=rcD3P7B6oJe4rZw08e_TZRUsWbPxZW3Tw@mail.gmail.com>
Comments: In-reply-to Richard Guo <guofenglinux@gmail.com>
	message dated "Tue, 01 Aug 2023 16:44:27 +0800"
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <3070525.1690896014.1@sss.pgh.pa.us>
Date: Tue, 01 Aug 2023 09:20:14 -0400
Message-ID: <3070526.1690896014@sss.pgh.pa.us>
Archived-At: <https://www.postgresql.org/message-id/3070526.1690896014%40sss.pgh.pa.us>
Precedence: bulk

Richard Guo <guofenglinux@gmail.com> writes:
> In this case what we need to do is to adjust the TableSampleClause to
> refer to the correct child relations.  We can do that with the help of
> adjust_appendrel_attrs_multilevel().  One problem is that the
> TableSampleClause is stored in RangeTblEntry, and it does not seem like
> a good practice to alter RangeTblEntry in this place.

Ugh.  That's why we didn't think to adjust it, obviously.  You are right
that we can't just modify the RTE on the fly, since it's shared with
other non-parameterized paths.

> So what I'm thinking is that maybe we can add a new type of path, named
> SampleScanPath, to have the TableSampleClause per path.  Then we can
> safely reparameterize the TableSampleClause as needed for each
> SampleScanPath.  That's what the attached patch does.

Alternatively, could we postpone the reparameterization until
createplan.c?  Having to build reparameterized expressions we might
not use seems expensive, and it would also contribute to the memory
bloat being complained of in nearby threads.

> There are some other plan types that do not have a separate path type
> but may have lateral references in expressions stored in RangeTblEntry,
> such as FunctionScan, TableFuncScan and ValuesScan.  But it's not clear
> to me if there are such problems with them too.

Hmm, well, not for partition-wise join anyway.

			regards, tom lane