Re: pre-commit triggers

Lists: pgsql-hackers
From: Andrew Dunstan <andrew(at)dunslane(dot)net>
To: PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: pre-commit triggers
Date: 2013-11-15 18:01:48
Message-ID: 5286618C.60301@dunslane.net
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers


Attached is a patch to provide a new event trigger that will fire on
transaction commit. I have tried to make certain that it fires at a
sufficiently early stage in the commit process that some of the evils
mentioned in previous discussions on this topic aren't relevant.

The triggers don't fire if there is no real XID, so only actual data
changes should cause the trigger to fire. They also don't fire in single
user mode, so that if you do something stupid like create a trigger that
unconditionally raises an error you have a way to recover.

This is intended to be somewhat similar to the same feature in the
Firebird database, and the initial demand came from a client migrating
from that system to Postgres.

cheers

andrew

Attachment Content-Type Size
txn-commit-triggers-1.patch text/x-patch 6.8 KB

From: Peter Eisentraut <peter_e(at)gmx(dot)net>
To: Andrew Dunstan <andrew(at)dunslane(dot)net>
Cc: PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: pre-commit triggers
Date: 2013-11-16 02:07:58
Message-ID: 1384567678.19006.8.camel@vanquo.pezone.net
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On Fri, 2013-11-15 at 13:01 -0500, Andrew Dunstan wrote:
> Attached is a patch to provide a new event trigger that will fire on
> transaction commit.

xact.c: In function ‘CommitTransaction’:
xact.c:1835:3: warning: implicit declaration of function ‘PreCommitTriggersFire’ [-Wimplicit-function-declaration]


From: Andrew Dunstan <andrew(at)dunslane(dot)net>
To: Peter Eisentraut <peter_e(at)gmx(dot)net>
Cc: PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: pre-commit triggers
Date: 2013-11-16 02:21:37
Message-ID: 5286D6B1.4070706@dunslane.net
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers


On 11/15/2013 09:07 PM, Peter Eisentraut wrote:
> On Fri, 2013-11-15 at 13:01 -0500, Andrew Dunstan wrote:
>> Attached is a patch to provide a new event trigger that will fire on
>> transaction commit.
> xact.c: In function ‘CommitTransaction’:
> xact.c:1835:3: warning: implicit declaration of function ‘PreCommitTriggersFire’ [-Wimplicit-function-declaration]
>
>
>

Oops. missed a #include. Revised patch attached.

cheers

andrew

Attachment Content-Type Size
txn-commit-triggers-2.patch text/x-patch 7.0 KB

From: Hannu Krosing <hannu(at)2ndQuadrant(dot)com>
To: Andrew Dunstan <andrew(at)dunslane(dot)net>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: pre-commit triggers
Date: 2013-11-16 20:00:57
Message-ID: 5287CEF9.3020908@2ndQuadrant.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On 11/15/2013 07:01 PM, Andrew Dunstan wrote:
>
> Attached is a patch to provide a new event trigger that will fire on
> transaction commit. I have tried to make certain that it fires at a
> sufficiently early stage in the commit process that some of the evils
> mentioned in previous discussions on this topic aren't relevant.
>
> The triggers don't fire if there is no real XID, so only actual data
> changes should cause the trigger to fire.
I have not looked at the patch, but does it also run pre-rollback ?

If not, how hard would it be to make it so ?

> They also don't fire in single user mode, so that if you do something
> stupid like create a trigger that unconditionally raises an error you
> have a way to recover.
>
> This is intended to be somewhat similar to the same feature in the
> Firebird database, and the initial demand came from a client migrating
> from that system to Postgres.
>
> cheers
>
> andrew
>
>

--
Hannu Krosing
PostgreSQL Consultant
Performance, Scalability and High Availability
2ndQuadrant Nordic OÜ


From: Andrew Dunstan <andrew(at)dunslane(dot)net>
To: Hannu Krosing <hannu(at)2ndQuadrant(dot)com>
Cc: PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: pre-commit triggers
Date: 2013-11-16 20:31:15
Message-ID: 5287D613.6060300@dunslane.net
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers


On 11/16/2013 03:00 PM, Hannu Krosing wrote:
> On 11/15/2013 07:01 PM, Andrew Dunstan wrote:
>>
>> Attached is a patch to provide a new event trigger that will fire on
>> transaction commit. I have tried to make certain that it fires at a
>> sufficiently early stage in the commit process that some of the evils
>> mentioned in previous discussions on this topic aren't relevant.
>>
>> The triggers don't fire if there is no real XID, so only actual data
>> changes should cause the trigger to fire.
> I have not looked at the patch, but does it also run pre-rollback ?
>
> If not, how hard would it be to make it so ?
>

No it doesn't.

The things you can do once a rollback has been initiated are extremely
limited, so I'm not sure value there would be in such a thing.

The requirements I was given specifically excluded this, so I haven't
looked at it, but I suspect the answer to your second question is "quite
hard". But feel free to prove me wrong :-)

cheers

andrew


From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Hannu Krosing <hannu(at)2ndQuadrant(dot)com>
Cc: Andrew Dunstan <andrew(at)dunslane(dot)net>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: pre-commit triggers
Date: 2013-11-17 00:42:58
Message-ID: 10149.1384648978@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Hannu Krosing <hannu(at)2ndQuadrant(dot)com> writes:
> I have not looked at the patch, but does it also run pre-rollback ?

error in trigger -> instant infinite loop.

Besides, exactly what would you do in such a trigger? Not modify
the database, for certain, because we're about to roll back.

regards, tom lane


From: Hannu Krosing <hannu(at)2ndQuadrant(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Andrew Dunstan <andrew(at)dunslane(dot)net>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: pre-commit triggers
Date: 2013-11-17 08:39:26
Message-ID: 528880BE.6070508@2ndQuadrant.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On 11/17/2013 01:42 AM, Tom Lane wrote:
> Hannu Krosing <hannu(at)2ndQuadrant(dot)com> writes:
>> I have not looked at the patch, but does it also run pre-rollback ?
> error in trigger -> instant infinite loop.
Means this needs to have some kind of recursion depth limit, like python

>>> def x():
... return x()
...
>>> x()
... (a few thousand messages like the following) ...
File "<stdin>", line 2, in x
RuntimeError: maximum recursion depth exceeded

>
> Besides, exactly what would you do in such a trigger?
The use case would be telling another system about the rollback.

Basically sending a "ignore what I told you to do" message

So it would send a network message, a signal or writing something to
external file.

> Not modify
> the database, for certain, because we're about to roll back.
>
> regards, tom lane

Cheers

--
Hannu Krosing
PostgreSQL Consultant
Performance, Scalability and High Availability
2ndQuadrant Nordic OÜ


From: Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>
To: Hannu Krosing <hannu(at)2ndQuadrant(dot)com>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Andrew Dunstan <andrew(at)dunslane(dot)net>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: pre-commit triggers
Date: 2013-11-17 15:20:01
Message-ID: 20131117152001.GC6220@eldon.alvh.no-ip.org
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Hannu Krosing wrote:

> So it would send a network message, a signal or writing something to
> external file.

If you're OK with a C function, you could try registering a callback,
see RegisterXactCallback().

--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services


From: Hannu Krosing <hannu(at)2ndQuadrant(dot)com>
To: Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Hannu Krosing <hannu(at)2ndQuadrant(dot)com>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Andrew Dunstan <andrew(at)dunslane(dot)net>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: pre-commit triggers
Date: 2013-11-17 16:43:26
Message-ID: 5288F22E.2080506@2ndQuadrant.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On 11/17/2013 04:20 PM, Alvaro Herrera wrote:
> Hannu Krosing wrote:
>
>> So it would send a network message, a signal or writing something to
>> external file.
> If you're OK with a C function, you could try registering a callback,
> see RegisterXactCallback().
>
I already have an implementation doing just that, thoughg having a
trigger would be perhaps clearer :)

And I suspect that calling a pl/* function after the ROLLBACK has
actually happened due to
error is a no-go anyway, so it has to be C.

Cheers

--
Hannu Krosing
PostgreSQL Consultant
Performance, Scalability and High Availability
2ndQuadrant Nordic OÜ


From: Andres Freund <andres(at)2ndquadrant(dot)com>
To: Hannu Krosing <hannu(at)2ndQuadrant(dot)com>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Andrew Dunstan <andrew(at)dunslane(dot)net>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: pre-commit triggers
Date: 2013-11-17 18:31:36
Message-ID: 20131117183136.GA19184@alap2.anarazel.de
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On 2013-11-17 09:39:26 +0100, Hannu Krosing wrote:
> > Besides, exactly what would you do in such a trigger?
> The use case would be telling another system about the rollback.
>
> Basically sending a "ignore what I told you to do" message

But you can't rely on it - if e.g. the server restarted/crashed, there
won't be any messages about it. In that light, I really don't see what
you could do with it.

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services


From: Hannu Krosing <hannu(at)2ndQuadrant(dot)com>
To: Andres Freund <andres(at)2ndquadrant(dot)com>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Andrew Dunstan <andrew(at)dunslane(dot)net>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: pre-commit triggers
Date: 2013-11-17 19:04:57
Message-ID: 52891359.40101@2ndQuadrant.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On 11/17/2013 07:31 PM, Andres Freund wrote:
> On 2013-11-17 09:39:26 +0100, Hannu Krosing wrote:
>>> Besides, exactly what would you do in such a trigger?
>> The use case would be telling another system about the rollback.
>>
>> Basically sending a "ignore what I told you to do" message
> But you can't rely on it - if e.g. the server restarted/crashed, there
> won't be any messages about it. In that light, I really don't see what
> you could do with it.
I can get the info about non-commit earlier :)

At some point I can call back into the database and see if the
transaction is still running.

Cheers

--
Hannu Krosing
PostgreSQL Consultant
Performance, Scalability and High Availability
2ndQuadrant Nordic OÜ


From: Noah Misch <noah(at)leadboat(dot)com>
To: Andrew Dunstan <andrew(at)dunslane(dot)net>
Cc: PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: pre-commit triggers
Date: 2013-11-19 05:45:29
Message-ID: 20131119054529.GA952344@tornado.leadboat.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On Fri, Nov 15, 2013 at 01:01:48PM -0500, Andrew Dunstan wrote:
> The triggers don't fire if there is no real XID, so only actual data
> changes should cause the trigger to fire.

What's the advantage of this provision? Without it, an individual trigger
could make the same check and drop out quickly. A trigger not wanting it
can't so easily work around its presence, though. Heretofore, skipping XID
assignment has been an implementation detail that improves performance without
otherwise calling user attention to itself. This provision would make the
decision to acquire an XID (where optional) affect application behavior.

Thanks,
nm

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


From: Andrew Dunstan <andrew(at)dunslane(dot)net>
To: Noah Misch <noah(at)leadboat(dot)com>
Cc: PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: pre-commit triggers
Date: 2013-11-19 13:54:49
Message-ID: 528B6DA9.8000100@dunslane.net
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers


On 11/19/2013 12:45 AM, Noah Misch wrote:
> On Fri, Nov 15, 2013 at 01:01:48PM -0500, Andrew Dunstan wrote:
>> The triggers don't fire if there is no real XID, so only actual data
>> changes should cause the trigger to fire.
> What's the advantage of this provision? Without it, an individual trigger
> could make the same check and drop out quickly. A trigger not wanting it
> can't so easily work around its presence, though. Heretofore, skipping XID
> assignment has been an implementation detail that improves performance without
> otherwise calling user attention to itself. This provision would make the
> decision to acquire an XID (where optional) affect application behavior.
>

Mainly speed. How is the trigger (especially if not written in C) going
to check the same thing?

Conventional triggers don't fire except on data changing events, so this
seemed consistent with that.

Perhaps my understanding of when XIDs are acquired is insufficient. When
exactly is it optional?

cheers

andrew


From: Noah Misch <noah(at)leadboat(dot)com>
To: Andrew Dunstan <andrew(at)dunslane(dot)net>
Cc: PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: pre-commit triggers
Date: 2013-11-19 16:28:29
Message-ID: 20131119162829.GA955931@tornado.leadboat.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On Tue, Nov 19, 2013 at 08:54:49AM -0500, Andrew Dunstan wrote:
>
> On 11/19/2013 12:45 AM, Noah Misch wrote:
> >On Fri, Nov 15, 2013 at 01:01:48PM -0500, Andrew Dunstan wrote:
> >>The triggers don't fire if there is no real XID, so only actual data
> >>changes should cause the trigger to fire.
> >What's the advantage of this provision? Without it, an individual trigger
> >could make the same check and drop out quickly. A trigger not wanting it
> >can't so easily work around its presence, though. Heretofore, skipping XID
> >assignment has been an implementation detail that improves performance without
> >otherwise calling user attention to itself. This provision would make the
> >decision to acquire an XID (where optional) affect application behavior.
> >
>
>
> Mainly speed. How is the trigger (especially if not written in C)
> going to check the same thing?

Probably through a thin C function calling GetCurrentTransactionIdIfAny(). If
using C is not an option, one could query pg_locks.

> Conventional triggers don't fire except on data changing events, so
> this seemed consistent with that.

The definitions of "data changing event" differ, though. An UPDATE that finds
no rows to change will fire statement-level triggers, but this commit trigger
would not fire.

> Perhaps my understanding of when XIDs are acquired is insufficient.
> When exactly is it optional?

The following commands force XID assignment, but I think that's an
implementation detail rather than a consequence of their identity as
data-changing events:

SELECT ... FOR <lockmode>
NOTIFY
PREPARE TRANSACTION (gets an XID even if nothing else had done so)

Also, parents of XID-bearing subtransactions always have XIDs, even if all
subtransactions that modified data have aborted. This, too, is an
implementation artifact.

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


From: Andres Freund <andres(at)2ndquadrant(dot)com>
To: Andrew Dunstan <andrew(at)dunslane(dot)net>
Cc: PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: pre-commit triggers
Date: 2013-11-19 16:42:35
Message-ID: 20131119164235.GE19293@alap2.anarazel.de
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Hi,

On 2013-11-15 13:01:48 -0500, Andrew Dunstan wrote:
> Attached is a patch to provide a new event trigger that will fire on
> transaction commit. I have tried to make certain that it fires at a
> sufficiently early stage in the commit process that some of the evils
> mentioned in previous discussions on this topic aren't relevant.
>
> The triggers don't fire if there is no real XID, so only actual data changes
> should cause the trigger to fire. They also don't fire in single user mode,
> so that if you do something stupid like create a trigger that
> unconditionally raises an error you have a way to recover.
>
> This is intended to be somewhat similar to the same feature in the Firebird
> database, and the initial demand came from a client migrating from that
> system to Postgres.

Could you explain a bit what the use case of this is and why it's not
sufficient to allow constraint triggers to work on a statement level?
"Just" that there would be multiple ones fired?

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services


From: Robert Haas <robertmhaas(at)gmail(dot)com>
To: Noah Misch <noah(at)leadboat(dot)com>
Cc: Andrew Dunstan <andrew(at)dunslane(dot)net>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: pre-commit triggers
Date: 2013-11-19 16:46:10
Message-ID: CA+Tgmobxo5xyvOagRwWp2+=K1ubbH7TAf+tP3Ra6WLJR5H-Onw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On Tue, Nov 19, 2013 at 12:45 AM, Noah Misch <noah(at)leadboat(dot)com> wrote:
> On Fri, Nov 15, 2013 at 01:01:48PM -0500, Andrew Dunstan wrote:
>> The triggers don't fire if there is no real XID, so only actual data
>> changes should cause the trigger to fire.
>
> What's the advantage of this provision? Without it, an individual trigger
> could make the same check and drop out quickly. A trigger not wanting it
> can't so easily work around its presence, though. Heretofore, skipping XID
> assignment has been an implementation detail that improves performance without
> otherwise calling user attention to itself. This provision would make the
> decision to acquire an XID (where optional) affect application behavior.

Yeah, I agree that that's an ugly wart. If we want a pre-commit
trigger that's only called for transactions that write data, we at
least need to name it appropriately.

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


From: Dimitri Fontaine <dimitri(at)2ndQuadrant(dot)fr>
To: Andrew Dunstan <andrew(at)dunslane(dot)net>
Cc: Noah Misch <noah(at)leadboat(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: pre-commit triggers
Date: 2013-11-19 17:27:33
Message-ID: m2fvqss3ne.fsf@2ndQuadrant.fr
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
> Perhaps my understanding of when XIDs are acquired is insufficient. When
> exactly is it optional?

My understanding of Noah's comment is that we would be exposing what
used to be an optimisation only implementation detail to the user, and
so we would need to properly document the current situation and would
probably be forbidden to change it in the future.

Then I guess it's back to the use cases: do we have use cases where it
would be interesting for the pre-commit trigger to only get fired when
an XID has been consumed?

I don't think so, because IIRC CREATE TEMP TABLE will consume an XID
even in an otherwise read-only transaction, and maybe the TEMP TABLE
writes will not be considered "actual writes" by the confused user.

What about specifying what notion of "data modifying" transactions
you're interested into and providing an SQL callable C function that the
trigger user might then use, or even a new WHEN clause?

Regards,
--
Dimitri Fontaine
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support


From: Josh Berkus <josh(at)agliodbs(dot)com>
To: pgsql-hackers(at)postgresql(dot)org
Subject: Re: pre-commit triggers
Date: 2013-11-19 20:45:27
Message-ID: 528BCDE7.7020909@agliodbs.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On 11/19/2013 08:42 AM, Andres Freund wrote:
> Could you explain a bit what the use case of this is and why it's not
> sufficient to allow constraint triggers to work on a statement level?
> "Just" that there would be multiple ones fired?

The main reason is to enforce arbitrary assertions which need
enforcement at the end of a transaction and not before. For example:

"each person record needs at least one record in the phone_numbers table"

or:

"no person may have more than one work and one home address which are
currently active"

You can't enforce this at the statement level because the
update/insert/deletes can happen in any order on the various tables.
The proposed patch is certainly an inefficient way to implement them
(since your checks get run regardless of which tables were touched), but
any other method would require a large and complex accounting
infrastructure to track which tables were modified and how.

This is the sort of thing the SQL committee covered in ASSERTIONS, but
of course they never specified any method for implementation.

--
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com


From: Andres Freund <andres(at)2ndquadrant(dot)com>
To: Josh Berkus <josh(at)agliodbs(dot)com>
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: Re: pre-commit triggers
Date: 2013-11-19 20:54:28
Message-ID: 20131119205428.GB29414@awork2.anarazel.de
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On 2013-11-19 12:45:27 -0800, Josh Berkus wrote:
> On 11/19/2013 08:42 AM, Andres Freund wrote:
> > Could you explain a bit what the use case of this is and why it's not
> > sufficient to allow constraint triggers to work on a statement level?
> > "Just" that there would be multiple ones fired?
>
> The main reason is to enforce arbitrary assertions which need
> enforcement at the end of a transaction and not before. For example:
> [...]
> You can't enforce this at the statement level because the
> update/insert/deletes can happen in any order on the various tables.

That's why I suggested adding statement level constraint triggers
(should be a farily small patch), which can be deferred till commit. The
problem there is that they can be triggered several times, but that can
relatively easily accounted for in user code.

I can't really say why, but commit time even triggers make me nervous...

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services


From: Andrew Dunstan <andrew(at)dunslane(dot)net>
To: Andres Freund <andres(at)2ndquadrant(dot)com>
Cc: Josh Berkus <josh(at)agliodbs(dot)com>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: pre-commit triggers
Date: 2013-11-19 21:04:12
Message-ID: 528BD24C.30303@dunslane.net
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers


On 11/19/2013 03:54 PM, Andres Freund wrote:
> On 2013-11-19 12:45:27 -0800, Josh Berkus wrote:
>> On 11/19/2013 08:42 AM, Andres Freund wrote:
>>> Could you explain a bit what the use case of this is and why it's not
>>> sufficient to allow constraint triggers to work on a statement level?
>>> "Just" that there would be multiple ones fired?
>> The main reason is to enforce arbitrary assertions which need
>> enforcement at the end of a transaction and not before. For example:
>> [...]
>> You can't enforce this at the statement level because the
>> update/insert/deletes can happen in any order on the various tables.
> That's why I suggested adding statement level constraint triggers
> (should be a farily small patch), which can be deferred till commit. The
> problem there is that they can be triggered several times, but that can
> relatively easily accounted for in user code.
>
> I can't really say why, but commit time even triggers make me nervous...

This feature is really extremely close to being a deferred constraint
trigger that is called once. The code that calls these event triggers
runs right before the code that runs the deferred triggers. That spot in
the code was chosen with some care, to try to reduce any risk from the
feature.

Putting the onus on the user to detect multiple invocations of the
trigger would make for MORE fragility, not less.

cheers

andrew


From: Andres Freund <andres(at)2ndquadrant(dot)com>
To: Andrew Dunstan <andrew(at)dunslane(dot)net>
Cc: Josh Berkus <josh(at)agliodbs(dot)com>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: pre-commit triggers
Date: 2013-11-19 21:23:02
Message-ID: 20131119212302.GC29414@awork2.anarazel.de
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On 2013-11-19 16:04:12 -0500, Andrew Dunstan wrote:
>
> On 11/19/2013 03:54 PM, Andres Freund wrote:
> >On 2013-11-19 12:45:27 -0800, Josh Berkus wrote:
> >>On 11/19/2013 08:42 AM, Andres Freund wrote:
> >>>Could you explain a bit what the use case of this is and why it's not
> >>>sufficient to allow constraint triggers to work on a statement level?
> >>>"Just" that there would be multiple ones fired?
> >>The main reason is to enforce arbitrary assertions which need
> >>enforcement at the end of a transaction and not before. For example:
> >>[...]
> >>You can't enforce this at the statement level because the
> >>update/insert/deletes can happen in any order on the various tables.
> >That's why I suggested adding statement level constraint triggers
> >(should be a farily small patch), which can be deferred till commit. The
> >problem there is that they can be triggered several times, but that can
> >relatively easily accounted for in user code.
> >
> >I can't really say why, but commit time even triggers make me nervous...

Don't get me wrong, I am not -1'ing the feature, just wondering whether
there might be better alternatives.

> This feature is really extremely close to being a deferred constraint
> trigger that is called once. The code that calls these event triggers runs
> right before the code that runs the deferred triggers. That spot in the code
> was chosen with some care, to try to reduce any risk from the feature.

Well, a) that code is battle tested b) it properly handles new events
being created during the invocation of a trigger c) it allows only
triggering when specific tables have been modified. That'd allow major
efficiency improvements in the usecase cited upthread.

I think the major advantage is that it doesn't depend on the relatively
obscure definition of "an xid has been assigned".

> Putting the onus on the user to detect multiple invocations of the trigger
> would make for MORE fragility, not less.

Yea, that's the major reason against it. Without that I'd say that's the
clear route. But maybe adding a AFTER STATEMENT ONCE (or better using an existing
keyword) is the way to go for that?

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services


From: Andrew Dunstan <andrew(at)dunslane(dot)net>
To: Andres Freund <andres(at)2ndquadrant(dot)com>
Cc: Josh Berkus <josh(at)agliodbs(dot)com>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: pre-commit triggers
Date: 2013-11-19 21:33:50
Message-ID: 528BD93E.5040005@dunslane.net
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers


On 11/19/2013 04:23 PM, Andres Freund wrote:
> On 2013-11-19 16:04:12 -0500, Andrew Dunstan wrote:
>> On 11/19/2013 03:54 PM, Andres Freund wrote:
>>> On 2013-11-19 12:45:27 -0800, Josh Berkus wrote:
>>>> On 11/19/2013 08:42 AM, Andres Freund wrote:
>>>>> Could you explain a bit what the use case of this is and why it's not
>>>>> sufficient to allow constraint triggers to work on a statement level?
>>>>> "Just" that there would be multiple ones fired?
>>>> The main reason is to enforce arbitrary assertions which need
>>>> enforcement at the end of a transaction and not before. For example:
>>>> [...]
>>>> You can't enforce this at the statement level because the
>>>> update/insert/deletes can happen in any order on the various tables.
>>> That's why I suggested adding statement level constraint triggers
>>> (should be a farily small patch), which can be deferred till commit. The
>>> problem there is that they can be triggered several times, but that can
>>> relatively easily accounted for in user code.
>>>
>>> I can't really say why, but commit time even triggers make me nervous...
> Don't get me wrong, I am not -1'ing the feature, just wondering whether
> there might be better alternatives.
>
>> This feature is really extremely close to being a deferred constraint
>> trigger that is called once. The code that calls these event triggers runs
>> right before the code that runs the deferred triggers. That spot in the code
>> was chosen with some care, to try to reduce any risk from the feature.
> Well, a) that code is battle tested b) it properly handles new events
> being created during the invocation of a trigger c) it allows only
> triggering when specific tables have been modified. That'd allow major
> efficiency improvements in the usecase cited upthread.
>
> I think the major advantage is that it doesn't depend on the relatively
> obscure definition of "an xid has been assigned".

That part or really orthogonal to the issue at hand I think. i.e. I
could, by removing about 30 characters, remove that restriction and
leave the rest intact.

>
>> Putting the onus on the user to detect multiple invocations of the trigger
>> would make for MORE fragility, not less.
> Yea, that's the major reason against it. Without that I'd say that's the
> clear route. But maybe adding a AFTER STATEMENT ONCE (or better using an existing
> keyword) is the way to go for that?

What if you need to have such a trigger on multiple tables? How many
times does it fire?

This feature would be nicely and easily defined - it will run each such
trigger once per transaction (modulo the xid issue).

Now maybe we could use an event trigger WHEN clause instead of always
applying the "xid must be real" rule. I'm not sure what that would look
like - i.e. what would be the filter variable or its possible values,
but it's possibly worth exploring.

cheers

andrew


From: Simon Riggs <simon(at)2ndQuadrant(dot)com>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: Noah Misch <noah(at)leadboat(dot)com>, Andrew Dunstan <andrew(at)dunslane(dot)net>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: pre-commit triggers
Date: 2013-11-24 14:42:48
Message-ID: CA+U5nM+fMoJnmmVpHw_5xicGcpULKLaOK3zBt2Hr_uop+NHJSQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On 19 November 2013 16:46, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
> On Tue, Nov 19, 2013 at 12:45 AM, Noah Misch <noah(at)leadboat(dot)com> wrote:
>> On Fri, Nov 15, 2013 at 01:01:48PM -0500, Andrew Dunstan wrote:
>>> The triggers don't fire if there is no real XID, so only actual data
>>> changes should cause the trigger to fire.
>>
>> What's the advantage of this provision? Without it, an individual trigger
>> could make the same check and drop out quickly. A trigger not wanting it
>> can't so easily work around its presence, though. Heretofore, skipping XID
>> assignment has been an implementation detail that improves performance without
>> otherwise calling user attention to itself. This provision would make the
>> decision to acquire an XID (where optional) affect application behavior.
>
> Yeah, I agree that that's an ugly wart. If we want a pre-commit
> trigger that's only called for transactions that write data, we at
> least need to name it appropriately.

It looks to me that this idea is horribly physical and seems likely to
be badly misused.

I don't see any way to use these that won't be quite ugly. There is no
trigger descriptor, so no way of writing a constraint sensibly, since
you'll need to make a constraint check for every commit by every user,
not just ones that touch the data you care about. And security goes
straight out the window, so these can't be used in normal application
development.

Plus we can already do this with RegisterXactCallback() as Alvaro
points out - so if its a hack we're after, then we already have it, no
patch required.

So this patch doesn't give us anything genuinely useful for
application developers, nor does it give us the thing that Josh is
looking for..
>The main reason is to enforce arbitrary assertions which need
> enforcement at the end of a transaction and not before.

I like the goal, but this is not the solution.

Josh also points out...
>This is the sort of thing the SQL committee covered in ASSERTIONS, but
> of course they never specified any method for implementation.

I think we should be thinking harder about how to implement
ASSERTIONs, possibly calling them ASSERTION TRIGGERs not pre-commit
write event triggers.

--
Simon Riggs http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services


From: Josh Berkus <josh(at)agliodbs(dot)com>
To: pgsql-hackers(at)postgresql(dot)org
Subject: Re: pre-commit triggers
Date: 2013-11-26 17:13:06
Message-ID: 5294D6A2.4020705@agliodbs.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On 11/24/2013 06:42 AM, Simon Riggs wrote:
> I think we should be thinking harder about how to implement
> ASSERTIONs, possibly calling them ASSERTION TRIGGERs not pre-commit
> write event triggers.

I don't know that anyone is working on this, though, or even plans to.

--
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com


From: Andrew Dunstan <andrew(at)dunslane(dot)net>
To: Simon Riggs <simon(at)2ndQuadrant(dot)com>
Cc: Robert Haas <robertmhaas(at)gmail(dot)com>, Noah Misch <noah(at)leadboat(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: pre-commit triggers
Date: 2013-11-26 17:37:26
Message-ID: 5294DC56.1070908@dunslane.net
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers


On 11/24/2013 09:42 AM, Simon Riggs wrote:

> It looks to me that this idea is horribly physical and seems likely to
> be badly misused.
>
> I don't see any way to use these that won't be quite ugly. There is no
> trigger descriptor, so no way of writing a constraint sensibly, since
> you'll need to make a constraint check for every commit by every user,
> not just ones that touch the data you care about. And security goes
> straight out the window, so these can't be used in normal application
> development.
>
> Plus we can already do this with RegisterXactCallback() as Alvaro
> points out - so if its a hack we're after, then we already have it, no
> patch required.

"Write a hack" is not normally advice I like to give or receive.

We're after a feature that at least one other RDBMS that we know of suports.

But leaving that aside, what are the restrictions, if any, in what can
be done in such a callback? Are we allowed to alter the database? If so,
what happens to FK constraints? Can we raise an ERROR exception?

cheers

andrew


From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Josh Berkus <josh(at)agliodbs(dot)com>
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: Re: pre-commit triggers
Date: 2013-11-26 17:45:26
Message-ID: 9974.1385487926@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Josh Berkus <josh(at)agliodbs(dot)com> writes:
> On 11/24/2013 06:42 AM, Simon Riggs wrote:
>> I think we should be thinking harder about how to implement
>> ASSERTIONs, possibly calling them ASSERTION TRIGGERs not pre-commit
>> write event triggers.

> I don't know that anyone is working on this, though, or even plans to.

Huh?
http://www.postgresql.org/message-id/1384486216.5008.17.camel@vanquo.pezone.net

It's far from committable, of course, but there is somebody working on it.

I tend to agree with the complaints that pre-commit triggers in the
proposed form would be pretty useless. You'd have to code in C to get
any useful information about what the transaction had done (and you'd
still not have much), but if you're coding in C there's already a hook
you can use for this.

regards, tom lane


From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Andrew Dunstan <andrew(at)dunslane(dot)net>
Cc: Simon Riggs <simon(at)2ndQuadrant(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, Noah Misch <noah(at)leadboat(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: pre-commit triggers
Date: 2013-11-26 18:04:30
Message-ID: 10386.1385489070@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
> "Write a hack" is not normally advice I like to give or receive.

> We're after a feature that at least one other RDBMS that we know of suports.

> But leaving that aside, what are the restrictions, if any, in what can
> be done in such a callback? Are we allowed to alter the database? If so,
> what happens to FK constraints? Can we raise an ERROR exception?

An XACT_EVENT_PRE_COMMIT action is fairly unconstrained, though if you're
planning to do something that might break FKs, you should do
AfterTriggerFireDeferred() afterwards. Actually it might be smart to
repeat the whole loop that's just before
"CallXactCallbacks(XACT_EVENT_PRE_COMMIT);" in CommitTransaction.

Of course, there's a certain chicken and egg question here. If you're
planning to modify the database in a way that would cause FK triggers to
fire, then this is not exactly the last thing that happens before commit,
is it? So I think this sounds more like fuzzy thinking than a valid
requirement.

regards, tom lane


From: Josh Berkus <josh(at)agliodbs(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: Re: pre-commit triggers
Date: 2013-11-26 18:17:23
Message-ID: 5294E5B3.8060703@agliodbs.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On 11/26/2013 09:45 AM, Tom Lane wrote:
> Josh Berkus <josh(at)agliodbs(dot)com> writes:
>> On 11/24/2013 06:42 AM, Simon Riggs wrote:
>>> I think we should be thinking harder about how to implement
>>> ASSERTIONs, possibly calling them ASSERTION TRIGGERs not pre-commit
>>> write event triggers.
>
>> I don't know that anyone is working on this, though, or even plans to.
>
> Huh?
> http://www.postgresql.org/message-id/1384486216.5008.17.camel@vanquo.pezone.net

Damn, I missed that. I'll have to check that out.

--
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com


From: Andrew Dunstan <andrew(at)dunslane(dot)net>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Simon Riggs <simon(at)2ndQuadrant(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, Noah Misch <noah(at)leadboat(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: pre-commit triggers
Date: 2013-11-26 18:50:37
Message-ID: 5294ED7D.8010001@dunslane.net
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers


On 11/26/2013 01:04 PM, Tom Lane wrote:
> Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
>> "Write a hack" is not normally advice I like to give or receive.
>> We're after a feature that at least one other RDBMS that we know of suports.
>> But leaving that aside, what are the restrictions, if any, in what can
>> be done in such a callback? Are we allowed to alter the database? If so,
>> what happens to FK constraints? Can we raise an ERROR exception?
> An XACT_EVENT_PRE_COMMIT action is fairly unconstrained, though if you're
> planning to do something that might break FKs, you should do
> AfterTriggerFireDeferred() afterwards. Actually it might be smart to
> repeat the whole loop that's just before
> "CallXactCallbacks(XACT_EVENT_PRE_COMMIT);" in CommitTransaction.
>
> Of course, there's a certain chicken and egg question here. If you're
> planning to modify the database in a way that would cause FK triggers to
> fire, then this is not exactly the last thing that happens before commit,
> is it? So I think this sounds more like fuzzy thinking than a valid
> requirement.

As far as I know the client isn't proposing to alter the database at
all. I'm just trying to get a clear understanding of the limitations of
this approach.

cheers

andrew