Re: Trigger violates foreign key constraint
Aleksander Alekseev writes: >> Perhaps we should leave the system triggers out of the discussion >> entirely? More or less like: >> >> If a foreign key constraint specifies referential actions (that >> is, cascading updates or deletes), those actions are performed via >> ordinary SQL update or delete commands on the referencing table. >> In particular, any triggers that exist on the referencing table >> will be fired for those changes. If such a trigger modifies or >> blocks the effect of one of these commands, the end result could >> be to break referential integrity. It is the trigger programmer's >> responsibility to avoid that. > That's perfect! Hearing no further comments, done like that. regards, tom lane
Re: Trigger violates foreign key constraint
Hi, > Perhaps we should leave the system triggers out of the discussion > entirely? More or less like: > > If a foreign key constraint specifies referential actions (that > is, cascading updates or deletes), those actions are performed via > ordinary SQL update or delete commands on the referencing table. > In particular, any triggers that exist on the referencing table > will be fired for those changes. If such a trigger modifies or > blocks the effect of one of these commands, the end result could > be to break referential integrity. It is the trigger programmer's > responsibility to avoid that. That's perfect! -- Best regards, Aleksander Alekseev
Re: Trigger violates foreign key constraint
Aleksander Alekseev writes: >> I agree with documenting this hazard, but I think it'd be better >> to do so in the "Triggers" chapter. There is no hazard unless >> you are writing user-defined triggers, which is surely far fewer >> people than use foreign keys. So I suggest something like the >> attached. > I don't mind changing the chapter, but I prefer the wording chosen in > v3. The explanation in v4 is somewhat hard to follow IMO. Well, I didn't like v3 because I thought it was wrong, or at least fairly misleading. The fact that we use triggers rather than some other mechanism to invoke referential updates isn't really relevant. The issue is that the update actions fire user-defined triggers, and it's those that are the (potential) problem. Perhaps we should leave the system triggers out of the discussion entirely? More or less like: If a foreign key constraint specifies referential actions (that is, cascading updates or deletes), those actions are performed via ordinary SQL update or delete commands on the referencing table. In particular, any triggers that exist on the referencing table will be fired for those changes. If such a trigger modifies or blocks the effect of one of these commands, the end result could be to break referential integrity. It is the trigger programmer's responsibility to avoid that. regards, tom lane
Re: Trigger violates foreign key constraint
Hi, > Laurenz Albe writes: > > Patch v3 is attached. > > I agree with documenting this hazard, but I think it'd be better > to do so in the "Triggers" chapter. There is no hazard unless > you are writing user-defined triggers, which is surely far fewer > people than use foreign keys. So I suggest something like the > attached. I don't mind changing the chapter, but I prefer the wording chosen in v3. The explanation in v4 is somewhat hard to follow IMO. -- Best regards, Aleksander Alekseev
Re: Trigger violates foreign key constraint
Laurenz Albe writes: > Patch v3 is attached. I agree with documenting this hazard, but I think it'd be better to do so in the "Triggers" chapter. There is no hazard unless you are writing user-defined triggers, which is surely far fewer people than use foreign keys. So I suggest something like the attached. regards, tom lane diff --git a/doc/src/sgml/trigger.sgml b/doc/src/sgml/trigger.sgml index a5390ff644..99d8b75fc5 100644 --- a/doc/src/sgml/trigger.sgml +++ b/doc/src/sgml/trigger.sgml @@ -354,6 +354,20 @@ to avoid infinite recursion in such scenarios. + +PostgreSQL implements foreign key +constraints via system-defined AFTER triggers. +If a foreign key constraint specifies referential actions (that is, +cascading updates or deletes), these triggers issue ordinary SQL +update or delete commands on the referencing table to perform the +actions. In particular, any user-defined triggers on the referencing +table will be fired. If such a trigger modifies or blocks the effect +of one of these commands, the end result could be to break referential +integrity. It is the trigger programmer's responsibility to avoid +that. (Similar problems can arise in any case where different +triggers are working at cross-purposes.) + + trigger
Re: Trigger violates foreign key constraint
On 22.12.2023 14:39, Laurenz Albe wrote: Yes, that is better - shorter and avoids passive mode. Changed. Thanks. Also I don't really like "This is not considered a bug" part, since it looks like an excuse. In a way, it is an excuse, so why not be honest about it. I still think that the "this is not a bug" style is not suitable for documentation. But I checked the documentation and found 3-4 more such places. Therefore, it's ok from me, especially since it really looks like a bug. The example you provided in your other message (cascading triggers fail if the table ovner has revoked the required permissions from herself) is not really about breaking foreign keys. You hit a surprising error, but referential integrity will be maintained. Yes, referential integrity will be maintained. But perhaps it is worth adding a section to the documentation about system triggers that are used to implement foreign keys. Now it is not mentioned anywhere, but questions and problems arise from time to time. Such a section named "System triggers" may be added as a separate chapter to Part VII. Internals or as a subsection to Chapter 38.Triggers. I thought about this after your recent excellent article [1], which has an introduction to system triggers. This does not negate the need for the patch being discussed. Patch v3 is attached. For me, it is ready for committer. 1. https://www.cybertec-postgresql.com/en/broken-foreign-keys-postgresql/ -- Pavel Luzanov Postgres Professional:https://postgrespro.com
Re: Trigger violates foreign key constraint
On Fri, 2023-12-22 at 10:59 +0300, Pavel Luzanov wrote: > Please, consider small suggestion to replace last sentence. > > - This is not considered a bug, and it is the responsibility of the user > to write triggers so that such problems are avoided. > + It is the trigger programmer's responsibility to avoid such scenarios. > > To be consistent with the sentence about recursive trigger calls: [1] > "It is the trigger programmer's responsibility to avoid infinite > recursion in such scenarios." Yes, that is better - shorter and avoids passive mode. Changed. > Also I don't really like "This is not considered a bug" part, since it > looks like an excuse. In a way, it is an excuse, so why not be honest about it. The example you provided in your other message (cascading triggers fail if the table ovner has revoked the required permissions from herself) is not really about breaking foreign keys. You hit a surprising error, but referential integrity will be maintained. Patch v3 is attached. Yours, Laurenz Albe From f47c149edd529dc7f1f39977b3d01ee501e19fab Mon Sep 17 00:00:00 2001 From: Laurenz Albe Date: Fri, 22 Dec 2023 12:33:40 +0100 Subject: [PATCH v3] Document foreign key internals Warn the user that foreign keys are implemented as triggers, and that user-defined triggers can interact with them and break referential integrity. Author: Laurenz Albe Reviewed-by: David G. Johnston, Pavel Luzanov Discussion: https://postgr.es/m/b81fe38fcc25a81be6e2e5b3fc1ff624130762fa.camel%40cybertec.at --- doc/src/sgml/ddl.sgml | 12 1 file changed, 12 insertions(+) diff --git a/doc/src/sgml/ddl.sgml b/doc/src/sgml/ddl.sgml index d2951cd754..03c5619e9e 100644 --- a/doc/src/sgml/ddl.sgml +++ b/doc/src/sgml/ddl.sgml @@ -1224,6 +1224,18 @@ CREATE TABLE posts ( syntax in the reference documentation for . + + + + Foreign key constraints are implemented as system triggers in + PostgreSQL. As such, they are subject to the + trigger firing rules described in . + In particular, user-defined triggers on the referencing table can cancel + or modify the effects of cascading deletes or updates, thereby breaking + referential integrity. This is not considered a bug, and it is the + trigger programmer's responsibility of avoid such problems. + + -- 2.43.0
Re: Trigger violates foreign key constraint
One more not documented issue with system triggers. It might be worth considering together. CREATE ROLE app_owner; CREATE TABLE t ( id int PRIMARY KEY, parent_id int REFERENCES t(id) ); ALTER TABLE t OWNER TO app_owner; -- No actions by application owner REVOKE ALL ON t FROM app_owner; INSERT INTO t VALUES (1,NULL); DELETE FROM t; ERROR: permission denied for table t CONTEXT: SQL statement "SELECT 1 FROM ONLY "public"."t" x WHERE "id" OPERATOR(pg_catalog.=) $1 FOR KEY SHARE OF x" It is not at all obvious why the superuser cannot delete the row that he just added. The reason is that system triggers are executed with the rights of the table owner, not the current role. But I can't find a description of this behavior in the documentation. -- Pavel Luzanov Postgres Professional: https://postgrespro.com
Re: Trigger violates foreign key constraint
I fully support this addition to the documentation. The legal possibility of breaking data consistency must be documented at least. Please, consider small suggestion to replace last sentence. - This is not considered a bug, and it is the responsibility of the user to write triggers so that such problems are avoided. + It is the trigger programmer's responsibility to avoid such scenarios. To be consistent with the sentence about recursive trigger calls: [1] "It is the trigger programmer's responsibility to avoid infinite recursion in such scenarios." Also I don't really like "This is not considered a bug" part, since it looks like an excuse. 1. https://www.postgresql.org/docs/devel/trigger-definition.html -- Pavel Luzanov Postgres Professional: https://postgrespro.com
Re: Trigger violates foreign key constraint
Hi, > Attached is a slightly modified version of the patch. The patch was marked as "Needs Review" so I decided to take a look. I believe it's a good patch. The text is well written, has the necessary references, it warns the user but doesn't scare him/her too much. > I couldn't make it clearer. I'd have to write an example to make it clearer, > and that would certainly be out of scope. Personally I don't think we should document particular steps to break FK constraints. In a similar situation when we documented the limitations about XID wraparound we didn't document particular steps to trigger XID wraparound. I don't recall encountering such examples in the documentation, so I don't think we should add them here, at least for consistency. All in all the patch seems to be as good as it will get. I suggest merging it. -- Best regards, Aleksander Alekseev
Re: Trigger violates foreign key constraint
Thanks for having a look at my patch!
On Mon, 2023-10-30 at 15:03 -0700, David G. Johnston wrote:
> On Mon, Oct 30, 2023 at 2:50 PM David G. Johnston
> wrote:
> > On Tue, Oct 3, 2023 at 12:52 AM Laurenz Albe
> > wrote:
> > > On Mon, 2023-10-02 at 09:49 -0400, Tom Lane wrote:
> > > > This is by design: triggers operate at a lower level than
> > > > foreign keys, so an ill-conceived trigger can break an FK constraint.
> > > > That's documented somewhere, though maybe not visibly enough.
> > >
> > > Not having found any documentation, I propose the attached caution.
> >
> > I dislike scaring the user like this without providing any context on what
> > conditions or actions are problematic.
I specifically *want* to scare^H^H^H^H^Halert the user, and I thought I
provided sufficient context and a link to a more detailed description of
how triggers behave.
What is unclear or lacking in the proposed wording?
In particular, other triggers
defined on the referencing table can cancel or modify the effects of
cascading delete or update, thereby breaking referential integrity.
> > The ON DELETE and ON UPDATE clauses of foreign keys are implemented as
> > system triggers
> > on the referenced table that invoke additional delete or update commands on
> > the
> > referencing table. The final outcome of these additional commands are not
> > checked -
> > it is the responsibility of the DBA to ensure that the user triggers on the
> > referencing table actually remove the rows they are requested to remove, or
> > update to NULL any referencing foreign key columns. In particular, before
> > row
> > triggers that return NULL will prevent the delete/update from occurring and
> > thus
> > result in a violated foreign key constraint.
I didn't plan to write a novel on the topic... and I don't think your wording is
clearer than mine. I went over my text again with the intent to add clarity,
but
apart from a few minor modifications ("other triggers" -> "user-defined
triggers")
I couldn't make it clearer. I'd have to write an example to make it clearer,
and that would certainly be out of scope.
> > Add sgml as needed, note the original patch missed adding ""
> > to PostgreSQL.
Ah, thanks for noticing! Fixed.
>
> Additionally, the existing place this is covered is here:
>
> [https://www.postgresql.org/docs/current/trigger-definition.html]
>
> We should probably add a note pointing back to the DDL chapter and that more
> concisely says.
>
> "Note: If this table also contains any foreign key constraints with on update
> or on delete clauses, then a failure to return the same row that was passed in
> for update and delete triggers is going to result in broken referential
> integrity
> for the affected row."
My patch already contains a link to this very section.
I tried to understand your sentence and had to read it several times. I don't
think that it adds clarity to my patch.
Attached is a slightly modified version of the patch.
Yours,
Laurenz Albe
From c1474f943b8fdb0f496c30cd87339e18d6e13a20 Mon Sep 17 00:00:00 2001
From: Laurenz Albe
Date: Tue, 31 Oct 2023 11:38:59 +0100
Subject: [PATCH] Document foreign key internals
Warn the user that foreign keys are implemented as triggers, and
that user-defined triggers can interact with them and break
referential integrity.
Author: Laurenz Albe
Reviewed-by: David G. Johnston
Discussion: https://postgr.es/m/b81fe38fcc25a81be6e2e5b3fc1ff624130762fa.camel%40cybertec.at
---
doc/src/sgml/ddl.sgml | 13 +
1 file changed, 13 insertions(+)
diff --git a/doc/src/sgml/ddl.sgml b/doc/src/sgml/ddl.sgml
index 075ff32991..2c1c06702e 100644
--- a/doc/src/sgml/ddl.sgml
+++ b/doc/src/sgml/ddl.sgml
@@ -1224,6 +1224,19 @@ CREATE TABLE posts (
syntax in the reference documentation for
.
+
+
+
+ Foreign key constraints are implemented as system triggers in
+ PostgreSQL. As such, they are subject to the
+ trigger firing rules described in .
+ In particular, user-defined triggers on the referencing table can cancel
+ or modify the effects of cascading deletes or updates, thereby breaking
+ referential integrity. This is not considered a bug, and it is the
+ responsibility of the user to write triggers so that such problems are
+ avoided.
+
+
--
2.41.0
Re: Trigger violates foreign key constraint
On Mon, Oct 30, 2023 at 2:50 PM David G. Johnston < david.g.johns...@gmail.com> wrote: > On Tue, Oct 3, 2023 at 12:52 AM Laurenz Albe > wrote: > >> On Mon, 2023-10-02 at 09:49 -0400, Tom Lane wrote: >> > This is by design: triggers operate at a lower level than >> > foreign keys, so an ill-conceived trigger can break an FK constraint. >> > That's documented somewhere, though maybe not visibly enough. >> >> Not having found any documentation, I propose the attached caution. >> >> > I dislike scaring the user like this without providing any context on what > conditions or actions are problematic. > > The ON DELETE and ON UPDATE clauses of foreign keys are implemented as > system triggers on the referenced table that invoke additional delete or > update commands on the referencing table. The final outcome of these > additional commands are not checked - it is the responsibility of the DBA > to ensure that the user triggers on the referencing table actually remove > the rows they are requested to remove, or update to NULL any referencing > foreign key columns. In particular, before row triggers that return NULL > will prevent the delete/update from occurring and thus result in a violated > foreign key constraint. > > Add sgml as needed, note the original patch missed adding "" > to PostgreSQL. > > Additionally, the existing place this is covered is here: """ Trigger functions invoked by per-statement triggers should always return NULL. Trigger functions invoked by per-row triggers can return a table row (a value of type HeapTuple) to the calling executor, if they choose. A row-level trigger fired before an operation has the following choices: It can return NULL to skip the operation for the current row. This instructs the executor to not perform the row-level operation that invoked the trigger (the insertion, modification, or deletion of a particular table row). For row-level INSERT and UPDATE triggers only, the returned row becomes the row that will be inserted or will replace the row being updated. This allows the trigger function to modify the row being inserted or updated. A row-level BEFORE trigger that does not intend to cause either of these behaviors must be careful to return as its result the same row that was passed in (that is, the NEW row for INSERT and UPDATE triggers, the OLD row for DELETE triggers). """ We should probably add a note pointing back to the DDL chapter and that more concisely says. "Note: If this table also contains any foreign key constraints with on update or on delete clauses, then a failure to return the same row that was passed in for update and delete triggers is going to result in broken referential integrity for the affected row." I do like "broken referential integrity" from the original patch over "violated foreign key constraint" - so that needs to be substituted in for the final part of my earlier proposal if we go with its more detailed wording. My issue with "violated" is that it sounds like the system is going to catch it at the end - broken doesn't have the same implication. David J.
Re: Trigger violates foreign key constraint
On Tue, Oct 3, 2023 at 12:52 AM Laurenz Albe wrote: > On Mon, 2023-10-02 at 09:49 -0400, Tom Lane wrote: > > This is by design: triggers operate at a lower level than > > foreign keys, so an ill-conceived trigger can break an FK constraint. > > That's documented somewhere, though maybe not visibly enough. > > Not having found any documentation, I propose the attached caution. > > I dislike scaring the user like this without providing any context on what conditions or actions are problematic. The ON DELETE and ON UPDATE clauses of foreign keys are implemented as system triggers on the referenced table that invoke additional delete or update commands on the referencing table. The final outcome of these additional commands are not checked - it is the responsibility of the DBA to ensure that the user triggers on the referencing table actually remove the rows they are requested to remove, or update to NULL any referencing foreign key columns. In particular, before row triggers that return NULL will prevent the delete/update from occurring and thus result in a violated foreign key constraint. Add sgml as needed, note the original patch missed adding "" to PostgreSQL. David J.
Re: Trigger violates foreign key constraint
The following review has been posted through the commitfest application: make installcheck-world: not tested Implements feature: not tested Spec compliant: not tested Documentation:tested, passed It seems like people have been talking about this problem since 2010 (https://stackoverflow.com/questions/3961825/foreign-keys-in-postgresql-can-be-violated-by-trigger), and we finally got our act together and put it in the official document today, only 13 years later. The new status of this patch is: Ready for Committer
Re: Trigger violates foreign key constraint
On Mon, Oct 02, 2023 at 09:49:53AM -0400, Tom Lane wrote: > Laurenz Albe writes: > > CREATE FUNCTION silly() RETURNS trigger LANGUAGE plpgsql AS 'BEGIN RETURN > > NULL; END;'; > > CREATE TRIGGER silly BEFORE DELETE ON child FOR EACH ROW EXECUTE FUNCTION > > silly(); > > > The trigger function cancels the cascaded delete on "child", and we are > > left with > > a row in "child" that references no row in "parent". > > Yes. This is by design: triggers operate at a lower level than > foreign keys, so an ill-conceived trigger can break an FK constraint. > That's documented somewhere, though maybe not visibly enough. > > There are good reasons to want triggers to be able to see and > react to FK-driven updates, I agree with that, but I also think it's a bug that other triggers can invalidate the constraint, without even going out of their way to do so. Ideally, triggers would be able to react, yet when all non-superuser-defined code settles, the constraint would still hold. While UNIQUE indexes over expressions aren't that strict, at least for those you need to commit the clear malfeasance of redefining an IMMUTABLE function. On Mon, Oct 02, 2023 at 12:02:17PM +0200, Laurenz Albe wrote: > Perhaps it would be enough to run "RI_FKey_noaction_del" after > "RI_FKey_cascade_del", although that would impact the performance. Yes. A cure that doubles the number of heap fetches would be worse than the disease, but a more-optimized version of this idea could work. The FK system could use a broader optimization-oriented rewrite, to deal with the unbounded memory usage and redundant I/O. If that happens, it could be a good time to plan for closing the trigger hole.
Re: Trigger violates foreign key constraint
On Mon, 2023-10-02 at 09:49 -0400, Tom Lane wrote: > This is by design: triggers operate at a lower level than > foreign keys, so an ill-conceived trigger can break an FK constraint. > That's documented somewhere, though maybe not visibly enough. Not having found any documentation, I propose the attached caution. Yours, Laurenz Albe From b6abd7dfdf1e25515ead092489efde0d239f7053 Mon Sep 17 00:00:00 2001 From: Laurenz Albe Date: Tue, 3 Oct 2023 09:20:54 +0200 Subject: [PATCH] Document foreign key internals Warn the user that foreign keys are implemented as triggers, and that user-defined triggers can interact with them and break referential integrity. Discussion: https://postgr.es/m/b81fe38fcc25a81be6e2e5b3fc1ff624130762fa.camel%40cybertec.at --- doc/src/sgml/ddl.sgml | 12 1 file changed, 12 insertions(+) diff --git a/doc/src/sgml/ddl.sgml b/doc/src/sgml/ddl.sgml index 075ff32991..8016b9fd81 100644 --- a/doc/src/sgml/ddl.sgml +++ b/doc/src/sgml/ddl.sgml @@ -1224,6 +1224,18 @@ CREATE TABLE posts ( syntax in the reference documentation for . + + + + Foreign key constraints are implemented as system triggers in PostgreSQL. + As such, they are subject to the trigger firing rules described in + . In particular, other triggers + defined on the referencing table can cancel or modify the effects of + cascading delete or update, thereby breaking referential integrity. + This is not considered a bug, and it is the responsibility of the user + to write triggers so that such problems are avoided. + + -- 2.41.0
Re: Trigger violates foreign key constraint
On Mon, 2023-10-02 at 09:49 -0400, Tom Lane wrote: > Laurenz Albe writes: > > CREATE FUNCTION silly() RETURNS trigger LANGUAGE plpgsql AS 'BEGIN RETURN > > NULL; END;'; > > CREATE TRIGGER silly BEFORE DELETE ON child FOR EACH ROW EXECUTE FUNCTION > > silly(); > > > The trigger function cancels the cascaded delete on "child", and we are > > left with > > a row in "child" that references no row in "parent". > > Yes. This is by design: triggers operate at a lower level than > foreign keys, so an ill-conceived trigger can break an FK constraint. > That's documented somewhere, though maybe not visibly enough. > > There are good reasons to want triggers to be able to see and > react to FK-driven updates, so it's unlikely that we'd want to > revisit that design decision, even if it hadn't already stood > for decades. Thanks for the clarification. I keep learning. I didn't find anything about that in the documentation or the READMEs in the source, but perhaps I didn't look well enough. Yours, Laurenz Albe
Re: Trigger violates foreign key constraint
Laurenz Albe writes: > CREATE FUNCTION silly() RETURNS trigger LANGUAGE plpgsql AS 'BEGIN RETURN > NULL; END;'; > CREATE TRIGGER silly BEFORE DELETE ON child FOR EACH ROW EXECUTE FUNCTION > silly(); > The trigger function cancels the cascaded delete on "child", and we are left > with > a row in "child" that references no row in "parent". Yes. This is by design: triggers operate at a lower level than foreign keys, so an ill-conceived trigger can break an FK constraint. That's documented somewhere, though maybe not visibly enough. There are good reasons to want triggers to be able to see and react to FK-driven updates, so it's unlikely that we'd want to revisit that design decision, even if it hadn't already stood for decades. regards, tom lane
Re: Trigger violates foreign key constraint
Perhaps it would be enough to run "RI_FKey_noaction_del" after "RI_FKey_cascade_del", although that would impact the performance. Yours, Laurenz Albe
