Re: Triggers on foreign tables

On Tue, Mar 18, 2014 at 09:31:06AM +0100, Ronan Dunklau wrote:
> Le mardi 18 mars 2014 03:54:19 Kouhei Kaigai a écrit :
> > > (1) To acquire the old tuple for UPDATE/DELETE operations, the patch
> > > closely
>  parallels our handling for INSTEAD OF triggers on views.  It
> > > adds a wholerow resjunk attribute, from which it constructs a HeapTuple
> > > before calling a trigger function.  This loses the system columns, an
> > > irrelevant
> > > consideration for views but meaningful for foreign tables.  postgres_fdw
> > > maintains the "ctid" system column (t_self), but triggers will always see
> > > an invalid t_self for the old tuple.  One way to fix this is to pass
> > > around
>  the old tuple data as ROW(ctid, oid, xmin, cmin, xmax, cmax,
> > > tableoid, wholerow).  That's fairly close to sufficient, but it doesn't
> > > cover t_ctid. Frankly, I would like to find a principled excuse to not
> > > worry about making foreign table system columns accessible from triggers.
> > >  Supporting system columns dramatically affects the mechanism, and what
> > > trigger is likely to care?  Unfortunately, that argument seems too weak. 
> > > Does anyone have a cleaner idea for keeping track of the system column
> > > values or a stronger argument for not bothering?
> > > 
> > 
> > Regarding to the first suggestion,
> > I think, it is better not to care about system columns on foreign tables,
> > because it fully depends on driver's implementation whether FDW fetches
> > "ctid" from its data source, or not.
> > Usually, system columns of foreign table, except for tableoid, are
> > nonsense.
>  Because of implementation reason, postgres_fdw fetches "ctid" of
> > remote tables on UPDATE / DELETE, it is not a common nature for all FDW
> > drivers. For example, we can assume an implementation that uses primary key
> > of remote table to identify the record to be updated or deleted. In this
> > case, local "ctid" does not have meaningful value.
> > So, fundamentally, we cannot guarantee FDW driver returns meaningful "ctid"
> > or other system columns.
> > 
> 
> I agree, I think it is somewhat clunky to have postgres_fdw use a feature that 
> is basically meaningless for other FDWs. Looking at some threads in this list, 
> it confused many people.

My own reasoning for accepting omission of system columns is more along the
lines of Robert's argument.  Regardless, three folks voting to do so and none
against suffices for me.  I documented the system columns limitation, made the
returningList change I mentioned, and committed the patch.

> This is off-topic, but maybe we could devise an API allowing for local "system 
> attributes" on foreign table. This would allow FDWs to carry attributes that 
> weren't declared as part of the table definition. This could then be used for 
> postgres_fdw ctid, as well as others foreign data wrappers equivalent of an 
> implicit "tuple id".

We could, but I discourage it.  System columns are a legacy feature; I doubt
we'd choose that design afresh today.  On the off-chance that you need the
value of a remote system column, you can already declare an ordinary foreign
table column for it.  I raised the issue because it's inconsistent for
RETURNING to convey system columns while tg_trigtuple/tg_newtuple do not, not
because acquiring system columns from foreign tables is notably useful.

Thanks,
nm

-- 
Noah Misch
EnterpriseDB                                 http://www.enterprisedb.com