Re: Bogus lateral-reference-propagation logic in create_lateral_join_info

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Amit Langote <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp>
Cc: David Rowley <david(dot)rowley(at)2ndquadrant(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Bogus lateral-reference-propagation logic in create_lateral_join_info
Date: 2019-02-06 16:23:22
Message-ID: 28479.1549470202@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Amit Langote <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp> writes:
> On 2019/02/06 12:11, Tom Lane wrote:
>> I didn't run this totally to bottom yet, but what seems to be
>> happening is that inheritance_planner is creating a partition-specific
>> subplan for the DELETE, and it's allowing AppendRelInfos from the
>> parent query to propagate into the subquery even though they have
>> nothing to do with that subquery.
>>
>> We could just decide that it's okay for code dealing with the subquery
>> to ignore the irrelevant AppendRelInfos (which is basically what my
>> draft patch did), but I find that to be an uncomfortable answer: it
>> seems *way* too likely to result in code that can mask real bugs.
>> I'd be happier fixing things so that inheritance_planner doesn't
>> propagate anything into the subquery that doesn't make sense in the
>> subquery's context. But perhaps that's unreasonably hard? Not enough
>> data yet.

> The target-relation specific entries in the append_rel_list of the
> original PlannerInfo are *only* for inheritance_planner to use, so it
> seems OK to ignore them during child target planning in any way possible,
> which in your patch's case is by ignoring the AppendRelInfos whose
> parent_relid fetches a NULL base rel.

I experimented with having inheritance_planner remove AppendRelInfos that
aren't relevant to the current child query. While it's quite easy to get
rid of everything at or above the current child, as per the attached,
that's not enough to stop initsplan.c from seeing irrelevant entries:
there might still be some linking siblings of the current child to their
children, or descendants of those. So I think we'll have to put up with
using a test-for-NULL hack in create_lateral_join_info. I'll adjust the
comment to explain why we need it.

I'm posting the attached mainly because I'm wondering if we should
apply it despite it not being able to remove every irrelevant entry.
In many cases (particularly with wide partition trees) it'd greatly
reduce the length of the append_rel_list passed down to each
subquery, and maybe that'd save enough cycles to make it worth doing
just on performance grounds. I've not attempted to measure though.

regards, tom lane

Attachment Content-Type Size
trim-append-rel-list-in-inheritance-planner.patch text/x-diff 2.2 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Andres Freund 2019-02-06 16:23:41 Re: PG_RE_THROW is mandatory (was Re: jsonpath)
Previous Message Thomas Munro 2019-02-06 16:21:09 Re: Undo logs