Horton

Re: Oversight in reparameterize_path_by_child leading to executor crash

Andrei Lepikhov <a.lepikhov@postgrespro.ru>

View thread
From: Andrei Lepikhov <a.lepikhov@postgrespro.ru>
To: Richard Guo <guofenglinux@gmail.com>
Cc: Tom Lane <tgl@sss.pgh.pa.us>, Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>, PostgreSQL-development <pgsql-hackers@postgresql.org>
Date: 2023-10-18T09:41:05Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Postpone reparameterization of paths until create_plan().

  2. Apply band-aid fix for an oversight in reparameterize_path_by_child.

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

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