Re: Oversight in reparameterize_path_by_child leading to executor crash

On Fri, Oct 13, 2023 at 6:18 PM Andrei Lepikhov <a(dot)lepikhov(at)postgrespro(dot)ru>
wrote:

> On 23/8/2023 12:37, Richard Guo wrote:
> > To fix it we may need to modify RelOptInfos for Path, BitmapHeapPath,
> > ForeignPath and CustomPath, and modify IndexOptInfos for IndexPath. It
> > seems that that is not easily done without postponing reparameterization
> > of paths until createplan.c.
> >
> > Attached is a patch which is v5 + fix for this new issue.
>
> Having looked into the patch for a while, I couldn't answer to myself
> for some stupid questions:

Thanks for reviewing this patch! I think these are great questions.

> 1. If we postpone parameterization until the plan creation, why do we
> still copy the path node in the FLAT_COPY_PATH macros? Do we really need
> it?

Good point. The NestPath's origin inner path should not be referenced
any more after the reparameterization, so it seems safe to adjust the
path itself, without the need of a flat-copy. I've done that in v8
patch.

> 2. I see big switches on path nodes. May it be time to create a
> path_walker function? I recall some thread where such a suggestion was
> declined, but I don't remember why.

I'm not sure. But this seems a separate topic, so maybe it's better to
discuss it separately?

> 3. Clause changing is postponed. Why does it not influence the
> calc_joinrel_size_estimate? We will use statistics on the parent table
> here. Am I wrong?

Hmm, I think the reparameterization does not change the estimated
size/costs. Quoting the comment

* The cost, number of rows, width and parallel path properties depend upon
* path->parent, which does not change during the translation.

Hi Tom, I'm wondering if you've had a chance to follow this issue. What
do you think about the proposed patch?

Thanks
Richard