Re: delta relations in AFTER triggers

From: Kevin Grittner <kgrittn(at)ymail(dot)com>
To: Kevin Grittner <kgrittn(at)ymail(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, David Fetter <david(at)fetter(dot)org>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>
Cc: Pg Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: delta relations in AFTER triggers
Date: 2014-06-21 18:06:26
Message-ID: 1403373986.39220.YahooMailNeo@web122305.mail.ne1.yahoo.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Kevin Grittner <kgrittn(at)ymail(dot)com> wrote:
> Kevin Grittner <kgrittn(at)ymail(dot)com> wrote:
>
>> I've already said that I now think we should use the standard
>> CREATE TRIGGER syntax to specify the transition tables, and that
>> if we do that we don't need the reloption that's in the patch.
>> If you ignore the 19 lines of new code to add that reloption,
>> absolutely 100% of the code changes in the patch so far are in
>> trigger.c and trigger.h.
>
> Although nobody has actually framed their feedback as a review, I
> feel that I have enough to work with to throw the patch into
> Waiting on Author status.  Since I started with the assumption
> that I was going to be using standard syntax and got a ways into
> that before convincing myself it was a bad idea, I should have a
> new version of the patch working that way in a couple days.

Here is v2.

This implements the standard syntax for transition tables, but
leaves transition row values alone.  (They remain as OLD and NEW in
plpgsql, for example.)  I took a first pass at the documentation
changes; I hope they are fairly readable.  I didn't create new
regression tests yet, since those will be a lot more interesting
when there is a PL to use with this.  That does mean there's not a
lot to test yet.  You can create triggers with transition tables
specified, and they should show correctly in psql and behave
correctly in pg_dump.

I think the new columns in pg_trigger should be null capable (which
currently means moving them to the variable part of the record).
This seems like it is better done once plpgsql support is there and
we have regression tests, since techniques for that seem a little
fragile.

I didn't change the tuplestores to TID because it seemed to me that
it would preclude using transition relations with FDW triggers, and
it seemed bad not to support that.  Does anyone see a way around
that, or feel that it's OK to not support FDW triggers in this
regard?

Does this look good otherwise, as far as it goes?

Unless people want the tuplestores changed to hold TIDs of the
tuples rather than the tuples themselves, or there are other
problems with the generation of the tuplestores, I think this is
done as far as capturing the data and passing it to the triggers.
I don't think any of the existing execution node types quite fit
this, although there are some which are similar enough to crib most
of the code from.  Have I missed a node type that can be bent to
this purpose?

What I'm looking for in this CF is to confirm the approach for
capturing the data, and get any suggestions people want to offer on
the PL implementations to use it -- at which point I think it can
be Returned with Feedback to be finished after this CF.  Now is a
great time to tell me what I've done wrong in the work so far, or
make suggestions for the next phase.  :-)

--
Kevin Grittner
EDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Attachment Content-Type Size
after-trigger-delta-relations-v2.patch text/x-diff 43.8 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Martijn van Oosterhout 2014-06-21 18:08:57 Re: Minmax indexes
Previous Message Pavel Stehule 2014-06-21 17:22:31 review: tab completion for set search_path TO