From: Jehan-Guillaume de Rorthais <jgdr@dalibo.com>
To: Alvaro Herrera <alvherre@alvh.no-ip.org>
Cc: pgsql-hackers <pgsql-hackers@postgresql.org>
Subject: [BUG] wrong FK constraint name when colliding name on ATTACH
Date: Thu, 1 Sep 2022 18:41:56 +0200
Message-ID: <20220901184156.738ebee5@karst> (raw)
In-Reply-To: <20220824104913.hgildshd4w4l6cu4@alvherre.pgsql>
References: <20220824122850.032f9639@karst>
<20220824104913.hgildshd4w4l6cu4@alvherre.pgsql>
Hi,
While studying and hacking on the parenting constraint issue, I found an
incoherent piece of code leading to badly chosen fk name. If a constraint
name collision is detected, while choosing a new name for the constraint,
the code uses fkconstraint->fk_attrs which is not yet populated:
/* No dice. Set up to create our own constraint */
fkconstraint = makeNode(Constraint);
if (ConstraintNameIsUsed(CONSTRAINT_RELATION,
RelationGetRelid(partRel),
NameStr(constrForm->conname)))
fkconstraint->conname =
ChooseConstraintName(RelationGetRelationName(partRel),
ChooseForeignKeyConstraintNameAddition(
fkconstraint->fk_attrs), // <= WOO000OPS
"fkey",
RelationGetNamespace(partRel), NIL);
else
fkconstraint->conname = pstrdup(NameStr(constrForm->conname));
fkconstraint->fk_upd_action = constrForm->confupdtype;
fkconstraint->fk_del_action = constrForm->confdeltype;
fkconstraint->deferrable = constrForm->condeferrable;
fkconstraint->initdeferred = constrForm->condeferred;
fkconstraint->fk_matchtype = constrForm->confmatchtype;
for (int i = 0; i < numfks; i++)
{
Form_pg_attribute att;
att = TupleDescAttr(RelationGetDescr(partRel),
mapped_conkey[i] - 1);
fkconstraint->fk_attrs = lappend(fkconstraint->fk_attrs, // <= POPULATING
makeString(NameStr(att->attname)));
}
The following SQL script showcase the bad constraint name:
DROP TABLE IF EXISTS parent, child1;
CREATE TABLE parent (
id bigint NOT NULL default 1,
no_part smallint NOT NULL,
id_abc bigint,
CONSTRAINT dummy_constr FOREIGN KEY (id_abc, no_part)
REFERENCES parent(id, no_part) ON UPDATE RESTRICT ON DELETE RESTRICT,
PRIMARY KEY (id, no_part)
)
PARTITION BY LIST (no_part);
CREATE TABLE child1 (
id bigint NOT NULL default 1,
no_part smallint NOT NULL,
id_abc bigint,
PRIMARY KEY (id, no_part),
CONSTRAINT dummy_constr CHECK ((no_part = 1))
);
ALTER TABLE parent ATTACH PARTITION child1 FOR VALUES IN ('1');
SELECT conname
FROM pg_constraint
WHERE conrelid = 'child1'::regclass
AND contype = 'f';
DROP TABLE
CREATE TABLE
CREATE TABLE
ALTER TABLE
conname
--------------
child1__fkey
(1 row)
The resulting constraint name "child1__fkey" is missing the attributes name the
original code wanted to add. The expected name is "child1_id_abc_no_part_fkey".
Find in attachment a simple fix, moving the name assignation after the
FK attributes are populated.
Regards,
Attachments:0001-Fix-FK-name-when-colliding-during-partition-attachme.patch (text/x-patch, 2.3K) download
From f1eeacb1eb3face38d76325666aff57019ef84c9 Mon Sep 17 00:00:00 2001
From: Jehan-Guillaume de Rorthais <jgdr@dalibo.com>
Date: Thu, 1 Sep 2022 17:41:55 +0200
Subject: [PATCH] Fix FK name when colliding during partition attachment
During ATLER TABLE ATTACH PARTITION, if the name of a parent's
foreign key constraint is already used on the partition, the code
try to choose another one before the FK attributes list has been
populated. The resulting constraint name was a strange
"<relname>__fkey" instead of "<relname>_<attrs>_fkey".
---
src/backend/commands/tablecmds.c | 20 ++++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.cindex dacc989d85..53b0f3a9c1 100644--- a/src/backend/commands/tablecmds.c+++ b/src/backend/commands/tablecmds.c@@ -10304,16 +10304,6 @@ CloneFkReferencing(List **wqueue, Relation parentRel, Relation partRel)
/* No dice. Set up to create our own constraint */
fkconstraint = makeNode(Constraint);
- if (ConstraintNameIsUsed(CONSTRAINT_RELATION,- RelationGetRelid(partRel),- NameStr(constrForm->conname)))- fkconstraint->conname =- ChooseConstraintName(RelationGetRelationName(partRel),- ChooseForeignKeyConstraintNameAddition(fkconstraint->fk_attrs),- "fkey",- RelationGetNamespace(partRel), NIL);- else- fkconstraint->conname = pstrdup(NameStr(constrForm->conname));
fkconstraint->fk_upd_action = constrForm->confupdtype;
fkconstraint->fk_del_action = constrForm->confdeltype;
fkconstraint->deferrable = constrForm->condeferrable;
@@ -10328,6 +10318,16 @@ CloneFkReferencing(List **wqueue, Relation parentRel, Relation partRel)
fkconstraint->fk_attrs = lappend(fkconstraint->fk_attrs,
makeString(NameStr(att->attname)));
}
+ if (ConstraintNameIsUsed(CONSTRAINT_RELATION,+ RelationGetRelid(partRel),+ NameStr(constrForm->conname)))+ fkconstraint->conname =+ ChooseConstraintName(RelationGetRelationName(partRel),+ ChooseForeignKeyConstraintNameAddition(fkconstraint->fk_attrs),+ "fkey",+ RelationGetNamespace(partRel), NIL);+ else+ fkconstraint->conname = pstrdup(NameStr(constrForm->conname));
indexOid = constrForm->conindid;
constrOid =
--
2.37.2
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: jgdr@dalibo.com, alvherre@alvh.no-ip.org
Subject: Re: [BUG] wrong FK constraint name when colliding name on ATTACH
In-Reply-To: <20220901184156.738ebee5@karst>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
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