Re: Oversight in reparameterize_path_by_child leading to executor crash

On Fri, Oct 13, 2023 at 6:18 PM Andrei Lepikhov <a.lepikhov@postgrespro.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