Re: simplifying foreign key/RI checks

Alvaro Herrera <alvherre@2ndquadrant.com> writes:
> I think we (I) should definitely pursue fixing whatever was broken by
> DETACH CONCURRENTLY, back to pg14, independently of this patch ...  but
> I would appreciate some insight into what the problem is.

Here's what I'm on about:

regression=# create table pk (f1 int primary key);
CREATE TABLE
regression=# insert into pk values(1);
INSERT 0 1
regression=# create table fk (f1 int references pk);
CREATE TABLE
regression=# begin isolation level repeatable read ;
BEGIN
regression=*# select * from pk;  -- to establish xact snapshot
 f1
----
  1
(1 row)

now, in another session, do:

regression=# insert into pk values(2);
INSERT 0 1

back at the RR transaction, we can't see that:

regression=*# select * from pk;  -- still no row 2
 f1
----
  1
(1 row)

so we get:

regression=*# insert into fk values(1);
INSERT 0 1
regression=*# insert into fk values(2);
ERROR:  insert or update on table "fk" violates foreign key constraint "fk_f1_fkey"
DETAIL:  Key (f1)=(2) is not present in table "pk".

IMO that behavior is correct.  If you use READ COMMITTED, then
SELECT can see row 2 as soon as it's committed, and so can the
FK check, and again that's correct.

In v13, the behavior is the same if "pk" is a partitioned table instead
of a plain one.  In HEAD, it's not:

regression=# drop table pk, fk;
DROP TABLE
regression=# create table pk (f1 int primary key) partition by list(f1);
CREATE TABLE
regression=# create table pk1 partition of pk for values in (1,2);
CREATE TABLE
regression=# insert into pk values(1);
INSERT 0 1
regression=# create table fk (f1 int references pk);
CREATE TABLE
regression=# begin isolation level repeatable read ;
BEGIN
regression=*# select * from pk;  -- to establish xact snapshot
 f1
----
  1
(1 row)

--- now insert row 2 in another session

regression=*# select * from pk;  -- still no row 2
 f1
----
  1
(1 row)

regression=*# insert into fk values(1);
INSERT 0 1
regression=*# insert into fk values(2);
INSERT 0 1
regression=*#

So I say that's busted, and the cause is this hunk from 71f4c8c6f:

@@ -392,11 +392,15 @@ RI_FKey_check(TriggerData *trigdata)

     /*
      * Now check that foreign key exists in PK table
+     *
+     * XXX detectNewRows must be true when a partitioned table is on the
+     * referenced side.  The reason is that our snapshot must be fresh
+     * in order for the hack in find_inheritance_children() to work.
      */
     ri_PerformCheck(riinfo, &qkey, qplan,
                     fk_rel, pk_rel,
                     NULL, newslot,
-                    false,
+                    pk_rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE,
                     SPI_OK_SELECT);

     if (SPI_finish() != SPI_OK_FINISH)

I think you need some signalling mechanism that's less global than
ActiveSnapshot to tell the partition-lookup machinery what to do
in this context.

            regards, tom lane