Re: simplifying foreign key/RI checks

Alvaro Herrera <alvherre(at)2ndquadrant(dot)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