Triggers on foreign tables

Hello.

I wanted to know what it would take to implement triggers on foreign tables.
It seems that statement-level triggers can work provided they are allowed in
the code.

Please find attached a simple POC patch that implement just that.

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 ?

--
Ronan Dunklau

On Tue, Sep 10, 2013 at 5:08 AM, Ronan Dunklau <rdunklau(at)gmail(dot)com> wrote:
> Hello.
>
> I wanted to know what it would take to implement triggers on foreign tables.
> It seems that statement-level triggers can work provided they are allowed in
> the code.
>
> Please find attached a simple POC patch that implement just that.
>
> 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 ?
As your patch is targeting the implementation of a new feature, please
consider adding this patch to the next commit fest that is going to
begin in a couple of days:
https://commitfest.postgresql.org/action/commitfest_view?id=19

More general information on how patches are processed is findable here:
http://wiki.postgresql.org/wiki/Submitting_a_Patch
Regards,
--
Michael

On Wednesday 11 September 2013 06:27:24 Michael Paquier wrote:
> As your patch is targeting the implementation of a new feature, please
> consider adding this patch to the next commit fest that is going to
> begin in a couple of days:
> https://commitfest.postgresql.org/action/commitfest_view?id=19

I intended to do that in the next couple of days if I don't get any feedback.
Thank you for the reminder, its done.

On 9/11/13 10:14 AM, Ronan Dunklau wrote:
> On Wednesday 11 September 2013 06:27:24 Michael Paquier wrote:
>> As your patch is targeting the implementation of a new feature, please
>> consider adding this patch to the next commit fest that is going to
>> begin in a couple of days:
>> https://commitfest.postgresql.org/action/commitfest_view?id=19
>
> I intended to do that in the next couple of days if I don't get any feedback.
> Thank you for the reminder, its done.

The documentation build fails:

openjade:trigger.sgml:72:9:E: end tag for "ACRONYM" omitted, but OMITTAG
NO was specified
openjade:trigger.sgml:70:56: start tag was here

On Thursday 12 September 2013 12:10:01 Peter Eisentraut wrote:
> The documentation build fails:
>
> openjade:trigger.sgml:72:9:E: end tag for "ACRONYM" omitted, but OMITTAG
> NO was specified
> openjade:trigger.sgml:70:56: start tag was here

Thank you, I took the time to install a working doc-building environment.
Please find attached a patch that corrects this.

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.
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?
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.

Any comment please.
--
KaiGai Kohei <kaigai(at)kaigai(dot)gr(dot)jp>

On 10/6/13 3:33 PM, Kohei KaiGai wrote:
> 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.
> 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?
> 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.
>
> Any comment please.

What happens if someone changes the record on the foreign side between when we've read it and we do the UPDATE?
--
Jim C. Nasby, Data Architect jim(at)nasby(dot)net
512.569.9461 (cell) http://jim.nasby.net

> What happens if someone changes the record on the foreign side between when
> we've read it and we do the UPDATE?
>
Concurrency control is job of FDW driver. It has to coordinate access to
the records to be fetched for update / delete.
In fact, postgres_fdw add "FOR UPDATE" to avoid concurrent update
when it issues 1st-stage query to remote server.

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

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.

> 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. Or, perform a second round-trip to the foreign data store to fetch the
whole tuple.

> 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.

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>

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.

>
> 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.

+1

> 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).

I apologize if this is not clear, since both my technical and english
vocabulary are maybe not precise enough to get my point accross.

> > 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.

+1

> 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 ?

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

+1

Thank you for taking the time to think this through.

--
Ronan Dunklau

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>

On Mon, Oct 14, 2013 at 5:24 PM, Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)jp> wrote:
>>> And, I also want some comments from committers, not only from mine.
>>
>> +1
>>
> +1

/me pokes head up. I know I'm going to annoy people with this
comment, but I feel like it's going to have to be made at some point
by somebody, so here goes: I don't see the point of this feature. If
you want a trigger on a table, why not set it on the remote side? A
trigger on the foreign table won't be enforced consistently; it'll
only work when the update is routed through the foreign table, not
when people access the underlying table on the remote side through any
other mechanism. The number of useful things you can do this way
seems fairly small. Perhaps you could use a row-level trigger for
RLS, to allow only certain rows on the foreign side to be updated, but
even that seems like a slightly strange design: generally it'd be
better to enforce the security as close to the target object as
possible.

There's another issue that concerns me here also: performance. IIUC,
an update of N tuples on the remote side currently requires N+1 server
round-trips. That is unspeakably awful, and we really oughta be
looking for ways to make that number go down, by pushing the whole
update to the remote side. But obviously that won't be possible if
there's a per-row trigger that has to be evaluated on the local side.
Now, assuming somebody comes up with code that implements that
optimization, we can just disable it when there are local-side
triggers. But, then you're back to having terrible performance. So
even if the use case for this seemed really broad, I tend to think
performance concerns would sink most of the possible real-world uses.

I could, of course, be all wet....

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Robert,

* Robert Haas (robertmhaas(at)gmail(dot)com) wrote:
> /me pokes head up. I know I'm going to annoy people with this
> comment, but I feel like it's going to have to be made at some point

Perhaps some folks will be annoyed- I'm not annoyed, but I don't
really agree. :)

> by somebody, so here goes: I don't see the point of this feature. If
> you want a trigger on a table, why not set it on the remote side? A
> trigger on the foreign table won't be enforced consistently; it'll
> only work when the update is routed through the foreign table, not
> when people access the underlying table on the remote side through any
> other mechanism. The number of useful things you can do this way
> seems fairly small. Perhaps you could use a row-level trigger for
> RLS, to allow only certain rows on the foreign side to be updated, but
> even that seems like a slightly strange design: generally it'd be
> better to enforce the security as close to the target object as
> possible.

I can certainly see use-cases for this, a very simple one being a way to
keep track of what's been updated/inserted/whatever through this
particular foreign table (essentially, an auditing table). The *remote*
side might not be ideal for tracking that information and you might want
the info locally and remotely anyway.

> There's another issue that concerns me here also: performance. IIUC,
> an update of N tuples on the remote side currently requires N+1 server
> round-trips. That is unspeakably awful, and we really oughta be
> looking for ways to make that number go down, by pushing the whole
> update to the remote side. But obviously that won't be possible if
> there's a per-row trigger that has to be evaluated on the local side.
> Now, assuming somebody comes up with code that implements that
> optimization, we can just disable it when there are local-side
> triggers. But, then you're back to having terrible performance. So
> even if the use case for this seemed really broad, I tend to think
> performance concerns would sink most of the possible real-world uses.

Performance, while a concern, should probably be secondary when there
are valid use-cases for this where the performance wouldn't be a problem
for users.

Thanks,

Stephen

2013/10/15 Robert Haas <robertmhaas(at)gmail(dot)com>:
> On Mon, Oct 14, 2013 at 5:24 PM, Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)jp> wrote:
>>>> And, I also want some comments from committers, not only from mine.
>>>
>>> +1
>>>
>> +1
>
> /me pokes head up. I know I'm going to annoy people with this
> comment, but I feel like it's going to have to be made at some point
> by somebody, so here goes: I don't see the point of this feature. If
> you want a trigger on a table, why not set it on the remote side? A
> trigger on the foreign table won't be enforced consistently; it'll
> only work when the update is routed through the foreign table, not
> when people access the underlying table on the remote side through any
> other mechanism. The number of useful things you can do this way
> seems fairly small. Perhaps you could use a row-level trigger for
> RLS, to allow only certain rows on the foreign side to be updated, but
> even that seems like a slightly strange design: generally it'd be
> better to enforce the security as close to the target object as
> possible.
>
One reason we should support local triggers is that not all the data
source of FDW support remote trigger. It required FDW drivers to
have RDBMS as its backend, but no realistic assumption.
For example, file_fdw is unavailable to implement remote triggers.

One thing I'd like to know is, where is the goal of FDW feature.
It seems to me, FDW goes into a feature to manage external data
set as if regular tables. If it is right understanding, things we try to
support on foreign table is things we're supporting on regular tables,
such as triggers.

> There's another issue that concerns me here also: performance. IIUC,
> an update of N tuples on the remote side currently requires N+1 server
> round-trips. That is unspeakably awful, and we really oughta be
> looking for ways to make that number go down, by pushing the whole
> update to the remote side. But obviously that won't be possible if
> there's a per-row trigger that has to be evaluated on the local side.
> Now, assuming somebody comes up with code that implements that
> optimization, we can just disable it when there are local-side
> triggers. But, then you're back to having terrible performance. So
> even if the use case for this seemed really broad, I tend to think
> performance concerns would sink most of the possible real-world uses.
>
We often have some case that we cannot apply fully optimized path
because of some reasons, like view has security-barrier, qualifier
contained volatile functions, and so on...
Trigger may be a factor to prevent fully optimized path, however,
it depends on the situation which one shall be prioritized; performance
or functionality.

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

Le mardi 15 octobre 2013 09:47:31 Robert Haas a écrit :
> On Mon, Oct 14, 2013 at 5:24 PM, Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)jp> wrote:
> >>> And, I also want some comments from committers, not only from mine.
> >>
> >> +1
> >
> > +1
>
> /me pokes head up. I know I'm going to annoy people with this
> comment, but I feel like it's going to have to be made at some point
> by somebody, so here goes: I don't see the point of this feature. If
> you want a trigger on a table, why not set it on the remote side? A
> trigger on the foreign table won't be enforced consistently; it'll
> only work when the update is routed through the foreign table, not
> when people access the underlying table on the remote side through any
> other mechanism. The number of useful things you can do this way
> seems fairly small. Perhaps you could use a row-level trigger for
> RLS, to allow only certain rows on the foreign side to be updated, but
> even that seems like a slightly strange design: generally it'd be
> better to enforce the security as close to the target object as
> possible.
>
> There's another issue that concerns me here also: performance. IIUC,
> an update of N tuples on the remote side currently requires N+1 server
> round-trips. That is unspeakably awful, and we really oughta be
> looking for ways to make that number go down, by pushing the whole
> update to the remote side. But obviously that won't be possible if
> there's a per-row trigger that has to be evaluated on the local side.
> Now, assuming somebody comes up with code that implements that
> optimization, we can just disable it when there are local-side
> triggers. But, then you're back to having terrible performance. So
> even if the use case for this seemed really broad, I tend to think
> performance concerns would sink most of the possible real-world uses.
>
> I could, of course, be all wet....

Even if the row-level trigger functionality is controversial, in terms of
provided features VS performance, the original patch I submitted enables
statement-level triggers.

Although my goal was to implement row-level triggers and I hit a wall pretty
quickly, would it make sense to have statement-level triggers without their
row counterparts ?

I also think that pushing the whole update to the remote side will not be
possible in all cases, and like Kohei wrote, it may be an acceptable loss to
not be able to push it when a trigger is present. If we want to push the whole
update, we will have to cope with local joins and function evaluations in all
cases. IMO, triggers are just a special case of these limiting factors.

> 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.

Ah, that makes sense: I was understanding tupleid as a synonym for ctid.

> >> 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.

I think that the additional target-entries should be added in core, because
that would require additional work from FDW drivers, and the code would be
duplicated amongst all FDW drivers that wish to support triggers.

> > 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.

Or, as suggested above, add a special code path for foreign tables which would
execute the trigger as soon as possible instead of queuing it.

> 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.

So, if I understand you, the target list would be built as follow:

- core builds the target list, including every attribute
- this target list is updated by the FDW to add any junk attributes it wishes
to use
- in the case of an UPDATE, core duplicates the whole list of attributes
(including the added junk attributes), as another set of junk attributes.
Maybe we could duplicate only the updated attributes.

> Unfortunately, I have no special idea to handle old/new remote tuple
> on AfterTriggerSaveEvent(), except for keeping it as copy, but it is
> straightforward.

Maybe avoid it altogether in the case of a trigger on a remote foreign table ?

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

On Tue, Oct 15, 2013 at 4:17 PM, Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)jp> wrote:
> One reason we should support local triggers is that not all the data
> source of FDW support remote trigger. It required FDW drivers to
> have RDBMS as its backend, but no realistic assumption.
> For example, file_fdw is unavailable to implement remote triggers.

True, but gosh, updates via file_fdw are gonna be so slow I can't
believe anybody'd use it for something real....

> One thing I'd like to know is, where is the goal of FDW feature.
> It seems to me, FDW goes into a feature to manage external data
> set as if regular tables. If it is right understanding, things we try to
> support on foreign table is things we're supporting on regular tables,
> such as triggers.

I generally agree with that.

> We often have some case that we cannot apply fully optimized path
> because of some reasons, like view has security-barrier, qualifier
> contained volatile functions, and so on...
> Trigger may be a factor to prevent fully optimized path, however,
> it depends on the situation which one shall be prioritized; performance
> or functionality.

Sure. I mean, I guess if there are enough people that want this, I
suppose I ought not stand in the way. It just seems like a lot of
work for a feature of very marginal utility.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

2013/10/16 Robert Haas <robertmhaas(at)gmail(dot)com>:
> On Tue, Oct 15, 2013 at 4:17 PM, Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)jp> wrote:
>> One reason we should support local triggers is that not all the data
>> source of FDW support remote trigger. It required FDW drivers to
>> have RDBMS as its backend, but no realistic assumption.
>> For example, file_fdw is unavailable to implement remote triggers.
>
> True, but gosh, updates via file_fdw are gonna be so slow I can't
> believe anybody'd use it for something real....
>
How about another example? I have implemented a column-oriented
data store with read/write capability, using FDW APIs.
The role of FDW driver here is to translate its data format between
column-oriented and row-oriented, but no trigger capability itself.

>> One thing I'd like to know is, where is the goal of FDW feature.
>> It seems to me, FDW goes into a feature to manage external data
>> set as if regular tables. If it is right understanding, things we try to
>> support on foreign table is things we're supporting on regular tables,
>> such as triggers.
>
> I generally agree with that.
>
>> We often have some case that we cannot apply fully optimized path
>> because of some reasons, like view has security-barrier, qualifier
>> contained volatile functions, and so on...
>> Trigger may be a factor to prevent fully optimized path, however,
>> it depends on the situation which one shall be prioritized; performance
>> or functionality.
>
> Sure. I mean, I guess if there are enough people that want this, I
> suppose I ought not stand in the way. It just seems like a lot of
> work for a feature of very marginal utility.
>
The reason why I'm interested in this feature is, row-level triggers on
foreign tables will become a piece to implement table partitioning
that contains multiple foreign tables.
Probably, it also connects to the effort of custom-plan node (in the
future) that enables to replace Append node by custom logic, to kick
multiple concurrent remote queries on behalf of partitioned foreign table.

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

On Wed, Oct 16, 2013 at 2:20 PM, Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)jp> wrote:
>> True, but gosh, updates via file_fdw are gonna be so slow I can't
>> believe anybody'd use it for something real....
>>
> How about another example? I have implemented a column-oriented
> data store with read/write capability, using FDW APIs.
> The role of FDW driver here is to translate its data format between
> column-oriented and row-oriented, but no trigger capability itself.

OK, that's a believable example. Call me convinced. :-)

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company