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 
> <david.g.johns...@gmail.com> wrote:
> > On Tue, Oct 3, 2023 at 12:52 AM Laurenz Albe <laurenz.a...@cybertec.at> 
> > 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 "<productname>" 
> > 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 <laurenz.a...@cybertec.at>
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
     <xref linkend="sql-createtable"/>.
    </para>
+
+   <note>
+    <para>
+     Foreign key constraints are implemented as system triggers in
+     <productname>PostgreSQL</productname>.  As such, they are subject to the
+     trigger firing rules described in <xref linkend="trigger-definition"/>.
+     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.
+    </para>
+   </note>
   </sect2>
 
   <sect2 id="ddl-constraints-exclusion">
-- 
2.41.0

Reply via email to