Re: Fix inconsistencies for v12

On 2019/05/28 20:26, Amit Kapila wrote:
> On Tue, May 28, 2019 at 12:29 PM Amit Langote wrote:
>> 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?

Hmm, I can't think of any way of doing than other than manual inspection.
We are sure that no piece of core code relies on it in the ExecInitNode()
code path. Apparently FDWs may, as we've found out here. Now that I've
looked around, maybe other loadable modules may too, by way of (only?)
Custom nodes. I don't see any other way to hook into ExecInitNode(), so
maybe that's it.

So, maybe reword a bit as:

diff --git a/src/backend/executor/nodeModifyTable.c
b/src/backend/executor/nodeModifyTable.c
index a3c0e91543..95545c9472 100644
--- a/src/backend/executor/nodeModifyTable.c
+++ b/src/backend/executor/nodeModifyTable.c
@@ -2316,7 +2316,7 @@ ExecInitModifyTable(ModifyTable *node, EState
*estate, int eflags)
* verify that the proposed target relations are valid and open their
* indexes for insertion of new index entries. Note we *must* set
* estate->es_result_relation_info correctly while we initialize each
- * sub-plan; ExecContextForcesOids depends on that!
+ * sub-plan; external modules such as FDWs may depend on that.

Thanks,
Amit