Thread: BUG #18516: Foreign key data integrity is not validated when reenabled the trigger on tables

The following bug has been logged on the website:

Bug reference:      18516
Logged by:          Muzammil Q
Email address:      mujjamil995@gmail.com
PostgreSQL version: 15.6
Operating system:   rehl7
Description:

Hi,
I come up with a weird issue in the PostgreSQL 15.6 version. To update the
data in the tables I generally disable all triggers on the table using the
superuser privilege and enable the after the data copy to avoid the checking
the parent and child tables sequence. During the enable process FK
constraint should check the data integrity between the child and parent
tables. but somehow this is skipped and trigger enabled without any issue
and later during the data restoration this issue is identified.

Please check the below steps and share your feedback on this.

To recreate the issue you can follow the below steps: -

a)Create parent and child tables
test_db=# create table database_login (id int generated always as
identity,dbname varchar, login varchar);
test_db=# create table database ( id int generated always as identity,
server_name varchar,dbname varchar primary key);
CREATE TABLE

b)Add FK constraint 
test_db=# alter table database_login add constraint dbname_fk FOREIGN
KEY(dbname) REFERENCES database(dbname);
ALTER TABLE
test_db=# \d database_login
                          Table "public.database_login"
 Column |       Type        | Collation | Nullable |           Default
--------+-------------------+-----------+----------+------------------------------
 id     | integer           |           | not null | generated always as
identity
 dbname | character varying |           |          |
 login  | character varying |           |          |
Foreign-key constraints:
    "dbname_fk" FOREIGN KEY (dbname) REFERENCES database(dbname)

test_db=# \d database
                                Table "public.database"
   Column    |       Type        | Collation | Nullable |
Default
-------------+-------------------+-----------+----------+------------------------------
 id          | integer           |           | not null | generated always
as identity
 server_name | character varying |           |          |
 dbname      | character varying |           | not null |
Indexes:
    "database_pkey" PRIMARY KEY, btree (dbname)
Referenced by:
    TABLE "database_login" CONSTRAINT "dbname_fk" FOREIGN KEY (dbname)
REFERENCES database(dbname)

c)Insert the test data into the tables

test_db=# insert into database(server_name,dbname)
values('test_server1','test_db_1');
INSERT 0 1
test_db=# select * from database;
 id | server_name  |  dbname
----+--------------+-----------
  1 | test_server1 | test_db_1
(1 row)

test_db=# insert into database_login(dbname,login)
values('test_db_1','abc_user');
INSERT 0 1

d) The Below error shows when the triggers are enabled 
test_db=# insert into database_login(dbname,login)
values('test_db_2','abc_user');
ERROR:  insert or update on table "database_login" violates foreign key
constraint "dbname_fk"
DETAIL:  Key (dbname)=(test_db_2) is not present in table "database".

e)Disable the trigger and update the data

test_db=# alter table database_login disable trigger all;
ALTER TABLE
test_db=# alter table database disable trigger all;
ALTER TABLE
test_db=# insert into database_login(dbname,login)
values('test_db_2','abc_user');
INSERT 0 1

f)Enable the trigger. At this point PostgreSQL should check the data
integrity between the tables. But this is not checked and PG is allowed to
enable the trigger and you can see the output of the database_login table in
which test_db_2 db entry is available and this is not present in the parent
database table. When you restore the dump to another schema then it will
give the fk violate error.

test_db=# alter table database  enable  trigger all;
ALTER TABLE
test_db=# alter table database_login enable  trigger all;
ALTER TABLE
test_db=# select * from database_login;
 id |  dbname   |  login
----+-----------+----------
  1 | test_db_1 | abc_user
  3 | test_db_2 | abc_user
(2 rows)

test_db=# select * from database;
 id | server_name  |  dbname
----+--------------+-----------
  1 | test_server1 | test_db_1
(1 row)

test_db=# \d database
                                Table "public.database"
   Column    |       Type        | Collation | Nullable |
Default
-------------+-------------------+-----------+----------+------------------------------
 id          | integer           |           | not null | generated always
as identity
 server_name | character varying |           |          |
 dbname      | character varying |           | not null |
Indexes:
    "database_pkey" PRIMARY KEY, btree (dbname)
Referenced by:
    TABLE "database_login" CONSTRAINT "dbname_fk" FOREIGN KEY (dbname)
REFERENCES database(dbname)

test_db=# \d database_login
                          Table "public.database_login"
 Column |       Type        | Collation | Nullable |           Default
--------+-------------------+-----------+----------+------------------------------
 id     | integer           |           | not null | generated always as
identity
 dbname | character varying |           |          |
 login  | character varying |           |          |
Foreign-key constraints:
    "dbname_fk" FOREIGN KEY (dbname) REFERENCES database(dbname)


On Thu, 2024-06-20 at 09:10 +0000, PG Bug reporting form wrote:
> I come up with a weird issue in the PostgreSQL 15.6 version. To update the
> data in the tables I generally disable all triggers on the table using the
> superuser privilege and enable the after the data copy to avoid the checking
> the parent and child tables sequence. During the enable process FK
> constraint should check the data integrity between the child and parent
> tables. but somehow this is skipped and trigger enabled without any issue
> and later during the data restoration this issue is identified.

You wish that enabling the triggers would check the foreign key constraint,
but it doesn't.  All it does is prevent new foreign key violations from
happening.

If you disable the system triggers that implement the foreign key, all
bets are off, and you have to make sure that only correct data are
loaded.  This potential for data corruption is why only superusers can
disable system triggers.

Leave the foreign key triggers enabled while you load data.

Yours,
Laurenz Albe



Thank you, Laurenz, for your valuable feedback. If the enabling system trigger doesn’t check the data integrity with the parent table, then this syntax (ALTER TABLE DISABLE/ENABLE TRIGGER ALL) should be removed from PostgreSQL. Otherwise, it will create confusion for the users.


On Thu, Jun 20, 2024 at 5:40 PM Laurenz Albe <laurenz.albe@cybertec.at> wrote:
On Thu, 2024-06-20 at 09:10 +0000, PG Bug reporting form wrote:
> I come up with a weird issue in the PostgreSQL 15.6 version. To update the
> data in the tables I generally disable all triggers on the table using the
> superuser privilege and enable the after the data copy to avoid the checking
> the parent and child tables sequence. During the enable process FK
> constraint should check the data integrity between the child and parent
> tables. but somehow this is skipped and trigger enabled without any issue
> and later during the data restoration this issue is identified.

You wish that enabling the triggers would check the foreign key constraint,
but it doesn't.  All it does is prevent new foreign key violations from
happening.

If you disable the system triggers that implement the foreign key, all
bets are off, and you have to make sure that only correct data are
loaded.  This potential for data corruption is why only superusers can
disable system triggers.

Leave the foreign key triggers enabled while you load data.

Yours,
Laurenz Albe


--


Thanks and Regards,
Mujjamil Khureshi
On 2024-Jun-20, Mujjamil k wrote:

> Thank you, Laurenz, for your valuable feedback. If the enabling system
> trigger doesn’t check the data integrity with the parent table, then this
> syntax (ALTER TABLE DISABLE/ENABLE TRIGGER *ALL*) should be removed from
> PostgreSQL. Otherwise, it will create confusion for the users.

Well, we're not doing that.

This is documented; the page
https://www.postgresql.org/docs/devel/sql-altertable.html
contains, under "DISABLE/ENABLE [ REPLICA | ALWAYS ] TRIGGER", the
following very explicit text:

: Disabling or enabling internally generated constraint triggers requires
: superuser privileges; it should be done with caution since of course the
: integrity of the constraint cannot be guaranteed if the triggers are not
: executed.

There's a reason you were forced to become superuser in order to do
this.  Anytime you're forced to become superuser, you should take a step
back and think carefully what you're doing.


I think it's not a bad idea to suggest that we could have a new command
ALTER TABLE .. DISABLE CONSTRAINT, which can be run by the table owner
and paired with a later ALTER TABLE ENABLE CONSTRAINT, which verifies
the constraint.

-- 
Álvaro Herrera               48°01'N 7°57'E  —  https://www.EnterpriseDB.com/
"Entristecido, Wutra                     (canción de Las Barreras)
echa a Freyr a rodar
y a nosotros al mar"



Thank you Alvaro for your feedback. 

Your suggestion of introducing a new command, ALTER TABLE .. DISABLE CONSTRAINT, which can be run by the table owner and paired with a later ALTER TABLE ENABLE CONSTRAINT, is a thoughtful approach. It would allow for more controlled management of constraints while maintaining data integrity.

On Thu, Jun 20, 2024 at 6:37 PM Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
On 2024-Jun-20, Mujjamil k wrote:

> Thank you, Laurenz, for your valuable feedback. If the enabling system
> trigger doesn’t check the data integrity with the parent table, then this
> syntax (ALTER TABLE DISABLE/ENABLE TRIGGER *ALL*) should be removed from
> PostgreSQL. Otherwise, it will create confusion for the users.

Well, we're not doing that.

This is documented; the page
https://www.postgresql.org/docs/devel/sql-altertable.html
contains, under "DISABLE/ENABLE [ REPLICA | ALWAYS ] TRIGGER", the
following very explicit text:

: Disabling or enabling internally generated constraint triggers requires
: superuser privileges; it should be done with caution since of course the
: integrity of the constraint cannot be guaranteed if the triggers are not
: executed.

There's a reason you were forced to become superuser in order to do
this.  Anytime you're forced to become superuser, you should take a step
back and think carefully what you're doing.


I think it's not a bad idea to suggest that we could have a new command
ALTER TABLE .. DISABLE CONSTRAINT, which can be run by the table owner
and paired with a later ALTER TABLE ENABLE CONSTRAINT, which verifies
the constraint.

--
Álvaro Herrera               48°01'N 7°57'E  —  https://www.EnterpriseDB.com/
"Entristecido, Wutra                     (canción de Las Barreras)
echa a Freyr a rodar
y a nosotros al mar"
Alvaro Herrera <alvherre@alvh.no-ip.org> writes:
> I think it's not a bad idea to suggest that we could have a new command
> ALTER TABLE .. DISABLE CONSTRAINT, which can be run by the table owner
> and paired with a later ALTER TABLE ENABLE CONSTRAINT, which verifies
> the constraint.

The existing, supported way to do this is to drop the foreign key
constraint and then later re-create it.

As noted, if you mess with the triggers directly then any subsequent
breakage is your responsibility.

            regards, tom lane