Re: Triggers on foreign tables

Le dimanche 23 mars 2014 02:44:26 Noah Misch a écrit :
> 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.

Thank you, I'm glad the patch found its way to the repository !

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

The idea here was to allow an FDW author to add columns which are not part of
the table definition, for example colums which are required to identify the
tuple remotely. Without system columns, a postgres_fdw user would have to
declare the ctid column on every table for a tuble to be identifiable. The
proposal would allow postgres_fdw to automatically inject an hidden (system ?)
column on every table for this ctid.

--
Ronan Dunklau
http://dalibo.com - http://dalibo.org