pg.ddx.io pgsql-hackers@postgresql.org mailing list archive
help / color / mirror / Atom feedForeign key validation failure in 18beta1
19+ messages / 5 participants
[nested] [flat]
Commits from this thread:
2025-06-05 e6f98d8848 Avoid bogus scans of partitions when marking FKs enforced
2025-06-05 cc733ed164 Avoid bogus scans of partitions when validating FKs to partitioned tables
* Foreign key validation failure in 18beta1
@ 2025-05-28 07:51 Antonin Houska <ah@cybertec.at>
0 siblings, 1 reply; 19+ messages in thread
From: Antonin Houska @ 2025-05-28 07:51 UTC (permalink / raw)
To: pgsql-hackers@lists.postgresql.org
I've come across an unexpected ERROR during validation of FK constraint in PG
18beta1. The same works in PG 17:
drop table if exists fk;
drop table if exists pk;
create table pk(i int primary key) partition by range (i);
create table pk_1 partition of pk for values from (0) to (1);
create table pk_2 partition of pk for values from (1) to (2);
insert into pk values (0), (1);
create table fk(i int);
insert into fk values (1);
-- Works if the FK constraint is created as valid.
--alter table fk add foreign key(i) references pk;
-- Fails if the FK constraint is created as NOT VALID and validated
-- afterwards.
alter table fk add foreign key(i) references pk not valid;
alter table fk validate constraint fk_i_fkey;
--
Antonin Houska
Web: https://www.cybertec-postgresql.com
^ permalink raw reply [nested|flat] 19+ messages in thread
* Re: Foreign key validation failure in 18beta1
@ 2025-05-28 10:54 Tender Wang <tndrwang@gmail.com>
parent: Antonin Houska <ah@cybertec.at>
0 siblings, 1 reply; 19+ messages in thread
From: Tender Wang @ 2025-05-28 10:54 UTC (permalink / raw)
To: Antonin Houska <ah@cybertec.at>; +Cc: pgsql-hackers@lists.postgresql.org, Alvaro Herrera <alvherre@alvh.no-ip.org>
Antonin Houska <ah@cybertec.at> 于2025年5月28日周三 15:51写道：
> I've come across an unexpected ERROR during validation of FK constraint in
> PG
> 18beta1. The same works in PG 17:
>
> drop table if exists fk;
> drop table if exists pk;
> create table pk(i int primary key) partition by range (i);
> create table pk_1 partition of pk for values from (0) to (1);
> create table pk_2 partition of pk for values from (1) to (2);
> insert into pk values (0), (1);
> create table fk(i int);
> insert into fk values (1);
>
> -- Works if the FK constraint is created as valid.
> --alter table fk add foreign key(i) references pk;
>
> -- Fails if the FK constraint is created as NOT VALID and validated
> -- afterwards.
> alter table fk add foreign key(i) references pk not valid;
> alter table fk validate constraint fk_i_fkey;
>
git bisect shows since below commit, the failure started.
commit b663b9436e7509b5e73c8c372539f067cd6e66c1
Author: Álvaro Herrera <alvherre@alvh.no-ip.org>
Date: Thu Jan 23 15:54:38 2025 +0100
Allow NOT VALID foreign key constraints on partitioned tables
--
Thanks,
Tender Wang
^ permalink raw reply [nested|flat] 19+ messages in thread
* Re: Foreign key validation failure in 18beta1
@ 2025-05-28 12:12 Tender Wang <tndrwang@gmail.com>
parent: Tender Wang <tndrwang@gmail.com>
0 siblings, 1 reply; 19+ messages in thread
From: Tender Wang @ 2025-05-28 12:12 UTC (permalink / raw)
To: Alvaro Herrera <alvherre@alvh.no-ip.org>; +Cc: pgsql-hackers@lists.postgresql.org, Antonin Houska <ah@cybertec.at>
Tender Wang <tndrwang@gmail.com> 于2025年5月28日周三 18:54写道：
>
>
> Antonin Houska <ah@cybertec.at> 于2025年5月28日周三 15:51写道：
>
>> I've come across an unexpected ERROR during validation of FK constraint
>> in PG
>> 18beta1. The same works in PG 17:
>>
>> drop table if exists fk;
>> drop table if exists pk;
>> create table pk(i int primary key) partition by range (i);
>> create table pk_1 partition of pk for values from (0) to (1);
>> create table pk_2 partition of pk for values from (1) to (2);
>> insert into pk values (0), (1);
>> create table fk(i int);
>> insert into fk values (1);
>>
>> -- Works if the FK constraint is created as valid.
>> --alter table fk add foreign key(i) references pk;
>>
>> -- Fails if the FK constraint is created as NOT VALID and validated
>> -- afterwards.
>> alter table fk add foreign key(i) references pk not valid;
>> alter table fk validate constraint fk_i_fkey;
>>
>
> git bisect shows since below commit, the failure started.
>
> commit b663b9436e7509b5e73c8c372539f067cd6e66c1
> Author: Álvaro Herrera <alvherre@alvh.no-ip.org>
> Date: Thu Jan 23 15:54:38 2025 +0100
>
> Allow NOT VALID foreign key constraints on partitioned tables
>
>
I dided the codes, in QueueFKConstraintValidation(), we add three
newconstraint for the
fk rel, because the pk rel is partition table.
During phase 3 of AlterTable, in ATRewriteTables(),
call validateForeignKeyConstraint() three times.
The first time the pk rel is pk, and it's ok.
The second time the pk rel is only pk_1, and the type(1) is not in pk_1, so
an error is reported.
In this case, the two children newconstraint should not be added to the
queue.
--
Thanks,
Tender Wang
^ permalink raw reply [nested|flat] 19+ messages in thread
* Re: Foreign key validation failure in 18beta1
@ 2025-05-28 12:26 Alvaro Herrera <alvherre@alvh.no-ip.org>
parent: Tender Wang <tndrwang@gmail.com>
0 siblings, 1 reply; 19+ messages in thread
From: Alvaro Herrera @ 2025-05-28 12:26 UTC (permalink / raw)
To: Tender Wang <tndrwang@gmail.com>; +Cc: pgsql-hackers@lists.postgresql.org, Antonin Houska <ah@cybertec.at>; Amul Sul <sulamul@gmail.com>
On 2025-May-28, Tender Wang wrote:
> I dided the codes, in QueueFKConstraintValidation(), we add three
> newconstraint for the
> fk rel, because the pk rel is partition table.
>
> During phase 3 of AlterTable, in ATRewriteTables(),
> call validateForeignKeyConstraint() three times.
> The first time the pk rel is pk, and it's ok.
> The second time the pk rel is only pk_1, and the type(1) is not in pk_1, so
> an error is reported.
>
> In this case, the two children newconstraint should not be added to the
> queue.
Yeah, I reached the same conclusion and this is the preliminary fix I
had written for it. I don't like that I had to duplicate a few lines of
code, but maybe it's not too bad. Also the comments need to be
clarified a bit more.
Clearly b663b9436e75 lacked tests.
--
Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/
"Sallah, I said NO camels! That's FIVE camels; can't you count?"
(Indiana Jones)
Attachments:
0001-Don-t-validate-referenced-side-FK-subconstraints.patch (text/x-diff, 2.2K) download
From 4c11da193991b95de8baf7689a9938ca9bb171aa Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C3=81lvaro=20Herrera?= <alvherre@kurilemu.de>
Date: Wed, 28 May 2025 14:22:58 +0200
Subject: [PATCH] Don't validate referenced-side FK subconstraints
Silly bug in b663b9436e75.
Reported-by: Antonin Houska <ah@cybertec.at>
Discussion: https://postgr.es/m/26983.1748418675@localhost
---
src/backend/commands/tablecmds.c | 24 ++++++++++++++++++++++--
1 file changed, 22 insertions(+), 2 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 54ad38247aa..581c59f117b 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -12988,8 +12988,10 @@ QueueFKConstraintValidation(List **wqueue, Relation conrel, Relation rel,
}
/*
- * If the table at either end of the constraint is partitioned, we need to
- * recurse and handle every constraint that is a child of this constraint.
+ * If the referencing table is partitioned, we need to recurse and handle
+ * every constraint that is a child of this constraint. Also update the
+ * sub-constraints on the referenced side of the constraint to appear as
+ * validated; no actual validation is needed for them.
*/
if (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE ||
get_rel_relkind(con->confrelid) == RELKIND_PARTITIONED_TABLE)
@@ -13021,6 +13023,24 @@ QueueFKConstraintValidation(List **wqueue, Relation conrel, Relation rel,
if (childcon->convalidated)
continue;
+ /*
+ * For child constraints in the referenced side of the constraint,
+ * we don't need to do any actual validation, but we do need to
+ * update the status to validated.
+ */
+ if (childcon->confrelid != con->confrelid)
+ {
+ copyTuple = heap_copytuple(childtup);
+ copy_con = (Form_pg_constraint) GETSTRUCT(copyTuple);
+ copy_con->convalidated = true;
+ CatalogTupleUpdate(conrel, ©Tuple->t_self, copyTuple);
+
+ InvokeObjectPostAlterHook(ConstraintRelationId, con->oid, 0);
+
+ heap_freetuple(copyTuple);
+ continue;
+ }
+
childrel = table_open(childcon->conrelid, lockmode);
QueueFKConstraintValidation(wqueue, conrel, childrel, childtup,
--
2.39.5
* Re: Foreign key validation failure in 18beta1
@ 2025-05-28 12:38 Tender Wang <tndrwang@gmail.com>
parent: Alvaro Herrera <alvherre@alvh.no-ip.org>
0 siblings, 2 replies; 19+ messages in thread
From: Tender Wang @ 2025-05-28 12:38 UTC (permalink / raw)
To: Alvaro Herrera <alvherre@alvh.no-ip.org>; +Cc: pgsql-hackers@lists.postgresql.org, Antonin Houska <ah@cybertec.at>; Amul Sul <sulamul@gmail.com>
Alvaro Herrera <alvherre@alvh.no-ip.org> 于2025年5月28日周三 20:26写道：
> On 2025-May-28, Tender Wang wrote:
>
> > I dided the codes, in QueueFKConstraintValidation(), we add three
> > newconstraint for the
> > fk rel, because the pk rel is partition table.
> >
> > During phase 3 of AlterTable, in ATRewriteTables(),
> > call validateForeignKeyConstraint() three times.
> > The first time the pk rel is pk, and it's ok.
> > The second time the pk rel is only pk_1, and the type(1) is not in pk_1,
> so
> > an error is reported.
> >
> > In this case, the two children newconstraint should not be added to the
> > queue.
>
> Yeah, I reached the same conclusion and this is the preliminary fix I
> had written for it. I don't like that I had to duplicate a few lines of
> code, but maybe it's not too bad. Also the comments need to be
> clarified a bit more.
>
If the child table is still a partitioned table, the patch seems not work.
I figure out a quick fix as the attached. I add a bool argument into
the QueueFKConstraintValidation().
If it is true, it means we recursively call QueueFKConstraintValidation(),
then we don't add the newconstraint to the queue.
I'm not sure about this fix. Any thoughts?
--
Thanks,
Tender Wang
Attachments:
fk_val.diff (text/plain, 2.5K) download
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 54ad38247aa..e11242e9b69 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -431,7 +431,7 @@ static ObjectAddress ATExecValidateConstraint(List **wqueue,
Relation rel, char *constrName,
bool recurse, bool recursing, LOCKMODE lockmode);
static void QueueFKConstraintValidation(List **wqueue, Relation conrel, Relation rel,
- HeapTuple contuple, LOCKMODE lockmode);
+ HeapTuple contuple, LOCKMODE lockmode, bool recursing);
static void QueueCheckConstraintValidation(List **wqueue, Relation conrel, Relation rel,
char *constrName, HeapTuple contuple,
bool recurse, bool recursing, LOCKMODE lockmode);
@@ -11867,7 +11867,7 @@ AttachPartitionForeignKey(List **wqueue,
/* Use the same lock as for AT_ValidateConstraint */
QueueFKConstraintValidation(wqueue, conrel, partition, partcontup,
- ShareUpdateExclusiveLock);
+ ShareUpdateExclusiveLock, false);
ReleaseSysCache(partcontup);
table_close(conrel, RowExclusiveLock);
}
@@ -12919,7 +12919,7 @@ ATExecValidateConstraint(List **wqueue, Relation rel, char *constrName,
{
if (con->contype == CONSTRAINT_FOREIGN)
{
- QueueFKConstraintValidation(wqueue, conrel, rel, tuple, lockmode);
+ QueueFKConstraintValidation(wqueue, conrel, rel, tuple, lockmode, false);
}
else if (con->contype == CONSTRAINT_CHECK)
{
@@ -12953,7 +12953,7 @@ ATExecValidateConstraint(List **wqueue, Relation rel, char *constrName,
*/
static void
QueueFKConstraintValidation(List **wqueue, Relation conrel, Relation rel,
- HeapTuple contuple, LOCKMODE lockmode)
+ HeapTuple contuple, LOCKMODE lockmode, bool recursing)
{
Form_pg_constraint con;
AlteredTableInfo *tab;
@@ -12983,8 +12983,11 @@ QueueFKConstraintValidation(List **wqueue, Relation conrel, Relation rel,
newcon->qual = (Node *) fkconstraint;
/* Find or create work queue entry for this table */
- tab = ATGetQueueEntry(wqueue, rel);
- tab->constraints = lappend(tab->constraints, newcon);
+ if (!recursing)
+ {
+ tab = ATGetQueueEntry(wqueue, rel);
+ tab->constraints = lappend(tab->constraints, newcon);
+ }
}
/*
@@ -13024,7 +13027,7 @@ QueueFKConstraintValidation(List **wqueue, Relation conrel, Relation rel,
childrel = table_open(childcon->conrelid, lockmode);
QueueFKConstraintValidation(wqueue, conrel, childrel, childtup,
- lockmode);
+ lockmode, true);
table_close(childrel, NoLock);
}
* Re: Foreign key validation failure in 18beta1
@ 2025-05-29 04:35 Tender Wang <tndrwang@gmail.com>
parent: Tender Wang <tndrwang@gmail.com>
1 sibling, 0 replies; 19+ messages in thread
From: Tender Wang @ 2025-05-29 04:35 UTC (permalink / raw)
To: Alvaro Herrera <alvherre@alvh.no-ip.org>; +Cc: pgsql-hackers@lists.postgresql.org, Antonin Houska <ah@cybertec.at>; Amul Sul <sulamul@gmail.com>
Tender Wang <tndrwang@gmail.com> 于2025年5月28日周三 20:38写道：
>
>
> Alvaro Herrera <alvherre@alvh.no-ip.org> 于2025年5月28日周三 20:26写道：
>
>> On 2025-May-28, Tender Wang wrote:
>>
>> > I dided the codes, in QueueFKConstraintValidation(), we add three
>> > newconstraint for the
>> > fk rel, because the pk rel is partition table.
>> >
>> > During phase 3 of AlterTable, in ATRewriteTables(),
>> > call validateForeignKeyConstraint() three times.
>> > The first time the pk rel is pk, and it's ok.
>> > The second time the pk rel is only pk_1, and the type(1) is not in
>> pk_1, so
>> > an error is reported.
>> >
>> > In this case, the two children newconstraint should not be added to the
>> > queue.
>>
>> Yeah, I reached the same conclusion and this is the preliminary fix I
>> had written for it. I don't like that I had to duplicate a few lines of
>> code, but maybe it's not too bad. Also the comments need to be
>> clarified a bit more.
>>
>
> If the child table is still a partitioned table, the patch seems not work.
>
I found a case that proves what I said above.
create table pk(i int, j int, primary key(i,j)) partition by range (i);
create table pk_1 partition of pk for values from (0) to (1) partition by
list(j);
create table pk_2 partition of pk for values from (1) to (2) partition by
list(j);
create table pk_1_1 partition of pk_1 for values in (1);
create table pk_2_1 partition of pk_2 for values in (2);
create table fk(i int, j int);
alter table fk add foreign key(i, j) references pk not valid;
postgres=# select oid ,conname , contype,
convalidated,conrelid,conparentid,confrelid from pg_constraint where oid >=
16384;
oid | conname | contype | convalidated | conrelid | conparentid |
confrelid
-------+---------------+---------+--------------+----------+-------------+-----------
16422 | fk_i_j_fkey | f | f | 16419 |
0 | 16384
16425 | fk_i_j_fkey_1 | f | f | 16419 | 16422 |
16391
16428 | fk_i_j_fkey_2 | f | f | 16419 | 16425 |
16405
16431 | fk_i_j_fkey_3 | f | f | 16419 | 16422 |
16398
16434 | fk_i_j_fkey_4 | f | f | 16419 | 16431 |
16412
alter table fk validate constraint fk_i_j_fkey;
postgres=# select oid ,conname , contype,
convalidated,conrelid,conparentid,confrelid from pg_constraint where oid >=
16384;
oid | conname | contype | convalidated | conrelid | conparentid |
confrelid
-------+---------------+---------+--------------+----------+-------------+-----------
16428 | fk_i_j_fkey_2 | f | f | 16419 | 16425 |
16405
16434 | fk_i_j_fkey_4 | f | f | 16419 | 16431 |
16412
16425 | fk_i_j_fkey_1 | f | t | 16419 | 16422 |
16391
16431 | fk_i_j_fkey_3 | f | t | 16419 | 16422 |
16398
16422 | fk_i_j_fkey | f | t | 16419 |
0 | 16384
The fk_i_j_fkey_2 and fk_i_j_fkey_4 are still invalid with your patch.
> I figure out a quick fix as the attached. I add a bool argument into
> the QueueFKConstraintValidation().
> If it is true, it means we recursively call QueueFKConstraintValidation(),
> then we don't add the newconstraint to the queue.
>
> I'm not sure about this fix. Any thoughts?
>
--
Thanks,
Tender Wang
^ permalink raw reply [nested|flat] 19+ messages in thread
* Re: Foreign key validation failure in 18beta1
@ 2025-05-29 07:08 jian he <jian.universality@gmail.com>
parent: Tender Wang <tndrwang@gmail.com>
1 sibling, 1 reply; 19+ messages in thread
From: jian he @ 2025-05-29 07:08 UTC (permalink / raw)
To: Tender Wang <tndrwang@gmail.com>; +Cc: Alvaro Herrera <alvherre@alvh.no-ip.org>; pgsql-hackers@lists.postgresql.org, Antonin Houska <ah@cybertec.at>; Amul Sul <sulamul@gmail.com>
On Wed, May 28, 2025 at 8:38 PM Tender Wang <tndrwang@gmail.com> wrote:
>
>
>
> Alvaro Herrera <alvherre@alvh.no-ip.org> 于2025年5月28日周三 20:26写道：
>>
>> On 2025-May-28, Tender Wang wrote:
>>
>> > I dided the codes, in QueueFKConstraintValidation(), we add three
>> > newconstraint for the
>> > fk rel, because the pk rel is partition table.
>> >
>> > During phase 3 of AlterTable, in ATRewriteTables(),
>> > call validateForeignKeyConstraint() three times.
>> > The first time the pk rel is pk, and it's ok.
>> > The second time the pk rel is only pk_1, and the type(1) is not in pk_1, so
>> > an error is reported.
>> >
>> > In this case, the two children newconstraint should not be added to the
>> > queue.
>>
>> Yeah, I reached the same conclusion and this is the preliminary fix I
>> had written for it. I don't like that I had to duplicate a few lines of
>> code, but maybe it's not too bad. Also the comments need to be
>> clarified a bit more.
>
>
> If the child table is still a partitioned table, the patch seems not work.
>
> I figure out a quick fix as the attached. I add a bool argument into the QueueFKConstraintValidation().
> If it is true, it means we recursively call QueueFKConstraintValidation(), then we don't add the newconstraint to the queue.
>
> I'm not sure about this fix. Any thoughts?
it will fail for the following case:
begin;
drop table if exists fk;
drop table if exists pk;
create table pk(i int primary key);
insert into pk values (0), (1);
create table fk(i int) partition by range (i);
create table fk_1 partition of fk for values from (0) to (1);
create table fk_2 partition of fk for values from (1) to (3);
insert into fk values (1),(2);
alter table fk add foreign key(i) references pk not valid;
commit;
alter table fk validate constraint fk_i_fkey; --error, should fail.
-----------
The attached *draft* patch is based on your idea.
The idea is that we only need to conditionally do
``tab->constraints = lappend(tab->constraints, newcon);`` within
QueueFKConstraintValidation.
but the catalog update needs to be done recursively.
Attachments:
v2-0001-foreign_key_validation.diff (text/x-patch, 3.2K) download
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 54ad38247aa..df8e02f1060 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -431,7 +431,7 @@ static ObjectAddress ATExecValidateConstraint(List **wqueue,
Relation rel, char *constrName,
bool recurse, bool recursing, LOCKMODE lockmode);
static void QueueFKConstraintValidation(List **wqueue, Relation conrel, Relation rel,
- HeapTuple contuple, LOCKMODE lockmode);
+ HeapTuple contuple, LOCKMODE lockmode, bool recursing);
static void QueueCheckConstraintValidation(List **wqueue, Relation conrel, Relation rel,
char *constrName, HeapTuple contuple,
bool recurse, bool recursing, LOCKMODE lockmode);
@@ -11867,7 +11867,7 @@ AttachPartitionForeignKey(List **wqueue,
/* Use the same lock as for AT_ValidateConstraint */
QueueFKConstraintValidation(wqueue, conrel, partition, partcontup,
- ShareUpdateExclusiveLock);
+ ShareUpdateExclusiveLock, false);
ReleaseSysCache(partcontup);
table_close(conrel, RowExclusiveLock);
}
@@ -12919,7 +12919,7 @@ ATExecValidateConstraint(List **wqueue, Relation rel, char *constrName,
{
if (con->contype == CONSTRAINT_FOREIGN)
{
- QueueFKConstraintValidation(wqueue, conrel, rel, tuple, lockmode);
+ QueueFKConstraintValidation(wqueue, conrel, rel, tuple, lockmode, false);
}
else if (con->contype == CONSTRAINT_CHECK)
{
@@ -12953,18 +12953,23 @@ ATExecValidateConstraint(List **wqueue, Relation rel, char *constrName,
*/
static void
QueueFKConstraintValidation(List **wqueue, Relation conrel, Relation rel,
- HeapTuple contuple, LOCKMODE lockmode)
+ HeapTuple contuple, LOCKMODE lockmode, bool recursing)
{
Form_pg_constraint con;
AlteredTableInfo *tab;
HeapTuple copyTuple;
Form_pg_constraint copy_con;
+ bool pk_is_partitioned;
+ bool fk_is_partitioned;
con = (Form_pg_constraint) GETSTRUCT(contuple);
Assert(con->contype == CONSTRAINT_FOREIGN);
Assert(!con->convalidated);
- if (rel->rd_rel->relkind == RELKIND_RELATION)
+ pk_is_partitioned = (get_rel_relkind(con->confrelid) == RELKIND_PARTITIONED_TABLE);
+ fk_is_partitioned = (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE);
+
+ if (rel->rd_rel->relkind == RELKIND_RELATION && !recursing)
{
NewConstraint *newcon;
Constraint *fkconstraint;
@@ -12991,8 +12996,7 @@ QueueFKConstraintValidation(List **wqueue, Relation conrel, Relation rel,
* If the table at either end of the constraint is partitioned, we need to
* recurse and handle every constraint that is a child of this constraint.
*/
- if (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE ||
- get_rel_relkind(con->confrelid) == RELKIND_PARTITIONED_TABLE)
+ if (pk_is_partitioned || fk_is_partitioned)
{
ScanKeyData pkey;
SysScanDesc pscan;
@@ -13024,7 +13028,7 @@ QueueFKConstraintValidation(List **wqueue, Relation conrel, Relation rel,
childrel = table_open(childcon->conrelid, lockmode);
QueueFKConstraintValidation(wqueue, conrel, childrel, childtup,
- lockmode);
+ lockmode, fk_is_partitioned ? false : true);
table_close(childrel, NoLock);
}
* Re: Foreign key validation failure in 18beta1
@ 2025-05-29 12:12 Amul Sul <sulamul@gmail.com>
parent: jian he <jian.universality@gmail.com>
0 siblings, 1 reply; 19+ messages in thread
From: Amul Sul @ 2025-05-29 12:12 UTC (permalink / raw)
To: jian he <jian.universality@gmail.com>; +Cc: Tender Wang <tndrwang@gmail.com>; Alvaro Herrera <alvherre@alvh.no-ip.org>; pgsql-hackers@lists.postgresql.org, Antonin Houska <ah@cybertec.at>
On Thu, May 29, 2025 at 12:38 PM jian he <jian.universality@gmail.com> wrote:
>
> On Wed, May 28, 2025 at 8:38 PM Tender Wang <tndrwang@gmail.com> wrote:
> >
> >
> >
> > Alvaro Herrera <alvherre@alvh.no-ip.org> 于2025年5月28日周三 20:26写道：
> >>
> >> On 2025-May-28, Tender Wang wrote:
> >>
> >> > [...]
> The attached *draft* patch is based on your idea.
>
> The idea is that we only need to conditionally do
> ``tab->constraints = lappend(tab->constraints, newcon);`` within
> QueueFKConstraintValidation.
> but the catalog update needs to be done recursively.
I like this approach, but I don’t think the flag name "recursing" is
appropriate, as the flag is meant to indicate whether we want to
enqueue constraints for validation or not.
Additionally, I noticed that we can skip opening child partitions
inside QueueFKConstraintValidation() when the referencing table is not
partitioned -- that is, when it is the same as the input rel. Based on
that, we can decide whether to queue the validation. However, I am not
sure this optimization is worth the added complexity in the code.
I have tried this in the attached patch (not the final version), with
a brief explanation in the code comments. If we choose to move forward
with this patch, I am happy to refine it and add proper tests.
Regards,
Amul
Attachments:
v3-0001-foreign_key_validation-WIP.patch (application/x-patch, 4.2K) download
From b2aaecf0df3c2ea15a84150ef0a91329587e4f20 Mon Sep 17 00:00:00 2001
From: Amul Sul <sulamul@gmail.com>
Date: Thu, 29 May 2025 17:22:44 +0530
Subject: [PATCH] POC - FK validation fix
---
src/backend/commands/tablecmds.c | 50 +++++++++++++++++++++++++++-----
1 file changed, 42 insertions(+), 8 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 54ad38247aa..f6172618abd 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -431,7 +431,8 @@ static ObjectAddress ATExecValidateConstraint(List **wqueue,
Relation rel, char *constrName,
bool recurse, bool recursing, LOCKMODE lockmode);
static void QueueFKConstraintValidation(List **wqueue, Relation conrel, Relation rel,
- HeapTuple contuple, LOCKMODE lockmode);
+ HeapTuple contuple, bool queueValidation,
+ LOCKMODE lockmode);
static void QueueCheckConstraintValidation(List **wqueue, Relation conrel, Relation rel,
char *constrName, HeapTuple contuple,
bool recurse, bool recursing, LOCKMODE lockmode);
@@ -11867,7 +11868,7 @@ AttachPartitionForeignKey(List **wqueue,
/* Use the same lock as for AT_ValidateConstraint */
QueueFKConstraintValidation(wqueue, conrel, partition, partcontup,
- ShareUpdateExclusiveLock);
+ true, ShareUpdateExclusiveLock);
ReleaseSysCache(partcontup);
table_close(conrel, RowExclusiveLock);
}
@@ -12919,7 +12920,8 @@ ATExecValidateConstraint(List **wqueue, Relation rel, char *constrName,
{
if (con->contype == CONSTRAINT_FOREIGN)
{
- QueueFKConstraintValidation(wqueue, conrel, rel, tuple, lockmode);
+ QueueFKConstraintValidation(wqueue, conrel, rel, tuple, true,
+ lockmode);
}
else if (con->contype == CONSTRAINT_CHECK)
{
@@ -12953,7 +12955,8 @@ ATExecValidateConstraint(List **wqueue, Relation rel, char *constrName,
*/
static void
QueueFKConstraintValidation(List **wqueue, Relation conrel, Relation rel,
- HeapTuple contuple, LOCKMODE lockmode)
+ HeapTuple contuple, bool queueValidation,
+ LOCKMODE lockmode)
{
Form_pg_constraint con;
AlteredTableInfo *tab;
@@ -12964,7 +12967,13 @@ QueueFKConstraintValidation(List **wqueue, Relation conrel, Relation rel,
Assert(con->contype == CONSTRAINT_FOREIGN);
Assert(!con->convalidated);
- if (rel->rd_rel->relkind == RELKIND_RELATION)
+ /*
+ * Sometimes we only want to update the pg_constraint entry without
+ * enqueueing it for validation; in such cases, queueValidation will be
+ * false.
+ */
+ if (queueValidation &&
+ rel->rd_rel->relkind == RELKIND_RELATION)
{
NewConstraint *newcon;
Constraint *fkconstraint;
@@ -13010,6 +13019,7 @@ QueueFKConstraintValidation(List **wqueue, Relation conrel, Relation rel,
{
Form_pg_constraint childcon;
Relation childrel;
+ bool childValidation = true;
childcon = (Form_pg_constraint) GETSTRUCT(childtup);
@@ -13021,11 +13031,35 @@ QueueFKConstraintValidation(List **wqueue, Relation conrel, Relation rel,
if (childcon->convalidated)
continue;
- childrel = table_open(childcon->conrelid, lockmode);
+ /*
+ * When the referenced table is partitioned, the referencing table
+ * may have multiple foreign key constraints -- one for each child
+ * table. These individual constraints do not require separate
+ * validation, as validating the constraint on the root partitioned
+ * table is sufficient.
+ *
+ * In other words, avoid enqueueing the same referencing table for
+ * validation again. However, we still need to update the
+ * constraint validation flags, so recursion is necessary.
+ */
+ if (childcon->conrelid == RelationGetRelid(rel))
+ {
+ /* No need to open it again */
+ childrel = rel;
+ childValidation = false;
+ }
+ else
+ childrel = table_open(childcon->conrelid, lockmode);
QueueFKConstraintValidation(wqueue, conrel, childrel, childtup,
- lockmode);
- table_close(childrel, NoLock);
+ childValidation, lockmode);
+
+ /*
+ * We skip opening the table if it is the same as the referencing
+ * table.
+ */
+ if (childcon->conrelid != RelationGetRelid(rel))
+ table_close(childrel, NoLock);
}
systable_endscan(pscan);
--
2.43.5
* Re: Foreign key validation failure in 18beta1
@ 2025-05-29 12:26 jian he <jian.universality@gmail.com>
parent: Amul Sul <sulamul@gmail.com>
0 siblings, 1 reply; 19+ messages in thread
From: jian he @ 2025-05-29 12:26 UTC (permalink / raw)
To: Amul Sul <sulamul@gmail.com>; +Cc: Tender Wang <tndrwang@gmail.com>; Alvaro Herrera <alvherre@alvh.no-ip.org>; pgsql-hackers@lists.postgresql.org, Antonin Houska <ah@cybertec.at>
On Thu, May 29, 2025 at 8:12 PM Amul Sul <sulamul@gmail.com> wrote:
>
> > >> > [...]
> > The attached *draft* patch is based on your idea.
> >
> > The idea is that we only need to conditionally do
> > ``tab->constraints = lappend(tab->constraints, newcon);`` within
> > QueueFKConstraintValidation.
> > but the catalog update needs to be done recursively.
>
> I like this approach, but I don’t think the flag name "recursing" is
> appropriate, as the flag is meant to indicate whether we want to
> enqueue constraints for validation or not.
>
Later, I came up with "need_validate", but it seems "queueValidation"
is better.
I just realized we have the same problem with ALTER FOREIGN KEY ENFORCED.
for example:
begin;
drop table if exists fk;
drop table if exists pk;
create table pk(i int, b int, primary key(i ,b)) partition by range (i);
create table pk_1 partition of pk for values from (0) to (10)
partition by list(b);
CREATE TABLE pk_1_p1 PARTITION OF pk_1 FOR VALUES IN (0, 1, 2);
create table pk_2 partition of pk for values from (10) to (12);
insert into pk values (0, 1), (1,2);
create table fk(i int, b int);
insert into fk values (0, 1), (1,3);
-- alter table fk add foreign key(i) references pk;
alter table fk add constraint fk_i_fkey foreign key(i, b) references
pk not enforced;
commit;
alter table fk alter constraint fk_i_fkey enforced; --error
delete from fk where i = 1 and b = 3;
alter table fk alter constraint fk_i_fkey enforced; --ok
^ permalink raw reply [nested|flat] 19+ messages in thread
* Re: Foreign key validation failure in 18beta1
@ 2025-05-29 12:58 Amul Sul <sulamul@gmail.com>
parent: jian he <jian.universality@gmail.com>
0 siblings, 1 reply; 19+ messages in thread
From: Amul Sul @ 2025-05-29 12:58 UTC (permalink / raw)
To: jian he <jian.universality@gmail.com>; +Cc: Tender Wang <tndrwang@gmail.com>; Alvaro Herrera <alvherre@alvh.no-ip.org>; pgsql-hackers@lists.postgresql.org, Antonin Houska <ah@cybertec.at>
On Thu, May 29, 2025 at 5:57 PM jian he <jian.universality@gmail.com> wrote:
>
> On Thu, May 29, 2025 at 8:12 PM Amul Sul <sulamul@gmail.com> wrote:
> >
> > > >> > [...]
> > > The attached *draft* patch is based on your idea.
> > >
> > > The idea is that we only need to conditionally do
> > > ``tab->constraints = lappend(tab->constraints, newcon);`` within
> > > QueueFKConstraintValidation.
> > > but the catalog update needs to be done recursively.
> >
> > I like this approach, but I don’t think the flag name "recursing" is
> > appropriate, as the flag is meant to indicate whether we want to
> > enqueue constraints for validation or not.
> >
>
> Later, I came up with "need_validate", but it seems "queueValidation"
> is better.
>
> I just realized we have the same problem with ALTER FOREIGN KEY ENFORCED.
> for example:
Yeah, I think adding a "currcon->confrelid == pkrelid" check before
enqueueing validation in ATExecAlterConstrEnforceability() would
address the issue, though I still need to test it thoroughly.
Regards,
Amul
^ permalink raw reply [nested|flat] 19+ messages in thread
* Re: Foreign key validation failure in 18beta1
@ 2025-05-30 08:06 jian he <jian.universality@gmail.com>
parent: Amul Sul <sulamul@gmail.com>
0 siblings, 1 reply; 19+ messages in thread
From: jian he @ 2025-05-30 08:06 UTC (permalink / raw)
To: Amul Sul <sulamul@gmail.com>; +Cc: Tender Wang <tndrwang@gmail.com>; Alvaro Herrera <alvherre@alvh.no-ip.org>; pgsql-hackers@lists.postgresql.org, Antonin Houska <ah@cybertec.at>
On Thu, May 29, 2025 at 8:58 PM Amul Sul <sulamul@gmail.com> wrote:
> >
> > I just realized we have the same problem with ALTER FOREIGN KEY ENFORCED.
> > for example:
>
> Yeah, I think adding a "currcon->confrelid == pkrelid" check before
> enqueueing validation in ATExecAlterConstrEnforceability() would
> address the issue, though I still need to test it thoroughly.
>
adding a "currcon->confrelid == pkrelid" will fix the problem.
I have used the attached scripts to test foreign key functionality:
ALTER TABLE VALIDATE CONSTRAINT and
ALTER TABLE ALTER CONSTRAINT ENFORCED.
v3-0001-foreign_key_validation-WIP also works fine.
but, I have one minor issue with the comment.
+ /*
+ * When the referenced table is partitioned, the referencing table
+ * may have multiple foreign key constraints -- one for each child
+ * table. These individual constraints do not require separate
+ * validation, as validating the constraint on the root partitioned
+ * table is sufficient.
The last sentence doesn't seem to explain why.
The following is what I came up with (my understanding):
""
If the primary key (PK) is partitioned, we *can't* queue validation (add a
NewConstraint on PK partition and validate it in phase3) for its partitions.
Validating foreign key constraints between primary key (PK) partitions and
foreign keys (FK) can fail. This is because some PK partitions may legitimately
lack corresponding FK values, which is acceptable but causes validation errors.
""
Attachments:
scratch3.sql (application/sql, 4.7K) download
^ permalink raw reply [nested|flat] 19+ messages in thread
* Re: Foreign key validation failure in 18beta1
@ 2025-05-30 10:31 Amul Sul <sulamul@gmail.com>
parent: jian he <jian.universality@gmail.com>
0 siblings, 1 reply; 19+ messages in thread
From: Amul Sul @ 2025-05-30 10:31 UTC (permalink / raw)
To: jian he <jian.universality@gmail.com>; +Cc: Tender Wang <tndrwang@gmail.com>; Alvaro Herrera <alvherre@alvh.no-ip.org>; pgsql-hackers@lists.postgresql.org, Antonin Houska <ah@cybertec.at>
On Fri, May 30, 2025 at 1:37 PM jian he <jian.universality@gmail.com> wrote:
>
> On Thu, May 29, 2025 at 8:58 PM Amul Sul <sulamul@gmail.com> wrote:
> > >
> > > I just realized we have the same problem with ALTER FOREIGN KEY ENFORCED.
> > > for example:
> >
> > Yeah, I think adding a "currcon->confrelid == pkrelid" check before
> > enqueueing validation in ATExecAlterConstrEnforceability() would
> > address the issue, though I still need to test it thoroughly.
> >
>
> adding a "currcon->confrelid == pkrelid" will fix the problem.
> I have used the attached scripts to test foreign key functionality:
> ALTER TABLE VALIDATE CONSTRAINT and
> ALTER TABLE ALTER CONSTRAINT ENFORCED.
>
Thanks for the checking.
>
> v3-0001-foreign_key_validation-WIP also works fine.
> but, I have one minor issue with the comment.
>
> + /*
> + * When the referenced table is partitioned, the referencing table
> + * may have multiple foreign key constraints -- one for each child
> + * table. These individual constraints do not require separate
> + * validation, as validating the constraint on the root partitioned
> + * table is sufficient.
>
> The last sentence doesn't seem to explain why.
> The following is what I came up with (my understanding):
> ""
> If the primary key (PK) is partitioned, we *can't* queue validation (add a
> NewConstraint on PK partition and validate it in phase3) for its partitions.
> Validating foreign key constraints between primary key (PK) partitions and
> foreign keys (FK) can fail. This is because some PK partitions may legitimately
> lack corresponding FK values, which is acceptable but causes validation errors.
> ""
I would prefer to use 'referenced' and 'referencing' tables instead of
'PK' and 'FK' tables, to stay consistent with the existing style. I
tried to improve the comment in the attached version with the
appropriate reason.
Kindly take a look at the attached version. I've also added the tests.
Thanks for your script -- all tests are passing with this patch.
Regards,
Amul
Attachments:
v4-0001-Skip-adding-action-based-foreign-key-constraints-.patch (application/octet-stream, 11.4K) download
From 5e4cff70da8872cb03a7391f5016cbd35d1bb9da Mon Sep 17 00:00:00 2001
From: Amul Sul <sulamul@gmail.com>
Date: Fri, 30 May 2025 15:30:21 +0530
Subject: [PATCH v4] Skip adding action-based foreign key constraints to the
phase 3 validation queue.
When the referenced table is partitioned, each partition will have its
own action trigger constraint to handle the appropriate behavior for
UPDATE or DELETE operations. However, these constraints should not be
used to validate data in the referencing table, as the referenced data
may be distributed across multiple partitions. Attempting validation
against a single partition could result in errors. Therefore,
validation of referencing table data should always be performed
against the referenced root partitioned table.
---
src/backend/commands/tablecmds.c | 55 +++++++++++++++++++----
src/test/regress/expected/foreign_key.out | 47 ++++++++++++-------
src/test/regress/sql/foreign_key.sql | 19 +++++---
3 files changed, 90 insertions(+), 31 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 54ad38247aa..e01b915a57c 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -431,7 +431,8 @@ static ObjectAddress ATExecValidateConstraint(List **wqueue,
Relation rel, char *constrName,
bool recurse, bool recursing, LOCKMODE lockmode);
static void QueueFKConstraintValidation(List **wqueue, Relation conrel, Relation rel,
- HeapTuple contuple, LOCKMODE lockmode);
+ HeapTuple contuple, bool queueValidation,
+ LOCKMODE lockmode);
static void QueueCheckConstraintValidation(List **wqueue, Relation conrel, Relation rel,
char *constrName, HeapTuple contuple,
bool recurse, bool recursing, LOCKMODE lockmode);
@@ -11867,7 +11868,7 @@ AttachPartitionForeignKey(List **wqueue,
/* Use the same lock as for AT_ValidateConstraint */
QueueFKConstraintValidation(wqueue, conrel, partition, partcontup,
- ShareUpdateExclusiveLock);
+ true, ShareUpdateExclusiveLock);
ReleaseSysCache(partcontup);
table_close(conrel, RowExclusiveLock);
}
@@ -12464,8 +12465,13 @@ ATExecAlterConstrEnforceability(List **wqueue, ATAlterConstraint *cmdcon,
/*
* Tell Phase 3 to check that the constraint is satisfied by existing
* rows.
+ *
+ * Note that validation should be performed against the referencing
+ * root table only, not its child partitions. See
+ * QueueFKConstraintValidation() for more details.
*/
- if (rel->rd_rel->relkind == RELKIND_RELATION)
+ if (rel->rd_rel->relkind == RELKIND_RELATION &&
+ currcon->confrelid == pkrelid)
{
AlteredTableInfo *tab;
NewConstraint *newcon;
@@ -12919,7 +12925,8 @@ ATExecValidateConstraint(List **wqueue, Relation rel, char *constrName,
{
if (con->contype == CONSTRAINT_FOREIGN)
{
- QueueFKConstraintValidation(wqueue, conrel, rel, tuple, lockmode);
+ QueueFKConstraintValidation(wqueue, conrel, rel, tuple, true,
+ lockmode);
}
else if (con->contype == CONSTRAINT_CHECK)
{
@@ -12953,7 +12960,8 @@ ATExecValidateConstraint(List **wqueue, Relation rel, char *constrName,
*/
static void
QueueFKConstraintValidation(List **wqueue, Relation conrel, Relation rel,
- HeapTuple contuple, LOCKMODE lockmode)
+ HeapTuple contuple, bool queueValidation,
+ LOCKMODE lockmode)
{
Form_pg_constraint con;
AlteredTableInfo *tab;
@@ -12964,7 +12972,13 @@ QueueFKConstraintValidation(List **wqueue, Relation conrel, Relation rel,
Assert(con->contype == CONSTRAINT_FOREIGN);
Assert(!con->convalidated);
- if (rel->rd_rel->relkind == RELKIND_RELATION)
+ /*
+ * Sometimes we only want to update the pg_constraint entry without
+ * enqueueing it for validation; in such cases, queueValidation will be
+ * false.
+ */
+ if (queueValidation &&
+ rel->rd_rel->relkind == RELKIND_RELATION)
{
NewConstraint *newcon;
Constraint *fkconstraint;
@@ -13010,6 +13024,7 @@ QueueFKConstraintValidation(List **wqueue, Relation conrel, Relation rel,
{
Form_pg_constraint childcon;
Relation childrel;
+ bool childValidation = true;
childcon = (Form_pg_constraint) GETSTRUCT(childtup);
@@ -13021,11 +13036,33 @@ QueueFKConstraintValidation(List **wqueue, Relation conrel, Relation rel,
if (childcon->convalidated)
continue;
- childrel = table_open(childcon->conrelid, lockmode);
+ /*
+ * When the referenced table is partitioned, the referencing table may have
+ * multiple foreign key constraints for action triggers -- one for each child
+ * partition. These are action-trigger constraints intended to enforce the
+ * appropriate behavior when UPDATE or DELETE operations are performed on the
+ * referenced partitions. See createForeignKeyActionTriggers() for more details.
+ *
+ * These constraints should not be used for validating data in the referencing
+ * table. This is because referenced key values may be distributed across
+ * multiple partitions, and therefore, validation should be performed only
+ * through the root referenced table.
+ */
+ if (childcon->conrelid == RelationGetRelid(rel))
+ {
+ /* No need to open it again */
+ childrel = rel;
+ childValidation = false;
+ }
+ else
+ childrel = table_open(childcon->conrelid, lockmode);
QueueFKConstraintValidation(wqueue, conrel, childrel, childtup,
- lockmode);
- table_close(childrel, NoLock);
+ childValidation, lockmode);
+
+ /* We have skip opening the table if it is the same input rel */
+ if (childcon->conrelid != RelationGetRelid(rel))
+ table_close(childrel, NoLock);
}
systable_endscan(pscan);
diff --git a/src/test/regress/expected/foreign_key.out b/src/test/regress/expected/foreign_key.out
index 4f3f280a439..8050a3c4aa3 100644
--- a/src/test/regress/expected/foreign_key.out
+++ b/src/test/regress/expected/foreign_key.out
@@ -1895,29 +1895,44 @@ WHERE conrelid::regclass::text like 'fk_partitioned_fk%' ORDER BY oid::regclass:
(5 rows)
DROP TABLE fk_partitioned_fk, fk_notpartitioned_pk;
--- NOT VALID foreign key on a non-partitioned table referencing a partitioned table
+-- NOT VALID and NOT ENFORCED foreign key on a non-partitioned table referencing a partitioned table
CREATE TABLE fk_partitioned_pk (a int, b int, PRIMARY KEY (a, b)) PARTITION BY RANGE (a, b);
CREATE TABLE fk_partitioned_pk_1 PARTITION OF fk_partitioned_pk FOR VALUES FROM (0,0) TO (1000,1000);
+CREATE TABLE fk_partitioned_pk_2 PARTITION OF fk_partitioned_pk FOR VALUES FROM (1000,1000) TO (2000,2000);
CREATE TABLE fk_notpartitioned_fk (b int, a int);
-ALTER TABLE fk_notpartitioned_fk ADD FOREIGN KEY (a, b) REFERENCES fk_partitioned_pk NOT VALID;
--- Constraint will be invalid.
-SELECT conname, convalidated FROM pg_constraint
+INSERT INTO fk_partitioned_pk VALUES(100,100), (1000,1000);
+INSERT INTO fk_notpartitioned_fk VALUES(100,100), (1000,1000);
+ALTER TABLE fk_notpartitioned_fk ADD CONSTRAINT fk_notpartitioned_fk_a_b_fkey
+ FOREIGN KEY (a, b) REFERENCES fk_partitioned_pk NOT VALID;
+ALTER TABLE fk_notpartitioned_fk ADD CONSTRAINT fk_notpartitioned_fk_a_b_fkey2
+ FOREIGN KEY (a, b) REFERENCES fk_partitioned_pk NOT ENFORCED;
+-- All constraints will be invalid, and _fkey2 constraints will not be enforced.
+SELECT conname, conenforced, convalidated FROM pg_constraint
WHERE conrelid = 'fk_notpartitioned_fk'::regclass ORDER BY oid::regclass::text;
- conname | convalidated
----------------------------------+--------------
- fk_notpartitioned_fk_a_b_fkey | f
- fk_notpartitioned_fk_a_b_fkey_1 | f
-(2 rows)
+ conname | conenforced | convalidated
+----------------------------------+-------------+--------------
+ fk_notpartitioned_fk_a_b_fkey | t | f
+ fk_notpartitioned_fk_a_b_fkey_1 | t | f
+ fk_notpartitioned_fk_a_b_fkey_2 | t | f
+ fk_notpartitioned_fk_a_b_fkey2 | f | f
+ fk_notpartitioned_fk_a_b_fkey2_1 | f | f
+ fk_notpartitioned_fk_a_b_fkey2_2 | f | f
+(6 rows)
ALTER TABLE fk_notpartitioned_fk VALIDATE CONSTRAINT fk_notpartitioned_fk_a_b_fkey;
--- All constraints are now valid.
-SELECT conname, convalidated FROM pg_constraint
+ALTER TABLE fk_notpartitioned_fk ALTER CONSTRAINT fk_notpartitioned_fk_a_b_fkey2 ENFORCED;
+-- All constraints are now valid and enforced.
+SELECT conname, conenforced, convalidated FROM pg_constraint
WHERE conrelid = 'fk_notpartitioned_fk'::regclass ORDER BY oid::regclass::text;
- conname | convalidated
----------------------------------+--------------
- fk_notpartitioned_fk_a_b_fkey | t
- fk_notpartitioned_fk_a_b_fkey_1 | t
-(2 rows)
+ conname | conenforced | convalidated
+----------------------------------+-------------+--------------
+ fk_notpartitioned_fk_a_b_fkey | t | t
+ fk_notpartitioned_fk_a_b_fkey_1 | t | t
+ fk_notpartitioned_fk_a_b_fkey_2 | t | t
+ fk_notpartitioned_fk_a_b_fkey2 | t | t
+ fk_notpartitioned_fk_a_b_fkey2_1 | t | t
+ fk_notpartitioned_fk_a_b_fkey2_2 | t | t
+(6 rows)
DROP TABLE fk_notpartitioned_fk, fk_partitioned_pk;
-- Test some other exotic foreign key features: MATCH SIMPLE, ON UPDATE/DELETE
diff --git a/src/test/regress/sql/foreign_key.sql b/src/test/regress/sql/foreign_key.sql
index 8159e363022..aaeb0f49694 100644
--- a/src/test/regress/sql/foreign_key.sql
+++ b/src/test/regress/sql/foreign_key.sql
@@ -1389,20 +1389,27 @@ WHERE conrelid::regclass::text like 'fk_partitioned_fk%' ORDER BY oid::regclass:
DROP TABLE fk_partitioned_fk, fk_notpartitioned_pk;
--- NOT VALID foreign key on a non-partitioned table referencing a partitioned table
+-- NOT VALID and NOT ENFORCED foreign key on a non-partitioned table referencing a partitioned table
CREATE TABLE fk_partitioned_pk (a int, b int, PRIMARY KEY (a, b)) PARTITION BY RANGE (a, b);
CREATE TABLE fk_partitioned_pk_1 PARTITION OF fk_partitioned_pk FOR VALUES FROM (0,0) TO (1000,1000);
+CREATE TABLE fk_partitioned_pk_2 PARTITION OF fk_partitioned_pk FOR VALUES FROM (1000,1000) TO (2000,2000);
CREATE TABLE fk_notpartitioned_fk (b int, a int);
-ALTER TABLE fk_notpartitioned_fk ADD FOREIGN KEY (a, b) REFERENCES fk_partitioned_pk NOT VALID;
+INSERT INTO fk_partitioned_pk VALUES(100,100), (1000,1000);
+INSERT INTO fk_notpartitioned_fk VALUES(100,100), (1000,1000);
+ALTER TABLE fk_notpartitioned_fk ADD CONSTRAINT fk_notpartitioned_fk_a_b_fkey
+ FOREIGN KEY (a, b) REFERENCES fk_partitioned_pk NOT VALID;
+ALTER TABLE fk_notpartitioned_fk ADD CONSTRAINT fk_notpartitioned_fk_a_b_fkey2
+ FOREIGN KEY (a, b) REFERENCES fk_partitioned_pk NOT ENFORCED;
--- Constraint will be invalid.
-SELECT conname, convalidated FROM pg_constraint
+-- All constraints will be invalid, and _fkey2 constraints will not be enforced.
+SELECT conname, conenforced, convalidated FROM pg_constraint
WHERE conrelid = 'fk_notpartitioned_fk'::regclass ORDER BY oid::regclass::text;
ALTER TABLE fk_notpartitioned_fk VALIDATE CONSTRAINT fk_notpartitioned_fk_a_b_fkey;
+ALTER TABLE fk_notpartitioned_fk ALTER CONSTRAINT fk_notpartitioned_fk_a_b_fkey2 ENFORCED;
--- All constraints are now valid.
-SELECT conname, convalidated FROM pg_constraint
+-- All constraints are now valid and enforced.
+SELECT conname, conenforced, convalidated FROM pg_constraint
WHERE conrelid = 'fk_notpartitioned_fk'::regclass ORDER BY oid::regclass::text;
DROP TABLE fk_notpartitioned_fk, fk_partitioned_pk;
--
2.43.5
* Re: Foreign key validation failure in 18beta1
@ 2025-06-01 12:35 jian he <jian.universality@gmail.com>
parent: Amul Sul <sulamul@gmail.com>
0 siblings, 1 reply; 19+ messages in thread
From: jian he @ 2025-06-01 12:35 UTC (permalink / raw)
To: Amul Sul <sulamul@gmail.com>; +Cc: Tender Wang <tndrwang@gmail.com>; Alvaro Herrera <alvherre@alvh.no-ip.org>; pgsql-hackers@lists.postgresql.org, Antonin Houska <ah@cybertec.at>
On Fri, May 30, 2025 at 6:32 PM Amul Sul <sulamul@gmail.com> wrote:
>
>
> Kindly take a look at the attached version. I've also added the tests.
> Thanks for your script -- all tests are passing with this patch.
>
hi.
+ * Note that validation should be performed against the referencing
+ * root table only, not its child partitions. See
+ * QueueFKConstraintValidation() for more details.
*/
if (rel->rd_rel->relkind == RELKIND_RELATION &&
currcon->confrelid == pkrelid)
{
AlteredTableInfo *tab;
NewConstraint *newcon;
newcon = (NewConstraint *) palloc0(sizeof(NewConstraint));
....
}
in the comments "referencing" should be "referenced"?
other than that, it looks good.
^ permalink raw reply [nested|flat] 19+ messages in thread
* Re: Foreign key validation failure in 18beta1
@ 2025-06-02 03:26 Amul Sul <sulamul@gmail.com>
parent: jian he <jian.universality@gmail.com>
0 siblings, 1 reply; 19+ messages in thread
From: Amul Sul @ 2025-06-02 03:26 UTC (permalink / raw)
To: jian he <jian.universality@gmail.com>; +Cc: Tender Wang <tndrwang@gmail.com>; Alvaro Herrera <alvherre@alvh.no-ip.org>; pgsql-hackers@lists.postgresql.org, Antonin Houska <ah@cybertec.at>
On Sun, Jun 1, 2025 at 6:05 PM jian he <jian.universality@gmail.com> wrote:
>
> On Fri, May 30, 2025 at 6:32 PM Amul Sul <sulamul@gmail.com> wrote:
> >
[...]
>
> + * Note that validation should be performed against the referencing
> + * root table only, not its child partitions. See
> + * QueueFKConstraintValidation() for more details.
> */
> if (rel->rd_rel->relkind == RELKIND_RELATION &&
> currcon->confrelid == pkrelid)
> {
> AlteredTableInfo *tab;
> NewConstraint *newcon;
> newcon = (NewConstraint *) palloc0(sizeof(NewConstraint));
> ....
> }
>
> in the comments "referencing" should be "referenced"?
>
Yes, fixed in the attached version.
> other than that, it looks good.
>
Thanks for the review.
Regards,
Amul
Attachments:
v5-0001-Skip-adding-action-based-foreign-key-constraints-.patch (application/x-patch, 11.4K) download
From d580da019e88aa953a53f937ccff89432d832a3c Mon Sep 17 00:00:00 2001
From: Amul Sul <sulamul@gmail.com>
Date: Mon, 2 Jun 2025 08:44:26 +0530
Subject: [PATCH v5] Skip adding action-based foreign key constraints to the
phase 3 validation queue.
When the referenced table is partitioned, each partition will have its
own action trigger constraint to handle the appropriate behavior for
UPDATE or DELETE operations. However, these constraints should not be
used to validate data in the referencing table, as the referenced data
may be distributed across multiple partitions. Attempting validation
against a single partition could result in errors. Therefore,
validation of referencing table data should always be performed
against the referenced root partitioned table.
---
src/backend/commands/tablecmds.c | 55 +++++++++++++++++++----
src/test/regress/expected/foreign_key.out | 47 ++++++++++++-------
src/test/regress/sql/foreign_key.sql | 19 +++++---
3 files changed, 90 insertions(+), 31 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index acf11e83c04..0ed637fa5de 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -431,7 +431,8 @@ static ObjectAddress ATExecValidateConstraint(List **wqueue,
Relation rel, char *constrName,
bool recurse, bool recursing, LOCKMODE lockmode);
static void QueueFKConstraintValidation(List **wqueue, Relation conrel, Relation rel,
- HeapTuple contuple, LOCKMODE lockmode);
+ HeapTuple contuple, bool queueValidation,
+ LOCKMODE lockmode);
static void QueueCheckConstraintValidation(List **wqueue, Relation conrel, Relation rel,
char *constrName, HeapTuple contuple,
bool recurse, bool recursing, LOCKMODE lockmode);
@@ -11867,7 +11868,7 @@ AttachPartitionForeignKey(List **wqueue,
/* Use the same lock as for AT_ValidateConstraint */
QueueFKConstraintValidation(wqueue, conrel, partition, partcontup,
- ShareUpdateExclusiveLock);
+ true, ShareUpdateExclusiveLock);
ReleaseSysCache(partcontup);
table_close(conrel, RowExclusiveLock);
}
@@ -12464,8 +12465,13 @@ ATExecAlterConstrEnforceability(List **wqueue, ATAlterConstraint *cmdcon,
/*
* Tell Phase 3 to check that the constraint is satisfied by existing
* rows.
+ *
+ * Note that validation should be performed against the referenced root
+ * table only, not its child partitions. See
+ * QueueFKConstraintValidation() for more details.
*/
- if (rel->rd_rel->relkind == RELKIND_RELATION)
+ if (rel->rd_rel->relkind == RELKIND_RELATION &&
+ currcon->confrelid == pkrelid)
{
AlteredTableInfo *tab;
NewConstraint *newcon;
@@ -12919,7 +12925,8 @@ ATExecValidateConstraint(List **wqueue, Relation rel, char *constrName,
{
if (con->contype == CONSTRAINT_FOREIGN)
{
- QueueFKConstraintValidation(wqueue, conrel, rel, tuple, lockmode);
+ QueueFKConstraintValidation(wqueue, conrel, rel, tuple, true,
+ lockmode);
}
else if (con->contype == CONSTRAINT_CHECK)
{
@@ -12953,7 +12960,8 @@ ATExecValidateConstraint(List **wqueue, Relation rel, char *constrName,
*/
static void
QueueFKConstraintValidation(List **wqueue, Relation conrel, Relation rel,
- HeapTuple contuple, LOCKMODE lockmode)
+ HeapTuple contuple, bool queueValidation,
+ LOCKMODE lockmode)
{
Form_pg_constraint con;
AlteredTableInfo *tab;
@@ -12964,7 +12972,13 @@ QueueFKConstraintValidation(List **wqueue, Relation conrel, Relation rel,
Assert(con->contype == CONSTRAINT_FOREIGN);
Assert(!con->convalidated);
- if (rel->rd_rel->relkind == RELKIND_RELATION)
+ /*
+ * Sometimes we only want to update the pg_constraint entry without
+ * enqueueing it for validation; in such cases, queueValidation will be
+ * false.
+ */
+ if (queueValidation &&
+ rel->rd_rel->relkind == RELKIND_RELATION)
{
NewConstraint *newcon;
Constraint *fkconstraint;
@@ -13010,6 +13024,7 @@ QueueFKConstraintValidation(List **wqueue, Relation conrel, Relation rel,
{
Form_pg_constraint childcon;
Relation childrel;
+ bool childValidation = true;
childcon = (Form_pg_constraint) GETSTRUCT(childtup);
@@ -13021,11 +13036,33 @@ QueueFKConstraintValidation(List **wqueue, Relation conrel, Relation rel,
if (childcon->convalidated)
continue;
- childrel = table_open(childcon->conrelid, lockmode);
+ /*
+ * When the referenced table is partitioned, the referencing table may have
+ * multiple foreign key constraints for action triggers -- one for each child
+ * partition. These are action-trigger constraints intended to enforce the
+ * appropriate behavior when UPDATE or DELETE operations are performed on the
+ * referenced partitions. See createForeignKeyActionTriggers() for more details.
+ *
+ * These constraints should not be used for validating data in the referencing
+ * table. This is because referenced key values may be distributed across
+ * multiple partitions, and therefore, validation should be performed only
+ * through the root referenced table.
+ */
+ if (childcon->conrelid == RelationGetRelid(rel))
+ {
+ /* No need to open it again */
+ childrel = rel;
+ childValidation = false;
+ }
+ else
+ childrel = table_open(childcon->conrelid, lockmode);
QueueFKConstraintValidation(wqueue, conrel, childrel, childtup,
- lockmode);
- table_close(childrel, NoLock);
+ childValidation, lockmode);
+
+ /* We have skip opening the table if it is the same input rel */
+ if (childcon->conrelid != RelationGetRelid(rel))
+ table_close(childrel, NoLock);
}
systable_endscan(pscan);
diff --git a/src/test/regress/expected/foreign_key.out b/src/test/regress/expected/foreign_key.out
index 4f3f280a439..8050a3c4aa3 100644
--- a/src/test/regress/expected/foreign_key.out
+++ b/src/test/regress/expected/foreign_key.out
@@ -1895,29 +1895,44 @@ WHERE conrelid::regclass::text like 'fk_partitioned_fk%' ORDER BY oid::regclass:
(5 rows)
DROP TABLE fk_partitioned_fk, fk_notpartitioned_pk;
--- NOT VALID foreign key on a non-partitioned table referencing a partitioned table
+-- NOT VALID and NOT ENFORCED foreign key on a non-partitioned table referencing a partitioned table
CREATE TABLE fk_partitioned_pk (a int, b int, PRIMARY KEY (a, b)) PARTITION BY RANGE (a, b);
CREATE TABLE fk_partitioned_pk_1 PARTITION OF fk_partitioned_pk FOR VALUES FROM (0,0) TO (1000,1000);
+CREATE TABLE fk_partitioned_pk_2 PARTITION OF fk_partitioned_pk FOR VALUES FROM (1000,1000) TO (2000,2000);
CREATE TABLE fk_notpartitioned_fk (b int, a int);
-ALTER TABLE fk_notpartitioned_fk ADD FOREIGN KEY (a, b) REFERENCES fk_partitioned_pk NOT VALID;
--- Constraint will be invalid.
-SELECT conname, convalidated FROM pg_constraint
+INSERT INTO fk_partitioned_pk VALUES(100,100), (1000,1000);
+INSERT INTO fk_notpartitioned_fk VALUES(100,100), (1000,1000);
+ALTER TABLE fk_notpartitioned_fk ADD CONSTRAINT fk_notpartitioned_fk_a_b_fkey
+ FOREIGN KEY (a, b) REFERENCES fk_partitioned_pk NOT VALID;
+ALTER TABLE fk_notpartitioned_fk ADD CONSTRAINT fk_notpartitioned_fk_a_b_fkey2
+ FOREIGN KEY (a, b) REFERENCES fk_partitioned_pk NOT ENFORCED;
+-- All constraints will be invalid, and _fkey2 constraints will not be enforced.
+SELECT conname, conenforced, convalidated FROM pg_constraint
WHERE conrelid = 'fk_notpartitioned_fk'::regclass ORDER BY oid::regclass::text;
- conname | convalidated
----------------------------------+--------------
- fk_notpartitioned_fk_a_b_fkey | f
- fk_notpartitioned_fk_a_b_fkey_1 | f
-(2 rows)
+ conname | conenforced | convalidated
+----------------------------------+-------------+--------------
+ fk_notpartitioned_fk_a_b_fkey | t | f
+ fk_notpartitioned_fk_a_b_fkey_1 | t | f
+ fk_notpartitioned_fk_a_b_fkey_2 | t | f
+ fk_notpartitioned_fk_a_b_fkey2 | f | f
+ fk_notpartitioned_fk_a_b_fkey2_1 | f | f
+ fk_notpartitioned_fk_a_b_fkey2_2 | f | f
+(6 rows)
ALTER TABLE fk_notpartitioned_fk VALIDATE CONSTRAINT fk_notpartitioned_fk_a_b_fkey;
--- All constraints are now valid.
-SELECT conname, convalidated FROM pg_constraint
+ALTER TABLE fk_notpartitioned_fk ALTER CONSTRAINT fk_notpartitioned_fk_a_b_fkey2 ENFORCED;
+-- All constraints are now valid and enforced.
+SELECT conname, conenforced, convalidated FROM pg_constraint
WHERE conrelid = 'fk_notpartitioned_fk'::regclass ORDER BY oid::regclass::text;
- conname | convalidated
----------------------------------+--------------
- fk_notpartitioned_fk_a_b_fkey | t
- fk_notpartitioned_fk_a_b_fkey_1 | t
-(2 rows)
+ conname | conenforced | convalidated
+----------------------------------+-------------+--------------
+ fk_notpartitioned_fk_a_b_fkey | t | t
+ fk_notpartitioned_fk_a_b_fkey_1 | t | t
+ fk_notpartitioned_fk_a_b_fkey_2 | t | t
+ fk_notpartitioned_fk_a_b_fkey2 | t | t
+ fk_notpartitioned_fk_a_b_fkey2_1 | t | t
+ fk_notpartitioned_fk_a_b_fkey2_2 | t | t
+(6 rows)
DROP TABLE fk_notpartitioned_fk, fk_partitioned_pk;
-- Test some other exotic foreign key features: MATCH SIMPLE, ON UPDATE/DELETE
diff --git a/src/test/regress/sql/foreign_key.sql b/src/test/regress/sql/foreign_key.sql
index 8159e363022..aaeb0f49694 100644
--- a/src/test/regress/sql/foreign_key.sql
+++ b/src/test/regress/sql/foreign_key.sql
@@ -1389,20 +1389,27 @@ WHERE conrelid::regclass::text like 'fk_partitioned_fk%' ORDER BY oid::regclass:
DROP TABLE fk_partitioned_fk, fk_notpartitioned_pk;
--- NOT VALID foreign key on a non-partitioned table referencing a partitioned table
+-- NOT VALID and NOT ENFORCED foreign key on a non-partitioned table referencing a partitioned table
CREATE TABLE fk_partitioned_pk (a int, b int, PRIMARY KEY (a, b)) PARTITION BY RANGE (a, b);
CREATE TABLE fk_partitioned_pk_1 PARTITION OF fk_partitioned_pk FOR VALUES FROM (0,0) TO (1000,1000);
+CREATE TABLE fk_partitioned_pk_2 PARTITION OF fk_partitioned_pk FOR VALUES FROM (1000,1000) TO (2000,2000);
CREATE TABLE fk_notpartitioned_fk (b int, a int);
-ALTER TABLE fk_notpartitioned_fk ADD FOREIGN KEY (a, b) REFERENCES fk_partitioned_pk NOT VALID;
+INSERT INTO fk_partitioned_pk VALUES(100,100), (1000,1000);
+INSERT INTO fk_notpartitioned_fk VALUES(100,100), (1000,1000);
+ALTER TABLE fk_notpartitioned_fk ADD CONSTRAINT fk_notpartitioned_fk_a_b_fkey
+ FOREIGN KEY (a, b) REFERENCES fk_partitioned_pk NOT VALID;
+ALTER TABLE fk_notpartitioned_fk ADD CONSTRAINT fk_notpartitioned_fk_a_b_fkey2
+ FOREIGN KEY (a, b) REFERENCES fk_partitioned_pk NOT ENFORCED;
--- Constraint will be invalid.
-SELECT conname, convalidated FROM pg_constraint
+-- All constraints will be invalid, and _fkey2 constraints will not be enforced.
+SELECT conname, conenforced, convalidated FROM pg_constraint
WHERE conrelid = 'fk_notpartitioned_fk'::regclass ORDER BY oid::regclass::text;
ALTER TABLE fk_notpartitioned_fk VALIDATE CONSTRAINT fk_notpartitioned_fk_a_b_fkey;
+ALTER TABLE fk_notpartitioned_fk ALTER CONSTRAINT fk_notpartitioned_fk_a_b_fkey2 ENFORCED;
--- All constraints are now valid.
-SELECT conname, convalidated FROM pg_constraint
+-- All constraints are now valid and enforced.
+SELECT conname, conenforced, convalidated FROM pg_constraint
WHERE conrelid = 'fk_notpartitioned_fk'::regclass ORDER BY oid::regclass::text;
DROP TABLE fk_notpartitioned_fk, fk_partitioned_pk;
--
2.43.5
* Re: Foreign key validation failure in 18beta1
@ 2025-06-02 16:26 Alvaro Herrera <alvherre@alvh.no-ip.org>
parent: Amul Sul <sulamul@gmail.com>
0 siblings, 1 reply; 19+ messages in thread
From: Alvaro Herrera @ 2025-06-02 16:26 UTC (permalink / raw)
To: Amul Sul <sulamul@gmail.com>; +Cc: jian he <jian.universality@gmail.com>; Tender Wang <tndrwang@gmail.com>; pgsql-hackers@lists.postgresql.org, Antonin Houska <ah@cybertec.at>
Hello
I find this coding somewhat confusing. Now you have a function
"QueueFkConstraintValidation" which may queue a FK queue constraint
validation, if the flag "queueValidation" is given, but it may also do
something else -- makes no sense IMO. I think it's better to split this
function in two; one does indeed validate, the other is a simple helper
that only sets the catalog flag, and does its own recursion. This
causes a bit of duplicate code, but is simpler to follow. (We could
also refactor the duplicate code by adding another helper routine, but
I'm not sure it's worth the trouble.)
I would propose something like this instead, what do you think?
I hate that pg_constraint.conparentid doesn't have a syscache or at
least an index :-(
--
Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/
"El miedo atento y previsor es la madre de la seguridad" (E. Burke)
Attachments:
v6-0001-Skip-adding-action-based-foreign-key-constraints-.patch (text/x-diff, 12.4K) download
From 16437c21eca0bb15e34ba6c7c951daabb280fcec Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C3=81lvaro=20Herrera?= <alvherre@kurilemu.de>
Date: Mon, 2 Jun 2025 18:19:13 +0200
Subject: [PATCH v6] Skip adding action-based foreign key constraints to the
phase 3 validation queue.
When the referenced table is partitioned, each partition will have its
own action trigger constraint to handle the appropriate behavior for
UPDATE or DELETE operations. However, these constraints should not be
used to validate data in the referencing table, as the referenced data
may be distributed across multiple partitions. Attempting validation
against a single partition could result in errors. Therefore,
validation of referencing table data should always be performed
against the referenced root partitioned table.
---
src/backend/commands/tablecmds.c | 113 ++++++++++++++++++++--
src/test/regress/expected/foreign_key.out | 47 ++++++---
src/test/regress/sql/foreign_key.sql | 19 ++--
3 files changed, 148 insertions(+), 31 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index acf11e83c04..84e4476e5c6 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -432,6 +432,7 @@ static ObjectAddress ATExecValidateConstraint(List **wqueue,
bool recurse, bool recursing, LOCKMODE lockmode);
static void QueueFKConstraintValidation(List **wqueue, Relation conrel, Relation rel,
HeapTuple contuple, LOCKMODE lockmode);
+static void markChildFKConstraintValidated(Relation conrel, HeapTuple contuple, Relation rel);
static void QueueCheckConstraintValidation(List **wqueue, Relation conrel, Relation rel,
char *constrName, HeapTuple contuple,
bool recurse, bool recursing, LOCKMODE lockmode);
@@ -12464,8 +12465,13 @@ ATExecAlterConstrEnforceability(List **wqueue, ATAlterConstraint *cmdcon,
/*
* Tell Phase 3 to check that the constraint is satisfied by existing
* rows.
+ *
+ * Note that validation should be performed against the referenced root
+ * table only, not its child partitions. See
+ * QueueFKConstraintValidation() for more details.
*/
- if (rel->rd_rel->relkind == RELKIND_RELATION)
+ if (rel->rd_rel->relkind == RELKIND_RELATION &&
+ currcon->confrelid == pkrelid)
{
AlteredTableInfo *tab;
NewConstraint *newcon;
@@ -12964,6 +12970,11 @@ QueueFKConstraintValidation(List **wqueue, Relation conrel, Relation rel,
Assert(con->contype == CONSTRAINT_FOREIGN);
Assert(!con->convalidated);
+ /*
+ * If we're processing a plain relation, it needs to be scanned by phase 3
+ * in order for the constraint to be validated. Find or create its work
+ * queue entry, and attach the constraint information to it.
+ */
if (rel->rd_rel->relkind == RELKIND_RELATION)
{
NewConstraint *newcon;
@@ -12988,8 +12999,15 @@ QueueFKConstraintValidation(List **wqueue, Relation conrel, Relation rel,
}
/*
- * If the table at either end of the constraint is partitioned, we need to
- * recurse and handle every constraint that is a child of this constraint.
+ * If the referencing table is partitioned, we need to scan every
+ * partition to report any potentially violating rows.
+ *
+ * If the referenced table is partitioned, we don't need to scan the
+ * partitions, but we need to mark those child constraint rows as
+ * validated. (This is just so that catalog state is consistent.)
+ *
+ * XXX this requires a seqscan of pg_constraint for each partitioning
+ * level.
*/
if (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE ||
get_rel_relkind(con->confrelid) == RELKIND_PARTITIONED_TABLE)
@@ -13009,23 +13027,100 @@ QueueFKConstraintValidation(List **wqueue, Relation conrel, Relation rel,
while (HeapTupleIsValid(childtup = systable_getnext(pscan)))
{
Form_pg_constraint childcon;
- Relation childrel;
childcon = (Form_pg_constraint) GETSTRUCT(childtup);
/*
- * If the child constraint has already been validated, no further
+ * If the child constraint is already marked validated, no further
* action is required for it or its descendants, as they are all
* valid.
*/
if (childcon->convalidated)
continue;
- childrel = table_open(childcon->conrelid, lockmode);
+ if (childcon->conrelid == RelationGetRelid(rel))
+ {
+ markChildFKConstraintValidated(conrel, childtup, rel);
+ }
+ else
+ {
+ Relation childrel;
- QueueFKConstraintValidation(wqueue, conrel, childrel, childtup,
- lockmode);
- table_close(childrel, NoLock);
+ childrel = table_open(childcon->conrelid, lockmode);
+
+ QueueFKConstraintValidation(wqueue, conrel, childrel, childtup,
+ lockmode);
+
+ /* We have skip opening the table if it is the same input rel */
+ table_close(childrel, NoLock);
+ }
+ }
+
+ systable_endscan(pscan);
+ }
+
+ /*
+ * Now update the catalog, while we have the door open.
+ */
+ copyTuple = heap_copytuple(contuple);
+ copy_con = (Form_pg_constraint) GETSTRUCT(copyTuple);
+ copy_con->convalidated = true;
+ CatalogTupleUpdate(conrel, ©Tuple->t_self, copyTuple);
+
+ InvokeObjectPostAlterHook(ConstraintRelationId, con->oid, 0);
+
+ heap_freetuple(copyTuple);
+}
+
+/*
+ * Helper for QueueFKConstraintValidation: recurse on the referenced-side
+ * relation, when it is a partitioned table.
+ */
+static void
+markChildFKConstraintValidated(Relation conrel, HeapTuple contuple, Relation rel)
+{
+ Form_pg_constraint con;
+ HeapTuple copyTuple;
+ Form_pg_constraint copy_con;
+
+ con = (Form_pg_constraint) GETSTRUCT(contuple);
+ Assert(con->contype == CONSTRAINT_FOREIGN);
+ Assert(!con->convalidated);
+ Assert(con->conrelid = RelationGetRelid(rel));
+
+ /*
+ * If this child refenced table is also partitioned, then we must process
+ * the constraints on its own partitions as well. Descend the hierarchy
+ * using the conparentid link.
+ *
+ * XXX this requires a seqscan of pg_constraint for each partitioning
+ * level.
+ */
+ if (get_rel_relkind(con->confrelid) == RELKIND_PARTITIONED_TABLE)
+ {
+ ScanKeyData pkey;
+ SysScanDesc pscan;
+ HeapTuple childtup;
+
+ ScanKeyInit(&pkey,
+ Anum_pg_constraint_conparentid,
+ BTEqualStrategyNumber, F_OIDEQ,
+ ObjectIdGetDatum(con->oid));
+
+ pscan = systable_beginscan(conrel, ConstraintParentIndexId,
+ true, NULL, 1, &pkey);
+
+ while (HeapTupleIsValid(childtup = systable_getnext(pscan)))
+ {
+ /*
+ * If the child constraint has already been validated, no further
+ * action is required for it or its descendants, as they are all
+ * valid.
+ */
+ if (((Form_pg_constraint) GETSTRUCT(childtup))->convalidated)
+ continue;
+
+ markChildFKConstraintValidated(conrel, childtup, rel);
}
systable_endscan(pscan);
diff --git a/src/test/regress/expected/foreign_key.out b/src/test/regress/expected/foreign_key.out
index 4f3f280a439..8050a3c4aa3 100644
--- a/src/test/regress/expected/foreign_key.out
+++ b/src/test/regress/expected/foreign_key.out
@@ -1895,29 +1895,44 @@ WHERE conrelid::regclass::text like 'fk_partitioned_fk%' ORDER BY oid::regclass:
(5 rows)
DROP TABLE fk_partitioned_fk, fk_notpartitioned_pk;
--- NOT VALID foreign key on a non-partitioned table referencing a partitioned table
+-- NOT VALID and NOT ENFORCED foreign key on a non-partitioned table referencing a partitioned table
CREATE TABLE fk_partitioned_pk (a int, b int, PRIMARY KEY (a, b)) PARTITION BY RANGE (a, b);
CREATE TABLE fk_partitioned_pk_1 PARTITION OF fk_partitioned_pk FOR VALUES FROM (0,0) TO (1000,1000);
+CREATE TABLE fk_partitioned_pk_2 PARTITION OF fk_partitioned_pk FOR VALUES FROM (1000,1000) TO (2000,2000);
CREATE TABLE fk_notpartitioned_fk (b int, a int);
-ALTER TABLE fk_notpartitioned_fk ADD FOREIGN KEY (a, b) REFERENCES fk_partitioned_pk NOT VALID;
--- Constraint will be invalid.
-SELECT conname, convalidated FROM pg_constraint
+INSERT INTO fk_partitioned_pk VALUES(100,100), (1000,1000);
+INSERT INTO fk_notpartitioned_fk VALUES(100,100), (1000,1000);
+ALTER TABLE fk_notpartitioned_fk ADD CONSTRAINT fk_notpartitioned_fk_a_b_fkey
+ FOREIGN KEY (a, b) REFERENCES fk_partitioned_pk NOT VALID;
+ALTER TABLE fk_notpartitioned_fk ADD CONSTRAINT fk_notpartitioned_fk_a_b_fkey2
+ FOREIGN KEY (a, b) REFERENCES fk_partitioned_pk NOT ENFORCED;
+-- All constraints will be invalid, and _fkey2 constraints will not be enforced.
+SELECT conname, conenforced, convalidated FROM pg_constraint
WHERE conrelid = 'fk_notpartitioned_fk'::regclass ORDER BY oid::regclass::text;
- conname | convalidated
----------------------------------+--------------
- fk_notpartitioned_fk_a_b_fkey | f
- fk_notpartitioned_fk_a_b_fkey_1 | f
-(2 rows)
+ conname | conenforced | convalidated
+----------------------------------+-------------+--------------
+ fk_notpartitioned_fk_a_b_fkey | t | f
+ fk_notpartitioned_fk_a_b_fkey_1 | t | f
+ fk_notpartitioned_fk_a_b_fkey_2 | t | f
+ fk_notpartitioned_fk_a_b_fkey2 | f | f
+ fk_notpartitioned_fk_a_b_fkey2_1 | f | f
+ fk_notpartitioned_fk_a_b_fkey2_2 | f | f
+(6 rows)
ALTER TABLE fk_notpartitioned_fk VALIDATE CONSTRAINT fk_notpartitioned_fk_a_b_fkey;
--- All constraints are now valid.
-SELECT conname, convalidated FROM pg_constraint
+ALTER TABLE fk_notpartitioned_fk ALTER CONSTRAINT fk_notpartitioned_fk_a_b_fkey2 ENFORCED;
+-- All constraints are now valid and enforced.
+SELECT conname, conenforced, convalidated FROM pg_constraint
WHERE conrelid = 'fk_notpartitioned_fk'::regclass ORDER BY oid::regclass::text;
- conname | convalidated
----------------------------------+--------------
- fk_notpartitioned_fk_a_b_fkey | t
- fk_notpartitioned_fk_a_b_fkey_1 | t
-(2 rows)
+ conname | conenforced | convalidated
+----------------------------------+-------------+--------------
+ fk_notpartitioned_fk_a_b_fkey | t | t
+ fk_notpartitioned_fk_a_b_fkey_1 | t | t
+ fk_notpartitioned_fk_a_b_fkey_2 | t | t
+ fk_notpartitioned_fk_a_b_fkey2 | t | t
+ fk_notpartitioned_fk_a_b_fkey2_1 | t | t
+ fk_notpartitioned_fk_a_b_fkey2_2 | t | t
+(6 rows)
DROP TABLE fk_notpartitioned_fk, fk_partitioned_pk;
-- Test some other exotic foreign key features: MATCH SIMPLE, ON UPDATE/DELETE
diff --git a/src/test/regress/sql/foreign_key.sql b/src/test/regress/sql/foreign_key.sql
index 8159e363022..aaeb0f49694 100644
--- a/src/test/regress/sql/foreign_key.sql
+++ b/src/test/regress/sql/foreign_key.sql
@@ -1389,20 +1389,27 @@ WHERE conrelid::regclass::text like 'fk_partitioned_fk%' ORDER BY oid::regclass:
DROP TABLE fk_partitioned_fk, fk_notpartitioned_pk;
--- NOT VALID foreign key on a non-partitioned table referencing a partitioned table
+-- NOT VALID and NOT ENFORCED foreign key on a non-partitioned table referencing a partitioned table
CREATE TABLE fk_partitioned_pk (a int, b int, PRIMARY KEY (a, b)) PARTITION BY RANGE (a, b);
CREATE TABLE fk_partitioned_pk_1 PARTITION OF fk_partitioned_pk FOR VALUES FROM (0,0) TO (1000,1000);
+CREATE TABLE fk_partitioned_pk_2 PARTITION OF fk_partitioned_pk FOR VALUES FROM (1000,1000) TO (2000,2000);
CREATE TABLE fk_notpartitioned_fk (b int, a int);
-ALTER TABLE fk_notpartitioned_fk ADD FOREIGN KEY (a, b) REFERENCES fk_partitioned_pk NOT VALID;
+INSERT INTO fk_partitioned_pk VALUES(100,100), (1000,1000);
+INSERT INTO fk_notpartitioned_fk VALUES(100,100), (1000,1000);
+ALTER TABLE fk_notpartitioned_fk ADD CONSTRAINT fk_notpartitioned_fk_a_b_fkey
+ FOREIGN KEY (a, b) REFERENCES fk_partitioned_pk NOT VALID;
+ALTER TABLE fk_notpartitioned_fk ADD CONSTRAINT fk_notpartitioned_fk_a_b_fkey2
+ FOREIGN KEY (a, b) REFERENCES fk_partitioned_pk NOT ENFORCED;
--- Constraint will be invalid.
-SELECT conname, convalidated FROM pg_constraint
+-- All constraints will be invalid, and _fkey2 constraints will not be enforced.
+SELECT conname, conenforced, convalidated FROM pg_constraint
WHERE conrelid = 'fk_notpartitioned_fk'::regclass ORDER BY oid::regclass::text;
ALTER TABLE fk_notpartitioned_fk VALIDATE CONSTRAINT fk_notpartitioned_fk_a_b_fkey;
+ALTER TABLE fk_notpartitioned_fk ALTER CONSTRAINT fk_notpartitioned_fk_a_b_fkey2 ENFORCED;
--- All constraints are now valid.
-SELECT conname, convalidated FROM pg_constraint
+-- All constraints are now valid and enforced.
+SELECT conname, conenforced, convalidated FROM pg_constraint
WHERE conrelid = 'fk_notpartitioned_fk'::regclass ORDER BY oid::regclass::text;
DROP TABLE fk_notpartitioned_fk, fk_partitioned_pk;
--
2.39.5
* Re: Foreign key validation failure in 18beta1
@ 2025-06-03 04:14 Amul Sul <sulamul@gmail.com>
parent: Alvaro Herrera <alvherre@alvh.no-ip.org>
0 siblings, 1 reply; 19+ messages in thread
From: Amul Sul @ 2025-06-03 04:14 UTC (permalink / raw)
To: Alvaro Herrera <alvherre@alvh.no-ip.org>; +Cc: jian he <jian.universality@gmail.com>; Tender Wang <tndrwang@gmail.com>; pgsql-hackers@lists.postgresql.org, Antonin Houska <ah@cybertec.at>
On Mon, Jun 2, 2025 at 9:56 PM Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
>
> Hello
>
> I find this coding somewhat confusing. Now you have a function
> "QueueFkConstraintValidation" which may queue a FK queue constraint
> validation, if the flag "queueValidation" is given, but it may also do
> something else -- makes no sense IMO.
I wouldn’t disagree with that.
> I think it's better to split this
> function in two; one does indeed validate, the other is a simple helper
> that only sets the catalog flag, and does its own recursion. This
> causes a bit of duplicate code, but is simpler to follow. (We could
> also refactor the duplicate code by adding another helper routine, but
> I'm not sure it's worth the trouble.)
>
> I would propose something like this instead, what do you think?
>
>
I found a third approach that requires only a few changes. The key
idea is to determine the root referenced table and pass it to
QueueFKConstraintValidation(). We would then enqueue phase 3
validation only if the constraint tuple’s confrelid matches that root
table -- similar to what is doing in ATExecAlterConstrEnforceability().
This would also ensure that the logic for adding/skipping phase 3
validation is consistent in both places.
Adding an extra parameter (i.e., pkrelid) doesn’t seem out of place,
as it aligns with other functions in the same file, such as
ATExecAlterConstraintInternal() and ATExecAlterConstrEnforceability().
Kindly take a look at the attached version and share your thoughts.
Regards,
Amul
Attachments:
v7-0001-Skip-adding-action-based-foreign-key-constraints-.patch (application/x-patch, 12.2K) download
From 10dd1def49327b65b5c5b30d405ebdf02811ecf7 Mon Sep 17 00:00:00 2001
From: Amul Sul <sulamul@gmail.com>
Date: Mon, 2 Jun 2025 08:44:26 +0530
Subject: [PATCH v7] Skip adding action-based foreign key constraints to the
phase 3 validation queue.
When the referenced table is partitioned, each partition will have its
own action trigger constraint to handle the appropriate behavior for
UPDATE or DELETE operations. However, these constraints should not be
used to validate data in the referencing table, as the referenced data
may be distributed across multiple partitions. Attempting validation
against a single partition could result in errors. Therefore,
validation of referencing table data should always be performed
against the referenced root partitioned table.
---
src/backend/commands/tablecmds.c | 53 +++++++++++++++++------
src/test/regress/expected/foreign_key.out | 47 +++++++++++++-------
src/test/regress/sql/foreign_key.sql | 19 +++++---
3 files changed, 84 insertions(+), 35 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index acf11e83c04..88417d65935 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -430,8 +430,8 @@ static void AlterConstrUpdateConstraintEntry(ATAlterConstraint *cmdcon, Relation
static ObjectAddress ATExecValidateConstraint(List **wqueue,
Relation rel, char *constrName,
bool recurse, bool recursing, LOCKMODE lockmode);
-static void QueueFKConstraintValidation(List **wqueue, Relation conrel, Relation rel,
- HeapTuple contuple, LOCKMODE lockmode);
+static void QueueFKConstraintValidation(List **wqueue, Relation conrel, Relation fkrel,
+ Oid pkrelid, HeapTuple contuple, LOCKMODE lockmode);
static void QueueCheckConstraintValidation(List **wqueue, Relation conrel, Relation rel,
char *constrName, HeapTuple contuple,
bool recurse, bool recursing, LOCKMODE lockmode);
@@ -11858,6 +11858,7 @@ AttachPartitionForeignKey(List **wqueue,
if (queueValidation)
{
Relation conrel;
+ Oid confrelid;
conrel = table_open(ConstraintRelationId, RowExclusiveLock);
@@ -11865,9 +11866,11 @@ AttachPartitionForeignKey(List **wqueue,
if (!HeapTupleIsValid(partcontup))
elog(ERROR, "cache lookup failed for constraint %u", partConstrOid);
+ confrelid = ((Form_pg_constraint) GETSTRUCT(partcontup))->confrelid;
+
/* Use the same lock as for AT_ValidateConstraint */
- QueueFKConstraintValidation(wqueue, conrel, partition, partcontup,
- ShareUpdateExclusiveLock);
+ QueueFKConstraintValidation(wqueue, conrel, partition, confrelid,
+ partcontup, ShareUpdateExclusiveLock);
ReleaseSysCache(partcontup);
table_close(conrel, RowExclusiveLock);
}
@@ -12464,8 +12467,13 @@ ATExecAlterConstrEnforceability(List **wqueue, ATAlterConstraint *cmdcon,
/*
* Tell Phase 3 to check that the constraint is satisfied by existing
* rows.
+ *
+ * Note that validation should be performed against the referenced root
+ * table only, not its child partitions. See
+ * QueueFKConstraintValidation() for more details.
*/
- if (rel->rd_rel->relkind == RELKIND_RELATION)
+ if (rel->rd_rel->relkind == RELKIND_RELATION &&
+ currcon->confrelid == pkrelid)
{
AlteredTableInfo *tab;
NewConstraint *newcon;
@@ -12919,7 +12927,8 @@ ATExecValidateConstraint(List **wqueue, Relation rel, char *constrName,
{
if (con->contype == CONSTRAINT_FOREIGN)
{
- QueueFKConstraintValidation(wqueue, conrel, rel, tuple, lockmode);
+ QueueFKConstraintValidation(wqueue, conrel, rel, con->confrelid,
+ tuple, lockmode);
}
else if (con->contype == CONSTRAINT_CHECK)
{
@@ -12952,8 +12961,8 @@ ATExecValidateConstraint(List **wqueue, Relation rel, char *constrName,
* for the specified relation and all its children.
*/
static void
-QueueFKConstraintValidation(List **wqueue, Relation conrel, Relation rel,
- HeapTuple contuple, LOCKMODE lockmode)
+QueueFKConstraintValidation(List **wqueue, Relation conrel, Relation fkrel,
+ Oid pkrelid, HeapTuple contuple, LOCKMODE lockmode)
{
Form_pg_constraint con;
AlteredTableInfo *tab;
@@ -12964,7 +12973,21 @@ QueueFKConstraintValidation(List **wqueue, Relation conrel, Relation rel,
Assert(con->contype == CONSTRAINT_FOREIGN);
Assert(!con->convalidated);
- if (rel->rd_rel->relkind == RELKIND_RELATION)
+ /*
+ * When the referenced table (pkrelid) is partitioned, the referencing
+ * table (fkrel) may have multiple foreign key constraints for action
+ * triggers—one for each referenced child partition. These action-trigger
+ * constraints are intended to enforce the correct behavior when UPDATE or
+ * DELETE operations are performed on the referenced partitions. See
+ * createForeignKeyActionTriggers() for more details.
+ *
+ * However, these constraints should not be used for validating data in the
+ * referencing table. Since referenced key values can be spread across
+ * multiple partitions, validation must be performed through the root
+ * referenced table (pkrelid) only.
+ */
+ if (fkrel->rd_rel->relkind == RELKIND_RELATION &&
+ con->confrelid == pkrelid)
{
NewConstraint *newcon;
Constraint *fkconstraint;
@@ -12983,7 +13006,7 @@ QueueFKConstraintValidation(List **wqueue, Relation conrel, Relation rel,
newcon->qual = (Node *) fkconstraint;
/* Find or create work queue entry for this table */
- tab = ATGetQueueEntry(wqueue, rel);
+ tab = ATGetQueueEntry(wqueue, fkrel);
tab->constraints = lappend(tab->constraints, newcon);
}
@@ -12991,7 +13014,7 @@ QueueFKConstraintValidation(List **wqueue, Relation conrel, Relation rel,
* If the table at either end of the constraint is partitioned, we need to
* recurse and handle every constraint that is a child of this constraint.
*/
- if (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE ||
+ if (fkrel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE ||
get_rel_relkind(con->confrelid) == RELKIND_PARTITIONED_TABLE)
{
ScanKeyData pkey;
@@ -13023,8 +13046,12 @@ QueueFKConstraintValidation(List **wqueue, Relation conrel, Relation rel,
childrel = table_open(childcon->conrelid, lockmode);
- QueueFKConstraintValidation(wqueue, conrel, childrel, childtup,
- lockmode);
+ /*
+ * NB: Note that pkrelid should be passed as-is during recursion,
+ * as it is required to identify the root referenced table.
+ */
+ QueueFKConstraintValidation(wqueue, conrel, childrel, pkrelid,
+ childtup, lockmode);
table_close(childrel, NoLock);
}
diff --git a/src/test/regress/expected/foreign_key.out b/src/test/regress/expected/foreign_key.out
index 4f3f280a439..8050a3c4aa3 100644
--- a/src/test/regress/expected/foreign_key.out
+++ b/src/test/regress/expected/foreign_key.out
@@ -1895,29 +1895,44 @@ WHERE conrelid::regclass::text like 'fk_partitioned_fk%' ORDER BY oid::regclass:
(5 rows)
DROP TABLE fk_partitioned_fk, fk_notpartitioned_pk;
--- NOT VALID foreign key on a non-partitioned table referencing a partitioned table
+-- NOT VALID and NOT ENFORCED foreign key on a non-partitioned table referencing a partitioned table
CREATE TABLE fk_partitioned_pk (a int, b int, PRIMARY KEY (a, b)) PARTITION BY RANGE (a, b);
CREATE TABLE fk_partitioned_pk_1 PARTITION OF fk_partitioned_pk FOR VALUES FROM (0,0) TO (1000,1000);
+CREATE TABLE fk_partitioned_pk_2 PARTITION OF fk_partitioned_pk FOR VALUES FROM (1000,1000) TO (2000,2000);
CREATE TABLE fk_notpartitioned_fk (b int, a int);
-ALTER TABLE fk_notpartitioned_fk ADD FOREIGN KEY (a, b) REFERENCES fk_partitioned_pk NOT VALID;
--- Constraint will be invalid.
-SELECT conname, convalidated FROM pg_constraint
+INSERT INTO fk_partitioned_pk VALUES(100,100), (1000,1000);
+INSERT INTO fk_notpartitioned_fk VALUES(100,100), (1000,1000);
+ALTER TABLE fk_notpartitioned_fk ADD CONSTRAINT fk_notpartitioned_fk_a_b_fkey
+ FOREIGN KEY (a, b) REFERENCES fk_partitioned_pk NOT VALID;
+ALTER TABLE fk_notpartitioned_fk ADD CONSTRAINT fk_notpartitioned_fk_a_b_fkey2
+ FOREIGN KEY (a, b) REFERENCES fk_partitioned_pk NOT ENFORCED;
+-- All constraints will be invalid, and _fkey2 constraints will not be enforced.
+SELECT conname, conenforced, convalidated FROM pg_constraint
WHERE conrelid = 'fk_notpartitioned_fk'::regclass ORDER BY oid::regclass::text;
- conname | convalidated
----------------------------------+--------------
- fk_notpartitioned_fk_a_b_fkey | f
- fk_notpartitioned_fk_a_b_fkey_1 | f
-(2 rows)
+ conname | conenforced | convalidated
+----------------------------------+-------------+--------------
+ fk_notpartitioned_fk_a_b_fkey | t | f
+ fk_notpartitioned_fk_a_b_fkey_1 | t | f
+ fk_notpartitioned_fk_a_b_fkey_2 | t | f
+ fk_notpartitioned_fk_a_b_fkey2 | f | f
+ fk_notpartitioned_fk_a_b_fkey2_1 | f | f
+ fk_notpartitioned_fk_a_b_fkey2_2 | f | f
+(6 rows)
ALTER TABLE fk_notpartitioned_fk VALIDATE CONSTRAINT fk_notpartitioned_fk_a_b_fkey;
--- All constraints are now valid.
-SELECT conname, convalidated FROM pg_constraint
+ALTER TABLE fk_notpartitioned_fk ALTER CONSTRAINT fk_notpartitioned_fk_a_b_fkey2 ENFORCED;
+-- All constraints are now valid and enforced.
+SELECT conname, conenforced, convalidated FROM pg_constraint
WHERE conrelid = 'fk_notpartitioned_fk'::regclass ORDER BY oid::regclass::text;
- conname | convalidated
----------------------------------+--------------
- fk_notpartitioned_fk_a_b_fkey | t
- fk_notpartitioned_fk_a_b_fkey_1 | t
-(2 rows)
+ conname | conenforced | convalidated
+----------------------------------+-------------+--------------
+ fk_notpartitioned_fk_a_b_fkey | t | t
+ fk_notpartitioned_fk_a_b_fkey_1 | t | t
+ fk_notpartitioned_fk_a_b_fkey_2 | t | t
+ fk_notpartitioned_fk_a_b_fkey2 | t | t
+ fk_notpartitioned_fk_a_b_fkey2_1 | t | t
+ fk_notpartitioned_fk_a_b_fkey2_2 | t | t
+(6 rows)
DROP TABLE fk_notpartitioned_fk, fk_partitioned_pk;
-- Test some other exotic foreign key features: MATCH SIMPLE, ON UPDATE/DELETE
diff --git a/src/test/regress/sql/foreign_key.sql b/src/test/regress/sql/foreign_key.sql
index 8159e363022..aaeb0f49694 100644
--- a/src/test/regress/sql/foreign_key.sql
+++ b/src/test/regress/sql/foreign_key.sql
@@ -1389,20 +1389,27 @@ WHERE conrelid::regclass::text like 'fk_partitioned_fk%' ORDER BY oid::regclass:
DROP TABLE fk_partitioned_fk, fk_notpartitioned_pk;
--- NOT VALID foreign key on a non-partitioned table referencing a partitioned table
+-- NOT VALID and NOT ENFORCED foreign key on a non-partitioned table referencing a partitioned table
CREATE TABLE fk_partitioned_pk (a int, b int, PRIMARY KEY (a, b)) PARTITION BY RANGE (a, b);
CREATE TABLE fk_partitioned_pk_1 PARTITION OF fk_partitioned_pk FOR VALUES FROM (0,0) TO (1000,1000);
+CREATE TABLE fk_partitioned_pk_2 PARTITION OF fk_partitioned_pk FOR VALUES FROM (1000,1000) TO (2000,2000);
CREATE TABLE fk_notpartitioned_fk (b int, a int);
-ALTER TABLE fk_notpartitioned_fk ADD FOREIGN KEY (a, b) REFERENCES fk_partitioned_pk NOT VALID;
+INSERT INTO fk_partitioned_pk VALUES(100,100), (1000,1000);
+INSERT INTO fk_notpartitioned_fk VALUES(100,100), (1000,1000);
+ALTER TABLE fk_notpartitioned_fk ADD CONSTRAINT fk_notpartitioned_fk_a_b_fkey
+ FOREIGN KEY (a, b) REFERENCES fk_partitioned_pk NOT VALID;
+ALTER TABLE fk_notpartitioned_fk ADD CONSTRAINT fk_notpartitioned_fk_a_b_fkey2
+ FOREIGN KEY (a, b) REFERENCES fk_partitioned_pk NOT ENFORCED;
--- Constraint will be invalid.
-SELECT conname, convalidated FROM pg_constraint
+-- All constraints will be invalid, and _fkey2 constraints will not be enforced.
+SELECT conname, conenforced, convalidated FROM pg_constraint
WHERE conrelid = 'fk_notpartitioned_fk'::regclass ORDER BY oid::regclass::text;
ALTER TABLE fk_notpartitioned_fk VALIDATE CONSTRAINT fk_notpartitioned_fk_a_b_fkey;
+ALTER TABLE fk_notpartitioned_fk ALTER CONSTRAINT fk_notpartitioned_fk_a_b_fkey2 ENFORCED;
--- All constraints are now valid.
-SELECT conname, convalidated FROM pg_constraint
+-- All constraints are now valid and enforced.
+SELECT conname, conenforced, convalidated FROM pg_constraint
WHERE conrelid = 'fk_notpartitioned_fk'::regclass ORDER BY oid::regclass::text;
DROP TABLE fk_notpartitioned_fk, fk_partitioned_pk;
--
2.43.5
* Re: Foreign key validation failure in 18beta1
@ 2025-06-04 03:18 jian he <jian.universality@gmail.com>
parent: Amul Sul <sulamul@gmail.com>
0 siblings, 1 reply; 19+ messages in thread
From: jian he @ 2025-06-04 03:18 UTC (permalink / raw)
To: Amul Sul <sulamul@gmail.com>; +Cc: Alvaro Herrera <alvherre@alvh.no-ip.org>; Tender Wang <tndrwang@gmail.com>; pgsql-hackers@lists.postgresql.org, Antonin Houska <ah@cybertec.at>
On Tue, Jun 3, 2025 at 12:14 PM Amul Sul <sulamul@gmail.com> wrote:
> >
>
> I found a third approach that requires only a few changes. The key
> idea is to determine the root referenced table and pass it to
> QueueFKConstraintValidation(). We would then enqueue phase 3
> validation only if the constraint tuple’s confrelid matches that root
> table -- similar to what is doing in ATExecAlterConstrEnforceability().
>
> This would also ensure that the logic for adding/skipping phase 3
> validation is consistent in both places.
>
indeed!
v7 is way more intuitive compared with v5, v6.
The commit message also looks fine.
^ permalink raw reply [nested|flat] 19+ messages in thread
* Re: Foreign key validation failure in 18beta1
@ 2025-06-05 17:01 Alvaro Herrera <alvherre@alvh.no-ip.org>
parent: jian he <jian.universality@gmail.com>
0 siblings, 1 reply; 19+ messages in thread
From: Alvaro Herrera @ 2025-06-05 17:01 UTC (permalink / raw)
To: jian he <jian.universality@gmail.com>; +Cc: Amul Sul <sulamul@gmail.com>; Tender Wang <tndrwang@gmail.com>; pgsql-hackers@lists.postgresql.org, Antonin Houska <ah@cybertec.at>
On 2025-Jun-04, jian he wrote:
> On Tue, Jun 3, 2025 at 12:14 PM Amul Sul <sulamul@gmail.com> wrote:
>
> > I found a third approach that requires only a few changes. The key
> > idea is to determine the root referenced table and pass it to
> > QueueFKConstraintValidation(). We would then enqueue phase 3
> > validation only if the constraint tuple’s confrelid matches that root
> > table -- similar to what is doing in ATExecAlterConstrEnforceability().
> >
> > This would also ensure that the logic for adding/skipping phase 3
> > validation is consistent in both places.
>
> indeed!
>
> v7 is way more intuitive compared with v5, v6.
Agreed, this version is better than the previous ones. I split it as
two commits, for NOT VALID and NOT ENFORCED separately, and modified some
comments a bit as well as the commit messages.
Thank you!
--
Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/
"Cuando mañana llegue pelearemos segun lo que mañana exija" (Mowgli)
^ permalink raw reply [nested|flat] 19+ messages in thread
* Re: Foreign key validation failure in 18beta1
@ 2025-06-06 04:25 Amul Sul <sulamul@gmail.com>
parent: Alvaro Herrera <alvherre@alvh.no-ip.org>
0 siblings, 0 replies; 19+ messages in thread
From: Amul Sul @ 2025-06-06 04:25 UTC (permalink / raw)
To: Alvaro Herrera <alvherre@alvh.no-ip.org>; +Cc: jian he <jian.universality@gmail.com>; Tender Wang <tndrwang@gmail.com>; pgsql-hackers@lists.postgresql.org, Antonin Houska <ah@cybertec.at>
On Thu, Jun 5, 2025 at 10:31 PM Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
>
> On 2025-Jun-04, jian he wrote:
>
> > On Tue, Jun 3, 2025 at 12:14 PM Amul Sul <sulamul@gmail.com> wrote:
> >
> >
> > v7 is way more intuitive compared with v5, v6.
>
> Agreed, this version is better than the previous ones. I split it as
> two commits, for NOT VALID and NOT ENFORCED separately, and modified some
> comments a bit as well as the commit messages.
>
Yeah, it makes sense to have this as a separate commit. Thank you!
Regards,
Amul
^ permalink raw reply [nested|flat] 19+ messages in thread
end of thread, other threads:[~2025-06-06 04:25 UTC | newest]
Thread overview: 19+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2025-05-28 07:51 Foreign key validation failure in 18beta1 Antonin Houska <ah@cybertec.at>
2025-05-28 10:54 ` Tender Wang <tndrwang@gmail.com>
2025-05-28 12:12 ` Tender Wang <tndrwang@gmail.com>
2025-05-28 12:26 ` Alvaro Herrera <alvherre@alvh.no-ip.org>
2025-05-28 12:38 ` Tender Wang <tndrwang@gmail.com>
2025-05-29 04:35 ` Tender Wang <tndrwang@gmail.com>
2025-05-29 07:08 ` jian he <jian.universality@gmail.com>
2025-05-29 12:12 ` Amul Sul <sulamul@gmail.com>
2025-05-29 12:26 ` jian he <jian.universality@gmail.com>
2025-05-29 12:58 ` Amul Sul <sulamul@gmail.com>
2025-05-30 08:06 ` jian he <jian.universality@gmail.com>
2025-05-30 10:31 ` Amul Sul <sulamul@gmail.com>
2025-06-01 12:35 ` jian he <jian.universality@gmail.com>
2025-06-02 03:26 ` Amul Sul <sulamul@gmail.com>
2025-06-02 16:26 ` Alvaro Herrera <alvherre@alvh.no-ip.org>
2025-06-03 04:14 ` Amul Sul <sulamul@gmail.com>
2025-06-04 03:18 ` jian he <jian.universality@gmail.com>
2025-06-05 17:01 ` Alvaro Herrera <alvherre@alvh.no-ip.org>
2025-06-06 04:25 ` Amul Sul <sulamul@gmail.com>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox