Re: [BUG] wrong FK constraint name when colliding name on ATTACH

Hi there,

I believe this very small bug and its fix are really trivial and could be push
out of the way quite quickly. It's just about a bad constraint name fixed by
moving one assignation after the next one. This could easily be fixed for next
round of releases.

Well, I hope I'm not wrong :)

Regards,

On Thu, 1 Sep 2022 18:41:56 +0200
Jehan-Guillaume de Rorthais <jgdr(at)dalibo(dot)com> wrote:

> 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,