Re: [v9.3] writable foreign tables

Hi,

2012/12/18 Ronan Dunklau <rdunklau(at)gmail(dot)com>:
> Hello.
>
> I've tried to implement this API for our Multicorn FDW, and I have a few
> questions about the API.
>
> First, I don't understand what are the requirements on the "rowid"
> pseudo-column values.
>
> In particular, this sentence from a previous mail makes it ambiguous to me:
>
>
>> At the beginning, I constructed the rowid pseudo-column using
>> INTERNALOID, but it made troubles when fetched tuples are
>> materialized prior to table modification.
>> So, the "rowid" argument of them are re-defined as "const char *".
>
> Does that mean that one should only store a cstring in the rowid
> pseudo-column ? Or can one store an arbitrary pointer ? Currently, I'm
> building a text value using cstring_to_text_with_len. Could there be a
> problem with that ?
>
All what we require on the rowid pseudo-column is it has capability to
identify a particular row on the remote side. In case of postgres_fdw,
ctid of the relevant table is sufficient for the purpose.
I don't recommend to set the rowid field an arbitrary pointer, because
scanned tuple may be materialized between scanning and modifying
foreign table, thus, the arbitrary pointer shall be dealt under the
assumption of cstring data type.

> Secondly, how does one use a returning clause ?
> I've tried to look at the postgres_fdw code, but it does not seem to handle
> that.
>
> For what its worth, knowing that the postgres_fdw is still in WIP status,
> here is a couple result of my experimentation with it:
>
> - Insert into a foreign table mapped to a table with a "before" trigger,
> using a returning clause, will return the values as they were before being
> modified by the trigger.
> - Same thing, but if the trigger prevents insertion (returning NULL), the
> "would-have-been" inserted row is still returned, although the insert
> statement reports zero rows.
>
Hmm. Do you want to see the "real" final contents of the rows being inserted,
don't you. Yep, the proposed interface does not have capability to modify
the supplied tuple on ExecForeignInsert or other methods.

Probably, it needs to adjust interface to allow FDW drivers to return
a modified HeapTuple or TupleTableSlot for RETURNING calculation.
(As trigger doing, it can return the given one as-is if no change)

Let me investigate the code around this topic.

> - Inserting into a table with default values does not work as intended,
> since missing values are replaced by a null value in the remote statement.
>
It might be a bug of my proof-of-concept portion at postgres_fdw.
The prepared INSERT statement should list up columns being actually
used only, not all ones unconditionally.

Thanks,

> What can be done to make the behaviour more consistent ?
>
> I'm very excited about this feature, thank you for making this possible.
>
> Regards,
> --
> Ronan Dunklau
>
> 2012/12/14 Albe Laurenz <laurenz(dot)albe(at)wien(dot)gv(dot)at>
>>
>> Kohei KaiGai wrote:
>> >> I came up with one more query that causes a problem:
>> [...]
>> >> This causes a deadlock, but one that is not detected;
>> >> the query just keeps hanging.
>> >>
>> >> The UPDATE in the CTE has the rows locked, so the
>> >> SELECT ... FOR UPDATE issued via the FDW connection will hang
>> >> indefinitely.
>> >>
>> >> I wonder if that's just a pathological corner case that we shouldn't
>> >> worry about. Loopback connections for FDWs themselves might not
>> >> be so rare, for example as a substitute for autonomous subtransactions.
>> >>
>> >> I guess it is not easily possible to detect such a situation or
>> >> to do something reasonable about it.
>> >
>> > It is not avoidable problem due to the nature of distributed database
>> > system,
>> > not only loopback scenario.
>> >
>> > In my personal opinion, I'd like to wait for someone implements
>> > distributed
>> > lock/transaction manager on top of the background worker framework being
>> > recently committed, to intermediate lock request.
>> > However, it will take massive amount of efforts to existing
>> > lock/transaction
>> > management layer, not only enhancement of FDW APIs. It is obviously out
>> > of scope in this patch.
>> >
>> > So, I'd like to suggest authors of FDW that support writable features to
>> > put
>> > mention about possible deadlock scenario in their documentation.
>> > At least, above writable CTE example is a situation that two different
>> > sessions
>> > concurrently update the "test" relation, thus, one shall be blocked.
>>
>> Fair enough.
>>
>> >> I tried to overhaul the documentation, see the attached patch.
>> >>
>> >> There was one thing that I was not certain of:
>> >> You say that for writable foreign tables, BeginForeignModify
>> >> and EndForeignModify *must* be implemented.
>> >> I thought that these were optional, and if you can do your work
>> > with just, say, ExecForeignDelete, you could do that.
>> >
>> > Yes, that's right. What I wrote was incorrect.
>> > If FDW driver does not require any state during modification of
>> > foreign tables, indeed, these are not mandatory handler.
>>
>> I have updated the documentation, that was all I changed in the
>> attached patches.
>>
>> > OK. I split the patch into two portion, part-1 is the APIs relevant
>> > patch, part-2 is relevant to postgres_fdw patch.
>>
>> Great.
>>
>> I'll mark the patch as "ready for committer".
>>
>> Yours,
>> Laurenz Albe
>>
>>
>> --
>> Sent via pgsql-hackers mailing list (pgsql-hackers(at)postgresql(dot)org)
>> To make changes to your subscription:
>> http://www.postgresql.org/mailpref/pgsql-hackers
>>
>

--
KaiGai Kohei <kaigai(at)kaigai(dot)gr(dot)jp>