On 2019/03/06 13:30, David Rowley wrote:
> On Wed, 6 Mar 2019 at 17:20, Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> wrote:
>>
>> On 2019/03/06 12:47, David Rowley wrote:
>>> It seems a bit light on detail to me. If I was a user I'd want to know
>>> what exactly the FDW needed to support this. Does it need a special
>>> partition move function? Looking at ExecFindPartition(), this check
>>> seems to be done in CheckValidResultRel() and is basically:
>>>
>>> case RELKIND_FOREIGN_TABLE:
>>> /* Okay only if the FDW supports it */
>>> fdwroutine = resultRelInfo->ri_FdwRoutine;
>>> switch (operation)
>>> {
>>> case CMD_INSERT:
>>> if (fdwroutine->ExecForeignInsert == NULL)
>>> ereport(ERROR,
>>> (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
>>> errmsg("cannot insert into foreign table \"%s\"",
>>> RelationGetRelationName(resultRel))));
>>>
>>> Alternatively, we could just remove the mention about "if the FDW
>>> supports it", since it's probably unlikely for an FDW not to support
>>> INSERT.
>>
>> AFAIK, there's no special support in FDWs for "tuple moving" as such. The
>> "if the FDW supports it" refers to the FDW's ability to handle tuple
>> routing. Note that moving/re-routing involves calling
>> ExecPrepareTupleRouting followed by ExecInsert on the new tupls after the
>> old tuple is deleted. If an FDW doesn't support tuple routing, then a
>> tuple cannot be moved into it. That's what that text is talking about.
>>
>> Maybe, we should reword it as "if the FDW supports tuple routing", so that
>> a reader doesn't go looking around for "tuple moving support" in FDWs.
>
> I think you missed my point. If there's no special support for "tuple
> moving", as you say, then what help is it to tell the user "if the FDW
> supports tuple routing"? The answer is, it's not any help. How would
> the user check such a fact?
Hmm, maybe getting the following error, like one would get in PG 10 when
using postgres_fdw-managed partitions:
ERROR: cannot route inserted tuples to a foreign table
Getting the above error is perhaps not the best way for a user to learn of
this fact, but maybe we (and hopefully other FDW authors) mention this in
the documentation?
> As far as I can tell, this is just the requirements as defined in
> CheckValidResultRel() for CMD_INSERT. Fragments of which I pasted
> above.
Only supporting INSERT doesn't suffice though. An FDW which intends to
support tuple routing and hence 1-way tuple moving needs to updated like
postgres_fdw was in PG 11.
Thanks,
Amit