Subject: Re: Oversight in reparameterize_path_by_child leading to executor crash

From: Tom Lane <[email-masked]>
Date: Tue, 01 Aug 2023 09:20:14 -0400

Richard Guo <[email-masked]> 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

---

From: Richard Guo <[email-masked]>
Date: Wed, 2 Aug 2023 19:02:38 +0800

> bloat being complained of in nearby threads.

I did think about this option but figured out that it seems beyond the
scope of just fixing SampleScan.  But if we want to optimize the
reparameterization mechanism besides fixing this crash, I think this
option is much better.  I drafted a patch as attached.

Thanks
Richard