Re: postgres_fdw: Oddity in pushing down inherited UPDATE/DELETE joins to remote servers

From: Etsuro Fujita <fujita(dot)etsuro(at)lab(dot)ntt(dot)co(dot)jp>
To: Amit Langote <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp>
Cc: Robert Haas <robertmhaas(at)gmail(dot)com>, Pg Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: postgres_fdw: Oddity in pushing down inherited UPDATE/DELETE joins to remote servers
Date: 2018-05-17 12:51:11
Message-ID: 5AFD7ABF.3060907@lab.ntt.co.jp
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

(2018/05/17 14:19), Amit Langote wrote:
> Looking at this for a bit, I wondered if this crash wouldn't have occurred
> if the "propagation" had also considered join relations in addition to
> simple relations. For example, if I changed inheritance_planner like the
> attached (not proposing that we consider committing it), reported crash
> doesn't occur. The fact that it's not currently that way means that
> somebody thought that there is no point in keeping all of those joinrels
> around until plan creation time.

One reason for that would be that we use the per-child PlannerInfo, not
the parent one, at plan creation time. Here is the code in
create_modifytable_plan:

/*
* In an inherited UPDATE/DELETE, reference the per-child modified
* subroot while creating Plans from Paths for the child rel.
This is
* a kluge, but otherwise it's too hard to ensure that Plan
creation
* functions (particularly in FDWs) don't depend on the contents of
* "root" matching what they saw at Path creation time. The main
* downside is that creation functions for Plans that might appear
* below a ModifyTable cannot expect to modify the contents of
"root"
* and have it "stick" for subsequent processing such as setrefs.c.
* That's not great, but it seems better than the alternative.
*/
subplan = create_plan_recurse(subroot, subpath, CP_EXACT_TLIST);

So, we don't need to accumulate the joinrel lists for child relations
into a single list and store that list into the parent PlannerInfo in
inheritance_planner, as in the patch you proposed. I think the change
by the commit is based on the same idea as that.

Best regards,
Etsuro Fujita

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Peter Eisentraut 2018-05-17 13:12:05 Re: lazy detoasting
Previous Message Michael Paquier 2018-05-17 12:48:54 Re: Postgres 11 release notes