On Tue, May 28, 2019 at 12:29 PM Amit Langote
<Langote_Amit_f8@lab.ntt.co.jp> wrote:
>
> On 2019/05/28 14:00, Alexander Lakhin wrote:
> > 28.05.2019 2:05, Amit Kapila wrote:
> >> ... If we read the comment atop ExecContextForcesOids
> >> [1] before it was removed, it seems to indicate that the
> >> initialization of es_result_relation_info for each subplan is for its
> >> usage in ExecContextForcesOids. I have run the regression tests with
> >> the attached patch (which removes changing es_result_relation_info in
> >> ExecInitModifyTable) and all the tests passed. Do you have any
> >> thoughts on this matter?
> >
> > I got a coredump with `make installcheck-world` (on postgres_fdw test):
> > Core was generated by `postgres: law contrib_regression [local]
> > UPDATE '.
> > Program terminated with signal SIGSEGV, Segmentation fault.
> > #0 0x00007ff1410ece98 in postgresBeginDirectModify
> > (node=0x560a563fab30, eflags=0) at postgres_fdw.c:2363
> > 2363 rtindex =
> > estate->es_result_relation_info->ri_RangeTableIndex;
> > (gdb) bt
> > #0 0x00007ff1410ece98 in postgresBeginDirectModify
> > (node=0x560a563fab30, eflags=0) at postgres_fdw.c:2363
> > #1 0x0000560a55979e62 in ExecInitForeignScan
> > (node=node@entry=0x560a56254dc0, estate=estate@entry=0x560a563f9ae8,
> > eflags=eflags@entry=0) at nodeForeignscan.c:227
> > #2 0x0000560a5594e123 in ExecInitNode (node=node@entry=0x560a56254dc0,
> > estate=estate@entry=0x560a563f9ae8,
> > eflags=eflags@entry=0) at execProcnode.c:277
> > ...
> > So It seems that this is not a dead code.
>
> > ... So it seems that
> > this comment at least diverged from the initial author's intent.
> > With this in mind, I am inclined to just remove it.
>
> Seeing that the crash occurs due to postgres_fdw relying on
> es_result_relation_info being set when initializing a "direct
> modification" plan on foreign tables managed by it, we could change the
> comment to say that instead. Note that allowing "direct modification" of
> foreign tables is a core feature, so there's no postgres_fdw-specific
> behavior here; there may be other FDWs that support "direct modification"
> plans and so likewise rely on es_result_relation_info being set.
>
Can we ensure some way that only FDW's rely on it not any other part
of the code?
--
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com