(2018/05/17 1:16), Robert Haas wrote:
On Fri, May 11, 2018 at 8:46 AM, Etsuro Fujita
<fujita.ets...@lab.ntt.co.jp> wrote:
I added an assertion test to make_modifytable to match that in
create_modifytable_path. Attached is an updated version of the patch.
Committed.
Thanks you!
Was it just good luck that this ever worked at all? I mean:
- if (rti< root->simple_rel_array_size&&
- root->simple_rel_array[rti] != NULL)
+ if (rti< subroot->simple_rel_array_size&&
+ subroot->simple_rel_array[rti] != NULL)
{
- RelOptInfo *resultRel = root->simple_rel_array[rti];
+ RelOptInfo *resultRel = subroot->simple_rel_array[rti];
fdwroutine = resultRel->fdwroutine;
}
else
{
- RangeTblEntry *rte = planner_rt_fetch(rti, root);
+ RangeTblEntry *rte = planner_rt_fetch(rti, subroot);
...an RTI is only meaningful relative to a particular PlannerInfo's
range table. It can't be right to just take an RTI for one
PlannerInfo and look it up in some other PlannerInfo's range table.
I think that can be right; because inheritance_planner generates the
simple_rel_array and simple_rte_array for the parent PlannerInfo so that
it allows us to do that. This is the logic that that function creates
the simple_rel_array for the parent PlannerInfo:
/*
* 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;
}
Best regards,
Etsuro Fujita
