From: Antonin Houska <ah@cybertec.at>
To: Corey Huinker <corey.huinker@gmail.com>
cc: emre@hasegeli.com, Adam Brusselback <adambrusselback@gmail.com>,
    Pavel Stehule <pavel.stehule@gmail.com>,
    Alvaro Herrera <alvherre@2ndquadrant.com>,
    PostgreSQL Hackers <pgsql-hackers@postgresql.org>
Subject: Re: Referential Integrity Checks with Statement-level Triggers
In-reply-to: 
 <CADkLM=diwD7cQW-kvuEEVLKW-P7EN0gqDYBdUm2vGR3zd+zz-A@mail.gmail.com>
References: 
 <CAFj8pRCQe9=tX5GmC-S_Dmms9tdU4OWGD2xaHZFm-yaMS1zmoQ@mail.gmail.com>
 <20181217172729.mjfkflaelii2boaj@alvherre.pgsql>
 <CAFj8pRBKcgMb1onbDz7abPL_aK6A6uUfbS5hpaQ=Og9nAskU7Q@mail.gmail.com>
 <CAMjNa7fPEUonQJDE2bY4KvCR+Ai7qUTT1XdUhgHYXJ8DMpx+tg@mail.gmail.com>
 <CAE2gYzxGYagVfHvG9bMAiz7vjGK7GQwVaY0YLddPditD1o6kcQ@mail.gmail.com>
 <CADkLM=diwD7cQW-kvuEEVLKW-P7EN0gqDYBdUm2vGR3zd+zz-A@mail.gmail.com>
Comments: In-reply-to Corey Huinker <corey.huinker@gmail.com>
   message dated "Wed, 23 Jan 2019 12:16:14 -0500."
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <17099.1550662686.1@localhost>
Content-Transfer-Encoding: quoted-printable
Date: Wed, 20 Feb 2019 12:38:06 +0100
Message-ID: <17100.1550662686@localhost>
Precedence: bulk

Corey Huinker <corey.huinker@gmail.com> wrote:

> Attached is a patch that refactors DELETE triggers to fire at the statem=
ent level.
> =

> I chose delete triggers partly out of simplicity, and partly because the=
re
> some before/after row linkage in the ON UPDATE CASCADE cases where state=
ment
> level triggers might not be feasible as we have currently implemented th=
em.

I tried to review this patch, also with the intention to learn more about
AFTER triggers internals. As for the code, I understood your idea and it
really seems like low hanging fruit. However in trigger.c I noticed a comm=
ent
that transition table is not supported for deferred constraints. Of course=
 I
tried to use this information to test your patch:

CREATE TABLE t_pk(i int PRIMARY KEY);
CREATE TABLE t_fk(i int REFERENCES t_pk ON DELETE NO ACTION DEFERRABLE);
INSERT INTO t_pk(i) VALUES (1);
INSERT INTO t_fk(i) VALUES (1);
BEGIN;
SET CONSTRAINTS t_fk_i_fkey DEFERRED;
DELETE FROM t_pk;
COMMIT;

server closed the connection unexpectedly
        This probably means the server terminated abnormally
        before or while processing the request.
The connection to the server was lost. Attempting reset: Failed.

Missing transition table really appears to be the problem:

#0  0x0000000000a8c2a8 in tuplestore_tuple_count (state=3D0x7f7f7f7f7f7f7f=
7f) at tuplestore.c:548
#1  0x00000000009b0e58 in ri_on_delete_restrict (trigdata=3D0x7ffe800c2890=
, is_no_action=3Dtrue) at ri_triggers.c:720
#2  0x00000000009b0d3f in RI_FKey_noaction_del (fcinfo=3D0x7ffe800c27a0) a=
t ri_triggers.c:607
#3  0x00000000006b8768 in ExecCallTriggerFunc (trigdata=3D0x7ffe800c2890, =
tgindx=3D0, finfo=3D0x1f08c10, instr=3D0x0, per_tuple_context=3D0x1f30690)=
 at trigger.c:2412
#4  0x00000000006bbb86 in AfterTriggerExecute (event=3D0x1f0ab10, rel=3D0x=
7fc71306ea70, trigdesc=3D0x1f089f8, finfo=3D0x1f08c10, instr=3D0x0, per_tu=
ple_context=3D0x1f30690, trig_tuple_slot1=3D0x0, trig_tuple_slot2=3D0x0)
    at trigger.c:4367
#5  0x00000000006bbf9e in afterTriggerInvokeEvents (events=3D0xf97950 <aft=
erTriggers+16>, firing_id=3D1, estate=3D0x1f086c8, delete_ok=3Dtrue) at tr=
igger.c:4560
#6  0x00000000006bc844 in AfterTriggerFireDeferred () at trigger.c:4996
#7  0x000000000055c989 in CommitTransaction () at xact.c:1987
#8  0x000000000055d6a4 in CommitTransactionCommand () at xact.c:2832

While the idea to use the transition table is good, this approach probably
requires the trigger engine (trigger.c) to be adjusted, and that in a
non-trivial way.

I'm also not sure if it's o.k. that performance related patch potentially
makes performance worse in some cases. If FK violations are checked at
statement boundary, the wasted effort / time can (at least in theory) be h=
igh
if early rows violate the FK.

Have you considered bulk processing of individual rows by row-level trigge=
r?
For IMMEDIATE constraints we'd have to ensure that the trigger is notified
that the current row is the last one from the current query, but that migh=
t
not be difficult.

-- =

Antonin Houska
https://www.cybertec-postgresql.com