Thread: Combination of Triggers and self-FKs produces inconsistent data

Version: 8.3.5
Install: self-compile on 64-bit Ubuntu Linux
    also reproduced by AndrewSN on another platform
Summary: self-referential FKs are not enforced properly in the
    presence of BEFORE triggers
Test Case:

-- create two tables, one of which is the master table (reftable) the
other of which is a child which contains a tree structure (treetab):

create table reftable(
    refid int primary key,
    refname text
);


create table treetab (
    id int primary key,
    parent int,
    refid int not null references reftable(refid) on delete cascade,
    name text
);

-- now create a trigger function to maintain the integrity of the trees
in treetab by "pulling up"
-- each node to its parent if intermediate nodes get deleted
-- this trigger is inherently flawed and won't work with the FK below

create function treemaint () returns trigger as $t$
begin
    update treetab set parent = OLD.parent
    where parent = OLD.id;
    return OLD;
end; $t$ language plpgsql;

create trigger treemaint_trg before delete on treetab
for each row execute procedure treemaint();

-- populate reftable

insert into reftable
select i, ( 'Ref' || i::TEXT ) from generate_series(1,100) as g(i);

-- populate treetab with 10 rows each pointing to reftable

insert into treetab (id, refid)
select i, (( i / 10::INT ) + 1 )
from generate_series (1,900) as g(i);

-- create trees in treetab.  for this simple example each treeset is
just a chain with each child node
-- pointing to one higher node

update treetab set parent = ( id - 1 )
where id >  (
    select min(id) from treetab tt2
    where tt2.refid = treetab.refid);

update treetab set "name" = ('tree' || parent::TEXT || '-' || id::TEXT);

-- now create a self-referential FK to enforce tree integrity.  This
logically breaks the trigger

alter table treetab add constraint selfref foreign key (parent)
references treetab (id);

-- show tree for id 45

select * from treetab where refid = 45;

  id  | parent | refid |    name
-----+--------+-------+-------------
  440 |        |    45 |
  441 |    440 |    45 | tree440-441
  442 |    441 |    45 | tree441-442
  443 |    442 |    45 | tree442-443
  444 |    443 |    45 | tree443-444
  445 |    444 |    45 | tree444-445
  446 |    445 |    45 | tree445-446
  447 |    446 |    45 | tree446-447
  448 |    447 |    45 | tree447-448
  449 |    448 |    45 | tree448-449


-- now, we're going to delete the tree.  This delete should fail with an
error because the
-- trigger will violate "selfref"

delete from reftable where refid = 45;

-- however, it doesn't fail.  it reports success, and some but not all
rows from treetab
-- are deleted, leaving the database in an inconsistent state.

select * from treetab where refid = 45;

  id  | parent | refid |    name
-----+--------+-------+-------------
  441 |        |    45 | tree440-441
  443 |    441 |    45 | tree442-443
  445 |    443 |    45 | tree444-445
  447 |    445 |    45 | tree446-447
  449 |    447 |    45 | tree448-449

-- this means we now have rows in the table which
-- violate the FK to reftable.

postgres=# select * from reftable where refid = 45;
  refid | refname
-------+---------
(0 rows)

Josh Berkus <josh@agliodbs.com> writes:
> Summary: self-referential FKs are not enforced properly in the
>     presence of BEFORE triggers

This isn't a bug.  If you create triggers that prevent the RI actions
from being taken, it's your own problem.

            regards, tom lane

Josh Berkus <josh@agliodbs.com> writes:
> Tom Lane wrote:
>> This isn't a bug.  If you create triggers that prevent the RI actions
>> from being taken, it's your own problem.

> Huh?  Since when was it OK by us to have data which violates a declared
> FK under *any* circumstances?

You can't have your cake and eat it too, Josh.  If we make the RI
mechanism operate at a level underneath triggers, then we'll lose all
sorts of useful capability that people are depending on.  A couple of
examples:

* the ability to log table changes caused by RI cascades

* the ability to maintain row update timestamps when the update is
  caused by an RI cascade


> Where in our docs does it say that
> Foreign Keys are not enforced if the table has triggers on it?

It doesn't say that, because it isn't true.  What is true is that if you
make a trigger that prevents updates from happening, it breaks RI
updates as well as directly-user-initiated updates.  Either way, you're
going to need to fix the trigger.

            regards, tom lane

Tom Lane wrote:
> Josh Berkus <josh@agliodbs.com> writes:
>> Summary: self-referential FKs are not enforced properly in the
>>     presence of BEFORE triggers
>
> This isn't a bug.  If you create triggers that prevent the RI actions
> from being taken, it's your own problem.

Huh?  Since when was it OK by us to have data which violates a declared
FK under *any* circumstances?  Where in our docs does it say that
Foreign Keys are not enforced if the table has triggers on it?

--Josh

Tom Lane <tgl@sss.pgh.pa.us> writes:

> It doesn't say that, because it isn't true.  What is true is that if you
> make a trigger that prevents updates from happening, it breaks RI
> updates as well as directly-user-initiated updates.

Can we detect that this happened and throw an error? I suspect not, though,
since we have no way to actually determine whether the user trigger didn't do
something else equivalent.

--
  Gregory Stark
  EnterpriseDB          http://www.enterprisedb.com
  Get trained by Bruce Momjian - ask me about EnterpriseDB's PostgreSQL training!

Tom,

> You can't have your cake and eat it too, Josh.  If we make the RI
> mechanism operate at a level underneath triggers, then we'll lose all
> sorts of useful capability that people are depending on.  A couple of
> examples:
>
> * the ability to log table changes caused by RI cascades
>
> * the ability to maintain row update timestamps when the update is
>   caused by an RI cascade

Yeah, I can see that there isn't an obvious fix.  However, at the end of
the day it means that RI in Postgres can be accidentally broken by user
action without removing or disabling the constraint.  This isn't a
comfortable thought; it sounds an awful lot like another OSS-DB.

Or to put it another way, we don't allow triggers to break UNIQUE
constraints or CHECK constraints. All of the other constraints operate
at a level below triggers.  Why are FKs different?

> It doesn't say that, because it isn't true.  What is true is that if you
> make a trigger that prevents updates from happening, it breaks RI
> updates as well as directly-user-initiated updates.

Again, if we're going to retain this issue, then it needs to be in the
documentation that RI isn't enforced on the results of triggers.
Because, polling 5 people on IRC who each have more than 3 years of
PostgreSQL experience ... and two of whom are code contributors ... this
issue surprised *all* of them.

> Either way, you're
> going to need to fix the trigger.

If you read to the end of the example, you'd see that I'm saying that
the trigger should *fail*, with an error.  Not work.

Throughout the history of the project, no functionality which ends in a
inconsistent data state has ever been acceptable which I can recall.
When did we change our policy?

--Josh Berkus