From: Tom Lane <tgl@sss.pgh.pa.us>
To: Alvaro Herrera <alvherre@alvh.no-ip.org>
cc: Kirill Reshke <reshkekirill@gmail.com>, pgsql-bugs@postgresql.org
Subject: Re: Command order bug in pg_dump
In-reply-to: <202504211744.4mzfm7wytiye@alvherre.pgsql>
References: <202504211744.4mzfm7wytiye@alvherre.pgsql>
Comments: In-reply-to Alvaro Herrera <alvherre@alvh.no-ip.org>
	message dated "Mon, 21 Apr 2025 19:44:27 +0200"
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="----- =_aaaaaaaaaa0"
Content-ID: <836717.1745264462.0@sss.pgh.pa.us>
Date: Mon, 21 Apr 2025 15:41:54 -0400
Message-ID: <836817.1745264514@sss.pgh.pa.us>
Archived-At: 
 <https://www.postgresql.org/message-id/836817.1745264514%40sss.pgh.pa.us>
Precedence: bulk

------- =_aaaaaaaaaa0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <836717.1745264462.1@sss.pgh.pa.us>

Alvaro Herrera <alvherre@alvh.no-ip.org> writes:
> Maybe we can change the naming policy so that these internal constraint
> objects have names that are unlikely to be chosen by users, maybe by
> suffixing "fkey_int" instead of "fkey", or something like that.  (We
> could even do "$1" and so on for this kind of constraint).  In
> hindsight, it isn't such a great idea to let the system choose the best
> name for an internal implementation object.

I experimented with the attached, which approximates "add some digits
to the name used for the parent constraint".  (We could refactor
ChooseConstraintName if we wanted a less approximate version of that
rule, but I'm not sure it's worth the trouble.)

The extent to which these derived names leak out to the user, as
illustrated by the regression test changes, makes me even less happy
about the fact that \d doesn't show them.  I think we really ought
to try to find a way to not need these entries.  But that is clearly
not v18 material at this point.

> I'd probably not change this in versions 13 and 14 at all in any case,
> because the code is too different.  I'm unsure whether this is enough of
> a bug to consider backpatching to 15-17; maybe we should just change 18
> at this point, since I haven't heard of a user complaining about this.

Kirill's complaint isn't that?  But I agree that changing this rule in
stable branches would probably be a net negative user experience,
seeing that the names are plenty user-visible.

			regards, tom lane


------- =_aaaaaaaaaa0
Content-Type: text/x-diff;
	name="wip-rename-child-foreign-key-constraints.patch";
	charset="us-ascii"
Content-ID: <836717.1745264462.2@sss.pgh.pa.us>
Content-Description: wip-rename-child-foreign-key-constraints.patch
Content-Transfer-Encoding: quoted-printable

diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/table=
cmds.c
index 265b1c397fb..3c263583451 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -10712,15 +10712,24 @@ addFkConstraint(addFkConstraintSides fkside,
 =

 	/*
 	 * Caller supplies us with a constraint name; however, it may be used in
-	 * this partition, so come up with a different one in that case.
+	 * this partition, so come up with a different one in that case.  Unless
+	 * truncation to NAMEDATALEN dictates otherwise, the new name will be th=
e
+	 * supplied name with digit(s) appended.
 	 */
 	if (ConstraintNameIsUsed(CONSTRAINT_RELATION,
 							 RelationGetRelid(rel),
 							 constraintname))
-		conname =3D ChooseConstraintName(RelationGetRelationName(rel),
-									   ChooseForeignKeyConstraintNameAddition(fkconstraint->fk_attrs=
),
+	{
+		char	   *trimname =3D pstrdup(constraintname);
+		int			trimlen =3D strlen(trimname);
+
+		if (trimlen > 5 && strcmp(trimname + trimlen - 5, "_fkey") =3D=3D 0)
+			trimname[trimlen - 5] =3D '\0';
+		conname =3D ChooseConstraintName(trimname,
+									   NULL,
 									   "fkey",
 									   RelationGetNamespace(rel), NIL);
+	}
 	else
 		conname =3D constraintname;
 =

diff --git a/src/test/regress/expected/constraints.out b/src/test/regress/=
expected/constraints.out
index 92e441a16cd..ad69d2fbbd6 100644
--- a/src/test/regress/expected/constraints.out
+++ b/src/test/regress/expected/constraints.out
@@ -641,9 +641,9 @@ CREATE TABLE parted_fk_naming_1 (
 );
 ALTER TABLE parted_fk_naming ATTACH PARTITION parted_fk_naming_1 FOR VALU=
ES IN ('1');
 SELECT conname FROM pg_constraint WHERE conrelid =3D 'parted_fk_naming_1'=
::regclass AND contype =3D 'f';
-            conname             =

---------------------------------
- parted_fk_naming_1_id_abc_fkey
+      conname      =

+-------------------
+ dummy_constr_fkey
 (1 row)
 =

 DROP TABLE parted_fk_naming;
diff --git a/src/test/regress/expected/foreign_key.out b/src/test/regress/=
expected/foreign_key.out
index c49abc3f0f6..bf68a4b1ab6 100644
--- a/src/test/regress/expected/foreign_key.out
+++ b/src/test/regress/expected/foreign_key.out
@@ -3306,7 +3306,7 @@ INSERT INTO fk_r_1 (id, p_id, p_jd) VALUES (2, 1, 2)=
; -- should fail
 ERROR:  insert or update on table "fk_r_1" violates foreign key constrain=
t "fk_r_p_id_p_jd_fkey"
 DETAIL:  Key (p_id, p_jd)=3D(1, 2) is not present in table "fk_p".
 DELETE FROM fk_p; -- should fail
-ERROR:  update or delete on table "fk_p_1_1" violates foreign key constra=
int "fk_r_1_p_id_p_jd_fkey1" on table "fk_r_1"
+ERROR:  update or delete on table "fk_p_1_1" violates foreign key constra=
int "fk_r_p_id_p_jd_fkey7" on table "fk_r_1"
 DETAIL:  Key (id, jd)=3D(1, 1) is still referenced from table "fk_r_1".
 ALTER TABLE fk_r ATTACH PARTITION fk_r_1 FOR VALUES IN (1);
 ALTER TABLE fk_r ATTACH PARTITION fk_r_2 FOR VALUES IN (2);
diff --git a/src/test/regress/expected/without_overlaps.out b/src/test/reg=
ress/expected/without_overlaps.out
index e38472079cc..32a9863bee0 100644
--- a/src/test/regress/expected/without_overlaps.out
+++ b/src/test/regress/expected/without_overlaps.out
@@ -2319,7 +2319,7 @@ UPDATE temporal_partitioned_rng SET valid_at =3D dat=
erange('2016-02-01', '2016-03-
 -- should fail:
 UPDATE temporal_partitioned_rng SET valid_at =3D daterange('2016-01-01', =
'2016-02-01')
   WHERE id =3D '[5,6)' AND valid_at =3D daterange('2018-01-01', '2018-02-=
01');
-ERROR:  update or delete on table "tp1" violates foreign key constraint "=
temporal_partitioned_fk_rng2rng_parent_id_valid_at_fkey" on table "tempora=
l_partitioned_fk_rng2rng"
+ERROR:  update or delete on table "tp1" violates foreign key constraint "=
temporal_partitioned_fk_rng2rng_fk_fkey" on table "temporal_partitioned_fk=
_rng2rng"
 DETAIL:  Key (id, valid_at)=3D([5,6), [2018-01-01,2018-02-01)) is still r=
eferenced from table "temporal_partitioned_fk_rng2rng".
 --
 -- partitioned FK referenced deletes NO ACTION
@@ -2331,7 +2331,7 @@ INSERT INTO temporal_partitioned_fk_rng2rng (id, val=
id_at, parent_id) VALUES ('[
 DELETE FROM temporal_partitioned_rng WHERE id =3D '[5,6)' AND valid_at =3D=
 daterange('2018-02-01', '2018-03-01');
 -- should fail:
 DELETE FROM temporal_partitioned_rng WHERE id =3D '[5,6)' AND valid_at =3D=
 daterange('2018-01-01', '2018-02-01');
-ERROR:  update or delete on table "tp1" violates foreign key constraint "=
temporal_partitioned_fk_rng2rng_parent_id_valid_at_fkey" on table "tempora=
l_partitioned_fk_rng2rng"
+ERROR:  update or delete on table "tp1" violates foreign key constraint "=
temporal_partitioned_fk_rng2rng_fk_fkey" on table "temporal_partitioned_fk=
_rng2rng"
 DETAIL:  Key (id, valid_at)=3D([5,6), [2018-01-01,2018-02-01)) is still r=
eferenced from table "temporal_partitioned_fk_rng2rng".
 --
 -- partitioned FK referenced updates CASCADE
@@ -2441,7 +2441,7 @@ UPDATE temporal_partitioned_mltrng SET valid_at =3D =
datemultirange(daterange('2016
 -- should fail:
 UPDATE temporal_partitioned_mltrng SET valid_at =3D datemultirange(datera=
nge('2016-01-01', '2016-02-01'))
   WHERE id =3D '[5,6)' AND valid_at =3D datemultirange(daterange('2018-01=
-01', '2018-02-01'));
-ERROR:  update or delete on table "tp1" violates foreign key constraint "=
temporal_partitioned_fk_mltrng2mltrng_parent_id_valid_at_fkey1" on table "=
temporal_partitioned_fk_mltrng2mltrng"
+ERROR:  update or delete on table "tp1" violates foreign key constraint "=
temporal_partitioned_fk_mltrng2mltrng_fk_fkey1" on table "temporal_partiti=
oned_fk_mltrng2mltrng"
 DETAIL:  Key (id, valid_at)=3D([5,6), {[2018-01-01,2018-02-01)}) is still=
 referenced from table "temporal_partitioned_fk_mltrng2mltrng".
 --
 -- partitioned FK referenced deletes NO ACTION
@@ -2453,7 +2453,7 @@ INSERT INTO temporal_partitioned_fk_mltrng2mltrng (i=
d, valid_at, parent_id) VALU
 DELETE FROM temporal_partitioned_mltrng WHERE id =3D '[5,6)' AND valid_at=
 =3D datemultirange(daterange('2018-02-01', '2018-03-01'));
 -- should fail:
 DELETE FROM temporal_partitioned_mltrng WHERE id =3D '[5,6)' AND valid_at=
 =3D datemultirange(daterange('2018-01-01', '2018-02-01'));
-ERROR:  update or delete on table "tp1" violates foreign key constraint "=
temporal_partitioned_fk_mltrng2mltrng_parent_id_valid_at_fkey1" on table "=
temporal_partitioned_fk_mltrng2mltrng"
+ERROR:  update or delete on table "tp1" violates foreign key constraint "=
temporal_partitioned_fk_mltrng2mltrng_fk_fkey1" on table "temporal_partiti=
oned_fk_mltrng2mltrng"
 DETAIL:  Key (id, valid_at)=3D([5,6), {[2018-01-01,2018-02-01)}) is still=
 referenced from table "temporal_partitioned_fk_mltrng2mltrng".
 --
 -- partitioned FK referenced updates CASCADE

------- =_aaaaaaaaaa0--