Re: Triggers on foreign tables

2013/10/10 Ronan Dunklau <rdunklau(at)gmail(dot)com>:
> Le dimanche 6 octobre 2013 22:33:23 Kohei KaiGai a écrit :
>> 2013/9/10 Ronan Dunklau <rdunklau(at)gmail(dot)com>:
>> > For row-level triggers, it seems more complicated. From what I understand,
>> > OLD/NEW tuples are fetched from the heap using their ctid (except for
>> > BEFORE INSERT triggers). How could this be adapted for foreign tables ?
>>
>> It seems to me the origin of difficulty to support row-level trigger
>> is that foreign table
>> does not have a back-door to see the older tuple to be updated, unlike
>> regular tables.
>> The core-PostgreSQL knows on-disk format to store tuples within
>> regular tables, thus,
>> GetTupleForTrigger() can fetch a tuple being identified by tupleid.
>> On the other hand, writable foreign table is also designed to identify
>> a remote tuple
>> with tupleid, as long as FDW module is responsible.
>
> It is my understanding that the tupleid is one way for a FDW to identify a
> tuple, but the AddForeignUpdateTargets hook allows for arbitrary resjunks
> columns to be added. It is these columns that are then used to identify the
> tuple to update. I don't think the tupleid itself can be used to identify a
> tuple for the trigger.
>
Sorry, I'm uncertain the point above.
Are you saying FDW driver may be able to handle well a case when
a remote tuple to be updated is different from a remote tuple being
fetched on the first stage? Or, am I misunderstanding?

From another standpoint, I'd like to cancel the above my suggestion,
because after-row update or delete trigger tries to fetch an older image
of tuple next to the actual update / delete jobs.
Even if FDW is responsible to identify a remote tuple using tupleid,
the identified tuple we can fetch is the newer image because FDW
driver already issues a remote query to update or delete the target
record.
So, soon or later, FDW shall be responsible to keep a complete tuple
image when foreign table has row-level triggers, until writer operation
is finished.

>> So, a straightforward idea is adding a new FDW interface to get an
>> older image of
>> the tuple to be updated. It makes possible to implement something similar to
>> GetTupleForTrigger() on foreign tables, even though it takes a
>> secondary query to
>> fetch a record from the remote server. Probably, it is an headache for us
>
>> One thing we pay attention is, the tuple to be updated is already
>> fetched from the
>> remote server and the tuple being fetched latest is (always?) the
>> target of update
>> or delete. It is not a heavy job for ForeignScanState node to hold a
>> copy of the latest
>> tuple. Isn't it an available way to reference relevant
>> ForeignScanState to get the older
>> image?
>
> It is however possible to have only an incomplete tuple.
>
> The FDW may have only fetched the tupleid-equivalent, and we would have to
> ensure that the reltargetlist contains every attribute that the trigger could
> need.
>
Does the incomplete tuple mean a tuple image but some of columns
are replaced with NULL due to optimization, as postgres_fdw is doing,
doesn't it?
If so, a solution is to enforce FDW driver to fetch all the columns if its
managing foreign table has row level triggers for update / delete.

> Or, perform a second round-trip to the foreign data store to fetch the
> whole tuple.
>
It might be a solution for before-row trigger, but not for after-row trigger,
unfortunately.

>> If my assumption is right, all we need to enhance are (1) add a store on
>> ForeignScanState to hold the last tuple on the scan stage, (2) add
>> GetForeignTupleForTrigger() to reference the store above on the relevant
>> ForeignScanState.
>
> I would add a 3) have a GetTupleForTrigger() hook that would get called with
> the stored tuple.
>
> I personnally don't like this approach: there is already (n+1) round trips to
> the foreign store (one during the scan phase, and one per tuple during the
> update/delete phase), we don't need another one per tuple for the triggers.
>
As I noted above, 2nd round trip is not a suitable design to support after-row
trigger. Likely, GetTupleForTrigger() hook shall perform to return a cached
older image being fetched on the first round of remote scan.
So, I guess every FDW driver will have similar implementation to handle
these the situation, thus it makes sense that PostgreSQL core has
a feature to support this handling; that keeps a tuple being fetched last
and returns older image for row-level triggers.

How about your opinion?

And, I also want some comments from committers, not only from mine.
--
KaiGai Kohei <kaigai(at)kaigai(dot)gr(dot)jp>