Re: Internal error XX000 with enable_partition_pruning=on, pg 11 beta1 on Debian

On 2018/08/08 8:09, Tom Lane wrote:
> Rushabh Lathia <rushabh(dot)lathia(at)gmail(dot)com> writes:
>> Consider the below case:
>
> I initially thought the rule might be messing stuff up, but you can get
> the same result without the rule by writing out the transformed query
> by hand:
>
> regression=# explain UPDATE pt_p1 SET a = 3 from pt
> WHERE pt.a = 2 and pt.a = pt_p1.a;
> ERROR: child rel 2 not found in append_rel_array
>
> With enable_partition_pruning=off this goes through without an error.
>
> I suspect the join pruning stuff is getting confused by the overlap
> between the two partitioning trees involved in the join; although the
> fact that one of them is the target rel must be related too, because
> if you just write a SELECT for this join it's fine.
>
> I rather doubt that this case worked before 1b54e91fa ... no time
> to look closer today, though.

The code pointed out by Rushabh indeed seems to be the culprit in this case.

/*
* The prunequal is presented to us as a qual for 'parentrel'.
* Frequently this rel is the same as targetpart, so we can skip
* an adjust_appendrel_attrs step. But it might not be, and then
* we have to translate. We update the prunequal parameter here,
* because in later iterations of the loop for child partitions,
* we want to translate from parent to child variables.
*/
if (parentrel != subpart)
{
int nappinfos;
AppendRelInfo **appinfos = find_appinfos_by_relids(root,
subpart->relids, &nappinfos);

prunequal = (List *) adjust_appendrel_attrs(root, (Node *)
prunequal,
nappinfos,
appinfos);
pfree(appinfos);
}

This code is looking for the case where we have to translate prunequal's
Vars from UNION ALL parent varno to actual partitioned parent's varno, but
the detection test (if (parentrel != subpart)) turns out to be unreliable,
as shown by this report. I think the test should be if (parent->relid !=
subpart->relid). Comparing pointers as is done now is fine without
inheritance_planner being involved, but not when it *is* involved, because
of the following piece of code in it that overwrites RelOptInfos:

/*
* We need to collect all the RelOptInfos from all child plans into
* the main PlannerInfo, since setrefs.c will need them. We use the
* last child's simple_rel_array (previous ones are too short), so we
* have to propagate forward the RelOptInfos that were already built
* in previous children.
*/
Assert(subroot->simple_rel_array_size >= save_rel_array_size);
for (rti = 1; rti < save_rel_array_size; rti++)
{
RelOptInfo *brel = save_rel_array[rti];

if (brel)
subroot->simple_rel_array[rti] = brel;
}
save_rel_array_size = subroot->simple_rel_array_size;
save_rel_array = subroot->simple_rel_array;
save_append_rel_array = subroot->append_rel_array;

With this, the RelOptInfos that would've been used to create Paths that
are currently under the subroot's final rel's best path, would no longer
be accessible through that subroot, because they're overwritten by the
corresponding ones in save_rel_array. Subsequently,
create_modifytable_plan() passes the 'subroot' to create_plan_recurse()
that will recurse down to make_parttionedrel_pruneinfo() via
create_append_plan(). The 'parentrel' RelOptInfo that's fetched off of
AppendPath is no longer reachable from 'subroot', because it's been
overwritten as mentioned above. 'subpart', the RelOptInfo (of the same RT
index) fetched from 'subroot' is thus not the same as 'parentrel'. So,
the if (parentrel != subpart) test is mistakenly satisfied, leading to the
failure of finding the needed AppendRelInfo, which makes sense, because
'subpart' is not really a child of anything.

Attached is a patch which modifies the if test to compare relids instead
of RelOptInfo pointers.

Thanks,
Amit