pg.ddx.io  pgsql-bugs@postgresql.org mailing list archive  
help / color / mirror / Atom feed
BUG #18741: Detaching a partition referencing a partitioned table fails with a trigger-related error
5+ messages / 4 participants
[nested] [flat]

* BUG #18741: Detaching a partition referencing a partitioned table fails with a trigger-related error
@ 2024-12-08 12:00  PG Bug reporting form <noreply@postgresql.org>
  0 siblings, 2 replies; 5+ messages in thread

From: PG Bug reporting form @ 2024-12-08 12:00 UTC (permalink / raw)
  To: pgsql-bugs@lists.postgresql.org; +Cc: exclusion@gmail.com

The following bug has been logged on the website:

Bug reference:      18741
Logged by:          Alexander Lakhin
Email address:      exclusion@gmail.com
PostgreSQL version: 17.2
Operating system:   Ubuntu 22.04
Description:        

The following script:
CREATE TABLE pt1 (a int PRIMARY KEY) PARTITION BY RANGE (a);
CREATE TABLE p1 PARTITION OF pt1 FOR VALUES FROM (0) TO (1);

CREATE TABLE pt2 (a int) PARTITION BY RANGE (a);
CREATE TABLE p2 (a int, FOREIGN KEY (a) REFERENCES pt1);

ALTER TABLE pt2 ATTACH PARTITION p2 FOR VALUES FROM (0) TO (1);
ALTER TABLE pt2 DETACH PARTITION p2;

fails with:
ERROR:  XX000: could not find ON INSERT check triggers of foreign key
constraint 16404
LOCATION:  GetForeignKeyCheckTriggers, tablecmds.c:11355

Reproduced on REL_15_STABLE (starting from f4566345c) .. master.



^ permalink  raw  reply  [nested|flat] 5+ messages in thread

* Re: BUG #18741: Detaching a partition referencing a partitioned table fails with a trigger-related error
@ 2024-12-09 02:56  Tender Wang <tndrwang@gmail.com>
  parent: PG Bug reporting form <noreply@postgresql.org>
  1 sibling, 1 reply; 5+ messages in thread

From: Tender Wang @ 2024-12-09 02:56 UTC (permalink / raw)
  To: exclusion@gmail.com; pgsql-bugs@lists.postgresql.org

PG Bug reporting form <noreply@postgresql.org> 于2024年12月8日周日 22:43写道:

> The following bug has been logged on the website:
>
> Bug reference:      18741
> Logged by:          Alexander Lakhin
> Email address:      exclusion@gmail.com
> PostgreSQL version: 17.2
> Operating system:   Ubuntu 22.04
> Description:
>
> The following script:
> CREATE TABLE pt1 (a int PRIMARY KEY) PARTITION BY RANGE (a);
> CREATE TABLE p1 PARTITION OF pt1 FOR VALUES FROM (0) TO (1);
>
> CREATE TABLE pt2 (a int) PARTITION BY RANGE (a);
> CREATE TABLE p2 (a int, FOREIGN KEY (a) REFERENCES pt1);
>
> ALTER TABLE pt2 ATTACH PARTITION p2 FOR VALUES FROM (0) TO (1);
> ALTER TABLE pt2 DETACH PARTITION p2;
>
> fails with:
> ERROR:  XX000: could not find ON INSERT check triggers of foreign key
> constraint 16404
> LOCATION:  GetForeignKeyCheckTriggers, tablecmds.c:11355
>
> Reproduced on REL_15_STABLE (starting from f4566345c) .. master.
>
>
Thanks for reporting. I can reproduce this on master.

We currently can handle two cases:
1. parent rel has foreign key, but attachrel doesn't have. So we just clone
parent's fks to attachrel.
2. parent rel and attachrel both have foreign keys, and these fks are
consistent, so we can still use attachrel fk not doing expensive clone.

If I understand correctly, we don't do well when parent rel no fks but
attachrel has one.
We can fix above case, but it seems not easy.

Would it work to reject this attaching directly?  This may affect users who
already do some attaching, like in this case.
If we allow the above case, we can still attach another partition without a
foreign key. However, maintaining consistent catalog data between parent
and child will take much work.

-- 
Thanks,
Tender Wang

^ permalink  raw  reply  [nested|flat] 5+ messages in thread

* Re: BUG #18741: Detaching a partition referencing a partitioned table fails with a trigger-related error
@ 2024-12-31 05:00  jian he <jian.universality@gmail.com>
  parent: Tender Wang <tndrwang@gmail.com>
  0 siblings, 0 replies; 5+ messages in thread

From: jian he @ 2024-12-31 05:00 UTC (permalink / raw)
  To: Tender Wang <tndrwang@gmail.com>; +Cc: exclusion@gmail.com; pgsql-bugs@lists.postgresql.org

On Mon, Dec 9, 2024 at 6:15 PM Tender Wang <tndrwang@gmail.com> wrote:
>
>
>
> PG Bug reporting form <noreply@postgresql.org> 于2024年12月8日周日 22:43写道:
>>
>> The following bug has been logged on the website:
>>
>> Bug reference:      18741
>> Logged by:          Alexander Lakhin
>> Email address:      exclusion@gmail.com
>> PostgreSQL version: 17.2
>> Operating system:   Ubuntu 22.04
>> Description:
>>
>> The following script:
>> CREATE TABLE pt1 (a int PRIMARY KEY) PARTITION BY RANGE (a);
>> CREATE TABLE p1 PARTITION OF pt1 FOR VALUES FROM (0) TO (1);
>>
>> CREATE TABLE pt2 (a int) PARTITION BY RANGE (a);
>> CREATE TABLE p2 (a int, FOREIGN KEY (a) REFERENCES pt1);
>>
>> ALTER TABLE pt2 ATTACH PARTITION p2 FOR VALUES FROM (0) TO (1);
>> ALTER TABLE pt2 DETACH PARTITION p2;
>>
>> fails with:
>> ERROR:  XX000: could not find ON INSERT check triggers of foreign key
>> constraint 16404
>> LOCATION:  GetForeignKeyCheckTriggers, tablecmds.c:11355

>
> Would it work to reject this attaching directly?  This may affect users who already do some attaching, like in this case.
> If we allow the above case, we can still attach another partition without a foreign key. However, maintaining consistent catalog data between parent and child will take much work.
>
In the above case, IMO, error out "ALTER TABLE pt2 ATTACH PARTITION p2
FOR VALUES FROM (0) TO (1);"
would be a good option to solve this problem.
otherwise what kind of  trigger/constraint will add to pt2?
currently it's none.

To minimize the impact of the change (reducing regression differences),

I think we don't need to error out for the following two cases:
ALTER TABLE <nameX> ATTACH PARTITION <nameY>
1. if the attachee (nameY) already referencing another non-partitioned
table, that should be fine.
2. if the attachee (nameY) referencing another partitioned table and
that partitioned table root table is attacher (nameX)

otherwise, if attachee (nameY) referencing another unrelated
partitioned table, we can error out.

Attachments:

  [text/x-patch] v1-0001-disallow-attach-partition-if-referencing-another-.patch (7.4K, ../../CACJufxHmFMuFQQxiXDd=FmvJs+06v2ekrbgb3OCtOFj--S0-Og@mail.gmail.com/2-v1-0001-disallow-attach-partition-if-referencing-another-.patch)
  download | inline diff:
From 215e8929bc8a61998047a8c060b4a0fd95c242f2 Mon Sep 17 00:00:00 2001
From: jian he <jian.universality@gmail.com>
Date: Tue, 31 Dec 2024 10:15:27 +0800
Subject: [PATCH v1 1/1] disallow attach partition if referencing another
 partitioned table

discussion: https://postgr.es/m/18741-e4ef6f7aa8a956cb@postgresql.org
---
 src/backend/commands/tablecmds.c          | 69 +++++++++++++++++++++--
 src/test/regress/expected/foreign_key.out | 20 +++++++
 src/test/regress/sql/foreign_key.sql      | 21 +++++++
 3 files changed, 104 insertions(+), 6 deletions(-)

diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 4937478262..314a1afd1d 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -11090,14 +11090,16 @@ CloneFkReferencing(List **wqueue, Relation parentRel, Relation partRel)
 	AttrMap    *attmap;
 	List	   *partFKs;
 	List	   *clone = NIL;
+	List		*fkpart	= NIL;
+	List		*fkparentRel = NIL;
 	ListCell   *cell;
 	Relation	trigrel;
 
 	/* obtain a list of constraints that we need to clone */
-	foreach(cell, RelationGetFKeyList(parentRel))
-	{
-		ForeignKeyCacheInfo *fk = lfirst(cell);
+	fkparentRel = RelationGetFKeyList(parentRel);
 
+	foreach_node(ForeignKeyCacheInfo, fk, fkparentRel)
+	{
 		/*
 		 * Refuse to attach a table as partition that this partitioned table
 		 * already has a foreign key to.  This isn't useful schema, which is
@@ -11118,6 +11120,60 @@ CloneFkReferencing(List **wqueue, Relation parentRel, Relation partRel)
 		clone = lappend_oid(clone, fk->conoid);
 	}
 
+	/*
+	 * we cannot attach a table as a partition if it is still referencing
+	 * another partitioned table. However, this restriction does not apply if
+	 * the referenced table is the partitioned table itself (i.e., `partRel`
+	 * referencing `parentRel`) or if both the table and the partitioned table
+	 * reference the same another partitioned table.
+	 *
+	 * so the following example should be OK.
+	 * CREATE TABLE pt2 (a int primary key) PARTITION BY RANGE (a);
+	 * CREATE TABLE p2 (a int not null, FOREIGN KEY (a) REFERENCES pt2);
+	 * ALTER TABLE pt2 ATTACH PARTITION p2 FOR VALUES FROM (0) TO (1);
+	 *
+	 * if the table is referencing non-partitioned table, that's OK.
+	*/
+	fkpart = RelationGetFKeyList(partRel);
+	foreach_node(ForeignKeyCacheInfo, fk_attrel, fkpart)
+	{
+		bool	skip = false;
+		foreach_node(ForeignKeyCacheInfo, fk, fkparentRel)
+		{
+			if (fk_attrel->confrelid == fk->confrelid)
+			{
+				skip = true;
+				break;
+			}
+		}
+
+		/*
+		 * if the attachee referencing a partitioned table and that partitioned table root
+		 * table is same as the attachee, then we don't error out.
+		*/
+		if (!skip && get_rel_relkind(fk_attrel->confrelid) == RELKIND_PARTITIONED_TABLE)
+		{
+			List	   *ancestors;
+			Oid			rootrelid;
+
+			ancestors = get_partition_ancestors(fk_attrel->confrelid);
+			if (ancestors == NIL)
+				rootrelid = fk_attrel->confrelid;
+			else
+				rootrelid = llast_oid(ancestors);
+
+			if (ancestors != NIL)
+				list_free(ancestors);
+
+			if (RelationGetRelid(parentRel) != rootrelid)
+				ereport(ERROR,
+						errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+						errmsg("table \"%s\" is still referenced by another partitioned table",
+								RelationGetRelationName(partRel)),
+						errdetail("table referenced by another partitioned table can not be attach as a partition."));
+		}
+	}
+
 	/*
 	 * Silently do nothing if there's nothing to do.  In particular, this
 	 * avoids throwing a spurious error for foreign tables.
@@ -19890,9 +19946,10 @@ DetachPartitionFinalize(Relation rel, Relation partRel, bool concurrent,
 		GetForeignKeyCheckTriggers(trigrel,
 								   fk->conoid, fk->confrelid, fk->conrelid,
 								   &insertTriggerOid, &updateTriggerOid);
-		Assert(OidIsValid(insertTriggerOid));
-		TriggerSetParentTrigger(trigrel, insertTriggerOid, InvalidOid,
-								RelationGetRelid(partRel));
+		// Assert(OidIsValid(insertTriggerOid));
+		if (OidIsValid(insertTriggerOid))
+			TriggerSetParentTrigger(trigrel, insertTriggerOid, InvalidOid,
+									RelationGetRelid(partRel));
 		Assert(OidIsValid(updateTriggerOid));
 		TriggerSetParentTrigger(trigrel, updateTriggerOid, InvalidOid,
 								RelationGetRelid(partRel));
diff --git a/src/test/regress/expected/foreign_key.out b/src/test/regress/expected/foreign_key.out
index 3f459f70ac..3399efe555 100644
--- a/src/test/regress/expected/foreign_key.out
+++ b/src/test/regress/expected/foreign_key.out
@@ -2946,6 +2946,26 @@ DETAIL:  drop cascades to table fkpart11.pk
 drop cascades to table fkpart11.fk_parted
 drop cascades to table fkpart11.fk_another
 drop cascades to function fkpart11.print_row()
+---BUG #18741 https://postgr.es/m/18741-e4ef6f7aa8a956cb@postgresql.org
+CREATE TABLE pk (a int PRIMARY KEY) PARTITION BY RANGE (a);
+ERROR:  relation "pk" already exists
+CREATE TABLE fk (a int) PARTITION BY RANGE (a);
+CREATE TABLE fk_1 (a int, FOREIGN KEY (a) REFERENCES pk);
+--should fail. fk_1 already referencing pk, but fk itself didn't
+ALTER TABLE fk ATTACH PARTITION fk_1 FOR VALUES FROM (0) TO (1);
+ERROR:  table "fk_1" is still referenced by another partitioned table
+DETAIL:  table referenced by another partitioned table can not be attach as a partition.
+DROP TABLE fk, fk_1, pk;
+CREATE TABLE pk (a int primary key) PARTITION BY RANGE (a);
+CREATE TABLE fk (a int not null, FOREIGN KEY (a) REFERENCES pk);
+ALTER TABLE pk ATTACH PARTITION fk FOR VALUES FROM (0) TO (1);
+DROP TABLE fk, pk;
+--should ok. since fk_1 referencing "pk" is non-partitioned table
+CREATE TABLE pk (a int primary key);
+CREATE TABLE fk (a int not null) partition by range(a);
+CREATE TABLE fk_1 (a int not null, FOREIGN KEY (a) REFERENCES pk) partition by range(a);
+ALTER TABLE fk ATTACH PARTITION fk_1 FOR VALUES FROM (0) TO (1);
+DROP TABLE fk, pk;
 -- When a table is attached as partition to a partitioned table that has
 -- a foreign key to another partitioned table, it acquires a clone of the
 -- FK.  Upon detach, this clone is not removed, but instead becomes an
diff --git a/src/test/regress/sql/foreign_key.sql b/src/test/regress/sql/foreign_key.sql
index 2e710e419c..d240b9d583 100644
--- a/src/test/regress/sql/foreign_key.sql
+++ b/src/test/regress/sql/foreign_key.sql
@@ -2087,6 +2087,27 @@ UPDATE fkpart11.pk SET a = 1 WHERE a = 2;
 
 DROP SCHEMA fkpart11 CASCADE;
 
+
+---BUG #18741 https://postgr.es/m/18741-e4ef6f7aa8a956cb@postgresql.org
+CREATE TABLE pk (a int PRIMARY KEY) PARTITION BY RANGE (a);
+CREATE TABLE fk (a int) PARTITION BY RANGE (a);
+CREATE TABLE fk_1 (a int, FOREIGN KEY (a) REFERENCES pk);
+--should fail. fk_1 already referencing pk, but fk itself didn't
+ALTER TABLE fk ATTACH PARTITION fk_1 FOR VALUES FROM (0) TO (1);
+DROP TABLE fk, fk_1, pk;
+
+CREATE TABLE pk (a int primary key) PARTITION BY RANGE (a);
+CREATE TABLE fk (a int not null, FOREIGN KEY (a) REFERENCES pk);
+ALTER TABLE pk ATTACH PARTITION fk FOR VALUES FROM (0) TO (1);
+DROP TABLE fk, pk;
+
+--should ok. since fk_1 referencing "pk" is non-partitioned table
+CREATE TABLE pk (a int primary key);
+CREATE TABLE fk (a int not null) partition by range(a);
+CREATE TABLE fk_1 (a int not null, FOREIGN KEY (a) REFERENCES pk) partition by range(a);
+ALTER TABLE fk ATTACH PARTITION fk_1 FOR VALUES FROM (0) TO (1);
+DROP TABLE fk, pk;
+
 -- When a table is attached as partition to a partitioned table that has
 -- a foreign key to another partitioned table, it acquires a clone of the
 -- FK.  Upon detach, this clone is not removed, but instead becomes an
-- 
2.34.1



^ permalink  raw  reply  [nested|flat] 5+ messages in thread

* Re: BUG #18741: Detaching a partition referencing a partitioned table fails with a trigger-related error
@ 2025-05-03 01:22  Tender Wang <tndrwang@gmail.com>
  parent: PG Bug reporting form <noreply@postgresql.org>
  1 sibling, 1 reply; 5+ messages in thread

From: Tender Wang @ 2025-05-03 01:22 UTC (permalink / raw)
  To: exclusion@gmail.com; pgsql-bugs@lists.postgresql.org; +Cc: Alvaro Herrera <alvherre@alvh.no-ip.org>

PG Bug reporting form <noreply@postgresql.org> 于2024年12月8日周日 22:43写道:

> The following bug has been logged on the website:
>
> Bug reference:      18741
> Logged by:          Alexander Lakhin
> Email address:      exclusion@gmail.com
> PostgreSQL version: 17.2
> Operating system:   Ubuntu 22.04
> Description:
>
> The following script:
> CREATE TABLE pt1 (a int PRIMARY KEY) PARTITION BY RANGE (a);
> CREATE TABLE p1 PARTITION OF pt1 FOR VALUES FROM (0) TO (1);
>
> CREATE TABLE pt2 (a int) PARTITION BY RANGE (a);
> CREATE TABLE p2 (a int, FOREIGN KEY (a) REFERENCES pt1);
>
> ALTER TABLE pt2 ATTACH PARTITION p2 FOR VALUES FROM (0) TO (1);
> ALTER TABLE pt2 DETACH PARTITION p2;
>
> fails with:
> ERROR:  XX000: could not find ON INSERT check triggers of foreign key
> constraint 16404
> LOCATION:  GetForeignKeyCheckTriggers, tablecmds.c:11355
>
> Reproduced on REL_15_STABLE (starting from f4566345c) .. master.
>
>
Hi,

This issue can't be reproduced on HEAD. I guess a commit happened to solve
this problem.
-- 
Thanks,
Tender Wang

^ permalink  raw  reply  [nested|flat] 5+ messages in thread

* Re: BUG #18741: Detaching a partition referencing a partitioned table fails with a trigger-related error
@ 2025-05-03 05:00  Alexander Lakhin <exclusion@gmail.com>
  parent: Tender Wang <tndrwang@gmail.com>
  0 siblings, 0 replies; 5+ messages in thread

From: Alexander Lakhin @ 2025-05-03 05:00 UTC (permalink / raw)
  To: Tender Wang <tndrwang@gmail.com>; +Cc: Alvaro Herrera <alvherre@alvh.no-ip.org>; pgsql-bugs@lists.postgresql.org

Hello Tender Wang,

03.05.2025 04:22, Tender Wang wrote:
>
> PG Bug reporting form <noreply@postgresql.org> 于2024年12月8日周日 22:43写道:
>
>     The following bug has been logged on the website:
>
>     Bug reference:      18741
>
>
> This issue can't be reproduced on HEAD. I guess a commit happened to solve this problem.

Thank you for the testing this!
Yes, according to `git bisect` it is fixed by 9b21f203d.

Best regards,
Alexander Lakhin
Neon (https://neon.tech)

^ permalink  raw  reply  [nested|flat] 5+ messages in thread


end of thread, other threads:[~2025-05-03 05:00 UTC | newest]

Thread overview: 5+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2024-12-08 12:00 BUG #18741: Detaching a partition referencing a partitioned table fails with a trigger-related error PG Bug reporting form <noreply@postgresql.org>
2024-12-09 02:56 ` Tender Wang <tndrwang@gmail.com>
2024-12-31 05:00   ` jian he <jian.universality@gmail.com>
2025-05-03 01:22 ` Tender Wang <tndrwang@gmail.com>
2025-05-03 05:00   ` Alexander Lakhin <exclusion@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