Re: Referential Integrity Checks with Statement-level Triggers

From: Antonin Houska <ah(at)cybertec(dot)at>
To: Corey Huinker <corey(dot)huinker(at)gmail(dot)com>
Cc: emre(at)hasegeli(dot)com, Adam Brusselback <adambrusselback(at)gmail(dot)com>, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Referential Integrity Checks with Statement-level Triggers
Date: 2019-02-20 11:38:06
Message-ID: 17100.1550662686@localhost
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Corey Huinker <corey(dot)huinker(at)gmail(dot)com> wrote:

> Attached is a patch that refactors DELETE triggers to fire at the statement level.
>
> I chose delete triggers partly out of simplicity, and partly because there
> some before/after row linkage in the ON UPDATE CASCADE cases where statement
> level triggers might not be feasible as we have currently implemented them.

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 comment
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=0x7f7f7f7f7f7f7f7f) at tuplestore.c:548
#1 0x00000000009b0e58 in ri_on_delete_restrict (trigdata=0x7ffe800c2890, is_no_action=true) at ri_triggers.c:720
#2 0x00000000009b0d3f in RI_FKey_noaction_del (fcinfo=0x7ffe800c27a0) at ri_triggers.c:607
#3 0x00000000006b8768 in ExecCallTriggerFunc (trigdata=0x7ffe800c2890, tgindx=0, finfo=0x1f08c10, instr=0x0, per_tuple_context=0x1f30690) at trigger.c:2412
#4 0x00000000006bbb86 in AfterTriggerExecute (event=0x1f0ab10, rel=0x7fc71306ea70, trigdesc=0x1f089f8, finfo=0x1f08c10, instr=0x0, per_tuple_context=0x1f30690, trig_tuple_slot1=0x0, trig_tuple_slot2=0x0)
at trigger.c:4367
#5 0x00000000006bbf9e in afterTriggerInvokeEvents (events=0xf97950 <afterTriggers+16>, firing_id=1, estate=0x1f086c8, delete_ok=true) at trigger.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 high
if early rows violate the FK.

Have you considered bulk processing of individual rows by row-level trigger?
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 might
not be difficult.

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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Michael Meskes 2019-02-20 11:41:17 Re: SQL statement PREPARE does not work in ECPG
Previous Message John Naylor 2019-02-20 11:34:12 Re: WIP: Avoid creation of the free space map for small tables