Обсуждение: Problem with trigger makes Detail record be invalid
I´m using Postgres 10 on ubuntu. suppose a simple Master/Detail structure like this: create table Master(ID integer primary key, name text); create table Detail(ID integer primary key, Master_ID Integer, OtherInfo text); alter table Detail add constraint FKMasterDetail foreign key (Master_ID) references Master(ID) on update cascade on delete cascade; Then insert some records on it: insert into Master(ID, Name) values(1,'First'), values(2,'Second'); insert into Detail(ID, Master_ID, OtherInfo) values(1,1,'Detail Information'), (2,2,'Detail Information2'); Then, if I delete on Master will delete on detail too. Fine. delete from Master where ID=1; But now, suppose I have a complex trigger before update or delete that runs on Detail table. create function DoAComplexJobOnDetail() returns trigger as $$ begin -- Do lots of things then Return new; --This is the error, because I´m returning new even for delete; end;$$ language plpgsql; create trigger DetailDoAComplexJob before update or delete on Detail for each row execute procedure DoAComplexJobOnDetail(); Then try to delete the other Master record. It will be deleted on Master but Detail record doesn´t and will obviously become invalid because the foreign key. delete from Master where ID=2; select * from Master; --will show no records. select * from Detail; --will show one record pointing to Master_ID=2, that doesn´t exist anymore. Is this a bug or it´s mine responsability to check that trigger result ? If that trigger responds incorrectly I think that no information could be executed. -- Sent from: http://www.postgresql-archive.org/PostgreSQL-general-f1843780.html
On 04/19/2018 10:55 AM, PegoraroF10 wrote: > I´m using Postgres 10 on ubuntu. > > suppose a simple Master/Detail structure like this: > > create table Master(ID integer primary key, name text); > create table Detail(ID integer primary key, Master_ID Integer, OtherInfo > text); > alter table Detail add constraint FKMasterDetail foreign key (Master_ID) > references Master(ID) on update cascade on delete cascade; > > Then insert some records on it: > insert into Master(ID, Name) values(1,'First'), values(2,'Second'); > insert into Detail(ID, Master_ID, OtherInfo) values(1,1,'Detail > Information'), (2,2,'Detail Information2'); > > Then, if I delete on Master will delete on detail too. Fine. > delete from Master where ID=1; > > But now, suppose I have a complex trigger before update or delete that runs > on Detail table. > create function DoAComplexJobOnDetail() returns trigger as $$ > begin > -- Do lots of things then > Return new; --This is the error, because I´m returning new even for > delete; That can be dealt with using TG_OP value to conditionally change what is RETURNed: https://www.postgresql.org/docs/10/static/plpgsql-trigger.html "TG_OP Data type text; a string of INSERT, UPDATE, DELETE, or TRUNCATE telling for which operation the trigger was fired. " See Example 42.4. A PL/pgSQL Trigger Procedure For Auditing at bottom of page. > end;$$ language plpgsql; > create trigger DetailDoAComplexJob before update or delete on Detail for > each row execute procedure DoAComplexJobOnDetail(); > > Then try to delete the other Master record. It will be deleted on Master but > Detail record doesn´t and will obviously become invalid because the foreign > key. > delete from Master where ID=2; > select * from Master; --will show no records. > select * from Detail; --will show one record pointing to Master_ID=2, that > doesn´t exist anymore. > > Is this a bug or it´s mine responsability to check that trigger result ? Without seeing exactly what the trigger function on Detail is doing that is not answerable. > If that trigger responds incorrectly I think that no information could be > executed. > > > > -- > Sent from: http://www.postgresql-archive.org/PostgreSQL-general-f1843780.html > -- Adrian Klaver adrian.klaver@aklaver.com
I know my trigger is incorrect. I know that I can use TG_OP to know what operation is being done. My question is ... > Is this a bug or it´s mine responsability to check that trigger result ? I think it´s a bug because if something got wrong on detail deletion and it was rolled back, how could be a parent record be deleted ? -- Sent from: http://www.postgresql-archive.org/PostgreSQL-general-f1843780.html
On 04/19/2018 11:30 AM, PegoraroF10 wrote: > I know my trigger is incorrect. I know that I can use TG_OP to know what > operation is being done. > My question is ... >> Is this a bug or it´s mine responsability to check that trigger result ? > > I think it´s a bug because if something got wrong on detail deletion and it > was rolled back, how could be a parent record be deleted ? In your example I saw no rollback or error message: "delete from Master where ID=2; select * from Master; --will show no records. select * from Detail; --will show one record pointing to Master_ID=2, that doesn´t exist anymore." Was there an error message? Then there is the fact that your trigger is doing something to the row BEFORE the delete or update and presumably modifying it. Without knowing what the function is doing or what it is actually returning then we are in full on guessing mode. > > > > -- > Sent from: http://www.postgresql-archive.org/PostgreSQL-general-f1843780.html > -- Adrian Klaver adrian.klaver@aklaver.com
On 04/19/2018 11:30 AM, PegoraroF10 wrote: > I know my trigger is incorrect. I know that I can use TG_OP to know what > operation is being done. > My question is ... >> Is this a bug or it´s mine responsability to check that trigger result ? > > I think it´s a bug because if something got wrong on detail deletion and it > was rolled back, how could be a parent record be deleted ? Another thought, are there are any other triggers on the Master and/or Detail tables? > > > > -- > Sent from: http://www.postgresql-archive.org/PostgreSQL-general-f1843780.html > -- Adrian Klaver adrian.klaver@aklaver.com
My point of view that there was a partial rollback, just on detail table. If I´ve done a delete from Master and I have a foreign key to it with cascade option, or all records should be deleted or no one should, this is my point. Did you see that Master table has no records and Detail table has one record ? I think you agree with me that we have a a detail record with no master, so it´s unusable, right ? I´m not able to do a backup/restore of this database because that record doesn´t match. -- Sent from: http://www.postgresql-archive.org/PostgreSQL-general-f1843780.html
Adrian Klaver <adrian.klaver@aklaver.com> writes: > On 04/19/2018 10:55 AM, PegoraroF10 wrote: >> Is this a bug or it´s mine responsability to check that trigger result ? > Without seeing exactly what the trigger function on Detail is doing that > is not answerable. I think the OP is complaining because his misimplemented trigger can break the consistency of the foreign key constraint. That is not a bug, it's an intentional design decision: triggers are lower-level than foreign key enforcement queries, and fire during such queries. It's easy to construct examples where people would be very unhappy if this were not so, because then FK-driven updates would not be seen by the table's triggers. It does mean that you have to be careful when writing a trigger. (I'm not sure that this issue is adequately documented, though. I'd have expected to find something about it in triggers.sgml and/or create_trigger.sgml, but in a quick look neither of them mentions foreign keys.) regards, tom lane
On 04/19/2018 11:52 AM, PegoraroF10 wrote: > My point of view that there was a partial rollback, just on detail table. If > I´ve done a delete from Master and I have a foreign key to it with cascade > option, or all records should be deleted or no one should, this is my point. Except you now have a trigger or possibly triggers that are altering the delete process and possibly counteracting the system trigger that is a FK. There have been enough instances of this show up on this list for me to know this is a distinct possibility. > > Did you see that Master table has no records and Detail table has one record > ? > I think you agree with me that we have a a detail record with no master, so > it´s unusable, right ? We have not seen the actual records, so I cannot say or agree/disagree. > > I´m not able to do a backup/restore of this database because that record > doesn´t match. > > > > -- > Sent from: http://www.postgresql-archive.org/PostgreSQL-general-f1843780.html > -- Adrian Klaver adrian.klaver@aklaver.com
2018-04-19 15:57 GMT-03:00 Tom Lane <tgl@sss.pgh.pa.us>:
>
> Adrian Klaver <adrian.klaver@aklaver.com> writes:
> > On 04/19/2018 10:55 AM, PegoraroF10 wrote:
> >> Is this a bug or it´s mine responsability to check that trigger result ?
>
> > Without seeing exactly what the trigger function on Detail is doing that
> > is not answerable.
>
> I think the OP is complaining because his misimplemented trigger can break
> the consistency of the foreign key constraint. That is not a bug, it's
> an intentional design decision: triggers are lower-level than foreign key
> enforcement queries, and fire during such queries. It's easy to construct
> examples where people would be very unhappy if this were not so, because
> then FK-driven updates would not be seen by the table's triggers. It does
> mean that you have to be careful when writing a trigger.
>
Yeap... it's already mentioned in stackoverflow in ptbr sometime ago [1] with a reproducible test case.
> (I'm not sure that this issue is adequately documented, though.
> I'd have expected to find something about it in triggers.sgml and/or
> create_trigger.sgml, but in a quick look neither of them mentions foreign
> keys.)
>
We don't have it properly documented... at the time I answered this question on pt-br stackoverflow I noticed the lack o documentation and unfortunately I completely forgot to propose a small patch for it.
> I'd have expected to find something about it in triggers.sgml and/or
> create_trigger.sgml, but in a quick look neither of them mentions foreign
> keys.)
>
We don't have it properly documented... at the time I answered this question on pt-br stackoverflow I noticed the lack o documentation and unfortunately I completely forgot to propose a small patch for it.
Regards,
[1] https://pt.stackoverflow.com/questions/256115/postgresql-foreign-keys-falhando/256398#256398
--
Fabrízio de Royes Mello Timbira - http://www.timbira.com.br/
PostgreSQL: Consultoria, Desenvolvimento, Suporte 24x7 e Treinamento
[1] https://pt.stackoverflow.com/questions/256115/postgresql-foreign-keys-falhando/256398#256398
--
Fabrízio de Royes Mello Timbira - http://www.timbira.com.br/
PostgreSQL: Consultoria, Desenvolvimento, Suporte 24x7 e Treinamento
(I'm not sure that this issue is adequately documented, though.
I'd have expected to find something about it in triggers.sgml and/or
create_trigger.sgml, but in a quick look neither of them mentions foreign
keys.)
I'm leading toward inadequate myself...though create_trigger.sgml does have:
"""
SQL specifies that BEFORE DELETE triggers on cascaded deletes fire after the cascaded DELETE completes. The PostgreSQL behavior is for BEFORE DELETE to always fire before the delete action, even a cascading one. This is considered more consistent. There is also nonstandard behavior if BEFORE triggers modify rows or prevent updates during an update that is caused by a referential action. This can lead to constraint violations or stored data that does not honor the referential constraint.
"""
And triggers.sgml has:
"""
A row-level BEFORE trigger that does not intend to cause [a delete] must be careful to return as its result the same row that was passed in (that is, the NEW row for INSERT and UPDATE triggers, the OLD row for DELETE triggers).
"""
There is a lot of surrounding text to sift through though - and the former is a "compatibility" comment. Warning blurb after the triggers.sgml quoted section about preventing the action from occurring potentially breaking FK constraints would be a reasonable response to this report.
I'd rather have a developer spend time coding up having an FK constraint define an AFTER STATEMENT trigger using a transition table and ensure that all FK constraints remain enforced for all changed records. Correctly or incorrectly written triggers do not have any liberty to violate FK constraints and the fact that they can is reasonably considered by the user base to be a bug.
David J.
=?UTF-8?Q?Fabr=C3=ADzio_de_Royes_Mello?= <fabrizio@timbira.com.br> writes: > 2018-04-19 15:57 GMT-03:00 Tom Lane <tgl@sss.pgh.pa.us>: >> (I'm not sure that this issue is adequately documented, though. >> I'd have expected to find something about it in triggers.sgml and/or >> create_trigger.sgml, but in a quick look neither of them mentions foreign >> keys.) > We don't have it properly documented... at the time I answered this > question on pt-br stackoverflow I noticed the lack o documentation and > unfortunately I completely forgot to propose a small patch for it. It strikes me that there are basically two things a trigger could do to break FK consistency: 1. Turn an FK-commanded update into a no-op by returning NULL. 2. Change the content of the FK-related columns during an FK-commanded update. Both of these apply only to BEFORE ROW triggers, of course. It might not be unreasonable or unduly expensive to throw an error for case #1. I don't think I want to get into the expense of checking for case #2, but covering case #1 would be enough to catch all of the reports of this type of problem that I can remember. IIRC, you can also break FK consistency with poorly-thought-out rules, but given that rules are close-to-deprecated, I'm not very concerned about sanding down rough edges in that case. (But if you feel like writing a documentation patch, please do, because we'd not be likely to back-patch a behavioral change like this even if we get around to making it.) regards, tom lane
On Thu, Apr 19, 2018 at 12:21 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Fabrízio de Royes Mello <fabrizio@timbira.com.br> writes:
> 2018-04-19 15:57 GMT-03:00 Tom Lane <tgl@sss.pgh.pa.us>:
>> (I'm not sure that this issue is adequately documented, though.
>> I'd have expected to find something about it in triggers.sgml and/or
>> create_trigger.sgml, but in a quick look neither of them mentions foreign
>> keys.)
> We don't have it properly documented... at the time I answered this
> question on pt-br stackoverflow I noticed the lack o documentation and
> unfortunately I completely forgot to propose a small patch for it.
It strikes me that there are basically two things a trigger could do to
break FK consistency:
1. Turn an FK-commanded update into a no-op by returning NULL.
2. Change the content of the FK-related columns during an FK-commanded
update.
Both of these apply only to BEFORE ROW triggers, of course.
It might not be unreasonable or unduly expensive to throw an error for
case #1. I don't think I want to get into the expense of checking
for case #2, but covering case #1 would be enough to catch all of the
reports of this type of problem that I can remember.
IIRC, you can also break FK consistency with poorly-thought-out rules,
but given that rules are close-to-deprecated, I'm not very concerned
about sanding down rough edges in that case.
(But if you feel like writing a documentation patch, please do, because
we'd not be likely to back-patch a behavioral change like this even
if we get around to making it.)
regards, tom lane
I'm gonna chime in here from a simple user perspective. I'm kinda shocked reading this thread that any of this is possible. I had always understood and relied on foreign keys being a _guarantee_ of referential integrity. I'd personally be in favor of at least an option to disallow this, even with a performance cost. Maybe you could even call it "Strict Mode." ;)
But regardless, I think some better documentation is in order, and not just in the triggers section. I'd suggest this be prominently mentioned as a big asterisk in any places that talk about constratints. This page seems like an obvious candidate: https://www.postgresql.org/docs/9.5/static/ddl-constraints.html), as it has nothing qualifying lots of statements such as "If a user attempts to store data in a column that would violate a constraint, an error is raised."
I do understand none of this happens unless you break it yourself, but it might change both how I write and test triggers, and how I might look at using other people's triggers or materials. Knowing my referential integrity can't be broken is a nice guard rail to have, but if you can't necessarily count on it, some prominent signs saying "warning, no guard rail ahead" seem like a good idea.
Thanks for listening!
Ken
![](http://agency-software.org/demo/client/images/agency_logo_small.png)
AGENCY Software
A Free Software data system
By and for non-profits
(253) 245-3801
learn more about AGENCY or
follow the discussion.
Correct, that delete done a partial commit. And this is absolutely unacceptable. I know I did that trigger incorrectly but referential integrity is obligatory. Imagine if I have a database crash and need to restore as soon as possible. How much time I´ll spend removing those records from a backup to get entire database restored properly. Well, I´ll review all my triggers. And I have hundreds of them. -- Sent from: http://www.postgresql-archive.org/PostgreSQL-general-f1843780.html
On 04/19/2018 06:49 PM, PegoraroF10 wrote: > Correct, that delete done a partial commit. And this is absolutely > unacceptable. Yet a known possible outcome. See the section on Triggers towards bottom of page: https://en.wikipedia.org/wiki/Foreign_key > I know I did that trigger incorrectly but referential integrity is > obligatory. I would agree if the FK relationship was entirely driven by the system trigger e.g: alter table Detail add constraint FKMasterDetail foreign key (Master_ID) references Master(ID) on update cascade on delete cascade; As soon as you added your UPDATE/DELETE trigger you took on responsibility for how the data was passed around. I understand that this was not communicated as well as it should be in the docs. > Imagine if I have a database crash and need to restore as soon as possible. > How much time I´ll spend removing those records from a backup to get entire > database restored properly. Myself, having written more then my fair share of poorly thought out trigger functions, I test new ones extensively before I release them into the wild. > > Well, I´ll review all my triggers. And I have hundreds of them. > > > -- Adrian Klaver adrian.klaver@aklaver.com
On 04/19/2018 06:49 PM, PegoraroF10 wrote:
I know I did that trigger incorrectly but referential integrity is
obligatory.
I would agree if the FK relationship was entirely driven by the system trigger e.g:
alter table Detail add constraint FKMasterDetail foreign key (Master_ID)
references Master(ID) on update cascade on delete cascade;
As soon as you added your UPDATE/DELETE trigger you took on responsibility for how the data was passed around.
Such responsibility is an artifact of our specific implementation and not an inherent property of writing triggers in the presence of FK constraints.
We've left a foot-gun laying around and should not be surprised when less experienced users pick it up and shoot themselves in the foot.
IOW, I do agree with the OP - its just an unfortunate reality that this isn't how things work today. Whether one can accept and work within this reality is a personal decision.
This does reinforce that testing the restoration of ones backups is important.
David J.
On 04/20/2018 07:21 AM, David G. Johnston wrote: > On 04/19/2018 06:49 PM, PegoraroF10 wrote: > On Fri, Apr 20, 2018 at 6:55 AM, Adrian Klaver > <adrian.klaver@aklaver.com <mailto:adrian.klaver@aklaver.com>>wrote: > > I know I did that trigger incorrectly but referential integrity is > obligatory. > > > I would agree if the FK relationship was entirely driven by the > system trigger e.g: > > alter table Detail add constraint FKMasterDetail foreign key (Master_ID) > references Master(ID) on update cascade on delete cascade; > > As soon as you added your UPDATE/DELETE trigger you took on > responsibility for how the data was passed around. > > > Such responsibility is an artifact of our specific implementation and > not an inherent property of writing triggers in the presence of FK > constraints. https://en.wikipedia.org/wiki/Foreign_key#Triggers > > We've left a foot-gun laying around and should not be surprised when > less experienced users pick it up and shoot themselves in the foot. > > IOW, I do agree with the OP - its just an unfortunate reality that this > isn't how things work today. Whether one can accept and work within > this reality is a personal decision. > > This does reinforce that testing the restoration of ones backups is > important. > > David J. > -- Adrian Klaver adrian.klaver@aklaver.com
Well, talking about responsabilities, I think one of responsabilities of a mature database is that it can only accept data it was configured for. If you try to store a timestamp in a integer field or a huge numeric value in a smallint field, Postgres will block you because that operation is not acceptable. So, it's not acceptable to break referential integrity, is it ? -- Sent from: http://www.postgresql-archive.org/PostgreSQL-general-f1843780.html
On 04/20/2018 01:30 PM, PegoraroF10 wrote: > Well, talking about responsabilities, I think one of responsabilities of a > mature database is that it can only accept data it was configured for. If > you try to store a timestamp in a integer field or a huge numeric value in a Actually there have been examples on this list where folks have stored a timestamp as seconds from an epoch in an integer field. Of course then someone has to know what that field really represents. This is not nit picking on my part so much as an example of end user inventiveness. To that end Postgres has many ways of coming to a solution for a problem. Unfortunately, there are paths to a solution can trip you up. This means there is often no simple answer to a problem. Basically, more choices means more pre-thinking, testing, re-thinking, repeat as needed. > smallint field, Postgres will block you because that operation is not > acceptable. > So, it's not acceptable to break referential integrity, is it ? That is a maybe: https://www.postgresql.org/docs/10/static/sql-altertable.html " DISABLE TRIGGER [ trigger_name | ALL | USER ] ALL Disable or enable all triggers belonging to the table. (This requires superuser privilege if any of the triggers are internally generated constraint triggers such as those that are used to implement foreign key constraints or deferrable uniqueness and exclusion constraints.) ADD table_constraint [ NOT VALID ] This form adds a new constraint to a table using the same syntax as CREATE TABLE, plus the option NOT VALID, which is currently only allowed for foreign key and CHECK constraints. If the constraint is marked NOT VALID, the potentially-lengthy initial check to verify that all rows in the table satisfy the constraint is skipped. The constraint will still be enforced against subsequent inserts or updates (that is, they'll fail unless there is a matching row in the referenced table, in the case of foreign keys; and they'll fail unless the new row matches the specified check constraints). But the database will not assume that the constraint holds for all rows in the table, until it is validated by using the VALIDATE CONSTRAINT option. " https://www.postgresql.org/docs/10/static/sql-createtrigger.html "SQL specifies that BEFORE DELETE triggers on cascaded deletes fire after the cascaded DELETE completes. The PostgreSQL behavior is for BEFORE DELETE to always fire before the delete action, even a cascading one. This is considered more consistent. There is also nonstandard behavior if BEFORE triggers modify rows or prevent updates during an update that is caused by a referential action. This can lead to constraint violations or stored data that does not honor the referential constraint." https://www.postgresql.org/docs/10/static/plpgsql-trigger.html "Row-level triggers fired BEFORE can return null to signal the trigger manager to skip the rest of the operation for this row (i.e., subsequent triggers are not fired, and the INSERT/UPDATE/DELETE does not occur for this row). " A certain amount of this came about because folks are dealing with messy data and want to get it into the database first, do the clean up there and then apply the RI constraints. Other folks have different ways of doing it. It comes done to personal choice. That means though you have to know that dangerous paths exist and how to avoid them. A lot of this is ingrained in the code and in use in the wild, so I would not expect there would be major changes in how things work. Instead as has already been indicated there maybe better documentation on the way detailing all the above. > > > > -- > Sent from: http://www.postgresql-archive.org/PostgreSQL-general-f1843780.html > > -- Adrian Klaver adrian.klaver@aklaver.com