(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