Unnecessary checks for new rows by some RI trigger functions? - Mailing list pgsql-hackers

From Antonin Houska
Subject Unnecessary checks for new rows by some RI trigger functions?
Date
Msg-id 22587.1550672960@localhost
Whole thread Raw
Responses Re: Unnecessary checks for new rows by some RI trigger functions?
List pgsql-hackers
When reviewing a related patch [1], I spent some time thinking about the
"detectNewRows" argument of ri_PerformCheck(). My understanding is that it
should (via the es_crosscheck_snapshot field of EState) prevent the executor
from accidentally deleting PK value that another transaction (whose data
changes the trigger's transaction does not see) might need.

However I find it confusing that the trigger functions pass detectNewRows=true
even if they only execute SELECT statement. I think that in these cases the
trigger functions 1) detect themselves that another transaction inserted
row(s) referencing the PK whose deletion is being checked, 2) do not try to
delete the PK anyway. Furthermore (AFAICS), only heap_update() and
heap_delete() functions receive the crosscheck snapshot, so ri_PerformCheck()
does not have to pass the crosscheck snapshot to SPI_execute_snapshot() for
SELECT queries.

Following is patch proposal. Is anything wrong about that?

diff --git a/src/backend/utils/adt/ri_triggers.c b/src/backend/utils/adt/ri_triggers.c
index e1aa3d0044..e235ad9cd0 100644
--- a/src/backend/utils/adt/ri_triggers.c
+++ b/src/backend/utils/adt/ri_triggers.c
@@ -574,7 +574,7 @@ ri_Check_Pk_Match(Relation pk_rel, Relation fk_rel,
     result = ri_PerformCheck(riinfo, &qkey, qplan,
                              fk_rel, pk_rel,
                              old_row, NULL,
-                             true,    /* treat like update */
+                             false,
                              SPI_OK_SELECT);

     if (SPI_finish() != SPI_OK_FINISH)
@@ -802,7 +802,7 @@ ri_restrict(TriggerData *trigdata, bool is_no_action)
             ri_PerformCheck(riinfo, &qkey, qplan,
                             fk_rel, pk_rel,
                             old_row, NULL,
-                            true,    /* must detect new rows */
+                            false,
                             SPI_OK_SELECT);

             if (SPI_finish() != SPI_OK_FINISH)


[1] https://commitfest.postgresql.org/22/1975/

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


pgsql-hackers by date:

Previous
From: Alvaro Herrera
Date:
Subject: Re: crash in pg_identify_object_as_address
Next
From: Arthur Zakirov
Date:
Subject: Re: [PROPOSAL] Shared Ispell dictionaries