Thread: BUG #6226: Broken foreign key stored on database (parent deleted with children still readable, BUG#6225 Update)

The following bug has been logged online:

Bug reference:      6226
Logged by:          Daniel Cristian Cruz
Email address:      danielcristian@gmail.com
PostgreSQL version: 9.1.1
Operating system:   "PostgreSQL 9.1.0 on x86_64-unknown-linux-gnu, compiled
by gcc (GCC) 4.1.2 20080704 (Red Hat 4.1.2-48), 64-bit"
Description:        Broken foreign key stored on database (parent deleted
with children still readable, BUG#6225 Update)
Details:

Hello,

Maybe my last message was very hard to read, English is not my natural
language and I'm with a strong headache (a real one, not about this
problem).

I've tested BUG #6225 in 9.1.1, and the problem persists. After deleting the
parent record, children record stills readable with a key that points to the
deleted record.

Script that shows the problem:

CREATE TABLE event (
 id_event serial primary key
);

CREATE TABLE repetition_conf (
 id_repetition_conf serial primary key,
 id_event integer,
 CONSTRAINT event_fk FOREIGN KEY (id_event) REFERENCES event (id_event)
  ON UPDATE NO ACTION ON DELETE CASCADE DEFERRABLE INITIALLY DEFERRED
   -- IF MAIN EVENT IS DELETED, REPETITION CONFIG IS DELETED TOO
);

CREATE TABLE repetition (
 id_repetition serial primary key,
 id_repetition_conf integer,
 id_event integer,
 CONSTRAINT repetition_conf_fk FOREIGN KEY (id_repetition_conf) REFERENCES
repetition_conf (id_repetition_conf)
  ON UPDATE NO ACTION ON DELETE CASCADE DEFERRABLE INITIALLY DEFERRED,
   -- IF REPETITION CONFIG IS DELETED, REPETITION IS DELETED TOO
 CONSTRAINT event_fk FOREIGN KEY (id_event) REFERENCES event (id_event)
  ON UPDATE NO ACTION ON DELETE SET NULL DEFERRABLE INITIALLY DEFERRED
   -- IF EVENT IS DELETED, SET NULL TO REFERENCE (BUSINESS LOGIC)
);

CREATE OR REPLACE FUNCTION remove_event()
  RETURNS trigger AS
$BODY$
BEGIN
 DELETE FROM event
  WHERE id_event = OLD.id_event;
 RETURN OLD;
END
$BODY$
  LANGUAGE plpgsql VOLATILE;

CREATE TRIGGER remove_repetition_event
  BEFORE DELETE
  ON repetition
  FOR EACH ROW
  WHEN ((OLD.id_event IS NOT NULL))
  EXECUTE PROCEDURE remove_event();
   -- IF REPETITION IS DELETED, EVENT IS DELETED TOO

-- ACTUAL INSERT STEPS
BEGIN;
INSERT INTO event (id_event) VALUES (DEFAULT); -- CREATE MAIN EVENT
INSERT INTO repetition_conf (id_event) VALUES
(CURRVAL('event_id_event_seq')); -- CREATE REPETITION CONFIG
-- THESE TWO NEXT STEPS ARE A LOOP IN A TRIGGER ON repetition_conf
INSERT INTO event (id_event) VALUES (DEFAULT);
INSERT INTO repetition (id_repetition, id_repetition_conf, id_event)
 VALUES (DEFAULT, CURRVAL('repetition_conf_id_repetition_conf_seq'),
CURRVAL('event_id_event_seq'));
COMMIT;

-- IN ANOTHER SESSION, DELETE THE MAIN EVENT, WHICH WILL DELETE CONFIG AND
REPETITIONS
BEGIN;
DELETE FROM event WHERE id_event = 1;
COMMIT;

-- NOW I HAD A FOREIGN KEY WITH NO PARENT...
SELECT *
FROM repetition
LEFT JOIN repetition_conf
 ON repetition.id_repetition_conf = repetition_conf.id_repetition_conf
WHERE repetition_conf.id_repetition_conf IS NULL;

Excerpts from Daniel Cristian Cruz's message of lun sep 26 15:29:33 -0300 2011:
>
> The following bug has been logged online:
>
> Bug reference:      6226
> Logged by:          Daniel Cristian Cruz
> Email address:      danielcristian@gmail.com
> PostgreSQL version: 9.1.1
> Operating system:   "PostgreSQL 9.1.0 on x86_64-unknown-linux-gnu, compiled
> by gcc (GCC) 4.1.2 20080704 (Red Hat 4.1.2-48), 64-bit"
> Description:        Broken foreign key stored on database (parent deleted
> with children still readable, BUG#6225 Update)
> Details:
>
> Hello,
>
> Maybe my last message was very hard to read, English is not my natural
> language and I'm with a strong headache (a real one, not about this
> problem).

Please see if bug #6123 applies to this case.
http://www.mail-archive.com/pgsql-hackers@postgresql.org/msg181541.html

--
Álvaro Herrera <alvherre@commandprompt.com>
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

2011/9/26 Alvaro Herrera <alvherre@commandprompt.com>

>
> Please see if bug #6123 applies to this case.
> http://www.mail-archive.com/pgsql-hackers@postgresql.org/msg181541.html


I guess, yes, it's related, because we had a trigger that deletes a row,
while other foreign key constraint is updating the row setting null.

But the thread is almost a book, and I am confused. It's a bug or do I need
to change my schema? Where can I change it to avoid it?

--=20
Daniel Cristian Cruz
=E3=82=AF=E3=83=AB=E3=82=BA =E3=82=AF=E3=83=AA=E3=82=B9=E3=83=81=E3=82=A2=
=E3=83=B3 =E3=83=80=E3=83=8B=E3=82=A8=E3=83=AB

Excerpts from Daniel Cristian Cruz's message of mié oct 05 10:00:36 -0300 2011:
> 2011/9/26 Alvaro Herrera <alvherre@commandprompt.com>
>
> >
> > Please see if bug #6123 applies to this case.
> > http://www.mail-archive.com/pgsql-hackers@postgresql.org/msg181541.html
>
>
> I guess, yes, it's related, because we had a trigger that deletes a row,
> while other foreign key constraint is updating the row setting null.
>
> But the thread is almost a book, and I am confused. It's a bug or do I need
> to change my schema?

Well, some people say it's a bug, others say it's not; and even if it
is, changing it means backwards incompatible behavior, so *if* it is
patched, it will only change the behavior on a future release, not
whatever you're using.

> Where can I change it to avoid it?

I don't know enough about the problem to help you there, sorry.

--
Álvaro Herrera <alvherre@commandprompt.com>
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

2011/10/5 Alvaro Herrera <alvherre@commandprompt.com>

> Well, some people say it's a bug, others say it's not; and even if it
> is, changing it means backwards incompatible behavior, so *if* it is
> patched, it will only change the behavior on a future release, not
> whatever you're using.


Well, I'll try to explain better, because it could be a bug, since after
deleting and cascading, a deleted row is still visible and has a foreign key
constraint violated with no error message.

Even if some people relies on this to build something, it is something not
desirable, because I expect a foreign key constraint to be always true.

The sample error case is:

-- My event table
CREATE TABLE event (
 id_event serial primary key
);

-- My event repetition configuration
CREATE TABLE repetition_conf (
 id_repetition_conf serial primary key,
 id_event integer,
 CONSTRAINT event_fk FOREIGN KEY (id_event) REFERENCES event (id_event)
  ON UPDATE NO ACTION ON DELETE CASCADE DEFERRABLE INITIALLY DEFERRED
   -- IF MAIN EVENT IS DELETED, REPETITION CONFIG IS DELETED TOO
);

-- Event repetition events
CREATE TABLE repetition (
 id_repetition serial primary key,
 id_repetition_conf integer,
 id_event integer,
 CONSTRAINT repetition_conf_fk FOREIGN KEY (id_repetition_conf) REFERENCES
repetition_conf (id_repetition_conf)
  ON UPDATE NO ACTION ON DELETE CASCADE DEFERRABLE INITIALLY DEFERRED,
   -- if I delete config, I delete all repetitions
 CONSTRAINT event_fk FOREIGN KEY (id_event) REFERENCES event (id_event)
  ON UPDATE NO ACTION ON DELETE SET NULL DEFERRABLE INITIALLY DEFERRED
   -- if repeated event is deleted, just set null to the reference, I need
to know there was a repetition
);

-- a trigger to remove an repetition event, when repetition is deleted
CREATE OR REPLACE FUNCTION remove_event()
  RETURNS trigger AS
$BODY$
BEGIN
 DELETE FROM event
  WHERE id_event =3D OLD.id_event;
 RETURN OLD;
END
$BODY$
  LANGUAGE plpgsql VOLATILE;

CREATE TRIGGER remove_repetition_event
  BEFORE DELETE
  ON repetition
  FOR EACH ROW
  WHEN ((OLD.id_event IS NOT NULL))
  EXECUTE PROCEDURE remove_event();

BEGIN;
-- create the main event
INSERT INTO event (id_event) VALUES (DEFAULT);
-- create the repetition config
INSERT INTO repetition_conf (id_event)
VALUES(CURRVAL('event_id_event_seq'));
-- create the repetition event
INSERT INTO event (id_event) VALUES (DEFAULT);
-- create the repetition link
INSERT INTO repetition (id_repetition, id_repetition_conf, id_event)
 VALUES (DEFAULT, CURRVAL('repetition_conf_id_repetition_conf_seq'),
 CURRVAL('event_id_event_seq'));
COMMIT;

-- delete an event with repetition in a transaction
-- *event *cascades to *repetition_conf*, then cascades to* repetition*,
then trigger cascades to* event* which set nulls back to *repetition*
BEGIN;
DELETE FROM event WHERE id_event =3D 1;
COMMIT;

-- now, there is a repetition pointing to a config, that is not visible
anymore
SELECT
 repetition.id_repetition_conf AS referenced_id,
 repetition_conf.id_repetition_conf AS reference
FROM repetition
LEFT JOIN repetition_conf
 ON repetition.id_repetition_conf =3D repetition_conf.id_repetition_conf
WHERE repetition_conf.id_repetition_conf IS NULL;


--=20
Daniel Cristian Cruz
=E3=82=AF=E3=83=AB=E3=82=BA =E3=82=AF=E3=83=AA=E3=82=B9=E3=83=81=E3=82=A2=
=E3=83=B3 =E3=83=80=E3=83=8B=E3=82=A8=E3=83=AB

Daniel Cristian Cruz <danielcristian@gmail.com> writes:
> 2011/10/5 Alvaro Herrera <alvherre@commandprompt.com>
>> Well, some people say it's a bug, others say it's not; and even if it
>> is, changing it means backwards incompatible behavior, so *if* it is
>> patched, it will only change the behavior on a future release, not
>> whatever you're using.

> Well, I'll try to explain better, because it could be a bug, since after
> deleting and cascading, a deleted row is still visible and has a foreign key
> constraint violated with no error message.

This schema arrangement is circular.  Consider what happens:

1. You issue a DELETE for the event row with id_event = 1.

2. This fires (as AFTER triggers) commands to do these things:

   a. DELETE FROM repetition_conf WHERE id_event = 1.
   b. UPDATE repetition SET id_event = NULL WHERE id_event = 1.
    (This one does nothing since there is no such row in repetition.)

   I don't recall whether we have any particular guarantees about the
   order these two things get done in, but in this example it doesn't
   matter.

3. The DELETE on repetition_conf deletes the lone repetition_conf row,
   and then fires an AFTER trigger to do

    DELETE FROM repetition WHERE id_repetition_conf = 1.

4. That DELETE finds it should delete the lone repetition row ...
   but wait!  *Before* it can delete that row, it must fire the
   BEFORE DELETE trigger.

5. The trigger function issues

    DELETE FROM event WHERE id_event = 2.

6. This fires (as AFTER triggers) commands to do these things:

   a. DELETE FROM repetition_conf WHERE id_event = 2.
    (This does nothing.)
   b. UPDATE repetition SET id_event = NULL WHERE id_event = 2.
    (This one finds and updates the lone repetition row.)

7. Now we exit the trigger function and are back at the step-3 DELETE
   command.  It was supposed to delete the repetition row.
   But by now, that row has already been updated, and under Postgres'
   visibility rules that means the previously-started delete can't
   delete it.  If it did delete it, it might be deleting data other
   than what you intended, since the row is now different from what
   it was when it was selected for deletion.


In my view, this whole thing is just an example of the hazards of
defining BEFORE triggers that can change the state of the target row.
That's best avoided, because there simply isn't any good semantics
for it.  But generally the rule in Postgres is first action past the
post wins, and in this case it's the UPDATE SET NULL that gets done
first.

The rough rule of thumb is that BEFORE triggers are good for adjusting
the contents of the row that's about to be stored, while AFTER triggers
are what to use to propagate changes elsewhere.  I'm not sure that
changing the BEFORE trigger to AFTER would make all your problems go
away, but it'd be worth a try.

Another point worth making here is that in Postgres, triggers run at a
lower level than foreign key constraints (since FKs are implemented
using triggers).  It *is possible* to cause an FK constraint to be
violated, if you install a trigger that interferes with the operation of
the FK action, which is what's happening here.  We don't consider that a
bug but a feature.  The only way to prevent it would be to not fire
triggers for updates caused by FK actions, which would be a cure worse
than the disease.

            regards, tom lane

Thanks, Tom. Now I got it.

I didn't tryed to change BEFORE to AFTER, but reminded something I learned
coding triggers a long time ago, and I forgot. This could be a fortune:

"The rough rule of thumb is that BEFORE triggers are good for adjusting
the contents of the row that's about to be stored, while AFTER triggers
are what to use to propagate changes elsewhere."

And the full version (from manual):

"Typically, row-level BEFORE triggers are used for checking or modifying the
data that will be inserted or updated. For example, a BEFORE trigger might
be used to insert the current time into a timestamp column, or to check that
two elements of the row are consistent. Row-level AFTER triggers are most
sensibly used to propagate the updates to other tables, or make consistency
checks against other tables. The reason for this division of labor is that
an AFTER trigger can be certain it is seeing the final value of the row,
while a BEFOREtrigger cannot; there might be other BEFORE triggers firing
after it. If you have no specific reason to make a trigger BEFORE or AFTER,
the BEFORE case is more efficient, since the information about the operation
doesn't have to be saved until end of statement."

I became a rusty DBA poluting pgsql-bugs, sorry. RTFM for myself. :)
--=20
Daniel Cristian Cruz
=E3=82=AF=E3=83=AB=E3=82=BA =E3=82=AF=E3=83=AA=E3=82=B9=E3=83=81=E3=82=A2=
=E3=83=B3 =E3=83=80=E3=83=8B=E3=82=A8=E3=83=AB

Q2hhbmdpbmcgQkVGT1JFIERFTEVURSB0byBBRlRFUiBERUxFVEUgd29ya2Vk
LgoKMjAxMS8xMC81IERhbmllbCBDcmlzdGlhbiBDcnV6IDxkYW5pZWxjcmlz
dGlhbkBnbWFpbC5jb20+Cgo+IC4uLgo+Ci0tIApEYW5pZWwgQ3Jpc3RpYW4g
Q3J1egrjgq/jg6vjgrog44Kv44Oq44K544OB44Ki44OzIOODgOODi+OCqOOD
qwo=