Hi Arseny, Thanks for the patch. I reviewed v1 (CF entry 7062) and tested it locally.
What I tested - Patch applies cleanly with "git apply" on master at 952bab8b74d (no rebase needed). Build with --enable-cassert has no warnings. - Old cluster: PostgreSQL 17.12, new cluster: master 952bab8b74d. - Your reproducer from the first message: without the patch, "pg_upgrade --check" reports "Clusters are compatible" and the real run fails in pg_restore with: constraint "fk_trn_acc" for relation "trn_part1" already exists. With the patch, --check fails with your new message and lists xxi.trn_part1.fk_trn_acc. - Cases that are correctly not reported: a VALID FK on the partition (it is linked to the parent, conparentid != 0), a NOT VALID FK that exists only on the partition, and a partition that was detached from the parent. - make -C src/bin/pg_upgrade check (master to master) passes with the patch. Comments 1. False positives. The check also fires when the restore would succeed, because it depends on the order in which pg_dump emits the constraints. I saw two such cases where the upgrade works without the patch but is now blocked by --check: (a) the partition sorts before the parent (e.g. parent "zz_trn", partition "aa_part", same FK statements as in your example), and (b) a three-level hierarchy where the leaf has its own NOT VALID FK with the same name as the root's FK. You mention that the check flags both orderings; I think that is acceptable given Álvaro's reservations, but it may be worth saying in the commit message or the error text that the upgrade "may" fail, and that the suggested fix is always safe. If it is easy, narrowing the query to the cases that actually fail would be nicer, but I could not find a simple catalog-only condition for that. 2. Report format. The report prints nspname.relname.conname without quoting, so with dots in names it is ambiguous (I tried schema c, table "p.y", constraint "fk.z", which prints c.p.y.fk.z). Other checks have the same style, so this is minor; quote_ident-style output or separate columns would be clearer. 3. Hint text. It says "ALTER TABLE ONLY partition_name DROP CONSTRAINT constraint_name". Maybe use schema.partition_name, since the report prints the schema, and mention that the parent's constraint still covers the partition after dropping it (the partition's own NOT VALID FK is redundant). 4. Root cause. The patch only adds a check; pg_dump output is unchanged. That is the option Álvaro mentioned (while saying he was not fully convinced of it), and since the partition's constraint is basically redundant and the problem only exists for 11-17 source clusters, a --check seems like the right scope to me rather than changing pg_dump. 5. The query and the version range (11 through 17) look right to me. Joining only the direct parent is fine for multi-level partitioning, since the intermediate partition inherits the root's FK under the same name. Test The patch has no test. I wrote a TAP test (src/bin/pg_upgrade/t/010_duplicate_fk_names.pl, plus the meson.build entry). It needs the "oldinstall" environment variable pointing to a version 11-17 installation and is skipped otherwise. It creates your reproducer plus two negative cases, checks that --check fails and lists the constraint, drops the partition constraint as suggested, and then checks that --check and the real pg_upgrade succeed. I ran it with oldinstall = PostgreSQL 17.12: without your patch it fails (--check does not fail), with your patch it passes (6 tests). Without oldinstall it is skipped. I am happy to send it as a patch to this thread if that is useful. Best regards, João Rito Em ter., 10 de nov. de 2026 às 19:46, Arseny Kositsin <[email protected]> escreveu: > > > > So why not just drop the constraint in the partition? Seems rather > > useless. If there's a valid reason to have a constraint that's > > different from the one in the parent table, then you can rename it. > > > > I guess we could add a check to "pg_upgrade --check" to report this kind > > of thing ... I'm not really convinced of this though. Anyhow, I don't > > feel inclined to try to have pg_upgrade/pg_dump handle this case cleanly. > > Thanks for the reply, and sorry for the long delay in responding. > > You're right that the constraint on the partition can simply be dropped > or renamed. However, since the failure occurs during pg_upgrade and > the error message is not very obvious, I think it would be helpful to > detect this situation with "pg_upgrade --check" beforehand, as you > suggested. > > Attached is a patch that implements this check. It detects NOT VALID > foreign keys on partitions that share a name with a foreign key on the > parent partitioned table and were not linked to it (conparentid = 0). > Such constraints may cause a name conflict during restore: pg_dump emits > them as separate ALTER TABLE ADD CONSTRAINT statements for both the > parent and the partition, and when the parent's constraint is restored > first, it is inherited by the partition, so the subsequent attempt to > create the same-named constraint on the partition errors out with > "constraint ... for relation ... already exists". > > Whether the failure actually occurs depends on the order in which > pg_dump emits the constraints, which is determined by alphabetical > sorting of table names; the check flags both cases. > > Only NOT VALID foreign keys are affected. VALID foreign keys on > partitions are matched and linked (conparentid is set) by > tryAttachPartitionForeignKey() during ATTACH PARTITION, so pg_dump > does not emit them separately. This matching logic excluded NOT VALID > constraints until PG 18, where it was fixed by commit b663b9436e7. > The check therefore applies to old clusters from PG 11 (when conparentid > was introduced) through PG 17. > > Other constraint types are not affected: CHECK constraints can have > duplicate names on partitions, but pg_dump emits them inline in > CREATE TABLE; PRIMARY KEY, UNIQUE, and EXCLUDE constraints create > indexes, so duplicate names are prevented by an index name conflict; > NOT NULL constraints as named entries in pg_constraint exist only since > PG 18. > > The patch is for the master branch. I did not add a separate test > because I did not find dedicated tests for other similar check > functions, but I am ready to add one if needed. > > Best regards, > Arseny Kositsyn.
