Re: Oversight in reparameterize_path_by_child leading to executor crash

On 18/10/2023 13:39, Richard Guo wrote:
>
> On Fri, Oct 13, 2023 at 6:18 PM Andrei Lepikhov
> <a(dot)lepikhov(at)postgrespro(dot)ru <mailto: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?

Agree.

> 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

Agree. I have looked at the code and figured it out - you're right. But
it seems strange: maybe I don't understand something. Why not estimate
selectivity for parameterized clauses based on leaf partition statistic,
not the parent one?

--
regards,
Andrey Lepikhov
Postgres Professional