Re: Triggers on foreign tables

From: Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)jp>
To: Ronan Dunklau <rdunklau(at)gmail(dot)com>
Cc: PG Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Triggers on foreign tables
Date: 2013-10-14 21:24:12
Message-ID: CADyhKSUGP6oJb1pybTiMOP3s5fg_yOkgUTo-7RBcLTNVaJ57Pw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

2013/10/13 Ronan Dunklau <rdunklau(at)gmail(dot)com>:
> Le samedi 12 octobre 2013 07:30:35 Kohei KaiGai a écrit :
>> 2013/10/10 Ronan Dunklau <rdunklau(at)gmail(dot)com>:
>
>> 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?
>
> I was not clear in the point above: what I meant was that, from my
> understanding, the FDW driver has no obligation to use the system-attribute
> "tupleid" to identify the remote tuple. IIRC, one of the early API proposal
> involved had the tupleid passed as an argument to the ExecForeign* hooks.
> Now, these hooks receive the whole "planslot" instead. This slot can store
> additional resjunks attributes (thanks to the "AddForeignUpdateTargets")
> which shouldn't be altered during the execution and are carried on until
> modify stage. So, these additional attributes can be used as identifiers
> instead of the tupleid.
>
> In fact, this is the approach I implemented for the multicorn fdw, and I
> believe it to be used in other FDWs as well (HadoopFDW does that, if I
> understand it correctly).
>
> So, it doesn't make sense to me to assume that the tupleid will be filled as
> valid remote identifier in the context of triggers.
>
Sorry, I might call it something like primary key, instead of 'tupleid'.
Apart from whether we can uniquely identify a particular remote record with
an attribute, what FDW needs here is "something to identify a remote record".
So, we were talking about same concept with different names.

>> 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.
>
> What I meant is that a DELETE statement, for example, does not build a scan-
> stage reltargetlist consisting of the whole set of attributes: the only
> attributes that are fetched are the ones built by addForeignUpdateTargets, and
> whatever additional attributes are involved in the DELETE statement (in the
> WHERE clause, for example).
>
DELETE statement indeed does not (need to) construct a complete tuple
image on scan stage, however, it does not prohibit FDW driver to keep the
latest record being fetched from remote server.
FDW driver easily knows whether relation has row-level trigger preliminary,
so here is no matter to keep a complete image internally if needed.
Or, it is perhaps available to add additional target-entries that is
needed to construct a complete tuple using AddForeignUpdateTargets()
callback.

>> 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?
>
> I like this idea, but I don't really see all the implications of such a
> design.
> Where would this tuple be stored ?
> From what I understand, after-triggers are queued for later execution, using
> the old/new tupleid for later retrieval (in the AfterTriggerEventData struct).
> This would need a major change to work with foreign tables. Would that involve
> a special path for executing those triggers ASAP ?
>
Ops, I oversight here. AfterTriggerSaveEvent() indeed saves only ctid of
tuples in regular tables, because it can re-construct a complete tuple
image from a particular ctid any time.
On the other hand, it is not available on foreign table due to its nature.
All we can do seems to me, probably, to save copy of before/after tuple
image on local memory, even though it consumes much memory than
regular tables.

The feature we need here might become a bit clear little by little.
A complete tuple image shall be fetched on the scan stage, if foreign
table has row-level trigger. The planSlot may make sense if it contains
(junk) attributes that is sufficient to re-construct an old tuple image.
Target-list of DELETE statement contains a junk ctid attribute. Here
is no reason why we cannot add junk attributes that reference each
regular attribute, isn't it? Also, target-list of UPDATE statement
contains new tuple image, however, it is available to add junk attributes
that just reference old value, as a carrier of old values from scan stage
to modify stage.
I want core PostgreSQL to support a part of these jobs. For example,
it may be able to add junk attribute to re-construct old tuple image on
rewriteTargetListUD(), if target relation has row-level triggers. Also, it
may be able to extract these old values from planSlot to construct
an old tuple image on GetTupleForTrigger(), if relation is foreign table.
Unfortunately, I have no special idea to handle old/new remote tuple
on AfterTriggerSaveEvent(), except for keeping it as copy, but it is
straightforward.

>> And, I also want some comments from committers, not only from mine.
>
> +1
>
+1

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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Josh Kupershmidt 2013-10-14 21:38:22 Re: Re: [BUGS] BUG #7873: pg_restore --clean tries to drop tables that don't exist
Previous Message Andres Freund 2013-10-14 21:07:50 Re: logical changeset generation v6.2