agora inbox for pgsql-hackers@postgresql.org
help / color / mirror / Atom feedpg_dump: assert failure sorting casts/transforms
8+ messages / 3 participants
[nested] [flat]
* pg_dump: assert failure sorting casts/transforms
@ 2026-08-20 10:20 Alexander Kukushkin <cyberdemn@gmail.com>
2026-08-20 11:03 ` Re: pg_dump: assert failure sorting casts/transforms Nitin Jadhav <nitinjadhavpostgres@gmail.com>
0 siblings, 1 reply; 8+ messages in thread
From: Alexander Kukushkin @ 2026-08-20 10:20 UTC (permalink / raw)
To: Pg Hackers <pgsql-hackers@lists.postgresql.org>
Hi hackers,
pg_dump's DOTypeNameCompare() can reach its Assert(false) fall-through
(pg_dump_sort.c) when a database contains two casts, or two transforms,
whose types share a typname across different schemas. On an
assertion-enabled build this aborts the dump:
pg_dump: pg_dump_sort.c:479: DOTypeNameCompare: Assertion `0' failed.
I hit this in the field with an extension that defines its own json type
alongside pg_catalog.json and casts to both, but it reproduces trivially
without any extension:
CREATE SCHEMA s;
CREATE TYPE public.tgt AS ENUM ('a');
CREATE TYPE s.tgt AS ENUM ('a');
CREATE TYPE public.src AS ENUM ('a');
CREATE CAST (public.src AS public.tgt) WITH INOUT;
CREATE CAST (public.src AS s.tgt) WITH INOUT;
$ pg_dump --schema-only ... # aborts on an --enable-cassert build
The cause: casts and transforms have no namespace of their own, and
getCasts() / getTransforms() build their sort name from the unqualified
type (and language) names. Two casts therefore tie on the full sort key
whenever their source/target type names match but the types live in
different schemas ("sourcetype tgt" in the example); transforms tie the same
way as "typname langname". Since DOTypeNameCompare() has no DO_CAST or
DO_TRANSFORM tiebreaker, such pairs fall through to the assert on master and
RL_19_STABLE branches.
The attached patch adds the missing tiebreakers, comparing the referenced
types by their full natural key via the existing pgTypeNameCompare()
(nspname, then typname) — the same helper already used for function
arguments and operator operands. For transforms, comparing trftype alone
is sufficient, since a name tie already implies the same unqualified typname
and language name. It also adds regression coverage to 002_pg_dump.pl (two
casts and two transforms sharing a typname across schemas), which aborts an
unpatched assert-enabled run and passes with the fix.
Regards,
--
Alexander Kukushkin
Attachments:
[text/x-patch] 0001-pg_dump-sort-casts-and-transforms-independent-of-OID.patch (5.5K, ../../CAFh8B=nb2KLugfF5pFgOLY2Q0db8KOqp=46kxvWNzgvTQeP8oQ@mail.gmail.com/3-0001-pg_dump-sort-casts-and-transforms-independent-of-OID.patch)
download | inline diff:
From 42ca91a5b69381860678a9bda66782fad30ae184 Mon Sep 17 00:00:00 2001
From: Alexander Kukushkin <cyberdemn@gmail.com>
Date: Thu, 20 Aug 2026 12:06:17 +0200
Subject: [PATCH] pg_dump: sort casts and transforms independent of OIDs
DOTypeNameCompare() sorts dumpable objects by (priority, namespace, name,
objType) and then an object-type-specific natural-key tiebreaker. Casts
and transforms have no namespace of their own, and getCasts() /
getTransforms() build their sort "name" from the *unqualified* type (and
language) names. Two casts therefore tie whenever their source and target
type names match while the types live in different schemas -- for example a
cast to pg_catalog.json and a cast to someext.json from the same source
type both get the sort name "sourcetype json". Transforms tie the same way
("typname langname").
With no DO_CAST or DO_TRANSFORM tiebreaker, such pairs reach the
Assert(false) fall-through added in commit 0decd5e89db (aborting
assert-enabled pg_dump), or on non-assert builds sort by OID, reintroducing
exactly the schema-diff instability that commit and its follow-ups
(b61a5c4bed7, 4921a5972a3, d49936f3028) have been eliminating.
Break the tie using the referenced types' full natural keys via the
existing pgTypeNameCompare() (nspname, then typname), the same helper
already used for function arguments and operator operands. For transforms,
comparing trftype alone suffices: a name tie already implies the same
unqualified typname and the same language name, so only the type's schema
can differ.
Add regression coverage to 002_pg_dump.pl: two casts and two transforms
whose types share a typname across schemas, which abort an unpatched
assert-enabled run and pass with the fix.
---
src/bin/pg_dump/pg_dump_sort.c | 28 +++++++++++++++++++++++
src/bin/pg_dump/t/002_pg_dump.pl | 39 ++++++++++++++++++++++++++++++++
2 files changed, 67 insertions(+)
diff --git a/src/bin/pg_dump/pg_dump_sort.c b/src/bin/pg_dump/pg_dump_sort.c
index 03e5c1c1116..6ff106d8ac3 100644
--- a/src/bin/pg_dump/pg_dump_sort.c
+++ b/src/bin/pg_dump/pg_dump_sort.c
@@ -342,6 +342,34 @@ DOTypeNameCompare(const void *p1, const void *p2)
if (cmpval != 0)
return cmpval;
}
+ else if (obj1->objType == DO_CAST)
+ {
+ CastInfo *cobj1 = *(CastInfo *const *) p1;
+ CastInfo *cobj2 = *(CastInfo *const *) p2;
+
+ /*
+ * The "name" is only the source and target type names, unqualified,
+ * so two casts tie whenever their types share typnames across
+ * different schemas. Break the tie by the source then target types'
+ * full natural keys.
+ */
+ cmpval = pgTypeNameCompare(cobj1->castsource, cobj2->castsource);
+ if (cmpval != 0)
+ return cmpval;
+ cmpval = pgTypeNameCompare(cobj1->casttarget, cobj2->casttarget);
+ if (cmpval != 0)
+ return cmpval;
+ }
+ else if (obj1->objType == DO_TRANSFORM)
+ {
+ TransformInfo *tobj1 = *(TransformInfo *const *) p1;
+ TransformInfo *tobj2 = *(TransformInfo *const *) p2;
+
+ /* Same unqualified-typname ambiguity as casts; break by type. */
+ cmpval = pgTypeNameCompare(tobj1->trftype, tobj2->trftype);
+ if (cmpval != 0)
+ return cmpval;
+ }
else if (obj1->objType == DO_ATTRDEF)
{
AttrDefInfo *adobj1 = *(AttrDefInfo *const *) p1;
diff --git a/src/bin/pg_dump/t/002_pg_dump.pl b/src/bin/pg_dump/t/002_pg_dump.pl
index 9258948b583..4307cca8525 100644
--- a/src/bin/pg_dump/t/002_pg_dump.pl
+++ b/src/bin/pg_dump/t/002_pg_dump.pl
@@ -2204,6 +2204,26 @@ my %tests = (
like => { %full_runs, section_pre_data => 1, },
},
+ 'CREATE CAST with typname shared across schemas' => {
+ create_order => 51,
+ create_sql => '
+ CREATE SCHEMA dump_cast_schema;
+ CREATE TYPE public.dump_cast_src AS ENUM (\'a\');
+ CREATE TYPE public.dump_cast_tgt AS ENUM (\'a\');
+ CREATE TYPE dump_cast_schema.dump_cast_tgt AS ENUM (\'a\');
+ CREATE CAST (public.dump_cast_src AS public.dump_cast_tgt) WITH INOUT;
+ CREATE CAST (public.dump_cast_src AS dump_cast_schema.dump_cast_tgt) WITH INOUT;',
+ regexp =>
+ qr/CREATE CAST \(public\.dump_cast_src AS public\.dump_cast_tgt\) WITH INOUT;/m,
+ like => { %full_runs, section_pre_data => 1, },
+ },
+
+ 'CREATE CAST to schema-qualified type sharing a typname' => {
+ regexp =>
+ qr/CREATE CAST \(public\.dump_cast_src AS dump_cast_schema\.dump_cast_tgt\) WITH INOUT;/m,
+ like => { %full_runs, section_pre_data => 1, },
+ },
+
'CREATE DATABASE postgres' => {
regexp => qr/^
\QCREATE DATABASE postgres WITH TEMPLATE = template0 \E
@@ -2927,6 +2947,25 @@ my %tests = (
like => { %full_runs, section_pre_data => 1, },
},
+ 'CREATE TRANSFORM with typname shared across schemas' => {
+ create_order => 34,
+ create_sql => '
+ CREATE SCHEMA dump_trf_schema;
+ CREATE TYPE public.dump_trf_type AS ENUM (\'a\');
+ CREATE TYPE dump_trf_schema.dump_trf_type AS ENUM (\'a\');
+ CREATE TRANSFORM FOR public.dump_trf_type LANGUAGE sql (FROM SQL WITH FUNCTION prsd_lextype(internal));
+ CREATE TRANSFORM FOR dump_trf_schema.dump_trf_type LANGUAGE sql (FROM SQL WITH FUNCTION prsd_lextype(internal));',
+ regexp =>
+ qr/CREATE TRANSFORM FOR public\.dump_trf_type LANGUAGE sql \(FROM SQL WITH FUNCTION pg_catalog\.prsd_lextype\(internal\)\);/m,
+ like => { %full_runs, section_pre_data => 1, },
+ },
+
+ 'CREATE TRANSFORM for schema-qualified type sharing a typname' => {
+ regexp =>
+ qr/CREATE TRANSFORM FOR dump_trf_schema\.dump_trf_type LANGUAGE sql \(FROM SQL WITH FUNCTION pg_catalog\.prsd_lextype\(internal\)\);/m,
+ like => { %full_runs, section_pre_data => 1, },
+ },
+
'CREATE LANGUAGE pltestlang' => {
create_order => 18,
create_sql => 'CREATE LANGUAGE pltestlang
--
2.34.1
^ permalink raw reply [nested|flat] 8+ messages in thread
* Re: pg_dump: assert failure sorting casts/transforms
2026-08-20 10:20 pg_dump: assert failure sorting casts/transforms Alexander Kukushkin <cyberdemn@gmail.com>
@ 2026-08-20 11:03 ` Nitin Jadhav <nitinjadhavpostgres@gmail.com>
2026-08-20 11:40 ` Re: pg_dump: assert failure sorting casts/transforms Alexander Kukushkin <cyberdemn@gmail.com>
0 siblings, 1 reply; 8+ messages in thread
From: Nitin Jadhav @ 2026-08-20 11:03 UTC (permalink / raw)
To: Alexander Kukushkin <cyberdemn@gmail.com>; +Cc: Pg Hackers <pgsql-hackers@lists.postgresql.org>
Hi Alexander,
The fix looks reasonable to me. It matches the existing natural-key
approach in DOTypeNameCompare(), and using pgTypeNameCompare() for the
referenced types seems like the right way to avoid falling back to OID
order.
One small test-coverage suggestion - the added cast test exercises the
casttarget tie-breaker, because both casts use the same source type
and only the target type differs by schema. Since the patch also adds
a castsource tie-breaker, it may be worth adding a symmetric case
where two source types share the same typname across schemas and cast
to the same target type. That would cover both new comparisons
explicitly.
Best Regards,
Nitin Jadhav
Azure Database for PostgreSQL
Microsoft
^ permalink raw reply [nested|flat] 8+ messages in thread
* Re: pg_dump: assert failure sorting casts/transforms
2026-08-20 10:20 pg_dump: assert failure sorting casts/transforms Alexander Kukushkin <cyberdemn@gmail.com>
2026-08-20 11:03 ` Re: pg_dump: assert failure sorting casts/transforms Nitin Jadhav <nitinjadhavpostgres@gmail.com>
@ 2026-08-20 11:40 ` Alexander Kukushkin <cyberdemn@gmail.com>
2026-08-21 07:40 ` Re: pg_dump: assert failure sorting casts/transforms Nitin Jadhav <nitinjadhavpostgres@gmail.com>
0 siblings, 1 reply; 8+ messages in thread
From: Alexander Kukushkin @ 2026-08-20 11:40 UTC (permalink / raw)
To: Nitin Jadhav <nitinjadhavpostgres@gmail.com>; +Cc: Pg Hackers <pgsql-hackers@lists.postgresql.org>
Hi,
On Thu, 20 Aug 2026 at 13:04, Nitin Jadhav <nitinjadhavpostgres@gmail.com>
wrote:
>
> One small test-coverage suggestion - the added cast test exercises the
> casttarget tie-breaker, because both casts use the same source type
> and only the target type differs by schema. Since the patch also adds
> a castsource tie-breaker, it may be worth adding a symmetric case
> where two source types share the same typname across schemas and cast
> to the same target type. That would cover both new comparisons
> explicitly.
Thank you for a review Nitin, here is a v2 version of the patch with
extended test coverage.
--
Regards,
--
Alexander Kukushkin
Attachments:
[text/x-patch] 0001-pg_dump-sort-casts-and-transforms-independent-of-OID-v2.patch (6.2K, ../../CAFh8B=nowWf0sDk01G7B=sa-TpGTObFEK=NoQMJQLzU_H6af=Q@mail.gmail.com/3-0001-pg_dump-sort-casts-and-transforms-independent-of-OID-v2.patch)
download | inline diff:
From ed11873c6421ff7290488b6d11ce5cb72f01eeb2 Mon Sep 17 00:00:00 2001
From: Alexander Kukushkin <akukushkin@microsoft.com>
Date: Thu, 20 Aug 2026 13:34:50 +0200
Subject: [PATCH] pg_dump: sort casts and transforms independent of OIDs
DOTypeNameCompare() sorts dumpable objects by (priority, namespace, name,
objType) and then an object-type-specific natural-key tiebreaker. Casts
and transforms have no namespace of their own, and getCasts() /
getTransforms() build their sort "name" from the *unqualified* type (and
language) names. Two casts therefore tie whenever their source and target
type names match while the types live in different schemas -- for example a
cast to pg_catalog.json and a cast to someext.json from the same source
type both get the sort name "sourcetype json". Transforms tie the same way
("typname langname").
With no DO_CAST or DO_TRANSFORM tiebreaker, such pairs reach the
Assert(false) fall-through added in commit 0decd5e89db (aborting
assert-enabled pg_dump), or on non-assert builds sort by OID, reintroducing
exactly the schema-diff instability that commit and its follow-ups
(b61a5c4bed7, 4921a5972a3, d49936f3028) have been eliminating.
Break the tie using the referenced types' full natural keys via the
existing pgTypeNameCompare() (nspname, then typname), the same helper
already used for function arguments and operator operands. For transforms,
comparing trftype alone suffices: a name tie already implies the same
unqualified typname and the same language name, so only the type's schema
can differ.
Add regression coverage to 002_pg_dump.pl: casts that tie on the target
type's schema and casts that tie on the source type's schema (exercising
both new comparisons), plus two transforms sharing a typname across
schemas. These abort an unpatched assert-enabled run and pass with the fix.
---
src/bin/pg_dump/pg_dump_sort.c | 28 ++++++++++++++++++
src/bin/pg_dump/t/002_pg_dump.pl | 49 ++++++++++++++++++++++++++++++++
2 files changed, 77 insertions(+)
diff --git a/src/bin/pg_dump/pg_dump_sort.c b/src/bin/pg_dump/pg_dump_sort.c
index 03e5c1c1116..6ff106d8ac3 100644
--- a/src/bin/pg_dump/pg_dump_sort.c
+++ b/src/bin/pg_dump/pg_dump_sort.c
@@ -342,6 +342,34 @@ DOTypeNameCompare(const void *p1, const void *p2)
if (cmpval != 0)
return cmpval;
}
+ else if (obj1->objType == DO_CAST)
+ {
+ CastInfo *cobj1 = *(CastInfo *const *) p1;
+ CastInfo *cobj2 = *(CastInfo *const *) p2;
+
+ /*
+ * The "name" is only the source and target type names, unqualified,
+ * so two casts tie whenever their types share typnames across
+ * different schemas. Break the tie by the source then target types'
+ * full natural keys.
+ */
+ cmpval = pgTypeNameCompare(cobj1->castsource, cobj2->castsource);
+ if (cmpval != 0)
+ return cmpval;
+ cmpval = pgTypeNameCompare(cobj1->casttarget, cobj2->casttarget);
+ if (cmpval != 0)
+ return cmpval;
+ }
+ else if (obj1->objType == DO_TRANSFORM)
+ {
+ TransformInfo *tobj1 = *(TransformInfo *const *) p1;
+ TransformInfo *tobj2 = *(TransformInfo *const *) p2;
+
+ /* Same unqualified-typname ambiguity as casts; break by type. */
+ cmpval = pgTypeNameCompare(tobj1->trftype, tobj2->trftype);
+ if (cmpval != 0)
+ return cmpval;
+ }
else if (obj1->objType == DO_ATTRDEF)
{
AttrDefInfo *adobj1 = *(AttrDefInfo *const *) p1;
diff --git a/src/bin/pg_dump/t/002_pg_dump.pl b/src/bin/pg_dump/t/002_pg_dump.pl
index 9258948b583..ae603bfb4c0 100644
--- a/src/bin/pg_dump/t/002_pg_dump.pl
+++ b/src/bin/pg_dump/t/002_pg_dump.pl
@@ -2204,6 +2204,36 @@ my %tests = (
like => { %full_runs, section_pre_data => 1, },
},
+ 'CREATE CAST with typname shared across schemas' => {
+ create_order => 51,
+ create_sql => '
+ CREATE SCHEMA dump_cast_schema;
+ CREATE TYPE public.dump_cast_src AS ENUM (\'a\');
+ CREATE TYPE public.dump_cast_tgt AS ENUM (\'a\');
+ CREATE TYPE dump_cast_schema.dump_cast_tgt AS ENUM (\'a\');
+ CREATE TYPE public.dump_cast_src2 AS ENUM (\'a\');
+ CREATE TYPE dump_cast_schema.dump_cast_src2 AS ENUM (\'a\');
+ CREATE CAST (public.dump_cast_src AS public.dump_cast_tgt) WITH INOUT;
+ CREATE CAST (public.dump_cast_src AS dump_cast_schema.dump_cast_tgt) WITH INOUT;
+ CREATE CAST (public.dump_cast_src2 AS public.dump_cast_tgt) WITH INOUT;
+ CREATE CAST (dump_cast_schema.dump_cast_src2 AS public.dump_cast_tgt) WITH INOUT;',
+ regexp =>
+ qr/CREATE CAST \(public\.dump_cast_src AS public\.dump_cast_tgt\) WITH INOUT;/m,
+ like => { %full_runs, section_pre_data => 1, },
+ },
+
+ 'CREATE CAST to schema-qualified type sharing a typname' => {
+ regexp =>
+ qr/CREATE CAST \(public\.dump_cast_src AS dump_cast_schema\.dump_cast_tgt\) WITH INOUT;/m,
+ like => { %full_runs, section_pre_data => 1, },
+ },
+
+ 'CREATE CAST from schema-qualified type sharing a typname' => {
+ regexp =>
+ qr/CREATE CAST \(dump_cast_schema\.dump_cast_src2 AS public\.dump_cast_tgt\) WITH INOUT;/m,
+ like => { %full_runs, section_pre_data => 1, },
+ },
+
'CREATE DATABASE postgres' => {
regexp => qr/^
\QCREATE DATABASE postgres WITH TEMPLATE = template0 \E
@@ -2927,6 +2957,25 @@ my %tests = (
like => { %full_runs, section_pre_data => 1, },
},
+ 'CREATE TRANSFORM with typname shared across schemas' => {
+ create_order => 34,
+ create_sql => '
+ CREATE SCHEMA dump_trf_schema;
+ CREATE TYPE public.dump_trf_type AS ENUM (\'a\');
+ CREATE TYPE dump_trf_schema.dump_trf_type AS ENUM (\'a\');
+ CREATE TRANSFORM FOR public.dump_trf_type LANGUAGE sql (FROM SQL WITH FUNCTION prsd_lextype(internal));
+ CREATE TRANSFORM FOR dump_trf_schema.dump_trf_type LANGUAGE sql (FROM SQL WITH FUNCTION prsd_lextype(internal));',
+ regexp =>
+ qr/CREATE TRANSFORM FOR public\.dump_trf_type LANGUAGE sql \(FROM SQL WITH FUNCTION pg_catalog\.prsd_lextype\(internal\)\);/m,
+ like => { %full_runs, section_pre_data => 1, },
+ },
+
+ 'CREATE TRANSFORM for schema-qualified type sharing a typname' => {
+ regexp =>
+ qr/CREATE TRANSFORM FOR dump_trf_schema\.dump_trf_type LANGUAGE sql \(FROM SQL WITH FUNCTION pg_catalog\.prsd_lextype\(internal\)\);/m,
+ like => { %full_runs, section_pre_data => 1, },
+ },
+
'CREATE LANGUAGE pltestlang' => {
create_order => 18,
create_sql => 'CREATE LANGUAGE pltestlang
--
2.34.1
^ permalink raw reply [nested|flat] 8+ messages in thread
* Re: pg_dump: assert failure sorting casts/transforms
2026-08-20 10:20 pg_dump: assert failure sorting casts/transforms Alexander Kukushkin <cyberdemn@gmail.com>
2026-08-20 11:03 ` Re: pg_dump: assert failure sorting casts/transforms Nitin Jadhav <nitinjadhavpostgres@gmail.com>
2026-08-20 11:40 ` Re: pg_dump: assert failure sorting casts/transforms Alexander Kukushkin <cyberdemn@gmail.com>
@ 2026-08-21 07:40 ` Nitin Jadhav <nitinjadhavpostgres@gmail.com>
2026-08-21 09:59 ` Re: pg_dump: assert failure sorting casts/transforms Alexander Kukushkin <cyberdemn@gmail.com>
0 siblings, 1 reply; 8+ messages in thread
From: Nitin Jadhav @ 2026-08-21 07:40 UTC (permalink / raw)
To: Alexander Kukushkin <cyberdemn@gmail.com>; +Cc: Pg Hackers <pgsql-hackers@lists.postgresql.org>
> Thank you for a review Nitin, here is a v2 version of the patch with extended test coverage.
Thanks Alexander, v2 addresses my earlier comment. The added src2
casts now cover the case where the source type's schema is the
distinguishing part of the cast key, so both the castsource and
casttarget comparisons are exercised.
I noticed one small test-completeness nit: the test creates four
casts, but I see regex checks for only three emitted CREATE CAST
statements. The unchecked one appears to be: CREATE CAST
(public.dump_cast_src2 AS public.dump_cast_tgt) WITH INOUT; Since this
is the public-side member of the new source-type ambiguity pair, would
it be worth adding a matching regexp for symmetry/completeness?
A couple of minor readability thoughts:
Now that the test covers two separate cases, target-side and
source-side ambiguity, perhaps the test type names could make that
distinction explicit. For example, instead of using dump_cast_src2,
names along these lines might make the intent clearer:
public.dump_cast_src_for_target_test,
public.dump_cast_src_for_source_test,dump_cast_schema.dump_cast_src_for_source_test.
That would make it easier to see which casts exercise the target-type
tie-breaker and which casts exercise the source-type tie-breaker.
The DO_TRANSFORM comment says "Same unqualified-typname ambiguity as
casts; break by type." That is understandable, but maybe it could
mention why comparing only trftype is enough here: the language name
has already been compared as part of dobj.name, so trftype is the
remaining natural-key field that can distinguish the two transform
objects.
These are minor; the code change itself looks reasonable to me.
Best Regards,
Nitin Jadhav
Azure Database for PostgreSQL
Microsoft
^ permalink raw reply [nested|flat] 8+ messages in thread
* Re: pg_dump: assert failure sorting casts/transforms
2026-08-20 10:20 pg_dump: assert failure sorting casts/transforms Alexander Kukushkin <cyberdemn@gmail.com>
2026-08-20 11:03 ` Re: pg_dump: assert failure sorting casts/transforms Nitin Jadhav <nitinjadhavpostgres@gmail.com>
2026-08-20 11:40 ` Re: pg_dump: assert failure sorting casts/transforms Alexander Kukushkin <cyberdemn@gmail.com>
2026-08-21 07:40 ` Re: pg_dump: assert failure sorting casts/transforms Nitin Jadhav <nitinjadhavpostgres@gmail.com>
@ 2026-08-21 09:59 ` Alexander Kukushkin <cyberdemn@gmail.com>
2026-08-22 13:59 ` Re: pg_dump: assert failure sorting casts/transforms Nitin Jadhav <nitinjadhavpostgres@gmail.com>
2026-09-09 00:03 ` Re: pg_dump: assert failure sorting casts/transforms Noah Misch <noah@leadboat.com>
0 siblings, 2 replies; 8+ messages in thread
From: Alexander Kukushkin @ 2026-08-21 09:59 UTC (permalink / raw)
To: Nitin Jadhav <nitinjadhavpostgres@gmail.com>; +Cc: Pg Hackers <pgsql-hackers@lists.postgresql.org>
On Fri, 21 Aug 2026 at 09:40, Nitin Jadhav <nitinjadhavpostgres@gmail.com>
wrote:
>
> These are minor; the code change itself looks reasonable to me.
Thank you Nitin,
here is v3 version of the patch addressing all nit-picks
Regards,
--
Alexander Kukushkin
Attachments:
[text/x-patch] 0001-pg_dump-sort-casts-and-transforms-independent-of-OID-v3.patch (6.7K, ../../CAFh8B=n-Dgffwdr68Or8x-psfGB1G0bMVR4n-NTiwJb8ro=oaA@mail.gmail.com/3-0001-pg_dump-sort-casts-and-transforms-independent-of-OID-v3.patch)
download | inline diff:
From ed11873c6421ff7290488b6d11ce5cb72f01eeb2 Mon Sep 17 00:00:00 2001
From: Alexander Kukushkin <akukushkin@microsoft.com>
Date: Thu, 20 Aug 2026 13:34:50 +0200
Subject: [PATCH] pg_dump: sort casts and transforms independent of OIDs
DOTypeNameCompare() sorts dumpable objects by (priority, namespace, name,
objType) and then an object-type-specific natural-key tiebreaker. Casts
and transforms have no namespace of their own, and getCasts() /
getTransforms() build their sort "name" from the *unqualified* type (and
language) names. Two casts therefore tie whenever their source and target
type names match while the types live in different schemas -- for example a
cast to pg_catalog.json and a cast to someext.json from the same source
type both get the sort name "sourcetype json". Transforms tie the same way
("typname langname").
With no DO_CAST or DO_TRANSFORM tiebreaker, such pairs reach the
Assert(false) fall-through added in commit 0decd5e89db (aborting
assert-enabled pg_dump), or on non-assert builds sort by OID, reintroducing
exactly the schema-diff instability that commit and its follow-ups
(b61a5c4bed7, 4921a5972a3, d49936f3028) have been eliminating.
Break the tie using the referenced types' full natural keys via the
existing pgTypeNameCompare() (nspname, then typname), the same helper
already used for function arguments and operator operands. For transforms,
comparing trftype alone suffices: a name tie already implies the same
unqualified typname and the same language name, so only the type's schema
can differ.
Add regression coverage to 002_pg_dump.pl: casts that tie on the target
type's schema and casts that tie on the source type's schema (exercising
both new comparisons), plus two transforms sharing a typname across
schemas. These abort an unpatched assert-enabled run and pass with the fix.
---
src/bin/pg_dump/pg_dump_sort.c | 32 ++++++++++++++++++++
src/bin/pg_dump/t/002_pg_dump.pl | 55 ++++++++++++++++++++++++++++++++
2 files changed, 87 insertions(+)
diff --git a/src/bin/pg_dump/pg_dump_sort.c b/src/bin/pg_dump/pg_dump_sort.c
index 03e5c1c1116..6ff106d8ac3 100644
--- a/src/bin/pg_dump/pg_dump_sort.c
+++ b/src/bin/pg_dump/pg_dump_sort.c
@@ -342,6 +342,38 @@ DOTypeNameCompare(const void *p1, const void *p2)
if (cmpval != 0)
return cmpval;
}
+ else if (obj1->objType == DO_CAST)
+ {
+ CastInfo *cobj1 = *(CastInfo *const *) p1;
+ CastInfo *cobj2 = *(CastInfo *const *) p2;
+
+ /*
+ * The "name" is only the source and target type names, unqualified,
+ * so two casts tie whenever their types share typnames across
+ * different schemas. Break the tie by the source then target types'
+ * full natural keys.
+ */
+ cmpval = pgTypeNameCompare(cobj1->castsource, cobj2->castsource);
+ if (cmpval != 0)
+ return cmpval;
+ cmpval = pgTypeNameCompare(cobj1->casttarget, cobj2->casttarget);
+ if (cmpval != 0)
+ return cmpval;
+ }
+ else if (obj1->objType == DO_TRANSFORM)
+ {
+ TransformInfo *tobj1 = *(TransformInfo *const *) p1;
+ TransformInfo *tobj2 = *(TransformInfo *const *) p2;
+
+ /*
+ * Same unqualified-typname ambiguity as casts. The language name
+ * was already compared as part of dobj.name, so trftype is the only
+ * remaining natural-key field that can break the tie.
+ */
+ cmpval = pgTypeNameCompare(tobj1->trftype, tobj2->trftype);
+ if (cmpval != 0)
+ return cmpval;
+ }
else if (obj1->objType == DO_ATTRDEF)
{
AttrDefInfo *adobj1 = *(AttrDefInfo *const *) p1;
diff --git a/src/bin/pg_dump/t/002_pg_dump.pl b/src/bin/pg_dump/t/002_pg_dump.pl
index 9258948b583..ae603bfb4c0 100644
--- a/src/bin/pg_dump/t/002_pg_dump.pl
+++ b/src/bin/pg_dump/t/002_pg_dump.pl
@@ -2204,6 +2204,42 @@ my %tests = (
like => { %full_runs, section_pre_data => 1, },
},
+ 'CREATE CAST to public target type sharing a typname' => {
+ create_order => 51,
+ create_sql => '
+ CREATE SCHEMA dump_cast_schema;
+ CREATE TYPE public.dump_cast_src_for_target_test AS ENUM (\'a\');
+ CREATE TYPE public.dump_cast_tgt AS ENUM (\'a\');
+ CREATE TYPE dump_cast_schema.dump_cast_tgt AS ENUM (\'a\');
+ CREATE TYPE public.dump_cast_src_for_source_test AS ENUM (\'a\');
+ CREATE TYPE dump_cast_schema.dump_cast_src_for_source_test AS ENUM (\'a\');
+ CREATE CAST (public.dump_cast_src_for_target_test AS public.dump_cast_tgt) WITH INOUT;
+ CREATE CAST (public.dump_cast_src_for_target_test AS dump_cast_schema.dump_cast_tgt) WITH INOUT;
+ CREATE CAST (public.dump_cast_src_for_source_test AS public.dump_cast_tgt) WITH INOUT;
+ CREATE CAST (dump_cast_schema.dump_cast_src_for_source_test AS public.dump_cast_tgt) WITH INOUT;',
+ regexp =>
+ qr/CREATE CAST \(public\.dump_cast_src_for_target_test AS public\.dump_cast_tgt\) WITH INOUT;/m,
+ like => { %full_runs, section_pre_data => 1, },
+ },
+
+ 'CREATE CAST to schema-qualified target type sharing a typname' => {
+ regexp =>
+ qr/CREATE CAST \(public\.dump_cast_src_for_target_test AS dump_cast_schema\.dump_cast_tgt\) WITH INOUT;/m,
+ like => { %full_runs, section_pre_data => 1, },
+ },
+
+ 'CREATE CAST from public source type sharing a typname' => {
+ regexp =>
+ qr/CREATE CAST \(public\.dump_cast_src_for_source_test AS public\.dump_cast_tgt\) WITH INOUT;/m,
+ like => { %full_runs, section_pre_data => 1, },
+ },
+
+ 'CREATE CAST from schema-qualified source type sharing a typname' => {
+ regexp =>
+ qr/CREATE CAST \(dump_cast_schema\.dump_cast_src_for_source_test AS public\.dump_cast_tgt\) WITH INOUT;/m,
+ like => { %full_runs, section_pre_data => 1, },
+ },
+
'CREATE DATABASE postgres' => {
regexp => qr/^
\QCREATE DATABASE postgres WITH TEMPLATE = template0 \E
@@ -2927,6 +2957,25 @@ my %tests = (
like => { %full_runs, section_pre_data => 1, },
},
+ 'CREATE TRANSFORM with typname shared across schemas' => {
+ create_order => 34,
+ create_sql => '
+ CREATE SCHEMA dump_trf_schema;
+ CREATE TYPE public.dump_trf_type AS ENUM (\'a\');
+ CREATE TYPE dump_trf_schema.dump_trf_type AS ENUM (\'a\');
+ CREATE TRANSFORM FOR public.dump_trf_type LANGUAGE sql (FROM SQL WITH FUNCTION prsd_lextype(internal));
+ CREATE TRANSFORM FOR dump_trf_schema.dump_trf_type LANGUAGE sql (FROM SQL WITH FUNCTION prsd_lextype(internal));',
+ regexp =>
+ qr/CREATE TRANSFORM FOR public\.dump_trf_type LANGUAGE sql \(FROM SQL WITH FUNCTION pg_catalog\.prsd_lextype\(internal\)\);/m,
+ like => { %full_runs, section_pre_data => 1, },
+ },
+
+ 'CREATE TRANSFORM for schema-qualified type sharing a typname' => {
+ regexp =>
+ qr/CREATE TRANSFORM FOR dump_trf_schema\.dump_trf_type LANGUAGE sql \(FROM SQL WITH FUNCTION pg_catalog\.prsd_lextype\(internal\)\);/m,
+ like => { %full_runs, section_pre_data => 1, },
+ },
+
'CREATE LANGUAGE pltestlang' => {
create_order => 18,
create_sql => 'CREATE LANGUAGE pltestlang
--
2.34.1
^ permalink raw reply [nested|flat] 8+ messages in thread
* Re: pg_dump: assert failure sorting casts/transforms
2026-08-20 10:20 pg_dump: assert failure sorting casts/transforms Alexander Kukushkin <cyberdemn@gmail.com>
2026-08-20 11:03 ` Re: pg_dump: assert failure sorting casts/transforms Nitin Jadhav <nitinjadhavpostgres@gmail.com>
2026-08-20 11:40 ` Re: pg_dump: assert failure sorting casts/transforms Alexander Kukushkin <cyberdemn@gmail.com>
2026-08-21 07:40 ` Re: pg_dump: assert failure sorting casts/transforms Nitin Jadhav <nitinjadhavpostgres@gmail.com>
2026-08-21 09:59 ` Re: pg_dump: assert failure sorting casts/transforms Alexander Kukushkin <cyberdemn@gmail.com>
@ 2026-08-22 13:59 ` Nitin Jadhav <nitinjadhavpostgres@gmail.com>
1 sibling, 0 replies; 8+ messages in thread
From: Nitin Jadhav @ 2026-08-22 13:59 UTC (permalink / raw)
To: Alexander Kukushkin <cyberdemn@gmail.com>; +Cc: Pg Hackers <pgsql-hackers@lists.postgresql.org>
> here is v3 version of the patch addressing all nit-picks
Thanks for the v3 patch. It addresses my earlier comments, and the fix
and regression coverage look good to me. I have no other comments.
One optional follow-up thought: the current tests exercise the
assertion-failure case well. Order-aware checks could additionally
verify the OID-independent ordering property and help detect future
regressions to an OID fallback. I realize the earlier natural-key
ordering changes do not consistently include explicit output-order
checks either, so I do not think this should delay the current patch
or require a v4. If there is interest, I can investigate a separate
follow-up patch for suitable coverage of this case and the earlier
OID-independence cases.
Since the issue also affects REL_19_STABLE, I think the fix should be
applied there along with master. No older back-branches appear to be
affected.
Best Regards,
Nitin Jadhav
Azure Database for PostgreSQL
Microsoft
^ permalink raw reply [nested|flat] 8+ messages in thread
* Re: pg_dump: assert failure sorting casts/transforms
2026-08-20 10:20 pg_dump: assert failure sorting casts/transforms Alexander Kukushkin <cyberdemn@gmail.com>
2026-08-20 11:03 ` Re: pg_dump: assert failure sorting casts/transforms Nitin Jadhav <nitinjadhavpostgres@gmail.com>
2026-08-20 11:40 ` Re: pg_dump: assert failure sorting casts/transforms Alexander Kukushkin <cyberdemn@gmail.com>
2026-08-21 07:40 ` Re: pg_dump: assert failure sorting casts/transforms Nitin Jadhav <nitinjadhavpostgres@gmail.com>
2026-08-21 09:59 ` Re: pg_dump: assert failure sorting casts/transforms Alexander Kukushkin <cyberdemn@gmail.com>
@ 2026-09-09 00:03 ` Noah Misch <noah@leadboat.com>
2026-10-02 07:25 ` Re: pg_dump: assert failure sorting casts/transforms Alexander Kukushkin <cyberdemn@gmail.com>
1 sibling, 1 reply; 8+ messages in thread
From: Noah Misch @ 2026-09-09 00:03 UTC (permalink / raw)
To: Alexander Kukushkin <cyberdemn@gmail.com>; +Cc: Nitin Jadhav <nitinjadhavpostgres@gmail.com>; Pg Hackers <pgsql-hackers@lists.postgresql.org>
On Fri, Aug 21, 2026 at 11:59:53AM +0200, Alexander Kukushkin wrote:
> here is v3 version of the patch addressing all nit-picks
Thanks.
> With no DO_CAST or DO_TRANSFORM tiebreaker, such pairs reach the
> Assert(false) fall-through added in commit 0decd5e89db (aborting
> assert-enabled pg_dump), or on non-assert builds sort by OID, reintroducing
> exactly the schema-diff instability that commit and its follow-ups
> (b61a5c4bed7, 4921a5972a3, d49936f3028) have been eliminating.
Since this is already the fourth follow-up to my original change, I had Opus 5
look for more ways to reach the assertion. It found one more:
D1 DO_POLICY: the "RLS enabled" pseudo-object borrows its table's relname, so
it ties with a policy named after that same table. An assert-enabled
pg_dump aborts; a production build orders the two by comparing a pg_class
OID against a pg_policy OID, which pg_upgrade inverts.
Let's fix that at the same time. Would you like to add that, or would you
like me to add it?
I'm attaching the larger Opus 5 report as an FYI. It found many other pg_dump
ordering problems distinct from the DOTypeNameCompare() assertion, and D3 is a
notable functional bug. They're off-topic for $SUBJECT, though.
From 99d7f92890217317005c6e5d6b0d41bf616e3fc5 Mon Sep 17 00:00:00 2001
From: Noah Misch <noah@leadboat.com>
Date: Thu, 3 Sep 2026 18:06:51 +0000
Subject: [PATCH 1/2] Audit report and regression tests for pg_dump dump-order
instability
Audit of what the DO_CAST/DO_TRANSFORM tiebreakers in the preceding commit do
not cover. Twelve confirmed defects; only one of them is another tie in
DOTypeNameCompare().
D1 DO_POLICY: the "RLS enabled" pseudo-object borrows its table's relname, so
it ties with a policy named after that same table. An assert-enabled
pg_dump aborts; a production build orders the two by comparing a pg_class
OID against a pg_policy OID, which pg_upgrade inverts.
D2 Dependency-loop repair picks its start point in dumpId order, so which
object is broken out of a cycle follows OID assignment. Reported only:
fixing it changes the dump of databases that dump fine today.
D3 getInherits() has no ORDER BY and pg_dump never reads inhseqno, so the
INHERITS list follows pg_inherits heap order. This is not only an
ordering defect: a plain dump/restore can permute the child's columns.
D4 dumpOpfamily()/dumpOpclass() order members by strategy number alone.
D5 getPolicies() builds the policy's TO role list with an unordered
sub-select over pg_roles.
D6 getPublications() does not order the FOR ALL TABLES EXCEPT list.
D7 dumpDatabaseConfig() does not order per-role database settings.
D8 append_depends_on_extension() does not order its rows.
D9 collectSecLabels() omits provider from its ORDER BY, as does the
shared-object path in dumputils.c.
D10 pg_dumpall's dumpTablespaces() says ORDER BY 1 on a select list whose
first column is oid; the sibling dumpRoles() says ORDER BY 2.
D11 dumpExtension() emits an extension's requires array under
--binary-upgrade in dependency-array order, which is OID-derived.
D12 getDefaultACLs() emits defaclacl in the backend's canonical order, which
aclitemsort() makes grantee-OID order.
Tests cover all of these but D2. Each fails, or aborts pg_dump, on the tree
without the sample fixes in the following commit. Two needed test
infrastructure rather than a test entry: D8 lives in test_pg_dump because
showing it needs one object with two extension dependencies and a bare initdb
has only plpgsql, and D9 adds a second label provider to dummy_seclabel.
This work is model-generated and unreviewed by a human; see PROVENANCE.md.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Newm1jZHVy54kfX1eRPDwB
---
DUMP_SORT_STABILITY_REPORT.md | 878 ++++++++++++++++++
PROVENANCE.md | 126 +++
src/bin/pg_dump/t/002_pg_dump.pl | 238 ++++-
src/bin/pg_dump/t/003_pg_dump_with_server.pl | 91 ++
.../modules/dummy_seclabel/dummy_seclabel.c | 26 +
src/test/modules/test_pg_dump/t/001_base.pl | 45 +
6 files changed, 1399 insertions(+), 5 deletions(-)
create mode 100644 DUMP_SORT_STABILITY_REPORT.md
create mode 100644 PROVENANCE.md
diff --git a/DUMP_SORT_STABILITY_REPORT.md b/DUMP_SORT_STABILITY_REPORT.md
new file mode 100644
index 0000000..6380580
--- /dev/null
+++ b/DUMP_SORT_STABILITY_REPORT.md
@@ -0,0 +1,878 @@
+# pg_dump dump-order stability: audit of what `sort_cast.patch` does not cover
+
+**Question asked:** after the DO_CAST / DO_TRANSFORM tiebreakers, are more sources of
+dump-order instability still lurking?
+
+**Answer:** yes -- **twelve**, but only **one** of them is another tie in
+`DOTypeNameCompare()`. The other eleven are outside the object sort: one in dependency-loop
+repair, ten in dump-time queries and emitters whose result order reaches the output text
+directly. One of those is not merely an ordering nuisance: it makes a plain `pg_dump`
+→ restore **silently permute an inherited table's column order**.
+
+| # | Where | What | Severity |
+|---|---|---|---|
+| D1 | `DOTypeNameCompare()` | the RLS-enabled pseudo-object ties with a policy named after its own table -- `pg_dump` **aborts** on an assert build | high |
+| D2 | `pg_dump_sort.c` loop repair | `TopoSort()` builds its failure list in dumpId order, so which object gets broken out of a dependency cycle follows OID assignment | medium |
+| D3 | `getInherits()` | no `ORDER BY`; `inhseqno` is never read at all, so the `INHERITS` list follows heap order -- **restores with the child's columns permuted** | high |
+| D4 | `dumpOpfamily()`, `dumpOpclass()` | member lists ordered only by `amopstrategy` / `amprocnum`, which is not a key | medium |
+| D5 | `getPolicies()` | the policy `TO` role list is built by an unordered sub-select | medium |
+| D6 | `getPublications()` | `FOR ALL TABLES EXCEPT (...)` list has no `ORDER BY` | medium |
+| D7 | `dumpDatabaseConfig()` | `ALTER ROLE ... IN DATABASE ... SET` lines have no `ORDER BY` | medium |
+| D8 | `append_depends_on_extension()` | `DEPENDS ON EXTENSION` lines have no `ORDER BY` | low |
+| D9 | `collectSecLabels()`, `dumputils.c` | `ORDER BY` omits `provider`, which is part of the key | low |
+| D10 | `pg_dumpall.c` `dumpTablespaces()` | `ORDER BY 1` on `SELECT oid, spcname, ...` sorts by **OID** | low |
+| D11 | `dumpExtension()` `--binary-upgrade` | the `requires` array is emitted in dependency-array order, which is OID-derived | low |
+| D12 | `getDefaultACLs()` | `defaclacl` is emitted in the backend's canonical **grantee-OID** order | medium |
+
+Every one of the twelve was reproduced twice: once by an agent that found it, once by an
+independent agent whose brief was to refute it. D1, D3, D4, D2 and D12 were additionally
+reproduced by hand, outside the agent framework; the commands are in this report.
+
+A twelfth candidate class -- `pg_dump` reproducing the element order of array-valued
+catalog columns (`aclitem[]`, `setconfig`, `reloptions`) -- was **rejected**, and the
+argument that killed it is worth reading: see [Considered and rejected](#considered-and-rejected).
+
+---
+
+## Method
+
+Three oracles, in increasing order of reach.
+
+**1. The stock assert-enabled build.** `DOTypeNameCompare()`'s fall-through is
+`Assert(false)`, so on an assert build a tie makes `pg_dump` abort. This is the oracle
+that matters most, because it is exactly what a developer or a buildfarm animal sees, and
+it needs no instrumentation.
+
+**2. A tie reporter.** An audit-only build (`ss-shuf-inst`) whose
+`sortDumpableObjectsByTypeName()` walks the sorted array afterwards and reports every
+adjacent pair for which the comparator reached the fall-through, with
+`describeDumpableObject()` output for both. Ties are necessarily adjacent after a sort, so
+one run enumerates *all* of them rather than aborting at the first. The `Assert` is
+disarmed in that build, which also makes it a faithful stand-in for a production build:
+with no environment variables set it falls through to `oidcmp()` exactly as a non-assert
+`pg_dump` does.
+
+**3. A pre-sort shuffle.** The same build permutes the object array before sorting when
+`PGDUMP_SHUFFLE_SEED` is set, and makes the fall-through return 0 rather than comparing
+OIDs, so a tie leaves the order genuinely up to `qsort`. Eight seeds, then diff. Dump
+output must not depend on the input permutation; if it does, the order is unstable --
+whatever the cause, including causes that never reach the comparator.
+
+Calibration: all three fire on the DO_POLICY case (D1) and all three are silent on the
+core regression database (2291 relations) and on control schemas. The shuffle oracle is
+blind to the D3..D11 class by construction -- those orders come from the *server's* result
+order, which is identical under every seed -- so that class had to be found by reading the
+queries and confirmed by diffing two independently built databases. Where a finding is of
+that kind, the report says so explicitly and gives the two-database pair.
+
+**Fan-out.** A first workflow ran 17 discovery agents -- 8 sweeping the 48
+`DumpableObjectType` values against their catalogs' natural keys, 4 code lenses (the
+topological sort; every dump-time query in `pg_dump.c`; `pg_dumpall.c` and the archive TOC;
+the history of the five commits that already fixed this class), and 5 empirical lanes
+(cross-schema name collisions for every schema-qualified object type; pg_dump's
+manufactured pseudo-objects; in-tree extensions; the regression corpus; a
+differential two-database generator). Their 54 raw candidates deduplicated to 49, each of
+which got an independent verification agent and, unless refuted, an independent adversarial
+judge told to refute it. 40 survived; those 40 describe **11 distinct mechanisms** --
+the same defect was found by up to 13 agents through different object types. A second
+workflow put one agent on each surviving mechanism to re-reproduce it from scratch, write
+its regression test and write a sample fix, plus two adversarial critics.
+
+**What this method cannot see.** Dump-order instability that requires a server version
+older than this tree (`pg_dump` supports back to 9.2 and builds several queries in
+version-dependent branches; only the modern branch was exercised), instability visible only
+under `pg_restore -j` scheduling, and anything needing a platform this box is not.
+
+---
+
+## Part 1 -- the object sort
+
+### D1. `DO_POLICY`: the RLS-enabled pseudo-object collides with a policy named after its table
+
+`getPolicies()` represents "row level security is enabled on this table" as a `PolicyInfo`
+with `polname == NULL`, and gives it the **table's** name:
+
+```c
+/* src/bin/pg_dump/pg_dump.c:4249 */
+polinfo->dobj.objType = DO_POLICY;
+polinfo->dobj.catId.tableoid = 0;
+polinfo->dobj.catId.oid = tbinfo->dobj.catId.oid;
+AssignDumpId(&polinfo->dobj);
+polinfo->dobj.namespace = tbinfo->dobj.namespace;
+polinfo->dobj.name = pg_strdup(tbinfo->dobj.name); /* <-- borrowed */
+polinfo->poltable = tbinfo;
+polinfo->polname = NULL;
+```
+
+A real policy gets `dobj.name = polname` and the same namespace, and the `DO_POLICY`
+tiebreaker compares only the table name:
+
+```c
+/* src/bin/pg_dump/pg_dump_sort.c */
+else if (obj1->objType == DO_POLICY)
+{
+ /* Sort by table name (table namespace was considered already) */
+ cmpval = strcmp(pobj1->poltable->dobj.name, pobj2->poltable->dobj.name);
+ if (cmpval != 0)
+ return cmpval;
+}
+```
+
+So for a policy whose `polname` equals its own table's `relname`, every step returns 0:
+same priority, same namespace, same name, same objType, same table. This is the same shape
+as the cast/transform defect -- a name that is not the object's own -- but arrived at from
+the other direction: instead of building a name out of other objects' unqualified names, it
+*borrows* one wholesale.
+
+Three statements reproduce it:
+
+```sql
+CREATE TABLE pol_t (i int);
+ALTER TABLE pol_t ENABLE ROW LEVEL SECURITY;
+CREATE POLICY pol_t ON pol_t USING (true);
+```
+
+```
+$ /home/nm/src/pg/ssrun/pgrun.sh plain t_policy.sql >/dev/null
+pg_dump: pg_dump_sort.c:511: DOTypeNameCompare: Assertion `0' failed.
+PG_DUMP FAILED (exit 134)
+ASSERTION FAILURE
+
+$ /home/nm/src/pg/ssrun/pgrun.sh tie t_policy.sql >/dev/null
+SORT TIE: objType 41 name "pol_t" nsp "public" | POLICY (ID 3523 OID 16384) | POLICY (ID 3524 OID 16387)
+TIES DETECTED
+```
+
+**On an assert-enabled build the database is simply not dumpable.** On a production build
+the tie falls through to `oidcmp()`, comparing the *table's* OID (the pseudo-object carries
+`catId.oid = table oid`, `tableoid = 0`) against the *pg_policy* OID. In a
+normally-built database the policy always postdates its table, so the order is stable by
+luck. It stops being stable exactly where `pg_upgrade` operates: relation OIDs are
+preserved across an upgrade, `pg_policy` OIDs are not. Give the table a high OID in the
+old cluster and the restored policy gets a low one, and the two logically identical
+databases dump in opposite orders:
+
+```
+== old cluster OIDs == == new cluster OIDs ==
+ table | 18784 table | 18784 (preserved)
+ policy | 18787 policy | 16384 (reassigned)
+
+$ diff -u old.dump new.dump
+--- Name: rls_demo; Type: ROW SECURITY; Schema: public; Owner: postgres
++-- Name: rls_demo rls_demo; Type: POLICY; Schema: public; Owner: postgres
+-ALTER TABLE public.rls_demo ENABLE ROW LEVEL SECURITY;
++CREATE POLICY rls_demo ON public.rls_demo USING (true);
+```
+
+(`/home/nm/src/pg/ssrun/oidflip.sh` -- it pads the OID counter, takes a
+`pg_dump --binary-upgrade --schema-only`, restores it into a second cluster started with
+`-b`, and dumps both with the assert-disarmed build. That is the flake the comment above
+`Assert(false)` predicts, reproduced deliberately.)
+
+The fix is the missing natural-key column. `pg_policy`'s key is `(polrelid, polname)`; the
+pseudo-object is the one row where `polname` is absent, so comparing "is `polname` NULL"
+after the table name completes the key:
+
+```c
+ /*
+ * The RLS-enabled pseudo-object (polname == NULL) borrows its name from
+ * its table, so it ties with a policy whose polname equals that table
+ * name. Sort the pseudo-object first, consistent with ENABLE ROW LEVEL
+ * SECURITY logically preceding the policies on the table.
+ */
+ if (pobj1->polname == NULL)
+ {
+ if (pobj2->polname != NULL)
+ return -1;
+ }
+ else if (pobj2->polname == NULL)
+ return 1;
+```
+
+Two non-NULL `polname`s on the same table cannot both survive to this point: `polname`
+*is* `dobj.name`, already compared at step 3.
+
+### Why nothing else in the comparator ties
+
+D1 is the only tie the audit found, and that claim was put to a dedicated adversarial
+critic whose brief was to falsify it. The reason the rest of the comparator is sound comes
+down to two observations that are worth recording, because they are what a future reviewer
+needs in order to check a new object type:
+
+1. **Constructed names are now closed.** Only three construction sites build a
+ `dobj.name` out of other names rather than copying a catalog column: `getCasts()` and
+ `getTransforms()` (fixed by `885a841`) and `getLOs()`, whose name is a large-object OID
+ range -- and a large object's OID *is* its identity, so that one is not a defect.
+2. **Borrowed names are safe wherever the borrower has its priority to itself.** Twelve
+ object types take their name from another object -- `DO_TABLE_ATTACH`,
+ `DO_INDEX_ATTACH`, `DO_ATTRDEF`, `DO_TABLE_DATA`, `DO_SEQUENCE_SET`,
+ `DO_REFRESH_MATVIEW`, `DO_REL_STATS`, `DO_SHELL_TYPE`, `DO_DUMMY_TYPE`,
+ `DO_PUBLICATION_REL`, `DO_PUBLICATION_TABLE_IN_SCHEMA`, `DO_SUBSCRIPTION_REL` -- and
+ every one of them is either alone at its priority level or separated from its
+ priority-mate by the `objType` comparison, and each has at most one instance per
+ borrowed-from object. `DO_POLICY` is the single case where a borrowed-name
+ pseudo-object shares both a priority *and* an `objType` with a genuinely named object.
+
+Two near misses are worth a note rather than a change, and both were refuted with
+structural arguments rather than merely not reproduced:
+
+* `DO_INDEX` takes its namespace from its table rather than from `pg_class.relnamespace`,
+ and has no tiebreaker. Today an index's `relnamespace` is pinned to its table's, so
+ `(namespace, name)` still reduces to `pg_class_relname_nsp_index`; the sort key is one
+ line narrower than the natural key, but nothing can exploit it.
+* the pseudo-objects built with `catId.tableoid = 0, catId.oid = 0`
+ (`DO_TABLE_ATTACH`, `DO_INDEX_ATTACH`, `DO_REL_STATS`) have no OID for the
+ `oidcmp()` safety net to fall back on, so if a future change did introduce a tie among
+ them, the fall-through would return 0 and the order would be pure `qsort` luck rather
+ than merely OID-dependent.
+
+---
+
+## Part 2 -- dependency-loop repair
+
+### D2. `TopoSort()` reports its failures in dumpId order, so loop repair follows OID assignment
+
+When the dependency graph has a cycle, `TopoSort()` fails and hands the objects it could
+not place to `findDependencyLoops()`, which finds a cycle and calls
+`repairDependencyLoop()` to break it -- by marking one object `separate`, so that (for
+example) a `CHECK` constraint moves out of `CREATE TABLE` into a post-data
+`ALTER TABLE ... ADD CONSTRAINT`, or one view of a mutually-recursive pair is emitted as a
+dummy `SELECT NULL::...` placeholder and rebuilt later with `CREATE OR REPLACE VIEW`.
+
+Which object gets chosen is decided by OID assignment order, not by name. Three links:
+
+```c
+/* pg_dump_sort.c:757 -- the failure list is rebuilt in dumpId order, discarding
+ * the name-sorted order the caller passed in */
+k = 0;
+for (j = 1; j <= maxDumpId; j++)
+{
+ if (beforeConstraints[j] != 0)
+ ordering[k++] = objs[idMap[j]];
+}
+```
+
+`findDependencyLoops()` then walks that array front to back, so `loop[0]` is the
+lowest-dumpId cycle member; and `repairDependencyLoop()`'s multi-object branches scan
+`loop[]` front to back and repair the *first* member of the type they are looking for.
+dumpIds are handed out by `AssignDumpId()` in catalog-scan order, and the scans are
+OID-ordered (`getTables()` ends `ORDER BY c.oid`; `getTypes()` and `getFuncs()` have no
+`ORDER BY` at all, so heap order). The whole repair decision therefore rides on which
+object was created first.
+
+Four statements, differing only in which of two domains is created first:
+
+```sql
+-- A -- B
+CREATE DOMAIN d1 AS int; CREATE DOMAIN d2 AS int;
+CREATE DOMAIN d2 AS int; CREATE DOMAIN d1 AS int;
+ALTER DOMAIN d1 ADD CONSTRAINT c1 CHECK ((CAST(VALUE AS int)::d2) IS NOT NULL);
+ALTER DOMAIN d2 ADD CONSTRAINT c2 CHECK ((CAST(VALUE AS int)::d1) IS NOT NULL);
+```
+
+```
+$ diff -u a.dump b.dump
+-CREATE DOMAIN public.d2 AS integer
+- CONSTRAINT c2 CHECK (((VALUE)::public.d1 IS NOT NULL));
++CREATE DOMAIN public.d1 AS integer
++ CONSTRAINT c1 CHECK (((VALUE)::public.d2 IS NOT NULL));
+-ALTER DOMAIN public.d1
+- ADD CONSTRAINT c1 CHECK (((VALUE)::public.d2 IS NOT NULL));
++ALTER DOMAIN public.d2
++ ADD CONSTRAINT c2 CHECK (((VALUE)::public.d1 IS NOT NULL));
+```
+
+A puts `c1` in a separate `ALTER DOMAIN` and inlines `c2`; B does the opposite. No
+assertion fires; the divergence is silent. The verification agent checked that the two
+databases are logically identical by projecting the whole catalog -- including the entire
+`pg_depend` graph with every OID rendered as `regclass`/`regprocedure`/`regtype` -- and
+diffing: no output. The same instability was demonstrated through four different repair
+paths (a table `CHECK` constraint via `BEGIN ATOMIC` functions, a domain `CHECK`
+constraint, the dummy-view choice in a view/rule cycle, and which column `DEFAULT` is split
+into a separate `ALTER TABLE ... SET DEFAULT`), and in one variant the dump flipped with
+*every OID identical* -- so dumpId order, not OID order as such, is the real input.
+
+**No sample fix is proposed for D2 and no test is committed for it.** The natural fix has
+two parts -- make `TopoSort()`'s failure list inherit the caller's name-sorted order, and
+make `repairDependencyLoop()` pick the minimum by natural key rather than the first in
+`loop[]` order -- and both change which object gets broken out in existing cases, i.e. they
+change dump output for databases that dump fine today. That is a judgement call about
+`pg_dump`'s output, not a mechanical key completion, so it is written up here and left to
+you. A test pinned to today's choice would only entrench the OID dependence; a test
+pinned to the fixed choice presumes the fix.
+
+---
+
+## Part 3 -- dump-time queries whose result order reaches the output
+
+Nine of the eleven findings are of one shape: a query whose rows are pasted into the dump
+in result order, ordered by less than a key -- or not ordered at all. None of them reaches
+`DOTypeNameCompare()`, so the tie and shuffle oracles are silent on all nine; each was
+established by reading the query, checking the plan, and diffing two independently built
+databases. They are listed worst first.
+
+### D3. `getInherits()` never reads `inhseqno`, and this permutes columns on restore
+
+```c
+/* src/bin/pg_dump/pg_dump.c:7696 */
+appendPQExpBufferStr(query, "SELECT inhrelid, inhparent FROM pg_inherits");
+```
+
+No `ORDER BY`, and `pg_dump` reads `inhseqno` **nowhere** -- `grep -rn inhseqno
+src/bin/pg_dump/` returns nothing. `flagInhTables()` appends parents in `PGresult` order
+(`common.c:323`) and nothing re-sorts, so the `INHERITS (...)` list at `pg_dump.c:17454`
+and the `--binary-upgrade` `ALTER TABLE ONLY ... INHERIT` at `pg_dump.c:17764` both follow
+`pg_inherits` **heap** order. `pg_inherits`'s natural key is `(inhrelid, inhseqno)`.
+
+Heap order diverges from `inhseqno` order as soon as a line pointer is reused, and it also
+just differs with creation order when other children's rows are interleaved. Seven
+statements, all ordinary DDL:
+
+```sql
+CREATE TABLE p1 (a int);
+CREATE TABLE p2 (b int);
+CREATE TABLE decoy () INHERITS (p1);
+CREATE TABLE ch (b int) INHERITS (p1);
+DROP TABLE decoy;
+VACUUM pg_inherits;
+ALTER TABLE ch INHERIT p2;
+```
+
+The catalog now says the parent order is `p1` then `p2`, and `ch`'s columns are `(a, b)`
+accordingly, but the two rows sit in the heap the other way round:
+
+```
+ ctid | inhparent | inhseqno attnum | attname
+-------+-----------+---------- --------+---------
+ (0,1) | p2 | 2 1 | a
+ (0,2) | p1 | 1 2 | b
+```
+
+and `pg_dump` emits the heap order:
+
+```sql
+CREATE TABLE public.ch (
+ b integer
+)
+INHERITS (public.p2, public.p1);
+```
+
+Restoring that gives `ch` the columns of `p2` first. **The column order changes:**
+
+```
+== ORIGINAL ch columns: == RESTORED ch columns:
+ 1 | a 1 | b
+ 2 | b 2 | a
+```
+
+This is not a spurious-schema-diff problem. A restored database in which a table's columns
+have swapped positions breaks `SELECT *`, `INSERT` without a column list, and every client
+that binds by position -- silently, with no error anywhere in the dump or the restore.
+(`pg_dump`'s own `COPY` statements carry explicit column lists, so the *data* lands in the
+right columns; it is the schema that moves.) Ordering the query by `(inhrelid, inhseqno)`
+fixes both the instability and the wrong restore, and needs no other change because
+`flagInhTables()` preserves `PGresult` order.
+
+### D4. `dumpOpfamily()` and `dumpOpclass()` order members by strategy alone
+
+```c
+/* pg_dump.c, dumpOpfamily(): both member queries */
+... "ORDER BY amopstrategy", /* pg_amop */
+... "ORDER BY amprocnum", /* pg_amproc */
+```
+
+`pg_amop`'s key is `(amopfamily, amoplefttype, amoprighttype, amopstrategy)` and
+`pg_amproc`'s is `(amprocfamily, amproclefttype, amprocrighttype, amprocnum)`. Within one
+family, every cross-type member pair shares a strategy number, so the sort key is not a
+key at all and the remaining order is the executor's -- which the judge traced to an index
+scan on `pg_depend`, i.e. ascending member OID. Adding the same two support functions in
+the opposite order permanently changes the dump:
+
+```sql
+CREATE OPERATOR FAMILY myfam USING btree;
+ALTER OPERATOR FAMILY myfam USING btree ADD FUNCTION 1 btint4cmp(int4, int4);
+ALTER OPERATOR FAMILY myfam USING btree ADD FUNCTION 1 btint8cmp(int8, int8);
+-- versus the same two ADDs in the opposite order
+```
+
+```
+$ diff -u a.dump b.dump
+ ALTER OPERATOR FAMILY public.myfam USING btree ADD
+- FUNCTION 1 (integer, integer) btint4cmp(integer,integer) ,
+- FUNCTION 1 (bigint, bigint) btint8cmp(bigint,bigint);
++ FUNCTION 1 (bigint, bigint) btint8cmp(bigint,bigint) ,
++ FUNCTION 1 (integer, integer) btint4cmp(integer,integer);
+```
+
+Deterministic, and it reproduces on every run. The `pg_amop` half of the same query pair
+has the identical missing key columns; the audit could not make the operator list flip
+(the plan it gets happens to be insensitive to insertion order), so that half is reported
+as latent rather than demonstrated. `dumpOpclass()`'s two queries are also latent for a
+different reason: only members whose left and right types both equal `opcintype` depend on
+the opclass rather than the family, so today at most one member per strategy reaches them
+-- access methods without an `amadjustmembers` hook are where that could stop holding.
+
+### D5. `getPolicies()` builds the `TO` role list with an unordered sub-select
+
+```c
+/* pg_dump.c:4278 */
+"CASE WHEN pol.polroles = '{0}' THEN NULL ELSE "
+" pg_catalog.array_to_string(ARRAY(SELECT pg_catalog.quote_ident(rolname) "
+" from pg_catalog.pg_roles "
+" WHERE oid = ANY(pol.polroles)), ', ') END AS polroles, "
+```
+
+The `ARRAY()` sub-select has no `ORDER BY` and plans as a seq scan on `pg_authid`, so the
+list follows role **creation** order -- neither the stored `polroles` array order (which
+`policy_role_list_to_array()` preserves from the `CREATE POLICY` text) nor `rolname` order.
+Two databases whose roles were created in the opposite order dump
+`CREATE POLICY p ON t TO alice, bob` versus `... TO bob, alice`. Ordering by each OID's
+position within `pol.polroles` (`unnest ... WITH ORDINALITY`) both stabilises it and makes
+the clause a faithful round trip of what the user wrote.
+
+### D6. `getPublications()` does not order the `FOR ALL TABLES EXCEPT` list
+
+```c
+/* pg_dump.c:4598, per publication, remoteVersion >= 190000 */
+"SELECT prrelid\n"
+"FROM pg_catalog.pg_publication_rel\n"
+"WHERE prpubid = %u AND prexcept"
+```
+
+The rows go into a `SimplePtrList` in arrival order and `dumpPublication()` walks it
+verbatim, so `CREATE PUBLICATION p FOR ALL TABLES EXCEPT (TABLE ONLY a, TABLE ONLY b)`
+follows `pg_publication_rel` heap order -- i.e. the order the tables were listed when the
+publication was created. Three statements per database reproduce it. This one is new
+code (v19), which makes it the cheapest of the nine to fix before it ships in a release.
+
+### D7. `dumpDatabaseConfig()` does not order per-role database settings
+
+```c
+/* pg_dump.c:3764 */
+"SELECT rolname, unnest(setconfig) FROM pg_db_role_setting s, pg_roles r "
+"WHERE setrole = r.oid AND setdatabase = '%u'::oid"
+```
+
+One row per `(role, database)`, no `ORDER BY`, and the plan seq-scans `pg_authid` on the
+probe side, so the `ALTER ROLE ... IN DATABASE ... SET` lines in a `--create` preamble come
+out in role-OID order. `ORDER BY 1` (`rolname`) is a complete key here, and the
+verification agent checked the one thing that could have gone wrong -- that sorting above
+the set-returning `unnest` does not permute the settings *within* a role -- by confirming
+the planner puts the sort below the `ProjectSet`.
+
+### D8. `append_depends_on_extension()` does not order its rows
+
+The query behind `ALTER ... DEPENDS ON EXTENSION` (`pg_dump.c:5702`) has no `ORDER BY`, so
+an object with two extension dependencies emits them in `pg_depend` row order -- the order
+the `ALTER ... DEPENDS ON EXTENSION` statements happened to run, and it changes if one is
+dropped and re-added. Affects every caller (`dumpFunc()`, `dumpTrigger()`, index and
+materialized-view paths). `ORDER BY 1` on the extension name is a complete key.
+
+### D9. `collectSecLabels()` omits `provider` from its `ORDER BY`
+
+```c
+/* pg_dump.c:16755 */
+"SELECT label, provider, classoid, objoid, objsubid "
+"FROM pg_catalog.pg_seclabels ORDER BY classoid, objoid, objsubid"
+```
+
+`pg_seclabel`'s key is `(objoid, classoid, objsubid, provider)`. Two providers labelling
+one object produce two rows that tie under that `ORDER BY`, and the server's sort is not
+stable, so the two `SECURITY LABEL FOR ...` statements come out in an order the catalog
+does not determine. `collectSecLabels()` is one of two sites; the shared-object path in `dumputils.c` has the
+same gap. Reaching it needs two registered label providers, which no in-tree module
+supplied, so the committed test adds a second provider to
+`src/test/modules/dummy_seclabel`, whose whole purpose is exercising this machinery.
+
+### D10. `pg_dumpall`'s `dumpTablespaces()` orders by OID
+
+```c
+/* pg_dumpall.c:1368 */
+"SELECT oid, spcname, ... FROM pg_catalog.pg_tablespace "
+"WHERE spcname !~ '^pg_' "
+"ORDER BY 1" /* select-list column 1 is oid */
+```
+
+Select-list column 1 is `oid`, so the whole per-tablespace block -- `CREATE TABLESPACE`,
+`ALTER TABLESPACE ... SET`, the ACL commands, `COMMENT`, `SECURITY LABEL` -- is emitted in
+OID order. That this is an off-by-one rather than intent is clear from the sibling
+`dumpRoles()` at `pg_dumpall.c:855`, which has the identical select-list shape
+(`SELECT oid, rolname, ...`) and says `ORDER BY 2`. `spcname` alone is a complete key
+(`pg_tablespace_spcname_index` is unique and `pg_tablespace` has no namespace).
+
+### D11. `dumpExtension()` emits the `requires` array in dependency-array order
+
+Under `--binary-upgrade`, `dumpExtension()` builds the seventh argument of
+`binary_upgrade_create_empty_extension()` by walking `extinfo->dobj.dependencies[]` and
+printing each `DO_EXTENSION` it finds (`pg_dump.c:11992`). Nothing ever sorts a
+`dependencies[]` array: `getDependencies()` ends `ORDER BY 1,2` -- `(classid, objid)`, with
+`refobjid` absent -- so all of one extension's requires-rows tie and arrive in scan order,
+which the backend wrote in *descending referenced-OID* order
+(`eliminate_duplicate_dependencies()` → `object_address_comparator()`, "Primary sort
+key is OID descending"). Two statements per database:
+
+```sql
+CREATE EXTENSION plperl; CREATE EXTENSION hstore_plperl CASCADE; -- ARRAY['hstore','plperl']
+CREATE EXTENSION hstore; CREATE EXTENSION hstore_plperl CASCADE; -- ARRAY['plperl','hstore']
+```
+
+Only four in-tree control files list more than one `requires` entry
+(`hstore_plperl`, `hstore_plperlu`, `hstore_plpython3u`, `ltree_plpython3u`), so the
+reachable surface is narrow, but this is on the `pg_upgrade` path, which is where the
+dump-comparison test lives.
+
+---
+
+## Considered and rejected
+
+### Array-valued catalog columns reproduced verbatim -- *mostly* not a defect
+
+This class was put to a dedicated adjudicator after the first workflow's judges split
+three-to-one on it. Its verdict: the two demonstrated cases are **not** defects, but the
+sweep had stopped one array short, and that one **is** -- see D12 below.
+
+#### The two demonstrated cases
+
+Several catalog columns are arrays whose element order is an artifact of the order the DDL
+was issued, and `pg_dump` reproduces that order. The audit demonstrated it twice:
+
+```sql
+GRANT SELECT ON acl_t TO r_aaa; GRANT SELECT ON acl_t TO r_bbb;
+-- versus the same two GRANTs in the opposite order
+```
+
+```
+=== A === === B ===
+GRANT SELECT ON TABLE public.acl_t TO r_aaa; GRANT SELECT ON TABLE public.acl_t TO r_bbb;
+GRANT SELECT ON TABLE public.acl_t TO r_bbb; GRANT SELECT ON TABLE public.acl_t TO r_aaa;
+```
+
+and the same for `pg_db_role_setting.setconfig` under `ALTER DATABASE ... SET`. Four
+agents split three-to-one on whether this belongs in the findings list. It does not, for
+three reasons, the third of which is decisive:
+
+1. **It inverts the defect definition.** The other findings are: identical catalog
+ content, different OIDs, different output. This is: *different* catalog content
+ (`relacl` genuinely holds a different array value), identical OIDs, different output.
+ `pg_dump` is reporting the catalog, not choosing an order.
+2. **It is a fixed point.** Dump, restore, dump again: the second dump equals the first.
+ None of the harms that motivate this class occur -- no `Assert`, no tie, no
+ `002_pg_upgrade.pl` mismatch.
+3. **`buildACLCommands()`'s order is load-bearing.** With a `WITH GRANT OPTION` chain, a
+ grant must be replayed after the grant that authorised it. A naive sort of the aclitem
+ list would produce a dump that **fails to restore**. Whatever is done here cannot be a
+ plain sort.
+
+The same argument covers every `*acl` column `pg_dump` feeds to `buildACLCommands()`
+(`relacl`, `typacl`, `proacl`, `nspacl`, `defaclacl`, `lanacl`, `fdwacl`, `srvacl`,
+`datacl`, `spcacl`, `lomacl`, parameter ACLs in `pg_dumpall`) and column-level ACLs, and it
+covers `reloptions`, `proconfig` and `attoptions` for reason 1 alone. Recorded here so it
+is not re-proposed.
+
+### Checked and found clean
+
+* **The core regression database** (2291 relations) -- no ties, and byte-identical dump
+ output across eight pre-sort shuffles. Also clean under `--with-statistics`,
+ `--no-owner`, `--no-privileges`, `--section=*`, `--schema-only`, `--data-only` and
+ `--binary-upgrade`.
+* **`TopoSort()` itself** -- given a fixed input order and a fixed dependency graph, its
+ output is deterministic; the binary heap is keyed on the input index. The instability in
+ D2 is in what feeds it on failure, not in the sort.
+* **The archive TOC** -- `-Fc` TOC order and single-threaded `pg_restore -f -` output
+ follow the same sorted list as the plain dump. (`pg_restore -j` deliberately does not,
+ as the comment above `Assert(false)` already says.)
+* **The rest of `pg_dumpall.c`** -- roles, role memberships, role GUC settings, databases
+ and subscriptions are all ordered by name; `dumpTablespaces()` (D10) is the only one that
+ is not.
+* **`getDependencies()`'s `ORDER BY 1,2`** -- incomplete as a key, but the only place a
+ `dependencies[]` array's order reaches the output is D11.
+* **The comparator's helper functions** -- `pgTypeNameCompare()` compares
+ `(nspname, typname)`, `accessMethodNameCompare()` compares `amname`; both are complete
+ for their catalogs, and both handle the not-found case by returning "equal" so the caller
+ falls through to its next basis for comparison.
+* **Comments** -- `collectComments()` orders by `(classoid, objoid, objsubid)`, which is
+ `pg_description`'s whole key; only the security-label sibling (D9) has a fourth key
+ column.
+
+---
+
+## Tests and sample fixes on this branch
+
+Eleven of the twelve findings have both a regression test and a sample fix. D2 has
+neither, for the reason given in Part 2.
+
+| # | Test | Sample fix |
+|---|---|---|
+| D1 | `002_pg_dump.pl`, policy named after its own table | `pg_dump_sort.c`: compare `polname == NULL` after the table name |
+| D3 | `002_pg_dump.pl`, `inh_order_child` | `pg_dump.c`: `ORDER BY inhrelid, inhseqno` |
+| D4 | `002_pg_dump.pl`, `op_family` | `pg_dump.c`: add the member type names to all four member queries |
+| D5 | `002_pg_dump.pl`, policy `p7` with a multi-role `TO` list | `pg_dump.c`: `unnest(polroles) WITH ORDINALITY` |
+| D6 | `002_pg_dump.pl`, publications `pub9`/`pub10` | `pg_dump.c`: `ORDER BY n.nspname, c.relname` |
+| D7 | `002_pg_dump.pl`, `ALTER ROLE ... IN DATABASE` | `pg_dump.c`: `ORDER BY rolname` |
+| D8 | `test_pg_dump/t/001_base.pl` | `pg_dump.c`: `ORDER BY e.extname` |
+| D9 | `003_pg_dump_with_server.pl` (+ a second provider in `dummy_seclabel`) | `pg_dump.c`, `dumputils.c`: add `provider` to both `ORDER BY`s |
+| D10 | `002_pg_dump.pl`, `CREATE TABLESPACE in name order` | `pg_dumpall.c`: `ORDER BY 1` → `ORDER BY 2` |
+| D11 | `003_pg_dump_with_server.pl` | `pg_dump.c`: sort the requires names with `pg_qsort_strcmp` |
+| D12 | `002_pg_dump.pl`, `ALTER DEFAULT PRIVILEGES grantees ... in name order` | `pg_dump.c`: re-sort `defaclacl` by aclitem text under `COLLATE "C"` |
+
+**The sample fixes are not proposed patches.** They exist so the branch is coherent -- the
+tests need something to pass against -- and so that "this test fails without the fix" is a
+statement someone can check. They are in their own commit and can be dropped wholesale.
+Four of them involve a judgement a committer should make rather than accept:
+
+* **D5** could instead be `ORDER BY rolname`. The committed fix preserves the order the
+ user wrote in `CREATE POLICY`, which round-trips; alphabetical order would be simpler but
+ would rewrite the clause. Both remove the OID dependence.
+* **D4** orders by the members' type names. Ordering by `regtype` output would have been
+ shorter, but that rendering depends on `search_path`, so the fix joins `pg_type` and
+ `pg_namespace` and orders by `(nspname, typname)` -- the same key
+ `pgTypeNameCompare()` uses.
+* **D1** sorts the RLS-enable pseudo-object *before* the policies on its table. Either
+ order is stable; this one matches `ENABLE ROW LEVEL SECURITY` logically preceding them.
+* **D12** sorts an ACL array, which the sibling `relacl` case shows can be unsafe. The
+ argument that it is safe *here* -- a default ACL's items all share one grantor, so there
+ is no grant chain to replay in order -- is the whole basis of the fix, and is the thing
+ to check before accepting it.
+
+Two findings needed test infrastructure rather than just a test entry. D8 lives in
+`src/test/modules/test_pg_dump` because showing it needs one object with **two** extension
+dependencies, and a bare `initdb` has exactly one extension (`plpgsql`); `src/bin/pg_dump`'s
+test install does not build contrib, so a test in `002_pg_dump.pl` would have to make the
+core pg_dump suite depend on contrib. `test_pg_dump` already installs its own extension
+and already owns the only existing `DEPENDS ON EXTENSION` coverage. D11 sidesteps the same
+problem differently: its test writes three throwaway control files into the test's temp
+directory and points `extension_control_path` at them, so it needs no contrib at all.
+
+## Verification
+
+Three runs of `meson test --suite setup --suite pg_dump --suite test_pg_dump --suite
+dummy_seclabel`, on this branch, in this order.
+
+**1. Everything applied: 13/13 pass**, including `002_pg_dump` with 13697 subtests.
+
+**2. All five product files reverted, tests kept: 3 suites fail.** `002_pg_dump` dies
+early:
+
+```
+# pg_dump: ../ss-audit/src/bin/pg_dump/pg_dump_sort.c:511: DOTypeNameCompare: Assertion `0' failed.
+# Failed test 'binary_upgrade: pg_dump runs'
+```
+
+That is D1 doing what it should -- and it is also why this run alone is not enough: the
+abort kills the dump before the ordering tests can be evaluated.
+
+**3. Only D1's fix applied, the other ten reverted: 3 suites fail, each test by its own
+name.** `002_pg_dump` now runs to completion and fails on exactly the new entries:
+
+```
+should dump CREATE TABLE inh_order_child (D3)
+should dump CREATE TABLE inh_order_child pg_upgrade (D3)
+should dump ALTER OPERATOR FAMILY dump_test.op_family USING btree (D4)
+should dump CREATE POLICY p7 ON test_table with a multi-role TO list (D5)
+should dump CREATE PUBLICATION pub9 / pub10 (D6)
+should dump ALTER ROLE ... IN DATABASE postgres SET, in role name order (D7)
+should dump CREATE TABLESPACE in name order (D10)
+should dump ALTER DEFAULT PRIVILEGES grantees are dumped in name order (D12)
+```
+
+`003_pg_dump_with_server` reports "failed 3 tests of 12" (D9 and D11), and
+`test_pg_dump/001_base` fails (D8). Every committed test fails for its own reason on the
+unfixed tree.
+
+Separately, each finding was re-checked outside the TAP suite by building the two databases
+the report describes and diffing the dumps with the unfixed and the fixed binary. All of
+D4, D5, D6, D7, D8, D11 and D12 go from UNSTABLE to STABLE; D1 stops aborting; D3 emits
+`INHERITS (public.p1, public.p2)` and restores the child with its columns in the original
+order; D10 emits the tablespaces in name order.
+
+**What is still unverified.** The completeness critic that examined the D1 claim -- 48
+object types, 55 construction sites, five SQL corpora each under 17 pg_dump option sets,
+all 59 contrib extensions, plus the regression database -- returned "claim holds", and
+named what it could not reach: cross-version dumps (pg_dump's older-server query branches),
+`DO_SUBSCRIPTION_REL`, multi-encoding collations, and catalog corruption. D2 is reported
+without a fix or a test by choice. Nothing else on this branch is unverified.
+
+### D12. `getDefaultACLs()`: `defaclacl` is emitted in grantee-OID order
+
+This one came out of the adjudication above, not out of discovery: the agent sent to settle
+whether array order is ever a defect reproduced both demonstrated cases, agreed they are
+not, and then checked the arrays the sweep had not. `pg_default_acl.defaclacl` is a
+different animal:
+
+```sql
+CREATE ROLE r_aaa; CREATE ROLE r_bbb; -- database A
+ALTER DEFAULT PRIVILEGES GRANT SELECT ON TABLES TO r_aaa;
+ALTER DEFAULT PRIVILEGES GRANT SELECT ON TABLES TO r_bbb;
+-- database B: identical, only the two CREATE ROLE lines swapped
+```
+
+```
+D12-defaclacl/OLD: UNSTABLE
+ -ALTER DEFAULT PRIVILEGES FOR ROLE postgres GRANT SELECT ON TABLES TO r_aaa;
+ +ALTER DEFAULT PRIVILEGES FOR ROLE postgres GRANT SELECT ON TABLES TO r_aaa;
+D12-defaclacl/NEW: STABLE (dumps identical)
+```
+
+The reason it is a defect where `relacl` is not: the backend **throws the DDL order away**.
+`ExecGrant_Default_Acl()` canonicalizes the array with `aclitemsort()`, which orders by
+grantee OID. So the stored order is not "what the user wrote", it is a function of role
+OIDs -- and a restore into a cluster that assigns different role OIDs produces a different
+canonical order. That also removes the objection that killed the `relacl` case: a default
+ACL cannot contain a chain of grants by different grantors (every item's grantor is
+`defaclrole`), so `buildACLCommands()`'s load-bearing replay order does not apply and
+sorting is safe.
+
+Sorting by the aclitem's text under `COLLATE "C"` in `getDefaultACLs()` fixes it.
+
+The same adjudicator's negative results are worth as much as the finding, and are why the
+`relacl` and `setconfig` cases stay rejected: it fuzzed 160 tables, 20 functions, 10 schemas
+and 10 types with random `GRANT`/`REVOKE` histories, non-owner grantors, `PUBLIC`, column
+privileges and three grant-option holders, then dumped, restored and re-dumped -- byte
+identical. It also ran a real `pg_upgrade` and confirmed that although the catalog arrays
+*are* rewritten, `pg_dump` already normalizes around it (it drops items matching
+`acldefault` and hoists owner self-grants into `firstsql`), so the dump comparison passes.
+And it confirmed that `relacl` order really is load-bearing, by replaying a grant chain in
+grantee-name order and getting `ERROR: permission denied for table t5`.
+
+---
+
+## Appendix A -- the instrumented build
+
+Applied to `sortDumpableObjectsByTypeName()` in `pg_dump_sort.c` for the audit build only;
+never committed.
+
+```c
+ /* PGDUMP_SHUFFLE_SEED=N: permute the array before sorting. */
+ {
+ const char *seedstr = getenv("PGDUMP_SHUFFLE_SEED");
+
+ instr_tie_zero = (getenv("PGDUMP_TIE_ZERO") != NULL);
+ if (seedstr != NULL && numObjs > 1)
+ {
+ srand((unsigned int) atoi(seedstr));
+ for (int i = numObjs - 1; i > 0; i--)
+ {
+ int j = rand() % (i + 1);
+ DumpableObject *tmp = objs[i];
+
+ objs[i] = objs[j];
+ objs[j] = tmp;
+ }
+ }
+ }
+
+ if (numObjs > 1)
+ qsort(objs, numObjs, sizeof(DumpableObject *), DOTypeNameCompare);
+
+ /* PGDUMP_TIE_REPORT=1: report every adjacent pair that reached the
+ * comparator's fall-through. Ties are adjacent after a sort, so this
+ * enumerates all of them. */
+ if (getenv("PGDUMP_TIE_REPORT") != NULL)
+ {
+ for (int i = 1; i < numObjs; i++)
+ {
+ instr_tie_fallthrough = false;
+ DOTypeNameCompare(&objs[i - 1], &objs[i]);
+ if (instr_tie_fallthrough)
+ {
+ char buf1[512], buf2[512];
+
+ describeDumpableObject(objs[i - 1], buf1, sizeof(buf1));
+ describeDumpableObject(objs[i], buf2, sizeof(buf2));
+ fprintf(stderr, "SORT TIE: objType %d name \"%s\" nsp \"%s\" | %s | %s\n",
+ (int) objs[i]->objType, objs[i]->name,
+ objs[i]->namespace ? objs[i]->namespace->dobj.name : "(none)",
+ buf1, buf2);
+ }
+ }
+ }
+```
+
+and, in `DOTypeNameCompare()`, the fall-through becomes
+
+```c
+ instr_tie_fallthrough = true;
+ if (instr_tie_zero)
+ return 0;
+ return oidcmp(obj1->catId.oid, obj2->catId.oid);
+```
+
+**The first version of this was wrong and reported nothing**, because disarming
+`Assert(false)` left the fall-through returning `oidcmp()`, so the reporter's
+"did these two compare equal?" test never fired. It was caught only by running the
+detector against a defect already known to be present. A detector that silently finds
+nothing is the failure mode that would have turned this report into "no defects", so
+calibrate any replacement the same way.
+
+## Appendix B -- minimal reproducers
+
+Each is a complete `.sql` for a fresh database. Where a finding is a two-database
+comparison, both variants are given; dump each with the stated options and diff, after
+normalizing pg_dump's random `\restrict` token
+(`sed -E 's/^(\\(un)?restrict) [A-Za-z0-9]+$/\1 XXX/'`).
+
+```sql
+-- D1: assert-enabled pg_dump aborts.
+CREATE TABLE pol_t (i int);
+ALTER TABLE pol_t ENABLE ROW LEVEL SECURITY;
+CREATE POLICY pol_t ON pol_t USING (true);
+
+-- D2: two databases, only the two CREATE DOMAIN lines swapped.
+CREATE DOMAIN d1 AS int;
+CREATE DOMAIN d2 AS int;
+ALTER DOMAIN d1 ADD CONSTRAINT c1 CHECK ((CAST(VALUE AS int)::d2) IS NOT NULL);
+ALTER DOMAIN d2 ADD CONSTRAINT c2 CHECK ((CAST(VALUE AS int)::d1) IS NOT NULL);
+
+-- D3: one database. Dump, restore, and compare ch's column order.
+CREATE TABLE p1 (a int);
+CREATE TABLE p2 (b int);
+CREATE TABLE decoy () INHERITS (p1);
+CREATE TABLE ch (b int) INHERITS (p1);
+DROP TABLE decoy;
+VACUUM pg_inherits;
+ALTER TABLE ch INHERIT p2;
+
+-- D4: two databases, the two ADD FUNCTION lines swapped.
+CREATE OPERATOR FAMILY myfam USING btree;
+ALTER OPERATOR FAMILY myfam USING btree ADD FUNCTION 1 btint4cmp(int4, int4);
+ALTER OPERATOR FAMILY myfam USING btree ADD FUNCTION 1 btint8cmp(int8, int8);
+
+-- D5: two databases, the two CREATE ROLE lines swapped.
+CREATE ROLE alice NOLOGIN; CREATE ROLE bob NOLOGIN;
+CREATE TABLE t (a int);
+CREATE POLICY p ON t TO alice, bob USING (true);
+
+-- D6: two databases, the EXCEPT list written in the opposite order.
+CREATE TABLE ta (x int); CREATE TABLE tb (x int);
+CREATE PUBLICATION p FOR ALL TABLES EXCEPT (TABLE ta, TABLE tb);
+
+-- D7: two databases, the two CREATE ROLE lines swapped. Dump with --create.
+CREATE ROLE ra NOLOGIN; CREATE ROLE rb NOLOGIN;
+ALTER ROLE ra IN DATABASE postgres SET work_mem='5MB';
+ALTER ROLE rb IN DATABASE postgres SET work_mem='6MB';
+
+-- D8: two databases, the two ALTER TRIGGER lines swapped.
+CREATE EXTENSION cube;
+CREATE TABLE t (a int);
+CREATE TRIGGER tg BEFORE UPDATE ON t FOR EACH ROW
+ EXECUTE FUNCTION suppress_redundant_updates_trigger();
+ALTER TRIGGER tg ON t DEPENDS ON EXTENSION plpgsql;
+ALTER TRIGGER tg ON t DEPENDS ON EXTENSION cube;
+
+-- D9: needs two registered label providers; see the committed test, which adds a
+-- second provider to src/test/modules/dummy_seclabel.
+
+-- D10: two clusters, the two CREATE TABLESPACE lines swapped. pg_dumpall --globals-only.
+SET allow_in_place_tablespaces = on;
+CREATE TABLESPACE ts_aaa LOCATION '';
+CREATE TABLESPACE ts_bbb LOCATION '';
+
+-- D11: two databases. Dump with --binary-upgrade.
+CREATE EXTENSION plperl; CREATE EXTENSION hstore_plperl CASCADE; -- variant A
+CREATE EXTENSION hstore; CREATE EXTENSION hstore_plperl CASCADE; -- variant B
+
+-- D12: two databases, the two CREATE ROLE lines swapped.
+CREATE ROLE r_aaa NOLOGIN; CREATE ROLE r_bbb NOLOGIN;
+ALTER DEFAULT PRIVILEGES GRANT SELECT ON TABLES TO r_aaa;
+ALTER DEFAULT PRIVILEGES GRANT SELECT ON TABLES TO r_bbb;
+```
diff --git a/PROVENANCE.md b/PROVENANCE.md
new file mode 100644
index 0000000..2b49ca3
--- /dev/null
+++ b/PROVENANCE.md
@@ -0,0 +1,126 @@
+# PROVENANCE
+
+Branch `dump-sort-stability-tests` is the output of an automated, model-driven audit that
+looked for sources of pg_dump dump-order instability **other than** the one fixed by the
+cast/transform patch the branch's first commit carries. Everything on the branch after
+that first commit -- the report, the regression tests, and the SAMPLE fixes -- was written
+by a language model. **No human wrote any of it, and no human has reviewed it.** This
+file records how it was produced, including what went wrong, so a reviewer can judge the
+work and reproduce every claim in it.
+
+---
+
+## Tooling
+
+* **Tool:** Claude Code (Anthropic's agentic CLI), using its Workflow feature -- a
+ deterministic JavaScript script that drives many subagents.
+* **Model:** Opus 5 (`claude-opus-5`) for the orchestrator and for every subagent.
+* **Run by:** the repository owner (Noah Misch), interactively, from
+ `/home/nm/src/pg/postgresql`.
+* **Date:** 2026-09-03.
+* **Human contribution:** the prompt below, and one mid-run question about whether work was
+ blocked. Nothing in the findings, the tests, the fixes or the report came from a human.
+
+## The prompt
+
+> ~/sort_cast.patch contains a reasonable-looking fix. I'm concerned that yet
+> more sources of dump sort instability are still lurking. Make a worklfow to
+> look for others and, if any found, write test cases covering them. Use your
+> own worktree; disregard the present dir except as repository to which to
+> attach your worktree.
+>
+> Commit the following on a fresh branch:
+> - A report. Prefix the report with [no defects] if that's so.
+> - Any tests written
+> - A PROVENANCE.md file containing model, prompt, etc.
+
+## Base
+
+* Upstream `master` at `6885b84` ("doc: Fix link on pg_dsm_registry_allocations page.").
+* Commit `885a841` is `~/sort_cast.patch` applied verbatim with `git am`. It is Alexander
+ Kukushkin's patch, **not** model-written; it is on the branch because the audit's whole
+ purpose was to find what that patch does not cover. Everything after it is the audit.
+
+## What was built to do the audit
+
+Three artifacts, all outside the branch, under `/home/nm/src/pg/`:
+
+* `ss-audit/` -- the branch worktree. Agents were given it **read-only**.
+* `ss-inst/` -- a stock assert-enabled install of `885a841`. Its `pg_dump` aborts when
+ `DOTypeNameCompare()` reaches `Assert(false)`, which is the audit's primary oracle
+ because it is exactly what a developer or buildfarm animal sees.
+* `ss-instr/` + `ss-shuf-inst/` -- the same commit with audit-only instrumentation in
+ `sortDumpableObjectsByTypeName()`: a pre-sort shuffle under `PGDUMP_SHUFFLE_SEED`, a
+ complete adjacent-pair tie report under `PGDUMP_TIE_REPORT`, and the comparator's
+ `Assert(false)` disarmed (which also makes it a faithful stand-in for a production
+ non-assert build). The instrumentation is reproduced in full in the report's appendix.
+ It was **never** committed to the branch.
+* `ssrun/pgrun.sh` -- runs a `.sql` file in a throwaway cluster and dumps it in `plain`,
+ `tie` or `shuffle` mode. `ssrun/regdata/` -- a prepared core-regression database
+ (243/243 tests passed, 2291 relations) used as the large realistic corpus.
+
+Calibration before any agent ran: all three oracles fire on a known defect and are silent
+on the regression database and on control schemas.
+
+## What the agents did
+
+**Workflow 1 -- discovery** (`wf_7ac43658-438`, 152 agents, 110 completed, 10.0M subagent
+tokens, 8h04m wall clock).
+
+* 17 discovery agents in parallel: 8 sweeping the 48 `DumpableObjectType` values against
+ their catalogs' natural keys, 4 code lenses (the topological sort; every dump-time query
+ in `pg_dump.c`; `pg_dumpall.c` and the archive TOC; the history of the five commits that
+ already fixed this class), and 5 empirical lanes (cross-schema name collisions; pg_dump's
+ manufactured pseudo-objects; in-tree extensions; the regression corpus; a differential
+ two-database generator).
+* 54 raw candidates, deduplicated to 49 by key.
+* Each candidate then got an independent verification agent, and each survivor an
+ independent adversarial judge whose brief was to **refute** it. 40 survived.
+* Those 40 describe **11 distinct mechanisms**; one mechanism was found independently by 13
+ different agents through 13 different object types.
+
+**Workflow 2 -- tests and fixes** (`wf_db8fc3d2-7db`, 12 agents, 12 completed, 1.8M
+subagent tokens): one agent per confirmed mechanism, each required to re-reproduce it from
+scratch before writing its regression test and sample fix, plus two adversarial critics --
+one told to falsify the claim that `DO_POLICY` is the only remaining comparator tie, one to
+settle a finding the first workflow's judges had split on.
+
+The first critic returned **"claim holds"** after enumerating all 48 `DumpableObjectType`
+values and their 55 construction sites and attacking the claim with five SQL corpora, each
+under 17 pg_dump option sets, plus all 59 contrib extensions and the regression database.
+The second **found a twelfth defect** (D12) while refuting the finding it was sent to
+adjudicate: the two array-order cases it was given are not defects, but the sweep had
+stopped one array short of `pg_default_acl.defaclacl`, which is. So the final count is
+**twelve**, not the eleven the discovery workflow produced.
+
+## What went wrong, and what was done about it
+
+* **The Anthropic API returned 529 Overloaded for about an hour.** It killed workflow 1's
+ entire Tests phase and its completeness critic (42 of the 152 agents), then two full
+ launches of workflow 2 (12 agents each, all failing at zero tokens). **No finding was
+ lost** -- discovery, verification and adjudication had all completed -- but no test was
+ written until the third launch of workflow 2 succeeded.
+* **The first tie-detector build was wrong and reported nothing.** Disarming
+ `Assert(false)` left the fall-through returning `oidcmp()`, so the reporter's
+ "did these two compare equal?" test never fired. Caught by running it against a defect
+ known to be present; fixed by having the fall-through set a flag the reporter reads.
+ Recorded here because a silent detector is the failure mode that would have made this
+ whole audit report "no defects".
+* **The shuffle oracle produced false positives** until `pg_dump`'s random `\restrict`
+ token was normalized away before diffing.
+* **The first dedup was too coarse**: 40 confirmed reports collapse to 11 mechanisms, but
+ the agents chose 40 different key strings, so the Tests phase was sized at 42 agents when
+ 10 would do. Consolidation was done by hand between the two workflows.
+* **`002_pg_dump.pl` cannot express every finding.** Which findings got a TAP test, which
+ did not, and why, is stated explicitly in the report -- no finding is quietly dropped.
+
+## How to check the work
+
+Every finding in the report carries the minimal SQL that produces it. For a comparator
+tie, run it under an assert-enabled `pg_dump` and watch the assertion fire. For an
+ordering finding, build the two databases the report gives and diff the dumps. The
+tests on this branch are the same reproducers expressed in `002_pg_dump.pl`; each one
+fails on `885a841` and passes with that finding's sample fix applied.
+
+The audit's own verification of the committed artifacts is described at the end of the
+report, including the result of reverting the sample fixes and re-running the suite.
diff --git a/src/bin/pg_dump/t/002_pg_dump.pl b/src/bin/pg_dump/t/002_pg_dump.pl
index 4b719e7..461463f 100644
--- a/src/bin/pg_dump/t/002_pg_dump.pl
+++ b/src/bin/pg_dump/t/002_pg_dump.pl
@@ -783,6 +783,33 @@ my %tests = (
unlike => { no_privs => 1, },
},
+ # The backend keeps a pg_default_acl entry's ACL array in grantee-OID order
+ # (ExecGrant_Default_Acl canonicalizes it with aclitemsort()), so emitting
+ # the GRANTs in array order would make the dump depend on the order the
+ # grantee roles happened to be created in. Two databases with the same
+ # default privileges must dump alike, and a dump/restore round trip must be
+ # order-stable even though the restore assigns new role OIDs. Create these
+ # roles in the reverse of their name order and require name order out.
+ 'ALTER DEFAULT PRIVILEGES grantees are dumped in name order' => {
+ create_order => 57,
+ create_sql => 'CREATE ROLE regress_dump_defacl_zzz;
+ CREATE ROLE regress_dump_defacl_aaa;
+ ALTER DEFAULT PRIVILEGES
+ FOR ROLE regress_dump_test_role
+ GRANT SELECT ON SEQUENCES
+ TO regress_dump_defacl_zzz, regress_dump_defacl_aaa;',
+ regexp => qr/^
+ \QALTER DEFAULT PRIVILEGES \E
+ \QFOR ROLE regress_dump_test_role \E
+ \QGRANT SELECT ON SEQUENCES TO regress_dump_defacl_aaa;\E\n
+ \QALTER DEFAULT PRIVILEGES \E
+ \QFOR ROLE regress_dump_test_role \E
+ \QGRANT SELECT ON SEQUENCES TO regress_dump_defacl_zzz;\E
+ /xm,
+ like => { %full_runs, section_post_data => 1, },
+ unlike => { no_privs => 1, },
+ },
+
'ALTER DEFAULT PRIVILEGES FOR ROLE regress_dump_test_role REVOKE SELECT'
=> {
create_order => 56,
@@ -815,6 +842,31 @@ my %tests = (
},
},
+ # dumpDatabaseConfig() must emit these in role name order. The roles are
+ # created in the reverse of that order, and the ALTER ROLE statements are
+ # issued in the reverse of that order too, so neither pg_authid OID order
+ # nor pg_db_role_setting heap order can produce the expected output by
+ # accident; only an explicit sort on rolname can.
+ 'ALTER ROLE ... IN DATABASE postgres SET, in role name order' => {
+ create_order => 28,
+ create_sql => '
+ CREATE ROLE regress_dump_role_z;
+ CREATE ROLE regress_dump_role_a;
+ ALTER ROLE regress_dump_role_z IN DATABASE postgres
+ SET work_mem = \'7MB\';
+ ALTER ROLE regress_dump_role_a IN DATABASE postgres
+ SET work_mem = \'6MB\';',
+ regexp => qr/^
+ \QALTER ROLE regress_dump_role_a IN DATABASE postgres SET work_mem TO '6MB';\E\n
+ \QALTER ROLE regress_dump_role_z IN DATABASE postgres SET work_mem TO '7MB';\E
+ /xm,
+
+ # These commands live in the DATABASE PROPERTIES entry, which only
+ # --create emits. pg_dumpall passes --create for other databases, but
+ # not for "postgres" unless --clean is given too.
+ like => { createdb => 1, },
+ },
+
'ALTER COLLATION test0 OWNER TO' => {
regexp => qr/^\QALTER COLLATION public.test0 OWNER TO \E.+;/m,
collation => 1,
@@ -884,10 +936,10 @@ my %tests = (
\QOPERATOR 4 >=(bigint,integer) ,\E\n\s+
\QOPERATOR 5 >(bigint,integer) ,\E\n\s+
\QFUNCTION 1 (integer, integer) btint4cmp(integer,integer) ,\E\n\s+
- \QFUNCTION 2 (bigint, bigint) btint8sortsupport(internal) ,\E\n\s+
\QFUNCTION 2 (integer, integer) btint4sortsupport(internal) ,\E\n\s+
- \QFUNCTION 4 (bigint, bigint) btequalimage(oid) ,\E\n\s+
- \QFUNCTION 4 (integer, integer) btequalimage(oid);\E
+ \QFUNCTION 2 (bigint, bigint) btint8sortsupport(internal) ,\E\n\s+
+ \QFUNCTION 4 (integer, integer) btequalimage(oid) ,\E\n\s+
+ \QFUNCTION 4 (bigint, bigint) btequalimage(oid);\E
/xm,
like =>
{ %full_runs, %dump_test_schema_runs, section_pre_data => 1, },
@@ -2156,6 +2208,30 @@ my %tests = (
},
},
+ # pg_dumpall must emit tablespaces in name order, not in pg_tablespace.oid
+ # order. These two are created in descending name order, so an OID-ordered
+ # dump emits _b before _a.
+ 'CREATE TABLESPACE in name order' => {
+ create_order => 2,
+ create_sql => q(
+ SET allow_in_place_tablespaces = on;
+ CREATE TABLESPACE regress_dump_tablespace_b
+ OWNER regress_dump_test_role LOCATION '';
+ CREATE TABLESPACE regress_dump_tablespace_a
+ OWNER regress_dump_test_role LOCATION ''),
+ regexp => qr/^
+ \QCREATE TABLESPACE regress_dump_tablespace_a OWNER regress_dump_test_role LOCATION '';\E
+ .*?
+ ^\QCREATE TABLESPACE regress_dump_tablespace_b OWNER regress_dump_test_role LOCATION '';\E
+ /xms,
+ like => {
+ pg_dumpall_dbprivs => 1,
+ pg_dumpall_exclude => 1,
+ pg_dumpall_globals => 1,
+ pg_dumpall_globals_clean => 1,
+ },
+ },
+
'CREATE DATABASE regression_invalid...' => {
create_order => 1,
create_sql => q(
@@ -3227,6 +3303,79 @@ my %tests = (
},
},
+ # The "RLS is enabled" pseudo-object borrows its table's relname, so it
+ # ties in the sort with a policy of that same name on that same table.
+ # Check that the marker still dumps ahead of the policy.
+ 'CREATE POLICY test_table ON test_table' => {
+ create_order => 27,
+ create_sql => 'CREATE POLICY test_table ON dump_test.test_table
+ USING (true);',
+ regexp => qr/^
+ \QALTER TABLE dump_test.test_table ENABLE ROW LEVEL SECURITY;\E\n.+
+ \QCREATE POLICY test_table ON dump_test.test_table USING (true);\E
+ /xms,
+ like => {
+ %full_runs,
+ %dump_test_schema_runs,
+ only_dump_test_table => 1,
+ section_post_data => 1,
+ },
+ unlike => {
+ exclude_dump_test_schema => 1,
+ exclude_test_table => 1,
+ no_policies => 1,
+ no_policies_restore => 1,
+ only_dump_measurement => 1,
+ },
+ },
+
+ 'CREATE POLICY p7 ON test_table with a multi-role TO list' => {
+ create_order => 28,
+ create_sql => 'CREATE ROLE regress_dump_policy_role_a;
+ CREATE ROLE regress_dump_policy_role_b;
+ CREATE POLICY p7 ON dump_test.test_table
+ TO regress_dump_policy_role_b, regress_dump_policy_role_a
+ USING (true);',
+ regexp => qr/^
+ \QCREATE POLICY p7 ON dump_test.test_table \E
+ \QTO regress_dump_policy_role_b, regress_dump_policy_role_a \E
+ \QUSING (true);\E
+ /xm,
+ like => {
+ %full_runs,
+ %dump_test_schema_runs,
+ only_dump_test_table => 1,
+ section_post_data => 1,
+ },
+ unlike => {
+ exclude_dump_test_schema => 1,
+ exclude_test_table => 1,
+ no_policies => 1,
+ no_policies_restore => 1,
+ only_dump_measurement => 1,
+ },
+ },
+
+ 'CREATE ROLE regress_dump_policy_role_a' => {
+ regexp => qr/^CREATE ROLE regress_dump_policy_role_a;/m,
+ like => {
+ pg_dumpall_dbprivs => 1,
+ pg_dumpall_exclude => 1,
+ pg_dumpall_globals => 1,
+ pg_dumpall_globals_clean => 1,
+ },
+ },
+
+ 'CREATE ROLE regress_dump_policy_role_b' => {
+ regexp => qr/^CREATE ROLE regress_dump_policy_role_b;/m,
+ like => {
+ pg_dumpall_dbprivs => 1,
+ pg_dumpall_exclude => 1,
+ pg_dumpall_globals => 1,
+ pg_dumpall_globals_clean => 1,
+ },
+ },
+
'CREATE PROPERTY GRAPH propgraph' => {
create_order => 20,
create_sql => 'CREATE PROPERTY GRAPH dump_test.propgraph;',
@@ -3323,7 +3472,7 @@ my %tests = (
create_sql =>
'CREATE PUBLICATION pub9 FOR ALL TABLES EXCEPT (TABLE dump_test.test_table, dump_test.test_second_table);',
regexp => qr/^
- \QCREATE PUBLICATION pub9 FOR ALL TABLES EXCEPT (TABLE ONLY dump_test.test_table, TABLE ONLY dump_test.test_second_table) WITH (publish = 'insert, update, delete, truncate');\E
+ \QCREATE PUBLICATION pub9 FOR ALL TABLES EXCEPT (TABLE ONLY dump_test.test_second_table, TABLE ONLY dump_test.test_table) WITH (publish = 'insert, update, delete, truncate');\E
/xm,
like => { %full_runs, section_post_data => 1, },
},
@@ -3333,7 +3482,7 @@ my %tests = (
create_sql =>
'CREATE PUBLICATION pub10 FOR ALL TABLES EXCEPT (TABLE dump_test.test_inheritance_parent);',
regexp => qr/^
- \QCREATE PUBLICATION pub10 FOR ALL TABLES EXCEPT (TABLE ONLY dump_test.test_inheritance_parent, TABLE ONLY dump_test.test_inheritance_child) WITH (publish = 'insert, update, delete, truncate');\E
+ \QCREATE PUBLICATION pub10 FOR ALL TABLES EXCEPT (TABLE ONLY dump_test.test_inheritance_child, TABLE ONLY dump_test.test_inheritance_parent) WITH (publish = 'insert, update, delete, truncate');\E
/xm,
like => { %full_runs, section_post_data => 1, },
},
@@ -4075,6 +4224,85 @@ my %tests = (
},
},
+ # The order of a table's parents is a logical property of the database:
+ # pg_inherits.inhseqno fixes it, and it determines the order of the
+ # child's inherited columns. Here inh_order_parent1 is re-attached after
+ # a NO INHERIT, so it has the *higher* inhseqno; VACUUM frees the line
+ # pointer of the removed pg_inherits row and the re-added one reuses it,
+ # putting the higher-inhseqno parent physically first. The INHERITS list
+ # must still come out in inhseqno order.
+ 'CREATE TABLE inh_order_parent1' => {
+ create_order => 101,
+ create_sql => 'CREATE TABLE dump_test.inh_order_parent1 (
+ col1 int
+ );',
+ regexp => qr/^
+ \QCREATE TABLE dump_test.inh_order_parent1 (\E\n
+ \s+\Qcol1 integer\E\n
+ \Q);\E\n
+ /xm,
+ like =>
+ { %full_runs, %dump_test_schema_runs, section_pre_data => 1, },
+ unlike => {
+ exclude_dump_test_schema => 1,
+ only_dump_measurement => 1,
+ },
+ },
+
+ 'CREATE TABLE inh_order_parent2' => {
+ create_order => 102,
+ create_sql => 'CREATE TABLE dump_test.inh_order_parent2 (
+ col1 int
+ );',
+ regexp => qr/^
+ \QCREATE TABLE dump_test.inh_order_parent2 (\E\n
+ \s+\Qcol1 integer\E\n
+ \Q);\E\n
+ /xm,
+ like =>
+ { %full_runs, %dump_test_schema_runs, section_pre_data => 1, },
+ unlike => {
+ exclude_dump_test_schema => 1,
+ only_dump_measurement => 1,
+ },
+ },
+
+ 'CREATE TABLE inh_order_child' => {
+ create_order => 103,
+ create_sql => 'CREATE TABLE dump_test.inh_order_child (
+ col2 int
+ ) INHERITS (dump_test.inh_order_parent1,
+ dump_test.inh_order_parent2);
+ ALTER TABLE dump_test.inh_order_child
+ NO INHERIT dump_test.inh_order_parent1;
+ VACUUM pg_catalog.pg_inherits;
+ ALTER TABLE dump_test.inh_order_child
+ INHERIT dump_test.inh_order_parent1;',
+ regexp => qr/^
+ \QCREATE TABLE dump_test.inh_order_child (\E\n
+ \s+\Qcol2 integer\E\n
+ \)\n
+ \QINHERITS (dump_test.inh_order_parent2, dump_test.inh_order_parent1);\E\n
+ /xm,
+ like => {
+ %full_runs, %dump_test_schema_runs, section_pre_data => 1,
+ },
+ unlike => {
+ binary_upgrade => 1,
+ exclude_dump_test_schema => 1,
+ only_dump_measurement => 1,
+ },
+ },
+
+ 'CREATE TABLE inh_order_child pg_upgrade' => {
+ regexp => qr/^
+ \QALTER TABLE ONLY dump_test.inh_order_child INHERIT dump_test.inh_order_parent2;\E\n
+ \QALTER TABLE ONLY dump_test.inh_order_child INHERIT dump_test.inh_order_parent1;\E\n
+ /xm,
+ like => { binary_upgrade => 1, },
+ },
+
+
'CREATE STATISTICS extended_stats_no_options' => {
create_order => 97,
create_sql => 'CREATE STATISTICS dump_test.test_ext_stats_no_options
diff --git a/src/bin/pg_dump/t/003_pg_dump_with_server.pl b/src/bin/pg_dump/t/003_pg_dump_with_server.pl
index 349add6..7c10e3a 100644
--- a/src/bin/pg_dump/t/003_pg_dump_with_server.pl
+++ b/src/bin/pg_dump/t/003_pg_dump_with_server.pl
@@ -47,4 +47,95 @@ command_ok(
],
"dump foreign server with no tables");
+#########################################
+# Verify that --binary-upgrade lists an extension's required extensions in
+# name order. pg_dump reads the requires list out of pg_depend, which
+# returns those rows in an order derived from the required extensions'
+# OIDs; without an explicit sort, two databases holding the same extensions
+# dump differently depending on the order the extensions were created in.
+
+mkdir "$tempdir/extension"
+ or die "could not create directory \"$tempdir/extension\": $!";
+foreach my $ext ('dump_test_ext_a', 'dump_test_ext_b', 'dump_test_ext_c')
+{
+ open my $cf, '>', "$tempdir/extension/$ext.control"
+ or die "could not create control file for $ext: $!";
+ print $cf "default_version = '1.0'\n";
+ print $cf "relocatable = true\n";
+ print $cf "requires = 'dump_test_ext_a,dump_test_ext_b'\n"
+ if $ext eq 'dump_test_ext_c';
+ close $cf;
+
+ # The extensions need no members, so an empty script will do.
+ open my $sf, '>', "$tempdir/extension/$ext--1.0.sql"
+ or die "could not create script file for $ext: $!";
+ close $sf;
+}
+
+my $sep = $windows_os ? ';' : ':';
+my $ext_path = $windows_os ? ($tempdir =~ s/\\/\\\\/gr) : $tempdir;
+
+# Create dump_test_ext_a before dump_test_ext_b, so that the requirement
+# that sorts first by name is the one with the smaller OID. pg_depend
+# hands back these rows in descending OID order, that is, in the reverse of
+# the order the dump must use.
+$node->safe_psql(
+ 'postgres', qq{
+ SET extension_control_path = '\$system$sep$ext_path';
+ CREATE EXTENSION dump_test_ext_a;
+ CREATE EXTENSION dump_test_ext_b;
+ CREATE EXTENSION dump_test_ext_c;});
+
+command_like(
+ [ 'pg_dump', '--port' => $port, '--binary-upgrade', 'postgres' ],
+ qr/\QSELECT pg_catalog.binary_upgrade_create_empty_extension('dump_test_ext_c', 'public', true, '1.0', NULL, NULL, ARRAY['dump_test_ext_a','dump_test_ext_b']::pg_catalog.text[]);\E/,
+ 'binary upgrade dumps required extensions in name order');
+
+#########################################
+# Verify that an object carrying labels from more than one security label
+# provider gets its SECURITY LABEL commands emitted in provider name order,
+# not in pg_seclabel/pg_shseclabel physical order. dummy_seclabel registers
+# a second provider, "dummy2", when dummy_seclabel.second_provider is turned
+# on before the module is loaded.
+
+SKIP:
+{
+ skip "dummy_seclabel module not installed", 6
+ unless $node->check_extension('dummy_seclabel');
+
+ # Label each object with "dummy2" before "dummy", that is, in the reverse
+ # of the order the dump has to use, so that emitting the labels in
+ # catalog order would produce the wrong output.
+ $node->safe_psql(
+ 'postgres', q|
+ SET dummy_seclabel.second_provider = on;
+ LOAD 'dummy_seclabel';
+ CREATE TABLE seclabel_order_tbl (a int);
+ SECURITY LABEL FOR dummy2 ON TABLE seclabel_order_tbl IS 'classified';
+ SECURITY LABEL FOR dummy ON TABLE seclabel_order_tbl IS 'classified';
+ SECURITY LABEL FOR dummy2 ON COLUMN seclabel_order_tbl.a IS 'classified';
+ SECURITY LABEL FOR dummy ON COLUMN seclabel_order_tbl.a IS 'classified';
+ SECURITY LABEL FOR dummy2 ON DATABASE postgres IS 'classified';
+ SECURITY LABEL FOR dummy ON DATABASE postgres IS 'classified';
+ |);
+
+ $node->command_like(
+ [ 'pg_dump', '--schema-only', 'postgres' ],
+ qr/^
+ \QSECURITY LABEL FOR dummy ON TABLE public.seclabel_order_tbl IS 'classified';\E\n
+ \QSECURITY LABEL FOR dummy2 ON TABLE public.seclabel_order_tbl IS 'classified';\E\n
+ \QSECURITY LABEL FOR dummy ON COLUMN public.seclabel_order_tbl.a IS 'classified';\E\n
+ \QSECURITY LABEL FOR dummy2 ON COLUMN public.seclabel_order_tbl.a IS 'classified';\E$
+ /xm,
+ 'security labels are dumped in provider order');
+
+ $node->command_like(
+ [ 'pg_dump', '--schema-only', '--create', 'postgres' ],
+ qr/^
+ \QSECURITY LABEL FOR dummy ON DATABASE postgres IS 'classified';\E\n
+ \QSECURITY LABEL FOR dummy2 ON DATABASE postgres IS 'classified';\E$
+ /xm,
+ 'shared security labels are dumped in provider order');
+}
+
done_testing();
diff --git a/src/test/modules/dummy_seclabel/dummy_seclabel.c b/src/test/modules/dummy_seclabel/dummy_seclabel.c
index 7277f61..909a1b7 100644
--- a/src/test/modules/dummy_seclabel/dummy_seclabel.c
+++ b/src/test/modules/dummy_seclabel/dummy_seclabel.c
@@ -15,12 +15,15 @@
#include "commands/seclabel.h"
#include "fmgr.h"
#include "miscadmin.h"
+#include "utils/guc.h"
#include "utils/rel.h"
PG_MODULE_MAGIC;
PG_FUNCTION_INFO_V1(dummy_seclabel_dummy);
+static bool dummy_seclabel_second_provider = false;
+
static void
dummy_object_relabel(const ObjectAddress *object, const char *seclabel)
{
@@ -47,6 +50,29 @@ void
_PG_init(void)
{
register_label_provider("dummy", dummy_object_relabel);
+
+ /*
+ * Optionally register a second provider. Tests that need two providers
+ * registered at the same time turn this on before the module is loaded.
+ * It defaults to off, so that the provider-less "SECURITY LABEL ON ... IS
+ * ..." syntax, which requires exactly one registered provider, keeps
+ * working.
+ */
+ DefineCustomBoolVariable("dummy_seclabel.second_provider",
+ "Also register a \"dummy2\" label provider.",
+ NULL,
+ &dummy_seclabel_second_provider,
+ false,
+ PGC_SUSET,
+ 0,
+ NULL,
+ NULL,
+ NULL);
+
+ MarkGUCPrefixReserved("dummy_seclabel");
+
+ if (dummy_seclabel_second_provider)
+ register_label_provider("dummy2", dummy_object_relabel);
}
/*
diff --git a/src/test/modules/test_pg_dump/t/001_base.pl b/src/test/modules/test_pg_dump/t/001_base.pl
index 3d65ce4..d9e1ea9 100644
--- a/src/test/modules/test_pg_dump/t/001_base.pl
+++ b/src/test/modules/test_pg_dump/t/001_base.pl
@@ -846,6 +846,51 @@ my %tests = (
},
},
+ 'CREATE TRIGGER extdepend_trig' => {
+ create_order => 12,
+ create_sql =>
+ 'CREATE TRIGGER extdepend_trig BEFORE UPDATE ON regress_pg_dump_schema.extdependtab
+ FOR EACH ROW EXECUTE FUNCTION suppress_redundant_updates_trigger();
+ ALTER TRIGGER extdepend_trig ON regress_pg_dump_schema.extdependtab DEPENDS ON EXTENSION test_pg_dump;
+ ALTER TRIGGER extdepend_trig ON regress_pg_dump_schema.extdependtab DEPENDS ON EXTENSION plpgsql;',
+ regexp => qr/^
+ \QCREATE TRIGGER extdepend_trig BEFORE UPDATE ON regress_pg_dump_schema.extdependtab FOR EACH ROW EXECUTE FUNCTION suppress_redundant_updates_trigger();\E\n
+ /xms,
+ like => {%pgdump_runs},
+ unlike => {
+ data_only => 1,
+ extension_schema => 1,
+ pg_dumpall_globals => 1,
+ privileged_internals => 1,
+ section_data => 1,
+ section_pre_data => 1,
+ # Excludes this schema as extension is not listed.
+ without_extension_explicit_schema => 1,
+ },
+ },
+
+ # The two ALTER TRIGGER ... DEPENDS ON EXTENSION statements above are
+ # executed test_pg_dump first, plpgsql second, but pg_dump must emit them
+ # in extension name order, so that the archive entry's text does not
+ # depend on pg_depend's physical row order.
+ 'ALTER TRIGGER DEPENDS ON extension in name order' => {
+ regexp => qr/^
+ \QALTER TRIGGER extdepend_trig ON regress_pg_dump_schema.extdependtab DEPENDS ON EXTENSION plpgsql;\E\n
+ \QALTER TRIGGER extdepend_trig ON regress_pg_dump_schema.extdependtab DEPENDS ON EXTENSION test_pg_dump;\E\n
+ /xms,
+ like => {%pgdump_runs},
+ unlike => {
+ data_only => 1,
+ extension_schema => 1,
+ pg_dumpall_globals => 1,
+ privileged_internals => 1,
+ section_data => 1,
+ section_pre_data => 1,
+ # Excludes this schema as extension is not listed.
+ without_extension_explicit_schema => 1,
+ },
+ },
+
# Objects not included in extension, part of schema created by extension
'CREATE TABLE regress_pg_dump_schema.external_tab' => {
create_order => 4,
--
2.49.0
From 2d0d0f3158afdc47e49afa68d87dd4823ad3d32c Mon Sep 17 00:00:00 2001
From: Noah Misch <noah@leadboat.com>
Date: Thu, 3 Sep 2026 18:07:02 +0000
Subject: [PATCH 2/2] SAMPLE fixes for the eleven testable dump-order defects
Not proposed patches. These exist so that the branch is coherent -- the tests
in the preceding commit need something to pass against -- and so that "this
test fails without the fix" is checkable. Drop this commit to see them fail.
Four involve a judgement rather than a mechanical key completion:
* D5 preserves the order the user wrote in CREATE POLICY, via unnest ... WITH
ORDINALITY. Plain ORDER BY rolname would be simpler but rewrites the
clause. Both remove the OID dependence.
* D4 joins pg_type and pg_namespace and orders by (nspname, typname) rather
than by regtype output, whose rendering depends on search_path.
* D1 sorts the RLS-enable pseudo-object before its table's policies. Either
order is stable.
* D12 sorts an ACL array, which buildACLCommands() warns can be unsafe. It is
safe here only because a default ACL's items all share one grantor, so there
is no grant chain to replay in order; that argument is the basis of the fix
and is the thing to check before accepting it.
Verified: with these applied, meson test over the pg_dump, test_pg_dump and
dummy_seclabel suites is 13/13 (002_pg_dump alone is 13697 subtests). With
all five product files reverted, pg_dump aborts on D1's assertion. With only
D1's fix applied, each remaining test fails by its own name.
This work is model-generated and unreviewed by a human; see PROVENANCE.md.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Newm1jZHVy54kfX1eRPDwB
---
src/bin/pg_dump/common.c | 8 +-
src/bin/pg_dump/dumputils.c | 8 +-
src/bin/pg_dump/pg_dump.c | 155 +++++++++++++++++++++++++++------
src/bin/pg_dump/pg_dump_sort.c | 12 +++
src/bin/pg_dump/pg_dumpall.c | 2 +-
5 files changed, 153 insertions(+), 32 deletions(-)
diff --git a/src/bin/pg_dump/common.c b/src/bin/pg_dump/common.c
index 047e1c6..38eb274 100644
--- a/src/bin/pg_dump/common.c
+++ b/src/bin/pg_dump/common.c
@@ -286,10 +286,10 @@ flagInhTables(Archive *fout, TableInfo *tblinfo, int numTables,
for (i = 0; i < numInherits; i++)
{
/*
- * Skip a hashtable lookup if it's same table as last time. This is
- * unlikely for the child, but less so for the parent. (Maybe we
- * should ask the backend for a sorted array to make it more likely?
- * Not clear the sorting effort would be repaid, though.)
+ * Skip a hashtable lookup if it's same table as last time.
+ * getInherits() sorts by inhrelid, so consecutive rows for the same
+ * child do come together; repeats of the same parent are less
+ * predictable.
*/
if (child == NULL ||
child->dobj.catId.oid != inhinfo[i].inhrelid)
diff --git a/src/bin/pg_dump/dumputils.c b/src/bin/pg_dump/dumputils.c
index a3835cc..d7b000f 100644
--- a/src/bin/pg_dump/dumputils.c
+++ b/src/bin/pg_dump/dumputils.c
@@ -681,10 +681,16 @@ void
buildShSecLabelQuery(const char *catalog_name, Oid objectId,
PQExpBuffer sql)
{
+ /*
+ * Sort by provider, the remaining column of pg_shseclabel's unique key
+ * (classoid and objoid are already fixed by the WHERE clause), so that
+ * the emitted commands do not depend on physical row order.
+ */
appendPQExpBuffer(sql,
"SELECT provider, label FROM pg_catalog.pg_shseclabel "
"WHERE classoid = 'pg_catalog.%s'::pg_catalog.regclass "
- "AND objoid = '%u'", catalog_name, objectId);
+ "AND objoid = '%u' "
+ "ORDER BY provider", catalog_name, objectId);
}
/*
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index db14834..436c17d 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -3760,10 +3760,16 @@ dumpDatabaseConfig(Archive *AH, PQExpBuffer outbuf,
PQclear(res);
- /* Now look for role-and-database-specific options */
+ /*
+ * Now look for role-and-database-specific options. Order by role name,
+ * so that the emitted commands don't depend on the roles' OIDs; rolname
+ * is a complete sort key, since pg_db_role_setting has at most one row
+ * per (setdatabase, setrole).
+ */
printfPQExpBuffer(buf, "SELECT rolname, unnest(setconfig) "
"FROM pg_db_role_setting s, pg_roles r "
- "WHERE setrole = r.oid AND setdatabase = '%u'::oid",
+ "WHERE setrole = r.oid AND setdatabase = '%u'::oid "
+ "ORDER BY 1",
dboid);
res = ExecuteSqlQuery(AH, buf->data, PGRES_TUPLES_OK);
@@ -4274,9 +4280,20 @@ getPolicies(Archive *fout, TableInfo tblinfo[], int numTables)
printfPQExpBuffer(query,
"SELECT pol.oid, pol.tableoid, pol.polrelid, pol.polname, pol.polcmd, ");
appendPQExpBufferStr(query, "pol.polpermissive, ");
+ /*
+ * The role names in the policy's TO clause must come out in the order
+ * they appear in polroles, which is the order they were written in
+ * CREATE POLICY. An unordered ARRAY() subquery would instead return them
+ * in pg_authid scan order, so two databases holding identical policies
+ * would dump differently whenever their roles occupy different physical
+ * positions or the planner picks a different scan for pg_authid.
+ */
appendPQExpBuffer(query,
"CASE WHEN pol.polroles = '{0}' THEN NULL ELSE "
- " pg_catalog.array_to_string(ARRAY(SELECT pg_catalog.quote_ident(rolname) from pg_catalog.pg_roles WHERE oid = ANY(pol.polroles)), ', ') END AS polroles, "
+ " pg_catalog.array_to_string(ARRAY(SELECT pg_catalog.quote_ident(r.rolname) "
+ "FROM pg_catalog.unnest(pol.polroles) WITH ORDINALITY AS u(roleoid, ord) "
+ "JOIN pg_catalog.pg_roles r ON (r.oid = u.roleoid) "
+ "ORDER BY u.ord), ', ') END AS polroles, "
"pg_catalog.pg_get_expr(pol.polqual, pol.polrelid) AS polqual, "
"pg_catalog.pg_get_expr(pol.polwithcheck, pol.polrelid) AS polwithcheck "
"FROM unnest('%s'::pg_catalog.oid[]) AS src(tbloid)\n"
@@ -4595,10 +4612,22 @@ getPublications(Archive *fout)
PGresult *res_tbls;
resetPQExpBuffer(query);
+ /*
+ * Sort the EXCEPT list by the excluded relations' names. The list
+ * is a set, and pg_publication_rel has no ordering column, so an
+ * unordered query would emit it in heap order and make two
+ * logically-identical databases dump differently. Sorting by
+ * prrelid would just trade heap order for OID order; use the
+ * referenced relation's natural key (nspname, relname), matching
+ * DOTypeNameCompare().
+ */
appendPQExpBuffer(query,
- "SELECT prrelid\n"
- "FROM pg_catalog.pg_publication_rel\n"
- "WHERE prpubid = %u AND prexcept",
+ "SELECT pr.prrelid\n"
+ "FROM pg_catalog.pg_publication_rel pr\n"
+ " JOIN pg_catalog.pg_class c ON c.oid = pr.prrelid\n"
+ " JOIN pg_catalog.pg_namespace n ON n.oid = c.relnamespace\n"
+ "WHERE pr.prpubid = %u AND pr.prexcept\n"
+ "ORDER BY n.nspname, c.relname",
pubinfo[i].dobj.catId.oid);
res_tbls = ExecuteSqlQuery(fout, query->data, PGRES_TUPLES_OK);
@@ -5677,6 +5706,10 @@ dumpSubscription(Archive *fout, const SubscriptionInfo *subinfo)
/*
* Given a "create query", append as many ALTER ... DEPENDS ON EXTENSION as
* the object needs.
+ *
+ * The statements are emitted in extension name order, so that the text of the
+ * object's archive entry is a function of the object's dependencies and not of
+ * the order in which those dependencies happen to appear in pg_depend.
*/
static void
append_depends_on_extension(Archive *fout,
@@ -5704,7 +5737,8 @@ append_depends_on_extension(Archive *fout,
"FROM pg_catalog.pg_depend d, pg_catalog.pg_extension e "
"WHERE d.refobjid = e.oid AND classid = '%s'::pg_catalog.regclass "
"AND objid = '%u'::pg_catalog.oid AND deptype = 'x' "
- "AND refclassid = 'pg_catalog.pg_extension'::pg_catalog.regclass",
+ "AND refclassid = 'pg_catalog.pg_extension'::pg_catalog.regclass "
+ "ORDER BY e.extname",
catalog,
dobj->catId.oid);
res = ExecuteSqlQuery(fout, query->data, PGRES_TUPLES_OK);
@@ -7692,8 +7726,18 @@ getInherits(Archive *fout, int *numInherits)
int i_inhrelid;
int i_inhparent;
- /* find all the inheritance information */
- appendPQExpBufferStr(query, "SELECT inhrelid, inhparent FROM pg_inherits");
+ /*
+ * Find all the inheritance information. ORDER BY inhseqno is essential:
+ * the order of a table's parents is a logical property of the database
+ * (inhseqno fixes the order of the child's inherited columns), while the
+ * physical order of pg_inherits rows is not, since a line pointer freed
+ * by NO INHERIT or DROP TABLE and then reclaimed by VACUUM gets reused by
+ * a later entry with a higher inhseqno. Sorting by inhrelid as well
+ * makes the "same table as last time" caching in flagInhTables() work.
+ */
+ appendPQExpBufferStr(query,
+ "SELECT inhrelid, inhparent FROM pg_inherits "
+ "ORDER BY inhrelid, inhseqno");
res = ExecuteSqlQuery(fout, query->data, PGRES_TUPLES_OK);
@@ -10570,12 +10614,26 @@ getDefaultACLs(Archive *fout)
* for the case of 'S' (DEFACLOBJ_SEQUENCE) which must be converted to
* 's'.
*/
+ /*
+ * The stored element order of defaclacl carries no information: the
+ * backend canonicalizes these arrays with aclitemsort(), which orders them
+ * by grantee OID. Dumping in that order would make our output depend on
+ * OID assignment, so re-sort by the aclitem's textual form, i.e. by
+ * grantee name. Unlike an object's own ACL, a default ACL cannot contain
+ * a chain of grants by different grantors -- every item's grantor is
+ * defaclrole -- so reordering is safe here.
+ */
appendPQExpBufferStr(query,
"SELECT oid, tableoid, "
"defaclrole, "
"defaclnamespace, "
"defaclobjtype, "
- "defaclacl, "
+ "CASE WHEN pg_catalog.array_length(defaclacl, 1) IS NULL "
+ "THEN defaclacl ELSE "
+ "(SELECT pg_catalog.array_agg(a ORDER BY "
+ "a::pg_catalog.text COLLATE pg_catalog.\"C\") "
+ "FROM pg_catalog.unnest(defaclacl) AS a) "
+ "END AS defaclacl, "
"CASE WHEN defaclnamespace = 0 THEN "
"acldefault(CASE WHEN defaclobjtype = 'S' "
"THEN 's'::\"char\" ELSE defaclobjtype END, "
@@ -11954,6 +12012,7 @@ dumpExtension(Archive *fout, const ExtensionInfo *extinfo)
*/
int i;
int n;
+ char **reqexts;
appendPQExpBufferStr(q, "-- For binary upgrade, create an empty extension and insert objects into it\n");
@@ -11989,7 +12048,14 @@ dumpExtension(Archive *fout, const ExtensionInfo *extinfo)
else
appendPQExpBufferStr(q, "NULL");
appendPQExpBufferStr(q, ", ");
- appendPQExpBufferStr(q, "ARRAY[");
+ /*
+ * Collect the names of the extensions this one requires. The
+ * dependency array is in the order getDependencies() read the
+ * pg_depend rows, which is a function of the required extensions'
+ * OIDs; sort the names so that the output depends only on the
+ * database's logical content.
+ */
+ reqexts = (char **) pg_malloc(extinfo->dobj.nDeps * sizeof(char *));
n = 0;
for (i = 0; i < extinfo->dobj.nDeps; i++)
{
@@ -11997,14 +12063,20 @@ dumpExtension(Archive *fout, const ExtensionInfo *extinfo)
extobj = findObjectByDumpId(extinfo->dobj.dependencies[i]);
if (extobj && extobj->objType == DO_EXTENSION)
- {
- if (n++ > 0)
- appendPQExpBufferChar(q, ',');
- appendStringLiteralAH(q, extobj->name, fout);
- }
+ reqexts[n++] = extobj->name;
+ }
+ qsort(reqexts, n, sizeof(char *), pg_qsort_strcmp);
+
+ appendPQExpBufferStr(q, "ARRAY[");
+ for (i = 0; i < n; i++)
+ {
+ if (i > 0)
+ appendPQExpBufferChar(q, ',');
+ appendStringLiteralAH(q, reqexts[i], fout);
}
appendPQExpBufferStr(q, "]::pg_catalog.text[]");
appendPQExpBufferStr(q, ");\n");
+ pg_free(reqexts);
}
if (extinfo->dobj.dump & DUMP_COMPONENT_DEFINITION)
@@ -14577,15 +14649,20 @@ dumpOpclass(Archive *fout, const OpclassInfo *opcinfo)
appendPQExpBuffer(query, "SELECT amopstrategy, "
"amopopr::pg_catalog.regoperator, "
"opfname AS sortfamily, "
- "nspname AS sortfamilynsp "
+ "n.nspname AS sortfamilynsp "
"FROM pg_catalog.pg_amop ao JOIN pg_catalog.pg_depend ON "
"(classid = 'pg_catalog.pg_amop'::pg_catalog.regclass AND objid = ao.oid) "
"LEFT JOIN pg_catalog.pg_opfamily f ON f.oid = amopsortfamily "
"LEFT JOIN pg_catalog.pg_namespace n ON n.oid = opfnamespace "
+ "JOIN pg_catalog.pg_type lt ON lt.oid = ao.amoplefttype "
+ "JOIN pg_catalog.pg_namespace ln ON ln.oid = lt.typnamespace "
+ "JOIN pg_catalog.pg_type rt ON rt.oid = ao.amoprighttype "
+ "JOIN pg_catalog.pg_namespace rn ON rn.oid = rt.typnamespace "
"WHERE refclassid = 'pg_catalog.pg_opclass'::pg_catalog.regclass "
"AND refobjid = '%u'::pg_catalog.oid "
"AND amopfamily = '%s'::pg_catalog.oid "
- "ORDER BY amopstrategy",
+ "ORDER BY amopstrategy, ln.nspname, lt.typname, "
+ "rn.nspname, rt.typname",
opcinfo->dobj.catId.oid,
opcfamily);
@@ -14639,12 +14716,19 @@ dumpOpclass(Archive *fout, const OpclassInfo *opcinfo)
"amproc::pg_catalog.regprocedure, "
"amproclefttype::pg_catalog.regtype, "
"amprocrighttype::pg_catalog.regtype "
- "FROM pg_catalog.pg_amproc ap, pg_catalog.pg_depend "
+ "FROM pg_catalog.pg_amproc ap, pg_catalog.pg_depend, "
+ "pg_catalog.pg_type lt, pg_catalog.pg_namespace ln, "
+ "pg_catalog.pg_type rt, pg_catalog.pg_namespace rn "
"WHERE refclassid = 'pg_catalog.pg_opclass'::pg_catalog.regclass "
"AND refobjid = '%u'::pg_catalog.oid "
"AND classid = 'pg_catalog.pg_amproc'::pg_catalog.regclass "
"AND objid = ap.oid "
- "ORDER BY amprocnum",
+ "AND lt.oid = ap.amproclefttype "
+ "AND ln.oid = lt.typnamespace "
+ "AND rt.oid = ap.amprocrighttype "
+ "AND rn.oid = rt.typnamespace "
+ "ORDER BY amprocnum, ln.nspname, lt.typname, "
+ "rn.nspname, rt.typname",
opcinfo->dobj.catId.oid);
res = ExecuteSqlQuery(fout, query->data, PGRES_TUPLES_OK);
@@ -14779,15 +14863,20 @@ dumpOpfamily(Archive *fout, const OpfamilyInfo *opfinfo)
appendPQExpBuffer(query, "SELECT amopstrategy, "
"amopopr::pg_catalog.regoperator, "
"opfname AS sortfamily, "
- "nspname AS sortfamilynsp "
+ "n.nspname AS sortfamilynsp "
"FROM pg_catalog.pg_amop ao JOIN pg_catalog.pg_depend ON "
"(classid = 'pg_catalog.pg_amop'::pg_catalog.regclass AND objid = ao.oid) "
"LEFT JOIN pg_catalog.pg_opfamily f ON f.oid = amopsortfamily "
"LEFT JOIN pg_catalog.pg_namespace n ON n.oid = opfnamespace "
+ "JOIN pg_catalog.pg_type lt ON lt.oid = ao.amoplefttype "
+ "JOIN pg_catalog.pg_namespace ln ON ln.oid = lt.typnamespace "
+ "JOIN pg_catalog.pg_type rt ON rt.oid = ao.amoprighttype "
+ "JOIN pg_catalog.pg_namespace rn ON rn.oid = rt.typnamespace "
"WHERE refclassid = 'pg_catalog.pg_opfamily'::pg_catalog.regclass "
"AND refobjid = '%u'::pg_catalog.oid "
"AND amopfamily = '%u'::pg_catalog.oid "
- "ORDER BY amopstrategy",
+ "ORDER BY amopstrategy, ln.nspname, lt.typname, "
+ "rn.nspname, rt.typname",
opfinfo->dobj.catId.oid,
opfinfo->dobj.catId.oid);
@@ -14799,12 +14888,19 @@ dumpOpfamily(Archive *fout, const OpfamilyInfo *opfinfo)
"amproc::pg_catalog.regprocedure, "
"amproclefttype::pg_catalog.regtype, "
"amprocrighttype::pg_catalog.regtype "
- "FROM pg_catalog.pg_amproc ap, pg_catalog.pg_depend "
+ "FROM pg_catalog.pg_amproc ap, pg_catalog.pg_depend, "
+ "pg_catalog.pg_type lt, pg_catalog.pg_namespace ln, "
+ "pg_catalog.pg_type rt, pg_catalog.pg_namespace rn "
"WHERE refclassid = 'pg_catalog.pg_opfamily'::pg_catalog.regclass "
"AND refobjid = '%u'::pg_catalog.oid "
"AND classid = 'pg_catalog.pg_amproc'::pg_catalog.regclass "
"AND objid = ap.oid "
- "ORDER BY amprocnum",
+ "AND lt.oid = ap.amproclefttype "
+ "AND ln.oid = lt.typnamespace "
+ "AND rt.oid = ap.amprocrighttype "
+ "AND rn.oid = rt.typnamespace "
+ "ORDER BY amprocnum, ln.nspname, lt.typname, "
+ "rn.nspname, rt.typname",
opfinfo->dobj.catId.oid);
res_procs = ExecuteSqlQuery(fout, query->data, PGRES_TUPLES_OK);
@@ -16731,7 +16827,8 @@ findSecLabels(Oid classoid, Oid objoid, SecLabelItem **items)
* Construct a table of all security labels available for database objects;
* also set the has-seclabel component flag for each relevant object.
*
- * The table is sorted by classoid/objid/objsubid for speed in lookup.
+ * The table is sorted by classoid/objid/objsubid/provider for speed in
+ * lookup.
*/
static void
collectSecLabels(Archive *fout)
@@ -16749,10 +16846,16 @@ collectSecLabels(Archive *fout)
query = createPQExpBuffer();
+ /*
+ * Sort by provider as well. It is the remaining column of pg_seclabel's
+ * unique key, so adding it makes the ordering total; without it, the
+ * order of the labels an object has from different providers would come
+ * from physical row order, making the dump unstable.
+ */
appendPQExpBufferStr(query,
"SELECT label, provider, classoid, objoid, objsubid "
"FROM pg_catalog.pg_seclabels "
- "ORDER BY classoid, objoid, objsubid");
+ "ORDER BY classoid, objoid, objsubid, provider");
res = ExecuteSqlQuery(fout, query->data, PGRES_TUPLES_OK);
diff --git a/src/bin/pg_dump/pg_dump_sort.c b/src/bin/pg_dump/pg_dump_sort.c
index 8ca0332..38c3a1c 100644
--- a/src/bin/pg_dump/pg_dump_sort.c
+++ b/src/bin/pg_dump/pg_dump_sort.c
@@ -394,6 +394,18 @@ DOTypeNameCompare(const void *p1, const void *p2)
pobj2->poltable->dobj.name);
if (cmpval != 0)
return cmpval;
+
+ /*
+ * getPolicies() represents "RLS is enabled on this table" as a
+ * PolicyInfo with null polname whose dobj.name is the table's relname.
+ * Policy names live in a per-table namespace disjoint from relation
+ * names, so such a marker ties with a real policy of that same name on
+ * that same table; whether polname is null is then the only remaining
+ * natural-key field. Sort the marker first.
+ */
+ cmpval = (pobj1->polname != NULL) - (pobj2->polname != NULL);
+ if (cmpval != 0)
+ return cmpval;
}
else if (obj1->objType == DO_RULE)
{
diff --git a/src/bin/pg_dump/pg_dumpall.c b/src/bin/pg_dump/pg_dumpall.c
index c53e77c..d867e9e 100644
--- a/src/bin/pg_dump/pg_dumpall.c
+++ b/src/bin/pg_dump/pg_dumpall.c
@@ -1373,7 +1373,7 @@ dumpTablespaces(PGconn *conn)
"pg_catalog.shobj_description(oid, 'pg_tablespace') "
"FROM pg_catalog.pg_tablespace "
"WHERE spcname !~ '^pg_' "
- "ORDER BY 1");
+ "ORDER BY 2");
if (PQntuples(res) > 0)
fprintf(OPF, "--\n-- Tablespaces\n--\n\n");
--
2.49.0
Attachments:
[text/plain] 0001-Audit-report-and-regression-tests-for-pg_dump-dump-o.patch (73.3K, ../../20260909000357.b2.noahmisch@microsoft.com/2-0001-Audit-report-and-regression-tests-for-pg_dump-dump-o.patch)
download | inline diff:
From 99d7f92890217317005c6e5d6b0d41bf616e3fc5 Mon Sep 17 00:00:00 2001
From: Noah Misch <noah@leadboat.com>
Date: Thu, 3 Sep 2026 18:06:51 +0000
Subject: [PATCH 1/2] Audit report and regression tests for pg_dump dump-order
instability
Audit of what the DO_CAST/DO_TRANSFORM tiebreakers in the preceding commit do
not cover. Twelve confirmed defects; only one of them is another tie in
DOTypeNameCompare().
D1 DO_POLICY: the "RLS enabled" pseudo-object borrows its table's relname, so
it ties with a policy named after that same table. An assert-enabled
pg_dump aborts; a production build orders the two by comparing a pg_class
OID against a pg_policy OID, which pg_upgrade inverts.
D2 Dependency-loop repair picks its start point in dumpId order, so which
object is broken out of a cycle follows OID assignment. Reported only:
fixing it changes the dump of databases that dump fine today.
D3 getInherits() has no ORDER BY and pg_dump never reads inhseqno, so the
INHERITS list follows pg_inherits heap order. This is not only an
ordering defect: a plain dump/restore can permute the child's columns.
D4 dumpOpfamily()/dumpOpclass() order members by strategy number alone.
D5 getPolicies() builds the policy's TO role list with an unordered
sub-select over pg_roles.
D6 getPublications() does not order the FOR ALL TABLES EXCEPT list.
D7 dumpDatabaseConfig() does not order per-role database settings.
D8 append_depends_on_extension() does not order its rows.
D9 collectSecLabels() omits provider from its ORDER BY, as does the
shared-object path in dumputils.c.
D10 pg_dumpall's dumpTablespaces() says ORDER BY 1 on a select list whose
first column is oid; the sibling dumpRoles() says ORDER BY 2.
D11 dumpExtension() emits an extension's requires array under
--binary-upgrade in dependency-array order, which is OID-derived.
D12 getDefaultACLs() emits defaclacl in the backend's canonical order, which
aclitemsort() makes grantee-OID order.
Tests cover all of these but D2. Each fails, or aborts pg_dump, on the tree
without the sample fixes in the following commit. Two needed test
infrastructure rather than a test entry: D8 lives in test_pg_dump because
showing it needs one object with two extension dependencies and a bare initdb
has only plpgsql, and D9 adds a second label provider to dummy_seclabel.
This work is model-generated and unreviewed by a human; see PROVENANCE.md.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Newm1jZHVy54kfX1eRPDwB
---
DUMP_SORT_STABILITY_REPORT.md | 878 ++++++++++++++++++
PROVENANCE.md | 126 +++
src/bin/pg_dump/t/002_pg_dump.pl | 238 ++++-
src/bin/pg_dump/t/003_pg_dump_with_server.pl | 91 ++
.../modules/dummy_seclabel/dummy_seclabel.c | 26 +
src/test/modules/test_pg_dump/t/001_base.pl | 45 +
6 files changed, 1399 insertions(+), 5 deletions(-)
create mode 100644 DUMP_SORT_STABILITY_REPORT.md
create mode 100644 PROVENANCE.md
diff --git a/DUMP_SORT_STABILITY_REPORT.md b/DUMP_SORT_STABILITY_REPORT.md
new file mode 100644
index 0000000..6380580
--- /dev/null
+++ b/DUMP_SORT_STABILITY_REPORT.md
@@ -0,0 +1,878 @@
+# pg_dump dump-order stability: audit of what `sort_cast.patch` does not cover
+
+**Question asked:** after the DO_CAST / DO_TRANSFORM tiebreakers, are more sources of
+dump-order instability still lurking?
+
+**Answer:** yes -- **twelve**, but only **one** of them is another tie in
+`DOTypeNameCompare()`. The other eleven are outside the object sort: one in dependency-loop
+repair, ten in dump-time queries and emitters whose result order reaches the output text
+directly. One of those is not merely an ordering nuisance: it makes a plain `pg_dump`
+→ restore **silently permute an inherited table's column order**.
+
+| # | Where | What | Severity |
+|---|---|---|---|
+| D1 | `DOTypeNameCompare()` | the RLS-enabled pseudo-object ties with a policy named after its own table -- `pg_dump` **aborts** on an assert build | high |
+| D2 | `pg_dump_sort.c` loop repair | `TopoSort()` builds its failure list in dumpId order, so which object gets broken out of a dependency cycle follows OID assignment | medium |
+| D3 | `getInherits()` | no `ORDER BY`; `inhseqno` is never read at all, so the `INHERITS` list follows heap order -- **restores with the child's columns permuted** | high |
+| D4 | `dumpOpfamily()`, `dumpOpclass()` | member lists ordered only by `amopstrategy` / `amprocnum`, which is not a key | medium |
+| D5 | `getPolicies()` | the policy `TO` role list is built by an unordered sub-select | medium |
+| D6 | `getPublications()` | `FOR ALL TABLES EXCEPT (...)` list has no `ORDER BY` | medium |
+| D7 | `dumpDatabaseConfig()` | `ALTER ROLE ... IN DATABASE ... SET` lines have no `ORDER BY` | medium |
+| D8 | `append_depends_on_extension()` | `DEPENDS ON EXTENSION` lines have no `ORDER BY` | low |
+| D9 | `collectSecLabels()`, `dumputils.c` | `ORDER BY` omits `provider`, which is part of the key | low |
+| D10 | `pg_dumpall.c` `dumpTablespaces()` | `ORDER BY 1` on `SELECT oid, spcname, ...` sorts by **OID** | low |
+| D11 | `dumpExtension()` `--binary-upgrade` | the `requires` array is emitted in dependency-array order, which is OID-derived | low |
+| D12 | `getDefaultACLs()` | `defaclacl` is emitted in the backend's canonical **grantee-OID** order | medium |
+
+Every one of the twelve was reproduced twice: once by an agent that found it, once by an
+independent agent whose brief was to refute it. D1, D3, D4, D2 and D12 were additionally
+reproduced by hand, outside the agent framework; the commands are in this report.
+
+A twelfth candidate class -- `pg_dump` reproducing the element order of array-valued
+catalog columns (`aclitem[]`, `setconfig`, `reloptions`) -- was **rejected**, and the
+argument that killed it is worth reading: see [Considered and rejected](#considered-and-rejected).
+
+---
+
+## Method
+
+Three oracles, in increasing order of reach.
+
+**1. The stock assert-enabled build.** `DOTypeNameCompare()`'s fall-through is
+`Assert(false)`, so on an assert build a tie makes `pg_dump` abort. This is the oracle
+that matters most, because it is exactly what a developer or a buildfarm animal sees, and
+it needs no instrumentation.
+
+**2. A tie reporter.** An audit-only build (`ss-shuf-inst`) whose
+`sortDumpableObjectsByTypeName()` walks the sorted array afterwards and reports every
+adjacent pair for which the comparator reached the fall-through, with
+`describeDumpableObject()` output for both. Ties are necessarily adjacent after a sort, so
+one run enumerates *all* of them rather than aborting at the first. The `Assert` is
+disarmed in that build, which also makes it a faithful stand-in for a production build:
+with no environment variables set it falls through to `oidcmp()` exactly as a non-assert
+`pg_dump` does.
+
+**3. A pre-sort shuffle.** The same build permutes the object array before sorting when
+`PGDUMP_SHUFFLE_SEED` is set, and makes the fall-through return 0 rather than comparing
+OIDs, so a tie leaves the order genuinely up to `qsort`. Eight seeds, then diff. Dump
+output must not depend on the input permutation; if it does, the order is unstable --
+whatever the cause, including causes that never reach the comparator.
+
+Calibration: all three fire on the DO_POLICY case (D1) and all three are silent on the
+core regression database (2291 relations) and on control schemas. The shuffle oracle is
+blind to the D3..D11 class by construction -- those orders come from the *server's* result
+order, which is identical under every seed -- so that class had to be found by reading the
+queries and confirmed by diffing two independently built databases. Where a finding is of
+that kind, the report says so explicitly and gives the two-database pair.
+
+**Fan-out.** A first workflow ran 17 discovery agents -- 8 sweeping the 48
+`DumpableObjectType` values against their catalogs' natural keys, 4 code lenses (the
+topological sort; every dump-time query in `pg_dump.c`; `pg_dumpall.c` and the archive TOC;
+the history of the five commits that already fixed this class), and 5 empirical lanes
+(cross-schema name collisions for every schema-qualified object type; pg_dump's
+manufactured pseudo-objects; in-tree extensions; the regression corpus; a
+differential two-database generator). Their 54 raw candidates deduplicated to 49, each of
+which got an independent verification agent and, unless refuted, an independent adversarial
+judge told to refute it. 40 survived; those 40 describe **11 distinct mechanisms** --
+the same defect was found by up to 13 agents through different object types. A second
+workflow put one agent on each surviving mechanism to re-reproduce it from scratch, write
+its regression test and write a sample fix, plus two adversarial critics.
+
+**What this method cannot see.** Dump-order instability that requires a server version
+older than this tree (`pg_dump` supports back to 9.2 and builds several queries in
+version-dependent branches; only the modern branch was exercised), instability visible only
+under `pg_restore -j` scheduling, and anything needing a platform this box is not.
+
+---
+
+## Part 1 -- the object sort
+
+### D1. `DO_POLICY`: the RLS-enabled pseudo-object collides with a policy named after its table
+
+`getPolicies()` represents "row level security is enabled on this table" as a `PolicyInfo`
+with `polname == NULL`, and gives it the **table's** name:
+
+```c
+/* src/bin/pg_dump/pg_dump.c:4249 */
+polinfo->dobj.objType = DO_POLICY;
+polinfo->dobj.catId.tableoid = 0;
+polinfo->dobj.catId.oid = tbinfo->dobj.catId.oid;
+AssignDumpId(&polinfo->dobj);
+polinfo->dobj.namespace = tbinfo->dobj.namespace;
+polinfo->dobj.name = pg_strdup(tbinfo->dobj.name); /* <-- borrowed */
+polinfo->poltable = tbinfo;
+polinfo->polname = NULL;
+```
+
+A real policy gets `dobj.name = polname` and the same namespace, and the `DO_POLICY`
+tiebreaker compares only the table name:
+
+```c
+/* src/bin/pg_dump/pg_dump_sort.c */
+else if (obj1->objType == DO_POLICY)
+{
+ /* Sort by table name (table namespace was considered already) */
+ cmpval = strcmp(pobj1->poltable->dobj.name, pobj2->poltable->dobj.name);
+ if (cmpval != 0)
+ return cmpval;
+}
+```
+
+So for a policy whose `polname` equals its own table's `relname`, every step returns 0:
+same priority, same namespace, same name, same objType, same table. This is the same shape
+as the cast/transform defect -- a name that is not the object's own -- but arrived at from
+the other direction: instead of building a name out of other objects' unqualified names, it
+*borrows* one wholesale.
+
+Three statements reproduce it:
+
+```sql
+CREATE TABLE pol_t (i int);
+ALTER TABLE pol_t ENABLE ROW LEVEL SECURITY;
+CREATE POLICY pol_t ON pol_t USING (true);
+```
+
+```
+$ /home/nm/src/pg/ssrun/pgrun.sh plain t_policy.sql >/dev/null
+pg_dump: pg_dump_sort.c:511: DOTypeNameCompare: Assertion `0' failed.
+PG_DUMP FAILED (exit 134)
+ASSERTION FAILURE
+
+$ /home/nm/src/pg/ssrun/pgrun.sh tie t_policy.sql >/dev/null
+SORT TIE: objType 41 name "pol_t" nsp "public" | POLICY (ID 3523 OID 16384) | POLICY (ID 3524 OID 16387)
+TIES DETECTED
+```
+
+**On an assert-enabled build the database is simply not dumpable.** On a production build
+the tie falls through to `oidcmp()`, comparing the *table's* OID (the pseudo-object carries
+`catId.oid = table oid`, `tableoid = 0`) against the *pg_policy* OID. In a
+normally-built database the policy always postdates its table, so the order is stable by
+luck. It stops being stable exactly where `pg_upgrade` operates: relation OIDs are
+preserved across an upgrade, `pg_policy` OIDs are not. Give the table a high OID in the
+old cluster and the restored policy gets a low one, and the two logically identical
+databases dump in opposite orders:
+
+```
+== old cluster OIDs == == new cluster OIDs ==
+ table | 18784 table | 18784 (preserved)
+ policy | 18787 policy | 16384 (reassigned)
+
+$ diff -u old.dump new.dump
+--- Name: rls_demo; Type: ROW SECURITY; Schema: public; Owner: postgres
++-- Name: rls_demo rls_demo; Type: POLICY; Schema: public; Owner: postgres
+-ALTER TABLE public.rls_demo ENABLE ROW LEVEL SECURITY;
++CREATE POLICY rls_demo ON public.rls_demo USING (true);
+```
+
+(`/home/nm/src/pg/ssrun/oidflip.sh` -- it pads the OID counter, takes a
+`pg_dump --binary-upgrade --schema-only`, restores it into a second cluster started with
+`-b`, and dumps both with the assert-disarmed build. That is the flake the comment above
+`Assert(false)` predicts, reproduced deliberately.)
+
+The fix is the missing natural-key column. `pg_policy`'s key is `(polrelid, polname)`; the
+pseudo-object is the one row where `polname` is absent, so comparing "is `polname` NULL"
+after the table name completes the key:
+
+```c
+ /*
+ * The RLS-enabled pseudo-object (polname == NULL) borrows its name from
+ * its table, so it ties with a policy whose polname equals that table
+ * name. Sort the pseudo-object first, consistent with ENABLE ROW LEVEL
+ * SECURITY logically preceding the policies on the table.
+ */
+ if (pobj1->polname == NULL)
+ {
+ if (pobj2->polname != NULL)
+ return -1;
+ }
+ else if (pobj2->polname == NULL)
+ return 1;
+```
+
+Two non-NULL `polname`s on the same table cannot both survive to this point: `polname`
+*is* `dobj.name`, already compared at step 3.
+
+### Why nothing else in the comparator ties
+
+D1 is the only tie the audit found, and that claim was put to a dedicated adversarial
+critic whose brief was to falsify it. The reason the rest of the comparator is sound comes
+down to two observations that are worth recording, because they are what a future reviewer
+needs in order to check a new object type:
+
+1. **Constructed names are now closed.** Only three construction sites build a
+ `dobj.name` out of other names rather than copying a catalog column: `getCasts()` and
+ `getTransforms()` (fixed by `885a841`) and `getLOs()`, whose name is a large-object OID
+ range -- and a large object's OID *is* its identity, so that one is not a defect.
+2. **Borrowed names are safe wherever the borrower has its priority to itself.** Twelve
+ object types take their name from another object -- `DO_TABLE_ATTACH`,
+ `DO_INDEX_ATTACH`, `DO_ATTRDEF`, `DO_TABLE_DATA`, `DO_SEQUENCE_SET`,
+ `DO_REFRESH_MATVIEW`, `DO_REL_STATS`, `DO_SHELL_TYPE`, `DO_DUMMY_TYPE`,
+ `DO_PUBLICATION_REL`, `DO_PUBLICATION_TABLE_IN_SCHEMA`, `DO_SUBSCRIPTION_REL` -- and
+ every one of them is either alone at its priority level or separated from its
+ priority-mate by the `objType` comparison, and each has at most one instance per
+ borrowed-from object. `DO_POLICY` is the single case where a borrowed-name
+ pseudo-object shares both a priority *and* an `objType` with a genuinely named object.
+
+Two near misses are worth a note rather than a change, and both were refuted with
+structural arguments rather than merely not reproduced:
+
+* `DO_INDEX` takes its namespace from its table rather than from `pg_class.relnamespace`,
+ and has no tiebreaker. Today an index's `relnamespace` is pinned to its table's, so
+ `(namespace, name)` still reduces to `pg_class_relname_nsp_index`; the sort key is one
+ line narrower than the natural key, but nothing can exploit it.
+* the pseudo-objects built with `catId.tableoid = 0, catId.oid = 0`
+ (`DO_TABLE_ATTACH`, `DO_INDEX_ATTACH`, `DO_REL_STATS`) have no OID for the
+ `oidcmp()` safety net to fall back on, so if a future change did introduce a tie among
+ them, the fall-through would return 0 and the order would be pure `qsort` luck rather
+ than merely OID-dependent.
+
+---
+
+## Part 2 -- dependency-loop repair
+
+### D2. `TopoSort()` reports its failures in dumpId order, so loop repair follows OID assignment
+
+When the dependency graph has a cycle, `TopoSort()` fails and hands the objects it could
+not place to `findDependencyLoops()`, which finds a cycle and calls
+`repairDependencyLoop()` to break it -- by marking one object `separate`, so that (for
+example) a `CHECK` constraint moves out of `CREATE TABLE` into a post-data
+`ALTER TABLE ... ADD CONSTRAINT`, or one view of a mutually-recursive pair is emitted as a
+dummy `SELECT NULL::...` placeholder and rebuilt later with `CREATE OR REPLACE VIEW`.
+
+Which object gets chosen is decided by OID assignment order, not by name. Three links:
+
+```c
+/* pg_dump_sort.c:757 -- the failure list is rebuilt in dumpId order, discarding
+ * the name-sorted order the caller passed in */
+k = 0;
+for (j = 1; j <= maxDumpId; j++)
+{
+ if (beforeConstraints[j] != 0)
+ ordering[k++] = objs[idMap[j]];
+}
+```
+
+`findDependencyLoops()` then walks that array front to back, so `loop[0]` is the
+lowest-dumpId cycle member; and `repairDependencyLoop()`'s multi-object branches scan
+`loop[]` front to back and repair the *first* member of the type they are looking for.
+dumpIds are handed out by `AssignDumpId()` in catalog-scan order, and the scans are
+OID-ordered (`getTables()` ends `ORDER BY c.oid`; `getTypes()` and `getFuncs()` have no
+`ORDER BY` at all, so heap order). The whole repair decision therefore rides on which
+object was created first.
+
+Four statements, differing only in which of two domains is created first:
+
+```sql
+-- A -- B
+CREATE DOMAIN d1 AS int; CREATE DOMAIN d2 AS int;
+CREATE DOMAIN d2 AS int; CREATE DOMAIN d1 AS int;
+ALTER DOMAIN d1 ADD CONSTRAINT c1 CHECK ((CAST(VALUE AS int)::d2) IS NOT NULL);
+ALTER DOMAIN d2 ADD CONSTRAINT c2 CHECK ((CAST(VALUE AS int)::d1) IS NOT NULL);
+```
+
+```
+$ diff -u a.dump b.dump
+-CREATE DOMAIN public.d2 AS integer
+- CONSTRAINT c2 CHECK (((VALUE)::public.d1 IS NOT NULL));
++CREATE DOMAIN public.d1 AS integer
++ CONSTRAINT c1 CHECK (((VALUE)::public.d2 IS NOT NULL));
+-ALTER DOMAIN public.d1
+- ADD CONSTRAINT c1 CHECK (((VALUE)::public.d2 IS NOT NULL));
++ALTER DOMAIN public.d2
++ ADD CONSTRAINT c2 CHECK (((VALUE)::public.d1 IS NOT NULL));
+```
+
+A puts `c1` in a separate `ALTER DOMAIN` and inlines `c2`; B does the opposite. No
+assertion fires; the divergence is silent. The verification agent checked that the two
+databases are logically identical by projecting the whole catalog -- including the entire
+`pg_depend` graph with every OID rendered as `regclass`/`regprocedure`/`regtype` -- and
+diffing: no output. The same instability was demonstrated through four different repair
+paths (a table `CHECK` constraint via `BEGIN ATOMIC` functions, a domain `CHECK`
+constraint, the dummy-view choice in a view/rule cycle, and which column `DEFAULT` is split
+into a separate `ALTER TABLE ... SET DEFAULT`), and in one variant the dump flipped with
+*every OID identical* -- so dumpId order, not OID order as such, is the real input.
+
+**No sample fix is proposed for D2 and no test is committed for it.** The natural fix has
+two parts -- make `TopoSort()`'s failure list inherit the caller's name-sorted order, and
+make `repairDependencyLoop()` pick the minimum by natural key rather than the first in
+`loop[]` order -- and both change which object gets broken out in existing cases, i.e. they
+change dump output for databases that dump fine today. That is a judgement call about
+`pg_dump`'s output, not a mechanical key completion, so it is written up here and left to
+you. A test pinned to today's choice would only entrench the OID dependence; a test
+pinned to the fixed choice presumes the fix.
+
+---
+
+## Part 3 -- dump-time queries whose result order reaches the output
+
+Nine of the eleven findings are of one shape: a query whose rows are pasted into the dump
+in result order, ordered by less than a key -- or not ordered at all. None of them reaches
+`DOTypeNameCompare()`, so the tie and shuffle oracles are silent on all nine; each was
+established by reading the query, checking the plan, and diffing two independently built
+databases. They are listed worst first.
+
+### D3. `getInherits()` never reads `inhseqno`, and this permutes columns on restore
+
+```c
+/* src/bin/pg_dump/pg_dump.c:7696 */
+appendPQExpBufferStr(query, "SELECT inhrelid, inhparent FROM pg_inherits");
+```
+
+No `ORDER BY`, and `pg_dump` reads `inhseqno` **nowhere** -- `grep -rn inhseqno
+src/bin/pg_dump/` returns nothing. `flagInhTables()` appends parents in `PGresult` order
+(`common.c:323`) and nothing re-sorts, so the `INHERITS (...)` list at `pg_dump.c:17454`
+and the `--binary-upgrade` `ALTER TABLE ONLY ... INHERIT` at `pg_dump.c:17764` both follow
+`pg_inherits` **heap** order. `pg_inherits`'s natural key is `(inhrelid, inhseqno)`.
+
+Heap order diverges from `inhseqno` order as soon as a line pointer is reused, and it also
+just differs with creation order when other children's rows are interleaved. Seven
+statements, all ordinary DDL:
+
+```sql
+CREATE TABLE p1 (a int);
+CREATE TABLE p2 (b int);
+CREATE TABLE decoy () INHERITS (p1);
+CREATE TABLE ch (b int) INHERITS (p1);
+DROP TABLE decoy;
+VACUUM pg_inherits;
+ALTER TABLE ch INHERIT p2;
+```
+
+The catalog now says the parent order is `p1` then `p2`, and `ch`'s columns are `(a, b)`
+accordingly, but the two rows sit in the heap the other way round:
+
+```
+ ctid | inhparent | inhseqno attnum | attname
+-------+-----------+---------- --------+---------
+ (0,1) | p2 | 2 1 | a
+ (0,2) | p1 | 1 2 | b
+```
+
+and `pg_dump` emits the heap order:
+
+```sql
+CREATE TABLE public.ch (
+ b integer
+)
+INHERITS (public.p2, public.p1);
+```
+
+Restoring that gives `ch` the columns of `p2` first. **The column order changes:**
+
+```
+== ORIGINAL ch columns: == RESTORED ch columns:
+ 1 | a 1 | b
+ 2 | b 2 | a
+```
+
+This is not a spurious-schema-diff problem. A restored database in which a table's columns
+have swapped positions breaks `SELECT *`, `INSERT` without a column list, and every client
+that binds by position -- silently, with no error anywhere in the dump or the restore.
+(`pg_dump`'s own `COPY` statements carry explicit column lists, so the *data* lands in the
+right columns; it is the schema that moves.) Ordering the query by `(inhrelid, inhseqno)`
+fixes both the instability and the wrong restore, and needs no other change because
+`flagInhTables()` preserves `PGresult` order.
+
+### D4. `dumpOpfamily()` and `dumpOpclass()` order members by strategy alone
+
+```c
+/* pg_dump.c, dumpOpfamily(): both member queries */
+... "ORDER BY amopstrategy", /* pg_amop */
+... "ORDER BY amprocnum", /* pg_amproc */
+```
+
+`pg_amop`'s key is `(amopfamily, amoplefttype, amoprighttype, amopstrategy)` and
+`pg_amproc`'s is `(amprocfamily, amproclefttype, amprocrighttype, amprocnum)`. Within one
+family, every cross-type member pair shares a strategy number, so the sort key is not a
+key at all and the remaining order is the executor's -- which the judge traced to an index
+scan on `pg_depend`, i.e. ascending member OID. Adding the same two support functions in
+the opposite order permanently changes the dump:
+
+```sql
+CREATE OPERATOR FAMILY myfam USING btree;
+ALTER OPERATOR FAMILY myfam USING btree ADD FUNCTION 1 btint4cmp(int4, int4);
+ALTER OPERATOR FAMILY myfam USING btree ADD FUNCTION 1 btint8cmp(int8, int8);
+-- versus the same two ADDs in the opposite order
+```
+
+```
+$ diff -u a.dump b.dump
+ ALTER OPERATOR FAMILY public.myfam USING btree ADD
+- FUNCTION 1 (integer, integer) btint4cmp(integer,integer) ,
+- FUNCTION 1 (bigint, bigint) btint8cmp(bigint,bigint);
++ FUNCTION 1 (bigint, bigint) btint8cmp(bigint,bigint) ,
++ FUNCTION 1 (integer, integer) btint4cmp(integer,integer);
+```
+
+Deterministic, and it reproduces on every run. The `pg_amop` half of the same query pair
+has the identical missing key columns; the audit could not make the operator list flip
+(the plan it gets happens to be insensitive to insertion order), so that half is reported
+as latent rather than demonstrated. `dumpOpclass()`'s two queries are also latent for a
+different reason: only members whose left and right types both equal `opcintype` depend on
+the opclass rather than the family, so today at most one member per strategy reaches them
+-- access methods without an `amadjustmembers` hook are where that could stop holding.
+
+### D5. `getPolicies()` builds the `TO` role list with an unordered sub-select
+
+```c
+/* pg_dump.c:4278 */
+"CASE WHEN pol.polroles = '{0}' THEN NULL ELSE "
+" pg_catalog.array_to_string(ARRAY(SELECT pg_catalog.quote_ident(rolname) "
+" from pg_catalog.pg_roles "
+" WHERE oid = ANY(pol.polroles)), ', ') END AS polroles, "
+```
+
+The `ARRAY()` sub-select has no `ORDER BY` and plans as a seq scan on `pg_authid`, so the
+list follows role **creation** order -- neither the stored `polroles` array order (which
+`policy_role_list_to_array()` preserves from the `CREATE POLICY` text) nor `rolname` order.
+Two databases whose roles were created in the opposite order dump
+`CREATE POLICY p ON t TO alice, bob` versus `... TO bob, alice`. Ordering by each OID's
+position within `pol.polroles` (`unnest ... WITH ORDINALITY`) both stabilises it and makes
+the clause a faithful round trip of what the user wrote.
+
+### D6. `getPublications()` does not order the `FOR ALL TABLES EXCEPT` list
+
+```c
+/* pg_dump.c:4598, per publication, remoteVersion >= 190000 */
+"SELECT prrelid\n"
+"FROM pg_catalog.pg_publication_rel\n"
+"WHERE prpubid = %u AND prexcept"
+```
+
+The rows go into a `SimplePtrList` in arrival order and `dumpPublication()` walks it
+verbatim, so `CREATE PUBLICATION p FOR ALL TABLES EXCEPT (TABLE ONLY a, TABLE ONLY b)`
+follows `pg_publication_rel` heap order -- i.e. the order the tables were listed when the
+publication was created. Three statements per database reproduce it. This one is new
+code (v19), which makes it the cheapest of the nine to fix before it ships in a release.
+
+### D7. `dumpDatabaseConfig()` does not order per-role database settings
+
+```c
+/* pg_dump.c:3764 */
+"SELECT rolname, unnest(setconfig) FROM pg_db_role_setting s, pg_roles r "
+"WHERE setrole = r.oid AND setdatabase = '%u'::oid"
+```
+
+One row per `(role, database)`, no `ORDER BY`, and the plan seq-scans `pg_authid` on the
+probe side, so the `ALTER ROLE ... IN DATABASE ... SET` lines in a `--create` preamble come
+out in role-OID order. `ORDER BY 1` (`rolname`) is a complete key here, and the
+verification agent checked the one thing that could have gone wrong -- that sorting above
+the set-returning `unnest` does not permute the settings *within* a role -- by confirming
+the planner puts the sort below the `ProjectSet`.
+
+### D8. `append_depends_on_extension()` does not order its rows
+
+The query behind `ALTER ... DEPENDS ON EXTENSION` (`pg_dump.c:5702`) has no `ORDER BY`, so
+an object with two extension dependencies emits them in `pg_depend` row order -- the order
+the `ALTER ... DEPENDS ON EXTENSION` statements happened to run, and it changes if one is
+dropped and re-added. Affects every caller (`dumpFunc()`, `dumpTrigger()`, index and
+materialized-view paths). `ORDER BY 1` on the extension name is a complete key.
+
+### D9. `collectSecLabels()` omits `provider` from its `ORDER BY`
+
+```c
+/* pg_dump.c:16755 */
+"SELECT label, provider, classoid, objoid, objsubid "
+"FROM pg_catalog.pg_seclabels ORDER BY classoid, objoid, objsubid"
+```
+
+`pg_seclabel`'s key is `(objoid, classoid, objsubid, provider)`. Two providers labelling
+one object produce two rows that tie under that `ORDER BY`, and the server's sort is not
+stable, so the two `SECURITY LABEL FOR ...` statements come out in an order the catalog
+does not determine. `collectSecLabels()` is one of two sites; the shared-object path in `dumputils.c` has the
+same gap. Reaching it needs two registered label providers, which no in-tree module
+supplied, so the committed test adds a second provider to
+`src/test/modules/dummy_seclabel`, whose whole purpose is exercising this machinery.
+
+### D10. `pg_dumpall`'s `dumpTablespaces()` orders by OID
+
+```c
+/* pg_dumpall.c:1368 */
+"SELECT oid, spcname, ... FROM pg_catalog.pg_tablespace "
+"WHERE spcname !~ '^pg_' "
+"ORDER BY 1" /* select-list column 1 is oid */
+```
+
+Select-list column 1 is `oid`, so the whole per-tablespace block -- `CREATE TABLESPACE`,
+`ALTER TABLESPACE ... SET`, the ACL commands, `COMMENT`, `SECURITY LABEL` -- is emitted in
+OID order. That this is an off-by-one rather than intent is clear from the sibling
+`dumpRoles()` at `pg_dumpall.c:855`, which has the identical select-list shape
+(`SELECT oid, rolname, ...`) and says `ORDER BY 2`. `spcname` alone is a complete key
+(`pg_tablespace_spcname_index` is unique and `pg_tablespace` has no namespace).
+
+### D11. `dumpExtension()` emits the `requires` array in dependency-array order
+
+Under `--binary-upgrade`, `dumpExtension()` builds the seventh argument of
+`binary_upgrade_create_empty_extension()` by walking `extinfo->dobj.dependencies[]` and
+printing each `DO_EXTENSION` it finds (`pg_dump.c:11992`). Nothing ever sorts a
+`dependencies[]` array: `getDependencies()` ends `ORDER BY 1,2` -- `(classid, objid)`, with
+`refobjid` absent -- so all of one extension's requires-rows tie and arrive in scan order,
+which the backend wrote in *descending referenced-OID* order
+(`eliminate_duplicate_dependencies()` → `object_address_comparator()`, "Primary sort
+key is OID descending"). Two statements per database:
+
+```sql
+CREATE EXTENSION plperl; CREATE EXTENSION hstore_plperl CASCADE; -- ARRAY['hstore','plperl']
+CREATE EXTENSION hstore; CREATE EXTENSION hstore_plperl CASCADE; -- ARRAY['plperl','hstore']
+```
+
+Only four in-tree control files list more than one `requires` entry
+(`hstore_plperl`, `hstore_plperlu`, `hstore_plpython3u`, `ltree_plpython3u`), so the
+reachable surface is narrow, but this is on the `pg_upgrade` path, which is where the
+dump-comparison test lives.
+
+---
+
+## Considered and rejected
+
+### Array-valued catalog columns reproduced verbatim -- *mostly* not a defect
+
+This class was put to a dedicated adjudicator after the first workflow's judges split
+three-to-one on it. Its verdict: the two demonstrated cases are **not** defects, but the
+sweep had stopped one array short, and that one **is** -- see D12 below.
+
+#### The two demonstrated cases
+
+Several catalog columns are arrays whose element order is an artifact of the order the DDL
+was issued, and `pg_dump` reproduces that order. The audit demonstrated it twice:
+
+```sql
+GRANT SELECT ON acl_t TO r_aaa; GRANT SELECT ON acl_t TO r_bbb;
+-- versus the same two GRANTs in the opposite order
+```
+
+```
+=== A === === B ===
+GRANT SELECT ON TABLE public.acl_t TO r_aaa; GRANT SELECT ON TABLE public.acl_t TO r_bbb;
+GRANT SELECT ON TABLE public.acl_t TO r_bbb; GRANT SELECT ON TABLE public.acl_t TO r_aaa;
+```
+
+and the same for `pg_db_role_setting.setconfig` under `ALTER DATABASE ... SET`. Four
+agents split three-to-one on whether this belongs in the findings list. It does not, for
+three reasons, the third of which is decisive:
+
+1. **It inverts the defect definition.** The other findings are: identical catalog
+ content, different OIDs, different output. This is: *different* catalog content
+ (`relacl` genuinely holds a different array value), identical OIDs, different output.
+ `pg_dump` is reporting the catalog, not choosing an order.
+2. **It is a fixed point.** Dump, restore, dump again: the second dump equals the first.
+ None of the harms that motivate this class occur -- no `Assert`, no tie, no
+ `002_pg_upgrade.pl` mismatch.
+3. **`buildACLCommands()`'s order is load-bearing.** With a `WITH GRANT OPTION` chain, a
+ grant must be replayed after the grant that authorised it. A naive sort of the aclitem
+ list would produce a dump that **fails to restore**. Whatever is done here cannot be a
+ plain sort.
+
+The same argument covers every `*acl` column `pg_dump` feeds to `buildACLCommands()`
+(`relacl`, `typacl`, `proacl`, `nspacl`, `defaclacl`, `lanacl`, `fdwacl`, `srvacl`,
+`datacl`, `spcacl`, `lomacl`, parameter ACLs in `pg_dumpall`) and column-level ACLs, and it
+covers `reloptions`, `proconfig` and `attoptions` for reason 1 alone. Recorded here so it
+is not re-proposed.
+
+### Checked and found clean
+
+* **The core regression database** (2291 relations) -- no ties, and byte-identical dump
+ output across eight pre-sort shuffles. Also clean under `--with-statistics`,
+ `--no-owner`, `--no-privileges`, `--section=*`, `--schema-only`, `--data-only` and
+ `--binary-upgrade`.
+* **`TopoSort()` itself** -- given a fixed input order and a fixed dependency graph, its
+ output is deterministic; the binary heap is keyed on the input index. The instability in
+ D2 is in what feeds it on failure, not in the sort.
+* **The archive TOC** -- `-Fc` TOC order and single-threaded `pg_restore -f -` output
+ follow the same sorted list as the plain dump. (`pg_restore -j` deliberately does not,
+ as the comment above `Assert(false)` already says.)
+* **The rest of `pg_dumpall.c`** -- roles, role memberships, role GUC settings, databases
+ and subscriptions are all ordered by name; `dumpTablespaces()` (D10) is the only one that
+ is not.
+* **`getDependencies()`'s `ORDER BY 1,2`** -- incomplete as a key, but the only place a
+ `dependencies[]` array's order reaches the output is D11.
+* **The comparator's helper functions** -- `pgTypeNameCompare()` compares
+ `(nspname, typname)`, `accessMethodNameCompare()` compares `amname`; both are complete
+ for their catalogs, and both handle the not-found case by returning "equal" so the caller
+ falls through to its next basis for comparison.
+* **Comments** -- `collectComments()` orders by `(classoid, objoid, objsubid)`, which is
+ `pg_description`'s whole key; only the security-label sibling (D9) has a fourth key
+ column.
+
+---
+
+## Tests and sample fixes on this branch
+
+Eleven of the twelve findings have both a regression test and a sample fix. D2 has
+neither, for the reason given in Part 2.
+
+| # | Test | Sample fix |
+|---|---|---|
+| D1 | `002_pg_dump.pl`, policy named after its own table | `pg_dump_sort.c`: compare `polname == NULL` after the table name |
+| D3 | `002_pg_dump.pl`, `inh_order_child` | `pg_dump.c`: `ORDER BY inhrelid, inhseqno` |
+| D4 | `002_pg_dump.pl`, `op_family` | `pg_dump.c`: add the member type names to all four member queries |
+| D5 | `002_pg_dump.pl`, policy `p7` with a multi-role `TO` list | `pg_dump.c`: `unnest(polroles) WITH ORDINALITY` |
+| D6 | `002_pg_dump.pl`, publications `pub9`/`pub10` | `pg_dump.c`: `ORDER BY n.nspname, c.relname` |
+| D7 | `002_pg_dump.pl`, `ALTER ROLE ... IN DATABASE` | `pg_dump.c`: `ORDER BY rolname` |
+| D8 | `test_pg_dump/t/001_base.pl` | `pg_dump.c`: `ORDER BY e.extname` |
+| D9 | `003_pg_dump_with_server.pl` (+ a second provider in `dummy_seclabel`) | `pg_dump.c`, `dumputils.c`: add `provider` to both `ORDER BY`s |
+| D10 | `002_pg_dump.pl`, `CREATE TABLESPACE in name order` | `pg_dumpall.c`: `ORDER BY 1` → `ORDER BY 2` |
+| D11 | `003_pg_dump_with_server.pl` | `pg_dump.c`: sort the requires names with `pg_qsort_strcmp` |
+| D12 | `002_pg_dump.pl`, `ALTER DEFAULT PRIVILEGES grantees ... in name order` | `pg_dump.c`: re-sort `defaclacl` by aclitem text under `COLLATE "C"` |
+
+**The sample fixes are not proposed patches.** They exist so the branch is coherent -- the
+tests need something to pass against -- and so that "this test fails without the fix" is a
+statement someone can check. They are in their own commit and can be dropped wholesale.
+Four of them involve a judgement a committer should make rather than accept:
+
+* **D5** could instead be `ORDER BY rolname`. The committed fix preserves the order the
+ user wrote in `CREATE POLICY`, which round-trips; alphabetical order would be simpler but
+ would rewrite the clause. Both remove the OID dependence.
+* **D4** orders by the members' type names. Ordering by `regtype` output would have been
+ shorter, but that rendering depends on `search_path`, so the fix joins `pg_type` and
+ `pg_namespace` and orders by `(nspname, typname)` -- the same key
+ `pgTypeNameCompare()` uses.
+* **D1** sorts the RLS-enable pseudo-object *before* the policies on its table. Either
+ order is stable; this one matches `ENABLE ROW LEVEL SECURITY` logically preceding them.
+* **D12** sorts an ACL array, which the sibling `relacl` case shows can be unsafe. The
+ argument that it is safe *here* -- a default ACL's items all share one grantor, so there
+ is no grant chain to replay in order -- is the whole basis of the fix, and is the thing
+ to check before accepting it.
+
+Two findings needed test infrastructure rather than just a test entry. D8 lives in
+`src/test/modules/test_pg_dump` because showing it needs one object with **two** extension
+dependencies, and a bare `initdb` has exactly one extension (`plpgsql`); `src/bin/pg_dump`'s
+test install does not build contrib, so a test in `002_pg_dump.pl` would have to make the
+core pg_dump suite depend on contrib. `test_pg_dump` already installs its own extension
+and already owns the only existing `DEPENDS ON EXTENSION` coverage. D11 sidesteps the same
+problem differently: its test writes three throwaway control files into the test's temp
+directory and points `extension_control_path` at them, so it needs no contrib at all.
+
+## Verification
+
+Three runs of `meson test --suite setup --suite pg_dump --suite test_pg_dump --suite
+dummy_seclabel`, on this branch, in this order.
+
+**1. Everything applied: 13/13 pass**, including `002_pg_dump` with 13697 subtests.
+
+**2. All five product files reverted, tests kept: 3 suites fail.** `002_pg_dump` dies
+early:
+
+```
+# pg_dump: ../ss-audit/src/bin/pg_dump/pg_dump_sort.c:511: DOTypeNameCompare: Assertion `0' failed.
+# Failed test 'binary_upgrade: pg_dump runs'
+```
+
+That is D1 doing what it should -- and it is also why this run alone is not enough: the
+abort kills the dump before the ordering tests can be evaluated.
+
+**3. Only D1's fix applied, the other ten reverted: 3 suites fail, each test by its own
+name.** `002_pg_dump` now runs to completion and fails on exactly the new entries:
+
+```
+should dump CREATE TABLE inh_order_child (D3)
+should dump CREATE TABLE inh_order_child pg_upgrade (D3)
+should dump ALTER OPERATOR FAMILY dump_test.op_family USING btree (D4)
+should dump CREATE POLICY p7 ON test_table with a multi-role TO list (D5)
+should dump CREATE PUBLICATION pub9 / pub10 (D6)
+should dump ALTER ROLE ... IN DATABASE postgres SET, in role name order (D7)
+should dump CREATE TABLESPACE in name order (D10)
+should dump ALTER DEFAULT PRIVILEGES grantees are dumped in name order (D12)
+```
+
+`003_pg_dump_with_server` reports "failed 3 tests of 12" (D9 and D11), and
+`test_pg_dump/001_base` fails (D8). Every committed test fails for its own reason on the
+unfixed tree.
+
+Separately, each finding was re-checked outside the TAP suite by building the two databases
+the report describes and diffing the dumps with the unfixed and the fixed binary. All of
+D4, D5, D6, D7, D8, D11 and D12 go from UNSTABLE to STABLE; D1 stops aborting; D3 emits
+`INHERITS (public.p1, public.p2)` and restores the child with its columns in the original
+order; D10 emits the tablespaces in name order.
+
+**What is still unverified.** The completeness critic that examined the D1 claim -- 48
+object types, 55 construction sites, five SQL corpora each under 17 pg_dump option sets,
+all 59 contrib extensions, plus the regression database -- returned "claim holds", and
+named what it could not reach: cross-version dumps (pg_dump's older-server query branches),
+`DO_SUBSCRIPTION_REL`, multi-encoding collations, and catalog corruption. D2 is reported
+without a fix or a test by choice. Nothing else on this branch is unverified.
+
+### D12. `getDefaultACLs()`: `defaclacl` is emitted in grantee-OID order
+
+This one came out of the adjudication above, not out of discovery: the agent sent to settle
+whether array order is ever a defect reproduced both demonstrated cases, agreed they are
+not, and then checked the arrays the sweep had not. `pg_default_acl.defaclacl` is a
+different animal:
+
+```sql
+CREATE ROLE r_aaa; CREATE ROLE r_bbb; -- database A
+ALTER DEFAULT PRIVILEGES GRANT SELECT ON TABLES TO r_aaa;
+ALTER DEFAULT PRIVILEGES GRANT SELECT ON TABLES TO r_bbb;
+-- database B: identical, only the two CREATE ROLE lines swapped
+```
+
+```
+D12-defaclacl/OLD: UNSTABLE
+ -ALTER DEFAULT PRIVILEGES FOR ROLE postgres GRANT SELECT ON TABLES TO r_aaa;
+ +ALTER DEFAULT PRIVILEGES FOR ROLE postgres GRANT SELECT ON TABLES TO r_aaa;
+D12-defaclacl/NEW: STABLE (dumps identical)
+```
+
+The reason it is a defect where `relacl` is not: the backend **throws the DDL order away**.
+`ExecGrant_Default_Acl()` canonicalizes the array with `aclitemsort()`, which orders by
+grantee OID. So the stored order is not "what the user wrote", it is a function of role
+OIDs -- and a restore into a cluster that assigns different role OIDs produces a different
+canonical order. That also removes the objection that killed the `relacl` case: a default
+ACL cannot contain a chain of grants by different grantors (every item's grantor is
+`defaclrole`), so `buildACLCommands()`'s load-bearing replay order does not apply and
+sorting is safe.
+
+Sorting by the aclitem's text under `COLLATE "C"` in `getDefaultACLs()` fixes it.
+
+The same adjudicator's negative results are worth as much as the finding, and are why the
+`relacl` and `setconfig` cases stay rejected: it fuzzed 160 tables, 20 functions, 10 schemas
+and 10 types with random `GRANT`/`REVOKE` histories, non-owner grantors, `PUBLIC`, column
+privileges and three grant-option holders, then dumped, restored and re-dumped -- byte
+identical. It also ran a real `pg_upgrade` and confirmed that although the catalog arrays
+*are* rewritten, `pg_dump` already normalizes around it (it drops items matching
+`acldefault` and hoists owner self-grants into `firstsql`), so the dump comparison passes.
+And it confirmed that `relacl` order really is load-bearing, by replaying a grant chain in
+grantee-name order and getting `ERROR: permission denied for table t5`.
+
+---
+
+## Appendix A -- the instrumented build
+
+Applied to `sortDumpableObjectsByTypeName()` in `pg_dump_sort.c` for the audit build only;
+never committed.
+
+```c
+ /* PGDUMP_SHUFFLE_SEED=N: permute the array before sorting. */
+ {
+ const char *seedstr = getenv("PGDUMP_SHUFFLE_SEED");
+
+ instr_tie_zero = (getenv("PGDUMP_TIE_ZERO") != NULL);
+ if (seedstr != NULL && numObjs > 1)
+ {
+ srand((unsigned int) atoi(seedstr));
+ for (int i = numObjs - 1; i > 0; i--)
+ {
+ int j = rand() % (i + 1);
+ DumpableObject *tmp = objs[i];
+
+ objs[i] = objs[j];
+ objs[j] = tmp;
+ }
+ }
+ }
+
+ if (numObjs > 1)
+ qsort(objs, numObjs, sizeof(DumpableObject *), DOTypeNameCompare);
+
+ /* PGDUMP_TIE_REPORT=1: report every adjacent pair that reached the
+ * comparator's fall-through. Ties are adjacent after a sort, so this
+ * enumerates all of them. */
+ if (getenv("PGDUMP_TIE_REPORT") != NULL)
+ {
+ for (int i = 1; i < numObjs; i++)
+ {
+ instr_tie_fallthrough = false;
+ DOTypeNameCompare(&objs[i - 1], &objs[i]);
+ if (instr_tie_fallthrough)
+ {
+ char buf1[512], buf2[512];
+
+ describeDumpableObject(objs[i - 1], buf1, sizeof(buf1));
+ describeDumpableObject(objs[i], buf2, sizeof(buf2));
+ fprintf(stderr, "SORT TIE: objType %d name \"%s\" nsp \"%s\" | %s | %s\n",
+ (int) objs[i]->objType, objs[i]->name,
+ objs[i]->namespace ? objs[i]->namespace->dobj.name : "(none)",
+ buf1, buf2);
+ }
+ }
+ }
+```
+
+and, in `DOTypeNameCompare()`, the fall-through becomes
+
+```c
+ instr_tie_fallthrough = true;
+ if (instr_tie_zero)
+ return 0;
+ return oidcmp(obj1->catId.oid, obj2->catId.oid);
+```
+
+**The first version of this was wrong and reported nothing**, because disarming
+`Assert(false)` left the fall-through returning `oidcmp()`, so the reporter's
+"did these two compare equal?" test never fired. It was caught only by running the
+detector against a defect already known to be present. A detector that silently finds
+nothing is the failure mode that would have turned this report into "no defects", so
+calibrate any replacement the same way.
+
+## Appendix B -- minimal reproducers
+
+Each is a complete `.sql` for a fresh database. Where a finding is a two-database
+comparison, both variants are given; dump each with the stated options and diff, after
+normalizing pg_dump's random `\restrict` token
+(`sed -E 's/^(\\(un)?restrict) [A-Za-z0-9]+$/\1 XXX/'`).
+
+```sql
+-- D1: assert-enabled pg_dump aborts.
+CREATE TABLE pol_t (i int);
+ALTER TABLE pol_t ENABLE ROW LEVEL SECURITY;
+CREATE POLICY pol_t ON pol_t USING (true);
+
+-- D2: two databases, only the two CREATE DOMAIN lines swapped.
+CREATE DOMAIN d1 AS int;
+CREATE DOMAIN d2 AS int;
+ALTER DOMAIN d1 ADD CONSTRAINT c1 CHECK ((CAST(VALUE AS int)::d2) IS NOT NULL);
+ALTER DOMAIN d2 ADD CONSTRAINT c2 CHECK ((CAST(VALUE AS int)::d1) IS NOT NULL);
+
+-- D3: one database. Dump, restore, and compare ch's column order.
+CREATE TABLE p1 (a int);
+CREATE TABLE p2 (b int);
+CREATE TABLE decoy () INHERITS (p1);
+CREATE TABLE ch (b int) INHERITS (p1);
+DROP TABLE decoy;
+VACUUM pg_inherits;
+ALTER TABLE ch INHERIT p2;
+
+-- D4: two databases, the two ADD FUNCTION lines swapped.
+CREATE OPERATOR FAMILY myfam USING btree;
+ALTER OPERATOR FAMILY myfam USING btree ADD FUNCTION 1 btint4cmp(int4, int4);
+ALTER OPERATOR FAMILY myfam USING btree ADD FUNCTION 1 btint8cmp(int8, int8);
+
+-- D5: two databases, the two CREATE ROLE lines swapped.
+CREATE ROLE alice NOLOGIN; CREATE ROLE bob NOLOGIN;
+CREATE TABLE t (a int);
+CREATE POLICY p ON t TO alice, bob USING (true);
+
+-- D6: two databases, the EXCEPT list written in the opposite order.
+CREATE TABLE ta (x int); CREATE TABLE tb (x int);
+CREATE PUBLICATION p FOR ALL TABLES EXCEPT (TABLE ta, TABLE tb);
+
+-- D7: two databases, the two CREATE ROLE lines swapped. Dump with --create.
+CREATE ROLE ra NOLOGIN; CREATE ROLE rb NOLOGIN;
+ALTER ROLE ra IN DATABASE postgres SET work_mem='5MB';
+ALTER ROLE rb IN DATABASE postgres SET work_mem='6MB';
+
+-- D8: two databases, the two ALTER TRIGGER lines swapped.
+CREATE EXTENSION cube;
+CREATE TABLE t (a int);
+CREATE TRIGGER tg BEFORE UPDATE ON t FOR EACH ROW
+ EXECUTE FUNCTION suppress_redundant_updates_trigger();
+ALTER TRIGGER tg ON t DEPENDS ON EXTENSION plpgsql;
+ALTER TRIGGER tg ON t DEPENDS ON EXTENSION cube;
+
+-- D9: needs two registered label providers; see the committed test, which adds a
+-- second provider to src/test/modules/dummy_seclabel.
+
+-- D10: two clusters, the two CREATE TABLESPACE lines swapped. pg_dumpall --globals-only.
+SET allow_in_place_tablespaces = on;
+CREATE TABLESPACE ts_aaa LOCATION '';
+CREATE TABLESPACE ts_bbb LOCATION '';
+
+-- D11: two databases. Dump with --binary-upgrade.
+CREATE EXTENSION plperl; CREATE EXTENSION hstore_plperl CASCADE; -- variant A
+CREATE EXTENSION hstore; CREATE EXTENSION hstore_plperl CASCADE; -- variant B
+
+-- D12: two databases, the two CREATE ROLE lines swapped.
+CREATE ROLE r_aaa NOLOGIN; CREATE ROLE r_bbb NOLOGIN;
+ALTER DEFAULT PRIVILEGES GRANT SELECT ON TABLES TO r_aaa;
+ALTER DEFAULT PRIVILEGES GRANT SELECT ON TABLES TO r_bbb;
+```
diff --git a/PROVENANCE.md b/PROVENANCE.md
new file mode 100644
index 0000000..2b49ca3
--- /dev/null
+++ b/PROVENANCE.md
@@ -0,0 +1,126 @@
+# PROVENANCE
+
+Branch `dump-sort-stability-tests` is the output of an automated, model-driven audit that
+looked for sources of pg_dump dump-order instability **other than** the one fixed by the
+cast/transform patch the branch's first commit carries. Everything on the branch after
+that first commit -- the report, the regression tests, and the SAMPLE fixes -- was written
+by a language model. **No human wrote any of it, and no human has reviewed it.** This
+file records how it was produced, including what went wrong, so a reviewer can judge the
+work and reproduce every claim in it.
+
+---
+
+## Tooling
+
+* **Tool:** Claude Code (Anthropic's agentic CLI), using its Workflow feature -- a
+ deterministic JavaScript script that drives many subagents.
+* **Model:** Opus 5 (`claude-opus-5`) for the orchestrator and for every subagent.
+* **Run by:** the repository owner (Noah Misch), interactively, from
+ `/home/nm/src/pg/postgresql`.
+* **Date:** 2026-09-03.
+* **Human contribution:** the prompt below, and one mid-run question about whether work was
+ blocked. Nothing in the findings, the tests, the fixes or the report came from a human.
+
+## The prompt
+
+> ~/sort_cast.patch contains a reasonable-looking fix. I'm concerned that yet
+> more sources of dump sort instability are still lurking. Make a worklfow to
+> look for others and, if any found, write test cases covering them. Use your
+> own worktree; disregard the present dir except as repository to which to
+> attach your worktree.
+>
+> Commit the following on a fresh branch:
+> - A report. Prefix the report with [no defects] if that's so.
+> - Any tests written
+> - A PROVENANCE.md file containing model, prompt, etc.
+
+## Base
+
+* Upstream `master` at `6885b84` ("doc: Fix link on pg_dsm_registry_allocations page.").
+* Commit `885a841` is `~/sort_cast.patch` applied verbatim with `git am`. It is Alexander
+ Kukushkin's patch, **not** model-written; it is on the branch because the audit's whole
+ purpose was to find what that patch does not cover. Everything after it is the audit.
+
+## What was built to do the audit
+
+Three artifacts, all outside the branch, under `/home/nm/src/pg/`:
+
+* `ss-audit/` -- the branch worktree. Agents were given it **read-only**.
+* `ss-inst/` -- a stock assert-enabled install of `885a841`. Its `pg_dump` aborts when
+ `DOTypeNameCompare()` reaches `Assert(false)`, which is the audit's primary oracle
+ because it is exactly what a developer or buildfarm animal sees.
+* `ss-instr/` + `ss-shuf-inst/` -- the same commit with audit-only instrumentation in
+ `sortDumpableObjectsByTypeName()`: a pre-sort shuffle under `PGDUMP_SHUFFLE_SEED`, a
+ complete adjacent-pair tie report under `PGDUMP_TIE_REPORT`, and the comparator's
+ `Assert(false)` disarmed (which also makes it a faithful stand-in for a production
+ non-assert build). The instrumentation is reproduced in full in the report's appendix.
+ It was **never** committed to the branch.
+* `ssrun/pgrun.sh` -- runs a `.sql` file in a throwaway cluster and dumps it in `plain`,
+ `tie` or `shuffle` mode. `ssrun/regdata/` -- a prepared core-regression database
+ (243/243 tests passed, 2291 relations) used as the large realistic corpus.
+
+Calibration before any agent ran: all three oracles fire on a known defect and are silent
+on the regression database and on control schemas.
+
+## What the agents did
+
+**Workflow 1 -- discovery** (`wf_7ac43658-438`, 152 agents, 110 completed, 10.0M subagent
+tokens, 8h04m wall clock).
+
+* 17 discovery agents in parallel: 8 sweeping the 48 `DumpableObjectType` values against
+ their catalogs' natural keys, 4 code lenses (the topological sort; every dump-time query
+ in `pg_dump.c`; `pg_dumpall.c` and the archive TOC; the history of the five commits that
+ already fixed this class), and 5 empirical lanes (cross-schema name collisions; pg_dump's
+ manufactured pseudo-objects; in-tree extensions; the regression corpus; a differential
+ two-database generator).
+* 54 raw candidates, deduplicated to 49 by key.
+* Each candidate then got an independent verification agent, and each survivor an
+ independent adversarial judge whose brief was to **refute** it. 40 survived.
+* Those 40 describe **11 distinct mechanisms**; one mechanism was found independently by 13
+ different agents through 13 different object types.
+
+**Workflow 2 -- tests and fixes** (`wf_db8fc3d2-7db`, 12 agents, 12 completed, 1.8M
+subagent tokens): one agent per confirmed mechanism, each required to re-reproduce it from
+scratch before writing its regression test and sample fix, plus two adversarial critics --
+one told to falsify the claim that `DO_POLICY` is the only remaining comparator tie, one to
+settle a finding the first workflow's judges had split on.
+
+The first critic returned **"claim holds"** after enumerating all 48 `DumpableObjectType`
+values and their 55 construction sites and attacking the claim with five SQL corpora, each
+under 17 pg_dump option sets, plus all 59 contrib extensions and the regression database.
+The second **found a twelfth defect** (D12) while refuting the finding it was sent to
+adjudicate: the two array-order cases it was given are not defects, but the sweep had
+stopped one array short of `pg_default_acl.defaclacl`, which is. So the final count is
+**twelve**, not the eleven the discovery workflow produced.
+
+## What went wrong, and what was done about it
+
+* **The Anthropic API returned 529 Overloaded for about an hour.** It killed workflow 1's
+ entire Tests phase and its completeness critic (42 of the 152 agents), then two full
+ launches of workflow 2 (12 agents each, all failing at zero tokens). **No finding was
+ lost** -- discovery, verification and adjudication had all completed -- but no test was
+ written until the third launch of workflow 2 succeeded.
+* **The first tie-detector build was wrong and reported nothing.** Disarming
+ `Assert(false)` left the fall-through returning `oidcmp()`, so the reporter's
+ "did these two compare equal?" test never fired. Caught by running it against a defect
+ known to be present; fixed by having the fall-through set a flag the reporter reads.
+ Recorded here because a silent detector is the failure mode that would have made this
+ whole audit report "no defects".
+* **The shuffle oracle produced false positives** until `pg_dump`'s random `\restrict`
+ token was normalized away before diffing.
+* **The first dedup was too coarse**: 40 confirmed reports collapse to 11 mechanisms, but
+ the agents chose 40 different key strings, so the Tests phase was sized at 42 agents when
+ 10 would do. Consolidation was done by hand between the two workflows.
+* **`002_pg_dump.pl` cannot express every finding.** Which findings got a TAP test, which
+ did not, and why, is stated explicitly in the report -- no finding is quietly dropped.
+
+## How to check the work
+
+Every finding in the report carries the minimal SQL that produces it. For a comparator
+tie, run it under an assert-enabled `pg_dump` and watch the assertion fire. For an
+ordering finding, build the two databases the report gives and diff the dumps. The
+tests on this branch are the same reproducers expressed in `002_pg_dump.pl`; each one
+fails on `885a841` and passes with that finding's sample fix applied.
+
+The audit's own verification of the committed artifacts is described at the end of the
+report, including the result of reverting the sample fixes and re-running the suite.
diff --git a/src/bin/pg_dump/t/002_pg_dump.pl b/src/bin/pg_dump/t/002_pg_dump.pl
index 4b719e7..461463f 100644
--- a/src/bin/pg_dump/t/002_pg_dump.pl
+++ b/src/bin/pg_dump/t/002_pg_dump.pl
@@ -783,6 +783,33 @@ my %tests = (
unlike => { no_privs => 1, },
},
+ # The backend keeps a pg_default_acl entry's ACL array in grantee-OID order
+ # (ExecGrant_Default_Acl canonicalizes it with aclitemsort()), so emitting
+ # the GRANTs in array order would make the dump depend on the order the
+ # grantee roles happened to be created in. Two databases with the same
+ # default privileges must dump alike, and a dump/restore round trip must be
+ # order-stable even though the restore assigns new role OIDs. Create these
+ # roles in the reverse of their name order and require name order out.
+ 'ALTER DEFAULT PRIVILEGES grantees are dumped in name order' => {
+ create_order => 57,
+ create_sql => 'CREATE ROLE regress_dump_defacl_zzz;
+ CREATE ROLE regress_dump_defacl_aaa;
+ ALTER DEFAULT PRIVILEGES
+ FOR ROLE regress_dump_test_role
+ GRANT SELECT ON SEQUENCES
+ TO regress_dump_defacl_zzz, regress_dump_defacl_aaa;',
+ regexp => qr/^
+ \QALTER DEFAULT PRIVILEGES \E
+ \QFOR ROLE regress_dump_test_role \E
+ \QGRANT SELECT ON SEQUENCES TO regress_dump_defacl_aaa;\E\n
+ \QALTER DEFAULT PRIVILEGES \E
+ \QFOR ROLE regress_dump_test_role \E
+ \QGRANT SELECT ON SEQUENCES TO regress_dump_defacl_zzz;\E
+ /xm,
+ like => { %full_runs, section_post_data => 1, },
+ unlike => { no_privs => 1, },
+ },
+
'ALTER DEFAULT PRIVILEGES FOR ROLE regress_dump_test_role REVOKE SELECT'
=> {
create_order => 56,
@@ -815,6 +842,31 @@ my %tests = (
},
},
+ # dumpDatabaseConfig() must emit these in role name order. The roles are
+ # created in the reverse of that order, and the ALTER ROLE statements are
+ # issued in the reverse of that order too, so neither pg_authid OID order
+ # nor pg_db_role_setting heap order can produce the expected output by
+ # accident; only an explicit sort on rolname can.
+ 'ALTER ROLE ... IN DATABASE postgres SET, in role name order' => {
+ create_order => 28,
+ create_sql => '
+ CREATE ROLE regress_dump_role_z;
+ CREATE ROLE regress_dump_role_a;
+ ALTER ROLE regress_dump_role_z IN DATABASE postgres
+ SET work_mem = \'7MB\';
+ ALTER ROLE regress_dump_role_a IN DATABASE postgres
+ SET work_mem = \'6MB\';',
+ regexp => qr/^
+ \QALTER ROLE regress_dump_role_a IN DATABASE postgres SET work_mem TO '6MB';\E\n
+ \QALTER ROLE regress_dump_role_z IN DATABASE postgres SET work_mem TO '7MB';\E
+ /xm,
+
+ # These commands live in the DATABASE PROPERTIES entry, which only
+ # --create emits. pg_dumpall passes --create for other databases, but
+ # not for "postgres" unless --clean is given too.
+ like => { createdb => 1, },
+ },
+
'ALTER COLLATION test0 OWNER TO' => {
regexp => qr/^\QALTER COLLATION public.test0 OWNER TO \E.+;/m,
collation => 1,
@@ -884,10 +936,10 @@ my %tests = (
\QOPERATOR 4 >=(bigint,integer) ,\E\n\s+
\QOPERATOR 5 >(bigint,integer) ,\E\n\s+
\QFUNCTION 1 (integer, integer) btint4cmp(integer,integer) ,\E\n\s+
- \QFUNCTION 2 (bigint, bigint) btint8sortsupport(internal) ,\E\n\s+
\QFUNCTION 2 (integer, integer) btint4sortsupport(internal) ,\E\n\s+
- \QFUNCTION 4 (bigint, bigint) btequalimage(oid) ,\E\n\s+
- \QFUNCTION 4 (integer, integer) btequalimage(oid);\E
+ \QFUNCTION 2 (bigint, bigint) btint8sortsupport(internal) ,\E\n\s+
+ \QFUNCTION 4 (integer, integer) btequalimage(oid) ,\E\n\s+
+ \QFUNCTION 4 (bigint, bigint) btequalimage(oid);\E
/xm,
like =>
{ %full_runs, %dump_test_schema_runs, section_pre_data => 1, },
@@ -2156,6 +2208,30 @@ my %tests = (
},
},
+ # pg_dumpall must emit tablespaces in name order, not in pg_tablespace.oid
+ # order. These two are created in descending name order, so an OID-ordered
+ # dump emits _b before _a.
+ 'CREATE TABLESPACE in name order' => {
+ create_order => 2,
+ create_sql => q(
+ SET allow_in_place_tablespaces = on;
+ CREATE TABLESPACE regress_dump_tablespace_b
+ OWNER regress_dump_test_role LOCATION '';
+ CREATE TABLESPACE regress_dump_tablespace_a
+ OWNER regress_dump_test_role LOCATION ''),
+ regexp => qr/^
+ \QCREATE TABLESPACE regress_dump_tablespace_a OWNER regress_dump_test_role LOCATION '';\E
+ .*?
+ ^\QCREATE TABLESPACE regress_dump_tablespace_b OWNER regress_dump_test_role LOCATION '';\E
+ /xms,
+ like => {
+ pg_dumpall_dbprivs => 1,
+ pg_dumpall_exclude => 1,
+ pg_dumpall_globals => 1,
+ pg_dumpall_globals_clean => 1,
+ },
+ },
+
'CREATE DATABASE regression_invalid...' => {
create_order => 1,
create_sql => q(
@@ -3227,6 +3303,79 @@ my %tests = (
},
},
+ # The "RLS is enabled" pseudo-object borrows its table's relname, so it
+ # ties in the sort with a policy of that same name on that same table.
+ # Check that the marker still dumps ahead of the policy.
+ 'CREATE POLICY test_table ON test_table' => {
+ create_order => 27,
+ create_sql => 'CREATE POLICY test_table ON dump_test.test_table
+ USING (true);',
+ regexp => qr/^
+ \QALTER TABLE dump_test.test_table ENABLE ROW LEVEL SECURITY;\E\n.+
+ \QCREATE POLICY test_table ON dump_test.test_table USING (true);\E
+ /xms,
+ like => {
+ %full_runs,
+ %dump_test_schema_runs,
+ only_dump_test_table => 1,
+ section_post_data => 1,
+ },
+ unlike => {
+ exclude_dump_test_schema => 1,
+ exclude_test_table => 1,
+ no_policies => 1,
+ no_policies_restore => 1,
+ only_dump_measurement => 1,
+ },
+ },
+
+ 'CREATE POLICY p7 ON test_table with a multi-role TO list' => {
+ create_order => 28,
+ create_sql => 'CREATE ROLE regress_dump_policy_role_a;
+ CREATE ROLE regress_dump_policy_role_b;
+ CREATE POLICY p7 ON dump_test.test_table
+ TO regress_dump_policy_role_b, regress_dump_policy_role_a
+ USING (true);',
+ regexp => qr/^
+ \QCREATE POLICY p7 ON dump_test.test_table \E
+ \QTO regress_dump_policy_role_b, regress_dump_policy_role_a \E
+ \QUSING (true);\E
+ /xm,
+ like => {
+ %full_runs,
+ %dump_test_schema_runs,
+ only_dump_test_table => 1,
+ section_post_data => 1,
+ },
+ unlike => {
+ exclude_dump_test_schema => 1,
+ exclude_test_table => 1,
+ no_policies => 1,
+ no_policies_restore => 1,
+ only_dump_measurement => 1,
+ },
+ },
+
+ 'CREATE ROLE regress_dump_policy_role_a' => {
+ regexp => qr/^CREATE ROLE regress_dump_policy_role_a;/m,
+ like => {
+ pg_dumpall_dbprivs => 1,
+ pg_dumpall_exclude => 1,
+ pg_dumpall_globals => 1,
+ pg_dumpall_globals_clean => 1,
+ },
+ },
+
+ 'CREATE ROLE regress_dump_policy_role_b' => {
+ regexp => qr/^CREATE ROLE regress_dump_policy_role_b;/m,
+ like => {
+ pg_dumpall_dbprivs => 1,
+ pg_dumpall_exclude => 1,
+ pg_dumpall_globals => 1,
+ pg_dumpall_globals_clean => 1,
+ },
+ },
+
'CREATE PROPERTY GRAPH propgraph' => {
create_order => 20,
create_sql => 'CREATE PROPERTY GRAPH dump_test.propgraph;',
@@ -3323,7 +3472,7 @@ my %tests = (
create_sql =>
'CREATE PUBLICATION pub9 FOR ALL TABLES EXCEPT (TABLE dump_test.test_table, dump_test.test_second_table);',
regexp => qr/^
- \QCREATE PUBLICATION pub9 FOR ALL TABLES EXCEPT (TABLE ONLY dump_test.test_table, TABLE ONLY dump_test.test_second_table) WITH (publish = 'insert, update, delete, truncate');\E
+ \QCREATE PUBLICATION pub9 FOR ALL TABLES EXCEPT (TABLE ONLY dump_test.test_second_table, TABLE ONLY dump_test.test_table) WITH (publish = 'insert, update, delete, truncate');\E
/xm,
like => { %full_runs, section_post_data => 1, },
},
@@ -3333,7 +3482,7 @@ my %tests = (
create_sql =>
'CREATE PUBLICATION pub10 FOR ALL TABLES EXCEPT (TABLE dump_test.test_inheritance_parent);',
regexp => qr/^
- \QCREATE PUBLICATION pub10 FOR ALL TABLES EXCEPT (TABLE ONLY dump_test.test_inheritance_parent, TABLE ONLY dump_test.test_inheritance_child) WITH (publish = 'insert, update, delete, truncate');\E
+ \QCREATE PUBLICATION pub10 FOR ALL TABLES EXCEPT (TABLE ONLY dump_test.test_inheritance_child, TABLE ONLY dump_test.test_inheritance_parent) WITH (publish = 'insert, update, delete, truncate');\E
/xm,
like => { %full_runs, section_post_data => 1, },
},
@@ -4075,6 +4224,85 @@ my %tests = (
},
},
+ # The order of a table's parents is a logical property of the database:
+ # pg_inherits.inhseqno fixes it, and it determines the order of the
+ # child's inherited columns. Here inh_order_parent1 is re-attached after
+ # a NO INHERIT, so it has the *higher* inhseqno; VACUUM frees the line
+ # pointer of the removed pg_inherits row and the re-added one reuses it,
+ # putting the higher-inhseqno parent physically first. The INHERITS list
+ # must still come out in inhseqno order.
+ 'CREATE TABLE inh_order_parent1' => {
+ create_order => 101,
+ create_sql => 'CREATE TABLE dump_test.inh_order_parent1 (
+ col1 int
+ );',
+ regexp => qr/^
+ \QCREATE TABLE dump_test.inh_order_parent1 (\E\n
+ \s+\Qcol1 integer\E\n
+ \Q);\E\n
+ /xm,
+ like =>
+ { %full_runs, %dump_test_schema_runs, section_pre_data => 1, },
+ unlike => {
+ exclude_dump_test_schema => 1,
+ only_dump_measurement => 1,
+ },
+ },
+
+ 'CREATE TABLE inh_order_parent2' => {
+ create_order => 102,
+ create_sql => 'CREATE TABLE dump_test.inh_order_parent2 (
+ col1 int
+ );',
+ regexp => qr/^
+ \QCREATE TABLE dump_test.inh_order_parent2 (\E\n
+ \s+\Qcol1 integer\E\n
+ \Q);\E\n
+ /xm,
+ like =>
+ { %full_runs, %dump_test_schema_runs, section_pre_data => 1, },
+ unlike => {
+ exclude_dump_test_schema => 1,
+ only_dump_measurement => 1,
+ },
+ },
+
+ 'CREATE TABLE inh_order_child' => {
+ create_order => 103,
+ create_sql => 'CREATE TABLE dump_test.inh_order_child (
+ col2 int
+ ) INHERITS (dump_test.inh_order_parent1,
+ dump_test.inh_order_parent2);
+ ALTER TABLE dump_test.inh_order_child
+ NO INHERIT dump_test.inh_order_parent1;
+ VACUUM pg_catalog.pg_inherits;
+ ALTER TABLE dump_test.inh_order_child
+ INHERIT dump_test.inh_order_parent1;',
+ regexp => qr/^
+ \QCREATE TABLE dump_test.inh_order_child (\E\n
+ \s+\Qcol2 integer\E\n
+ \)\n
+ \QINHERITS (dump_test.inh_order_parent2, dump_test.inh_order_parent1);\E\n
+ /xm,
+ like => {
+ %full_runs, %dump_test_schema_runs, section_pre_data => 1,
+ },
+ unlike => {
+ binary_upgrade => 1,
+ exclude_dump_test_schema => 1,
+ only_dump_measurement => 1,
+ },
+ },
+
+ 'CREATE TABLE inh_order_child pg_upgrade' => {
+ regexp => qr/^
+ \QALTER TABLE ONLY dump_test.inh_order_child INHERIT dump_test.inh_order_parent2;\E\n
+ \QALTER TABLE ONLY dump_test.inh_order_child INHERIT dump_test.inh_order_parent1;\E\n
+ /xm,
+ like => { binary_upgrade => 1, },
+ },
+
+
'CREATE STATISTICS extended_stats_no_options' => {
create_order => 97,
create_sql => 'CREATE STATISTICS dump_test.test_ext_stats_no_options
diff --git a/src/bin/pg_dump/t/003_pg_dump_with_server.pl b/src/bin/pg_dump/t/003_pg_dump_with_server.pl
index 349add6..7c10e3a 100644
--- a/src/bin/pg_dump/t/003_pg_dump_with_server.pl
+++ b/src/bin/pg_dump/t/003_pg_dump_with_server.pl
@@ -47,4 +47,95 @@ command_ok(
],
"dump foreign server with no tables");
+#########################################
+# Verify that --binary-upgrade lists an extension's required extensions in
+# name order. pg_dump reads the requires list out of pg_depend, which
+# returns those rows in an order derived from the required extensions'
+# OIDs; without an explicit sort, two databases holding the same extensions
+# dump differently depending on the order the extensions were created in.
+
+mkdir "$tempdir/extension"
+ or die "could not create directory \"$tempdir/extension\": $!";
+foreach my $ext ('dump_test_ext_a', 'dump_test_ext_b', 'dump_test_ext_c')
+{
+ open my $cf, '>', "$tempdir/extension/$ext.control"
+ or die "could not create control file for $ext: $!";
+ print $cf "default_version = '1.0'\n";
+ print $cf "relocatable = true\n";
+ print $cf "requires = 'dump_test_ext_a,dump_test_ext_b'\n"
+ if $ext eq 'dump_test_ext_c';
+ close $cf;
+
+ # The extensions need no members, so an empty script will do.
+ open my $sf, '>', "$tempdir/extension/$ext--1.0.sql"
+ or die "could not create script file for $ext: $!";
+ close $sf;
+}
+
+my $sep = $windows_os ? ';' : ':';
+my $ext_path = $windows_os ? ($tempdir =~ s/\\/\\\\/gr) : $tempdir;
+
+# Create dump_test_ext_a before dump_test_ext_b, so that the requirement
+# that sorts first by name is the one with the smaller OID. pg_depend
+# hands back these rows in descending OID order, that is, in the reverse of
+# the order the dump must use.
+$node->safe_psql(
+ 'postgres', qq{
+ SET extension_control_path = '\$system$sep$ext_path';
+ CREATE EXTENSION dump_test_ext_a;
+ CREATE EXTENSION dump_test_ext_b;
+ CREATE EXTENSION dump_test_ext_c;});
+
+command_like(
+ [ 'pg_dump', '--port' => $port, '--binary-upgrade', 'postgres' ],
+ qr/\QSELECT pg_catalog.binary_upgrade_create_empty_extension('dump_test_ext_c', 'public', true, '1.0', NULL, NULL, ARRAY['dump_test_ext_a','dump_test_ext_b']::pg_catalog.text[]);\E/,
+ 'binary upgrade dumps required extensions in name order');
+
+#########################################
+# Verify that an object carrying labels from more than one security label
+# provider gets its SECURITY LABEL commands emitted in provider name order,
+# not in pg_seclabel/pg_shseclabel physical order. dummy_seclabel registers
+# a second provider, "dummy2", when dummy_seclabel.second_provider is turned
+# on before the module is loaded.
+
+SKIP:
+{
+ skip "dummy_seclabel module not installed", 6
+ unless $node->check_extension('dummy_seclabel');
+
+ # Label each object with "dummy2" before "dummy", that is, in the reverse
+ # of the order the dump has to use, so that emitting the labels in
+ # catalog order would produce the wrong output.
+ $node->safe_psql(
+ 'postgres', q|
+ SET dummy_seclabel.second_provider = on;
+ LOAD 'dummy_seclabel';
+ CREATE TABLE seclabel_order_tbl (a int);
+ SECURITY LABEL FOR dummy2 ON TABLE seclabel_order_tbl IS 'classified';
+ SECURITY LABEL FOR dummy ON TABLE seclabel_order_tbl IS 'classified';
+ SECURITY LABEL FOR dummy2 ON COLUMN seclabel_order_tbl.a IS 'classified';
+ SECURITY LABEL FOR dummy ON COLUMN seclabel_order_tbl.a IS 'classified';
+ SECURITY LABEL FOR dummy2 ON DATABASE postgres IS 'classified';
+ SECURITY LABEL FOR dummy ON DATABASE postgres IS 'classified';
+ |);
+
+ $node->command_like(
+ [ 'pg_dump', '--schema-only', 'postgres' ],
+ qr/^
+ \QSECURITY LABEL FOR dummy ON TABLE public.seclabel_order_tbl IS 'classified';\E\n
+ \QSECURITY LABEL FOR dummy2 ON TABLE public.seclabel_order_tbl IS 'classified';\E\n
+ \QSECURITY LABEL FOR dummy ON COLUMN public.seclabel_order_tbl.a IS 'classified';\E\n
+ \QSECURITY LABEL FOR dummy2 ON COLUMN public.seclabel_order_tbl.a IS 'classified';\E$
+ /xm,
+ 'security labels are dumped in provider order');
+
+ $node->command_like(
+ [ 'pg_dump', '--schema-only', '--create', 'postgres' ],
+ qr/^
+ \QSECURITY LABEL FOR dummy ON DATABASE postgres IS 'classified';\E\n
+ \QSECURITY LABEL FOR dummy2 ON DATABASE postgres IS 'classified';\E$
+ /xm,
+ 'shared security labels are dumped in provider order');
+}
+
done_testing();
diff --git a/src/test/modules/dummy_seclabel/dummy_seclabel.c b/src/test/modules/dummy_seclabel/dummy_seclabel.c
index 7277f61..909a1b7 100644
--- a/src/test/modules/dummy_seclabel/dummy_seclabel.c
+++ b/src/test/modules/dummy_seclabel/dummy_seclabel.c
@@ -15,12 +15,15 @@
#include "commands/seclabel.h"
#include "fmgr.h"
#include "miscadmin.h"
+#include "utils/guc.h"
#include "utils/rel.h"
PG_MODULE_MAGIC;
PG_FUNCTION_INFO_V1(dummy_seclabel_dummy);
+static bool dummy_seclabel_second_provider = false;
+
static void
dummy_object_relabel(const ObjectAddress *object, const char *seclabel)
{
@@ -47,6 +50,29 @@ void
_PG_init(void)
{
register_label_provider("dummy", dummy_object_relabel);
+
+ /*
+ * Optionally register a second provider. Tests that need two providers
+ * registered at the same time turn this on before the module is loaded.
+ * It defaults to off, so that the provider-less "SECURITY LABEL ON ... IS
+ * ..." syntax, which requires exactly one registered provider, keeps
+ * working.
+ */
+ DefineCustomBoolVariable("dummy_seclabel.second_provider",
+ "Also register a \"dummy2\" label provider.",
+ NULL,
+ &dummy_seclabel_second_provider,
+ false,
+ PGC_SUSET,
+ 0,
+ NULL,
+ NULL,
+ NULL);
+
+ MarkGUCPrefixReserved("dummy_seclabel");
+
+ if (dummy_seclabel_second_provider)
+ register_label_provider("dummy2", dummy_object_relabel);
}
/*
diff --git a/src/test/modules/test_pg_dump/t/001_base.pl b/src/test/modules/test_pg_dump/t/001_base.pl
index 3d65ce4..d9e1ea9 100644
--- a/src/test/modules/test_pg_dump/t/001_base.pl
+++ b/src/test/modules/test_pg_dump/t/001_base.pl
@@ -846,6 +846,51 @@ my %tests = (
},
},
+ 'CREATE TRIGGER extdepend_trig' => {
+ create_order => 12,
+ create_sql =>
+ 'CREATE TRIGGER extdepend_trig BEFORE UPDATE ON regress_pg_dump_schema.extdependtab
+ FOR EACH ROW EXECUTE FUNCTION suppress_redundant_updates_trigger();
+ ALTER TRIGGER extdepend_trig ON regress_pg_dump_schema.extdependtab DEPENDS ON EXTENSION test_pg_dump;
+ ALTER TRIGGER extdepend_trig ON regress_pg_dump_schema.extdependtab DEPENDS ON EXTENSION plpgsql;',
+ regexp => qr/^
+ \QCREATE TRIGGER extdepend_trig BEFORE UPDATE ON regress_pg_dump_schema.extdependtab FOR EACH ROW EXECUTE FUNCTION suppress_redundant_updates_trigger();\E\n
+ /xms,
+ like => {%pgdump_runs},
+ unlike => {
+ data_only => 1,
+ extension_schema => 1,
+ pg_dumpall_globals => 1,
+ privileged_internals => 1,
+ section_data => 1,
+ section_pre_data => 1,
+ # Excludes this schema as extension is not listed.
+ without_extension_explicit_schema => 1,
+ },
+ },
+
+ # The two ALTER TRIGGER ... DEPENDS ON EXTENSION statements above are
+ # executed test_pg_dump first, plpgsql second, but pg_dump must emit them
+ # in extension name order, so that the archive entry's text does not
+ # depend on pg_depend's physical row order.
+ 'ALTER TRIGGER DEPENDS ON extension in name order' => {
+ regexp => qr/^
+ \QALTER TRIGGER extdepend_trig ON regress_pg_dump_schema.extdependtab DEPENDS ON EXTENSION plpgsql;\E\n
+ \QALTER TRIGGER extdepend_trig ON regress_pg_dump_schema.extdependtab DEPENDS ON EXTENSION test_pg_dump;\E\n
+ /xms,
+ like => {%pgdump_runs},
+ unlike => {
+ data_only => 1,
+ extension_schema => 1,
+ pg_dumpall_globals => 1,
+ privileged_internals => 1,
+ section_data => 1,
+ section_pre_data => 1,
+ # Excludes this schema as extension is not listed.
+ without_extension_explicit_schema => 1,
+ },
+ },
+
# Objects not included in extension, part of schema created by extension
'CREATE TABLE regress_pg_dump_schema.external_tab' => {
create_order => 4,
--
2.49.0
[text/plain] 0002-SAMPLE-fixes-for-the-eleven-testable-dump-order-defe.patch (18.8K, ../../20260909000357.b2.noahmisch@microsoft.com/3-0002-SAMPLE-fixes-for-the-eleven-testable-dump-order-defe.patch)
download | inline diff:
From 2d0d0f3158afdc47e49afa68d87dd4823ad3d32c Mon Sep 17 00:00:00 2001
From: Noah Misch <noah@leadboat.com>
Date: Thu, 3 Sep 2026 18:07:02 +0000
Subject: [PATCH 2/2] SAMPLE fixes for the eleven testable dump-order defects
Not proposed patches. These exist so that the branch is coherent -- the tests
in the preceding commit need something to pass against -- and so that "this
test fails without the fix" is checkable. Drop this commit to see them fail.
Four involve a judgement rather than a mechanical key completion:
* D5 preserves the order the user wrote in CREATE POLICY, via unnest ... WITH
ORDINALITY. Plain ORDER BY rolname would be simpler but rewrites the
clause. Both remove the OID dependence.
* D4 joins pg_type and pg_namespace and orders by (nspname, typname) rather
than by regtype output, whose rendering depends on search_path.
* D1 sorts the RLS-enable pseudo-object before its table's policies. Either
order is stable.
* D12 sorts an ACL array, which buildACLCommands() warns can be unsafe. It is
safe here only because a default ACL's items all share one grantor, so there
is no grant chain to replay in order; that argument is the basis of the fix
and is the thing to check before accepting it.
Verified: with these applied, meson test over the pg_dump, test_pg_dump and
dummy_seclabel suites is 13/13 (002_pg_dump alone is 13697 subtests). With
all five product files reverted, pg_dump aborts on D1's assertion. With only
D1's fix applied, each remaining test fails by its own name.
This work is model-generated and unreviewed by a human; see PROVENANCE.md.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Newm1jZHVy54kfX1eRPDwB
---
src/bin/pg_dump/common.c | 8 +-
src/bin/pg_dump/dumputils.c | 8 +-
src/bin/pg_dump/pg_dump.c | 155 +++++++++++++++++++++++++++------
src/bin/pg_dump/pg_dump_sort.c | 12 +++
src/bin/pg_dump/pg_dumpall.c | 2 +-
5 files changed, 153 insertions(+), 32 deletions(-)
diff --git a/src/bin/pg_dump/common.c b/src/bin/pg_dump/common.c
index 047e1c6..38eb274 100644
--- a/src/bin/pg_dump/common.c
+++ b/src/bin/pg_dump/common.c
@@ -286,10 +286,10 @@ flagInhTables(Archive *fout, TableInfo *tblinfo, int numTables,
for (i = 0; i < numInherits; i++)
{
/*
- * Skip a hashtable lookup if it's same table as last time. This is
- * unlikely for the child, but less so for the parent. (Maybe we
- * should ask the backend for a sorted array to make it more likely?
- * Not clear the sorting effort would be repaid, though.)
+ * Skip a hashtable lookup if it's same table as last time.
+ * getInherits() sorts by inhrelid, so consecutive rows for the same
+ * child do come together; repeats of the same parent are less
+ * predictable.
*/
if (child == NULL ||
child->dobj.catId.oid != inhinfo[i].inhrelid)
diff --git a/src/bin/pg_dump/dumputils.c b/src/bin/pg_dump/dumputils.c
index a3835cc..d7b000f 100644
--- a/src/bin/pg_dump/dumputils.c
+++ b/src/bin/pg_dump/dumputils.c
@@ -681,10 +681,16 @@ void
buildShSecLabelQuery(const char *catalog_name, Oid objectId,
PQExpBuffer sql)
{
+ /*
+ * Sort by provider, the remaining column of pg_shseclabel's unique key
+ * (classoid and objoid are already fixed by the WHERE clause), so that
+ * the emitted commands do not depend on physical row order.
+ */
appendPQExpBuffer(sql,
"SELECT provider, label FROM pg_catalog.pg_shseclabel "
"WHERE classoid = 'pg_catalog.%s'::pg_catalog.regclass "
- "AND objoid = '%u'", catalog_name, objectId);
+ "AND objoid = '%u' "
+ "ORDER BY provider", catalog_name, objectId);
}
/*
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index db14834..436c17d 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -3760,10 +3760,16 @@ dumpDatabaseConfig(Archive *AH, PQExpBuffer outbuf,
PQclear(res);
- /* Now look for role-and-database-specific options */
+ /*
+ * Now look for role-and-database-specific options. Order by role name,
+ * so that the emitted commands don't depend on the roles' OIDs; rolname
+ * is a complete sort key, since pg_db_role_setting has at most one row
+ * per (setdatabase, setrole).
+ */
printfPQExpBuffer(buf, "SELECT rolname, unnest(setconfig) "
"FROM pg_db_role_setting s, pg_roles r "
- "WHERE setrole = r.oid AND setdatabase = '%u'::oid",
+ "WHERE setrole = r.oid AND setdatabase = '%u'::oid "
+ "ORDER BY 1",
dboid);
res = ExecuteSqlQuery(AH, buf->data, PGRES_TUPLES_OK);
@@ -4274,9 +4280,20 @@ getPolicies(Archive *fout, TableInfo tblinfo[], int numTables)
printfPQExpBuffer(query,
"SELECT pol.oid, pol.tableoid, pol.polrelid, pol.polname, pol.polcmd, ");
appendPQExpBufferStr(query, "pol.polpermissive, ");
+ /*
+ * The role names in the policy's TO clause must come out in the order
+ * they appear in polroles, which is the order they were written in
+ * CREATE POLICY. An unordered ARRAY() subquery would instead return them
+ * in pg_authid scan order, so two databases holding identical policies
+ * would dump differently whenever their roles occupy different physical
+ * positions or the planner picks a different scan for pg_authid.
+ */
appendPQExpBuffer(query,
"CASE WHEN pol.polroles = '{0}' THEN NULL ELSE "
- " pg_catalog.array_to_string(ARRAY(SELECT pg_catalog.quote_ident(rolname) from pg_catalog.pg_roles WHERE oid = ANY(pol.polroles)), ', ') END AS polroles, "
+ " pg_catalog.array_to_string(ARRAY(SELECT pg_catalog.quote_ident(r.rolname) "
+ "FROM pg_catalog.unnest(pol.polroles) WITH ORDINALITY AS u(roleoid, ord) "
+ "JOIN pg_catalog.pg_roles r ON (r.oid = u.roleoid) "
+ "ORDER BY u.ord), ', ') END AS polroles, "
"pg_catalog.pg_get_expr(pol.polqual, pol.polrelid) AS polqual, "
"pg_catalog.pg_get_expr(pol.polwithcheck, pol.polrelid) AS polwithcheck "
"FROM unnest('%s'::pg_catalog.oid[]) AS src(tbloid)\n"
@@ -4595,10 +4612,22 @@ getPublications(Archive *fout)
PGresult *res_tbls;
resetPQExpBuffer(query);
+ /*
+ * Sort the EXCEPT list by the excluded relations' names. The list
+ * is a set, and pg_publication_rel has no ordering column, so an
+ * unordered query would emit it in heap order and make two
+ * logically-identical databases dump differently. Sorting by
+ * prrelid would just trade heap order for OID order; use the
+ * referenced relation's natural key (nspname, relname), matching
+ * DOTypeNameCompare().
+ */
appendPQExpBuffer(query,
- "SELECT prrelid\n"
- "FROM pg_catalog.pg_publication_rel\n"
- "WHERE prpubid = %u AND prexcept",
+ "SELECT pr.prrelid\n"
+ "FROM pg_catalog.pg_publication_rel pr\n"
+ " JOIN pg_catalog.pg_class c ON c.oid = pr.prrelid\n"
+ " JOIN pg_catalog.pg_namespace n ON n.oid = c.relnamespace\n"
+ "WHERE pr.prpubid = %u AND pr.prexcept\n"
+ "ORDER BY n.nspname, c.relname",
pubinfo[i].dobj.catId.oid);
res_tbls = ExecuteSqlQuery(fout, query->data, PGRES_TUPLES_OK);
@@ -5677,6 +5706,10 @@ dumpSubscription(Archive *fout, const SubscriptionInfo *subinfo)
/*
* Given a "create query", append as many ALTER ... DEPENDS ON EXTENSION as
* the object needs.
+ *
+ * The statements are emitted in extension name order, so that the text of the
+ * object's archive entry is a function of the object's dependencies and not of
+ * the order in which those dependencies happen to appear in pg_depend.
*/
static void
append_depends_on_extension(Archive *fout,
@@ -5704,7 +5737,8 @@ append_depends_on_extension(Archive *fout,
"FROM pg_catalog.pg_depend d, pg_catalog.pg_extension e "
"WHERE d.refobjid = e.oid AND classid = '%s'::pg_catalog.regclass "
"AND objid = '%u'::pg_catalog.oid AND deptype = 'x' "
- "AND refclassid = 'pg_catalog.pg_extension'::pg_catalog.regclass",
+ "AND refclassid = 'pg_catalog.pg_extension'::pg_catalog.regclass "
+ "ORDER BY e.extname",
catalog,
dobj->catId.oid);
res = ExecuteSqlQuery(fout, query->data, PGRES_TUPLES_OK);
@@ -7692,8 +7726,18 @@ getInherits(Archive *fout, int *numInherits)
int i_inhrelid;
int i_inhparent;
- /* find all the inheritance information */
- appendPQExpBufferStr(query, "SELECT inhrelid, inhparent FROM pg_inherits");
+ /*
+ * Find all the inheritance information. ORDER BY inhseqno is essential:
+ * the order of a table's parents is a logical property of the database
+ * (inhseqno fixes the order of the child's inherited columns), while the
+ * physical order of pg_inherits rows is not, since a line pointer freed
+ * by NO INHERIT or DROP TABLE and then reclaimed by VACUUM gets reused by
+ * a later entry with a higher inhseqno. Sorting by inhrelid as well
+ * makes the "same table as last time" caching in flagInhTables() work.
+ */
+ appendPQExpBufferStr(query,
+ "SELECT inhrelid, inhparent FROM pg_inherits "
+ "ORDER BY inhrelid, inhseqno");
res = ExecuteSqlQuery(fout, query->data, PGRES_TUPLES_OK);
@@ -10570,12 +10614,26 @@ getDefaultACLs(Archive *fout)
* for the case of 'S' (DEFACLOBJ_SEQUENCE) which must be converted to
* 's'.
*/
+ /*
+ * The stored element order of defaclacl carries no information: the
+ * backend canonicalizes these arrays with aclitemsort(), which orders them
+ * by grantee OID. Dumping in that order would make our output depend on
+ * OID assignment, so re-sort by the aclitem's textual form, i.e. by
+ * grantee name. Unlike an object's own ACL, a default ACL cannot contain
+ * a chain of grants by different grantors -- every item's grantor is
+ * defaclrole -- so reordering is safe here.
+ */
appendPQExpBufferStr(query,
"SELECT oid, tableoid, "
"defaclrole, "
"defaclnamespace, "
"defaclobjtype, "
- "defaclacl, "
+ "CASE WHEN pg_catalog.array_length(defaclacl, 1) IS NULL "
+ "THEN defaclacl ELSE "
+ "(SELECT pg_catalog.array_agg(a ORDER BY "
+ "a::pg_catalog.text COLLATE pg_catalog.\"C\") "
+ "FROM pg_catalog.unnest(defaclacl) AS a) "
+ "END AS defaclacl, "
"CASE WHEN defaclnamespace = 0 THEN "
"acldefault(CASE WHEN defaclobjtype = 'S' "
"THEN 's'::\"char\" ELSE defaclobjtype END, "
@@ -11954,6 +12012,7 @@ dumpExtension(Archive *fout, const ExtensionInfo *extinfo)
*/
int i;
int n;
+ char **reqexts;
appendPQExpBufferStr(q, "-- For binary upgrade, create an empty extension and insert objects into it\n");
@@ -11989,7 +12048,14 @@ dumpExtension(Archive *fout, const ExtensionInfo *extinfo)
else
appendPQExpBufferStr(q, "NULL");
appendPQExpBufferStr(q, ", ");
- appendPQExpBufferStr(q, "ARRAY[");
+ /*
+ * Collect the names of the extensions this one requires. The
+ * dependency array is in the order getDependencies() read the
+ * pg_depend rows, which is a function of the required extensions'
+ * OIDs; sort the names so that the output depends only on the
+ * database's logical content.
+ */
+ reqexts = (char **) pg_malloc(extinfo->dobj.nDeps * sizeof(char *));
n = 0;
for (i = 0; i < extinfo->dobj.nDeps; i++)
{
@@ -11997,14 +12063,20 @@ dumpExtension(Archive *fout, const ExtensionInfo *extinfo)
extobj = findObjectByDumpId(extinfo->dobj.dependencies[i]);
if (extobj && extobj->objType == DO_EXTENSION)
- {
- if (n++ > 0)
- appendPQExpBufferChar(q, ',');
- appendStringLiteralAH(q, extobj->name, fout);
- }
+ reqexts[n++] = extobj->name;
+ }
+ qsort(reqexts, n, sizeof(char *), pg_qsort_strcmp);
+
+ appendPQExpBufferStr(q, "ARRAY[");
+ for (i = 0; i < n; i++)
+ {
+ if (i > 0)
+ appendPQExpBufferChar(q, ',');
+ appendStringLiteralAH(q, reqexts[i], fout);
}
appendPQExpBufferStr(q, "]::pg_catalog.text[]");
appendPQExpBufferStr(q, ");\n");
+ pg_free(reqexts);
}
if (extinfo->dobj.dump & DUMP_COMPONENT_DEFINITION)
@@ -14577,15 +14649,20 @@ dumpOpclass(Archive *fout, const OpclassInfo *opcinfo)
appendPQExpBuffer(query, "SELECT amopstrategy, "
"amopopr::pg_catalog.regoperator, "
"opfname AS sortfamily, "
- "nspname AS sortfamilynsp "
+ "n.nspname AS sortfamilynsp "
"FROM pg_catalog.pg_amop ao JOIN pg_catalog.pg_depend ON "
"(classid = 'pg_catalog.pg_amop'::pg_catalog.regclass AND objid = ao.oid) "
"LEFT JOIN pg_catalog.pg_opfamily f ON f.oid = amopsortfamily "
"LEFT JOIN pg_catalog.pg_namespace n ON n.oid = opfnamespace "
+ "JOIN pg_catalog.pg_type lt ON lt.oid = ao.amoplefttype "
+ "JOIN pg_catalog.pg_namespace ln ON ln.oid = lt.typnamespace "
+ "JOIN pg_catalog.pg_type rt ON rt.oid = ao.amoprighttype "
+ "JOIN pg_catalog.pg_namespace rn ON rn.oid = rt.typnamespace "
"WHERE refclassid = 'pg_catalog.pg_opclass'::pg_catalog.regclass "
"AND refobjid = '%u'::pg_catalog.oid "
"AND amopfamily = '%s'::pg_catalog.oid "
- "ORDER BY amopstrategy",
+ "ORDER BY amopstrategy, ln.nspname, lt.typname, "
+ "rn.nspname, rt.typname",
opcinfo->dobj.catId.oid,
opcfamily);
@@ -14639,12 +14716,19 @@ dumpOpclass(Archive *fout, const OpclassInfo *opcinfo)
"amproc::pg_catalog.regprocedure, "
"amproclefttype::pg_catalog.regtype, "
"amprocrighttype::pg_catalog.regtype "
- "FROM pg_catalog.pg_amproc ap, pg_catalog.pg_depend "
+ "FROM pg_catalog.pg_amproc ap, pg_catalog.pg_depend, "
+ "pg_catalog.pg_type lt, pg_catalog.pg_namespace ln, "
+ "pg_catalog.pg_type rt, pg_catalog.pg_namespace rn "
"WHERE refclassid = 'pg_catalog.pg_opclass'::pg_catalog.regclass "
"AND refobjid = '%u'::pg_catalog.oid "
"AND classid = 'pg_catalog.pg_amproc'::pg_catalog.regclass "
"AND objid = ap.oid "
- "ORDER BY amprocnum",
+ "AND lt.oid = ap.amproclefttype "
+ "AND ln.oid = lt.typnamespace "
+ "AND rt.oid = ap.amprocrighttype "
+ "AND rn.oid = rt.typnamespace "
+ "ORDER BY amprocnum, ln.nspname, lt.typname, "
+ "rn.nspname, rt.typname",
opcinfo->dobj.catId.oid);
res = ExecuteSqlQuery(fout, query->data, PGRES_TUPLES_OK);
@@ -14779,15 +14863,20 @@ dumpOpfamily(Archive *fout, const OpfamilyInfo *opfinfo)
appendPQExpBuffer(query, "SELECT amopstrategy, "
"amopopr::pg_catalog.regoperator, "
"opfname AS sortfamily, "
- "nspname AS sortfamilynsp "
+ "n.nspname AS sortfamilynsp "
"FROM pg_catalog.pg_amop ao JOIN pg_catalog.pg_depend ON "
"(classid = 'pg_catalog.pg_amop'::pg_catalog.regclass AND objid = ao.oid) "
"LEFT JOIN pg_catalog.pg_opfamily f ON f.oid = amopsortfamily "
"LEFT JOIN pg_catalog.pg_namespace n ON n.oid = opfnamespace "
+ "JOIN pg_catalog.pg_type lt ON lt.oid = ao.amoplefttype "
+ "JOIN pg_catalog.pg_namespace ln ON ln.oid = lt.typnamespace "
+ "JOIN pg_catalog.pg_type rt ON rt.oid = ao.amoprighttype "
+ "JOIN pg_catalog.pg_namespace rn ON rn.oid = rt.typnamespace "
"WHERE refclassid = 'pg_catalog.pg_opfamily'::pg_catalog.regclass "
"AND refobjid = '%u'::pg_catalog.oid "
"AND amopfamily = '%u'::pg_catalog.oid "
- "ORDER BY amopstrategy",
+ "ORDER BY amopstrategy, ln.nspname, lt.typname, "
+ "rn.nspname, rt.typname",
opfinfo->dobj.catId.oid,
opfinfo->dobj.catId.oid);
@@ -14799,12 +14888,19 @@ dumpOpfamily(Archive *fout, const OpfamilyInfo *opfinfo)
"amproc::pg_catalog.regprocedure, "
"amproclefttype::pg_catalog.regtype, "
"amprocrighttype::pg_catalog.regtype "
- "FROM pg_catalog.pg_amproc ap, pg_catalog.pg_depend "
+ "FROM pg_catalog.pg_amproc ap, pg_catalog.pg_depend, "
+ "pg_catalog.pg_type lt, pg_catalog.pg_namespace ln, "
+ "pg_catalog.pg_type rt, pg_catalog.pg_namespace rn "
"WHERE refclassid = 'pg_catalog.pg_opfamily'::pg_catalog.regclass "
"AND refobjid = '%u'::pg_catalog.oid "
"AND classid = 'pg_catalog.pg_amproc'::pg_catalog.regclass "
"AND objid = ap.oid "
- "ORDER BY amprocnum",
+ "AND lt.oid = ap.amproclefttype "
+ "AND ln.oid = lt.typnamespace "
+ "AND rt.oid = ap.amprocrighttype "
+ "AND rn.oid = rt.typnamespace "
+ "ORDER BY amprocnum, ln.nspname, lt.typname, "
+ "rn.nspname, rt.typname",
opfinfo->dobj.catId.oid);
res_procs = ExecuteSqlQuery(fout, query->data, PGRES_TUPLES_OK);
@@ -16731,7 +16827,8 @@ findSecLabels(Oid classoid, Oid objoid, SecLabelItem **items)
* Construct a table of all security labels available for database objects;
* also set the has-seclabel component flag for each relevant object.
*
- * The table is sorted by classoid/objid/objsubid for speed in lookup.
+ * The table is sorted by classoid/objid/objsubid/provider for speed in
+ * lookup.
*/
static void
collectSecLabels(Archive *fout)
@@ -16749,10 +16846,16 @@ collectSecLabels(Archive *fout)
query = createPQExpBuffer();
+ /*
+ * Sort by provider as well. It is the remaining column of pg_seclabel's
+ * unique key, so adding it makes the ordering total; without it, the
+ * order of the labels an object has from different providers would come
+ * from physical row order, making the dump unstable.
+ */
appendPQExpBufferStr(query,
"SELECT label, provider, classoid, objoid, objsubid "
"FROM pg_catalog.pg_seclabels "
- "ORDER BY classoid, objoid, objsubid");
+ "ORDER BY classoid, objoid, objsubid, provider");
res = ExecuteSqlQuery(fout, query->data, PGRES_TUPLES_OK);
diff --git a/src/bin/pg_dump/pg_dump_sort.c b/src/bin/pg_dump/pg_dump_sort.c
index 8ca0332..38c3a1c 100644
--- a/src/bin/pg_dump/pg_dump_sort.c
+++ b/src/bin/pg_dump/pg_dump_sort.c
@@ -394,6 +394,18 @@ DOTypeNameCompare(const void *p1, const void *p2)
pobj2->poltable->dobj.name);
if (cmpval != 0)
return cmpval;
+
+ /*
+ * getPolicies() represents "RLS is enabled on this table" as a
+ * PolicyInfo with null polname whose dobj.name is the table's relname.
+ * Policy names live in a per-table namespace disjoint from relation
+ * names, so such a marker ties with a real policy of that same name on
+ * that same table; whether polname is null is then the only remaining
+ * natural-key field. Sort the marker first.
+ */
+ cmpval = (pobj1->polname != NULL) - (pobj2->polname != NULL);
+ if (cmpval != 0)
+ return cmpval;
}
else if (obj1->objType == DO_RULE)
{
diff --git a/src/bin/pg_dump/pg_dumpall.c b/src/bin/pg_dump/pg_dumpall.c
index c53e77c..d867e9e 100644
--- a/src/bin/pg_dump/pg_dumpall.c
+++ b/src/bin/pg_dump/pg_dumpall.c
@@ -1373,7 +1373,7 @@ dumpTablespaces(PGconn *conn)
"pg_catalog.shobj_description(oid, 'pg_tablespace') "
"FROM pg_catalog.pg_tablespace "
"WHERE spcname !~ '^pg_' "
- "ORDER BY 1");
+ "ORDER BY 2");
if (PQntuples(res) > 0)
fprintf(OPF, "--\n-- Tablespaces\n--\n\n");
--
2.49.0
^ permalink raw reply [nested|flat] 8+ messages in thread
* Re: pg_dump: assert failure sorting casts/transforms
2026-08-20 10:20 pg_dump: assert failure sorting casts/transforms Alexander Kukushkin <cyberdemn@gmail.com>
2026-08-20 11:03 ` Re: pg_dump: assert failure sorting casts/transforms Nitin Jadhav <nitinjadhavpostgres@gmail.com>
2026-08-20 11:40 ` Re: pg_dump: assert failure sorting casts/transforms Alexander Kukushkin <cyberdemn@gmail.com>
2026-08-21 07:40 ` Re: pg_dump: assert failure sorting casts/transforms Nitin Jadhav <nitinjadhavpostgres@gmail.com>
2026-08-21 09:59 ` Re: pg_dump: assert failure sorting casts/transforms Alexander Kukushkin <cyberdemn@gmail.com>
2026-09-09 00:03 ` Re: pg_dump: assert failure sorting casts/transforms Noah Misch <noah@leadboat.com>
@ 2026-10-02 07:25 ` Alexander Kukushkin <cyberdemn@gmail.com>
0 siblings, 0 replies; 8+ messages in thread
From: Alexander Kukushkin @ 2026-10-02 07:25 UTC (permalink / raw)
To: Noah Misch <noah@leadboat.com>; +Cc: Nitin Jadhav <nitinjadhavpostgres@gmail.com>; Pg Hackers <pgsql-hackers@lists.postgresql.org>
Hi Noah,
sorry that it took so long to get back.
On Wed, 9 Sept 2026 at 02:04, Noah Misch <noah@leadboat.com> wrote:
> Since this is already the fourth follow-up to my original change, I had
> Opus 5
> look for more ways to reach the assertion. It found one more:
>
> D1 DO_POLICY: the "RLS enabled" pseudo-object borrows its table's
> relname, so
> it ties with a policy named after that same table. An assert-enabled
> pg_dump aborts; a production build orders the two by comparing a
> pg_class
> OID against a pg_policy OID, which pg_upgrade inverts.
>
> Let's fix that at the same time. Would you like to add that, or would you
> like me to add it?
>
Please find the attached v4 version of the patch that also handles RLS
policies.
--
Regards,
--
Alexander Kukushkin
Attachments:
[text/x-patch] 0001-pg_dump-sort-casts-and-transforms-rls-v4.patch (8.6K, ../../CAFh8B=kyXjy=1NzACXsD7LFS3rfAsWmpdXbWDs5f4=1C8uG8pw@mail.gmail.com/3-0001-pg_dump-sort-casts-and-transforms-rls-v4.patch)
download | inline diff:
From 25e8adf022d7064d5861dba38b6120c05f0d0d43 Mon Sep 17 00:00:00 2001
From: Alexander Kukushkin <cyberdemn@gmail.com>
Date: Fri, 2 Oct 2026 09:19:20 +0200
Subject: [PATCH v4] pg_dump: sort casts, transforms, and RLS policies
independent of OIDs
DOTypeNameCompare() sorts dumpable objects by (priority, namespace, name,
objType) and then an object-type-specific natural-key tiebreaker. Three
object types reach that tiebreaker without a complete key and can tie,
falling through to the Assert(false) added in commit 0decd5e89db (aborting
assert-enabled pg_dump) or, on non-assert builds, to oidcmp() -- which
reintroduces exactly the schema-diff instability that commit and its
follow-ups (b61a5c4bed7, 4921a5972a3, d49936f3028) have been eliminating.
Casts and transforms build their sort "name" from the *unqualified* type
(and language) names, so two casts tie whenever their source and target
type names match while the types live in different schemas -- for example a
cast to pg_catalog.json and a cast to someext.json from the same source
type both get the sort name "sourcetype json". Transforms tie the same way
("typname langname"). Break those ties using the referenced types' full
natural keys via the existing pgTypeNameCompare() (nspname, then typname),
the same helper already used for function arguments and operator operands.
For transforms, comparing trftype alone suffices: a name tie already
implies the same unqualified typname and the same language name, so only
the type's schema can differ.
getPolicies() represents "row level security is enabled on this table" as a
PolicyInfo with polname == NULL that borrows the table's name, so it ties
with a real policy whose name equals that table's own name. The DO_POLICY
tiebreaker compared only the table name and then fell through. Break the
remaining tie on whether polname is NULL -- the one natural-key column the
pseudo-object lacks -- sorting the RLS-enable marker first.
Add regression coverage to 002_pg_dump.pl for all three cases: casts that
tie on the target type's schema and on the source type's schema (exercising
both new type comparisons), two transforms sharing a typname across
schemas, and a policy named after its own RLS-enabled table. These abort an
unpatched assert-enabled run and pass with the fix.
---
src/bin/pg_dump/pg_dump_sort.c | 41 ++++++++++++++++
src/bin/pg_dump/t/002_pg_dump.pl | 81 ++++++++++++++++++++++++++++++++
2 files changed, 122 insertions(+)
diff --git a/src/bin/pg_dump/pg_dump_sort.c b/src/bin/pg_dump/pg_dump_sort.c
index 4f3469d4396..86721dacde2 100644
--- a/src/bin/pg_dump/pg_dump_sort.c
+++ b/src/bin/pg_dump/pg_dump_sort.c
@@ -341,6 +341,38 @@ DOTypeNameCompare(const void *p1, const void *p2)
if (cmpval != 0)
return cmpval;
}
+ else if (obj1->objType == DO_CAST)
+ {
+ CastInfo *cobj1 = *(CastInfo *const *) p1;
+ CastInfo *cobj2 = *(CastInfo *const *) p2;
+
+ /*
+ * The "name" is only the source and target type names, unqualified,
+ * so two casts tie whenever their types share typnames across
+ * different schemas. Break the tie by the source then target types'
+ * full natural keys.
+ */
+ cmpval = pgTypeNameCompare(cobj1->castsource, cobj2->castsource);
+ if (cmpval != 0)
+ return cmpval;
+ cmpval = pgTypeNameCompare(cobj1->casttarget, cobj2->casttarget);
+ if (cmpval != 0)
+ return cmpval;
+ }
+ else if (obj1->objType == DO_TRANSFORM)
+ {
+ TransformInfo *tobj1 = *(TransformInfo *const *) p1;
+ TransformInfo *tobj2 = *(TransformInfo *const *) p2;
+
+ /*
+ * Same unqualified-typname ambiguity as casts. The language name
+ * was already compared as part of dobj.name, so trftype is the only
+ * remaining natural-key field that can break the tie.
+ */
+ cmpval = pgTypeNameCompare(tobj1->trftype, tobj2->trftype);
+ if (cmpval != 0)
+ return cmpval;
+ }
else if (obj1->objType == DO_ATTRDEF)
{
AttrDefInfo *adobj1 = *(AttrDefInfo *const *) p1;
@@ -361,6 +393,15 @@ DOTypeNameCompare(const void *p1, const void *p2)
pobj2->poltable->dobj.name);
if (cmpval != 0)
return cmpval;
+
+ /*
+ * The RLS-enabled pseudo-object has null polname but borrows its
+ * table's name, so it ties with a real policy of that name. Break by
+ * polname, sorting the pseudo-object first.
+ */
+ cmpval = (pobj1->polname != NULL) - (pobj2->polname != NULL);
+ if (cmpval != 0)
+ return cmpval;
}
else if (obj1->objType == DO_RULE)
{
diff --git a/src/bin/pg_dump/t/002_pg_dump.pl b/src/bin/pg_dump/t/002_pg_dump.pl
index 1299c837063..0a9561d99f8 100644
--- a/src/bin/pg_dump/t/002_pg_dump.pl
+++ b/src/bin/pg_dump/t/002_pg_dump.pl
@@ -2204,6 +2204,42 @@ my %tests = (
like => { %full_runs, section_pre_data => 1, },
},
+ 'CREATE CAST to public target type sharing a typname' => {
+ create_order => 51,
+ create_sql => '
+ CREATE SCHEMA dump_cast_schema;
+ CREATE TYPE public.dump_cast_src_for_target_test AS ENUM (\'a\');
+ CREATE TYPE public.dump_cast_tgt AS ENUM (\'a\');
+ CREATE TYPE dump_cast_schema.dump_cast_tgt AS ENUM (\'a\');
+ CREATE TYPE public.dump_cast_src_for_source_test AS ENUM (\'a\');
+ CREATE TYPE dump_cast_schema.dump_cast_src_for_source_test AS ENUM (\'a\');
+ CREATE CAST (public.dump_cast_src_for_target_test AS public.dump_cast_tgt) WITH INOUT;
+ CREATE CAST (public.dump_cast_src_for_target_test AS dump_cast_schema.dump_cast_tgt) WITH INOUT;
+ CREATE CAST (public.dump_cast_src_for_source_test AS public.dump_cast_tgt) WITH INOUT;
+ CREATE CAST (dump_cast_schema.dump_cast_src_for_source_test AS public.dump_cast_tgt) WITH INOUT;',
+ regexp =>
+ qr/CREATE CAST \(public\.dump_cast_src_for_target_test AS public\.dump_cast_tgt\) WITH INOUT;/m,
+ like => { %full_runs, section_pre_data => 1, },
+ },
+
+ 'CREATE CAST to schema-qualified target type sharing a typname' => {
+ regexp =>
+ qr/CREATE CAST \(public\.dump_cast_src_for_target_test AS dump_cast_schema\.dump_cast_tgt\) WITH INOUT;/m,
+ like => { %full_runs, section_pre_data => 1, },
+ },
+
+ 'CREATE CAST from public source type sharing a typname' => {
+ regexp =>
+ qr/CREATE CAST \(public\.dump_cast_src_for_source_test AS public\.dump_cast_tgt\) WITH INOUT;/m,
+ like => { %full_runs, section_pre_data => 1, },
+ },
+
+ 'CREATE CAST from schema-qualified source type sharing a typname' => {
+ regexp =>
+ qr/CREATE CAST \(dump_cast_schema\.dump_cast_src_for_source_test AS public\.dump_cast_tgt\) WITH INOUT;/m,
+ like => { %full_runs, section_pre_data => 1, },
+ },
+
'CREATE DATABASE postgres' => {
regexp => qr/^
\QCREATE DATABASE postgres WITH TEMPLATE = template0 \E
@@ -2927,6 +2963,25 @@ my %tests = (
like => { %full_runs, section_pre_data => 1, },
},
+ 'CREATE TRANSFORM with typname shared across schemas' => {
+ create_order => 34,
+ create_sql => '
+ CREATE SCHEMA dump_trf_schema;
+ CREATE TYPE public.dump_trf_type AS ENUM (\'a\');
+ CREATE TYPE dump_trf_schema.dump_trf_type AS ENUM (\'a\');
+ CREATE TRANSFORM FOR public.dump_trf_type LANGUAGE sql (FROM SQL WITH FUNCTION prsd_lextype(internal));
+ CREATE TRANSFORM FOR dump_trf_schema.dump_trf_type LANGUAGE sql (FROM SQL WITH FUNCTION prsd_lextype(internal));',
+ regexp =>
+ qr/CREATE TRANSFORM FOR public\.dump_trf_type LANGUAGE sql \(FROM SQL WITH FUNCTION pg_catalog\.prsd_lextype\(internal\)\);/m,
+ like => { %full_runs, section_pre_data => 1, },
+ },
+
+ 'CREATE TRANSFORM for schema-qualified type sharing a typname' => {
+ regexp =>
+ qr/CREATE TRANSFORM FOR dump_trf_schema\.dump_trf_type LANGUAGE sql \(FROM SQL WITH FUNCTION pg_catalog\.prsd_lextype\(internal\)\);/m,
+ like => { %full_runs, section_pre_data => 1, },
+ },
+
'CREATE LANGUAGE pltestlang' => {
create_order => 18,
create_sql => 'CREATE LANGUAGE pltestlang
@@ -3172,6 +3227,32 @@ my %tests = (
},
},
+ # The "RLS is enabled" pseudo-object borrows its table's relname, so it
+ # ties in the sort with a policy of that same name on that same table.
+ # Check that the marker still dumps ahead of the policy.
+ 'CREATE POLICY test_table ON test_table' => {
+ create_order => 28,
+ create_sql => 'CREATE POLICY test_table ON dump_test.test_table
+ USING (true);',
+ regexp => qr/^
+ \QALTER TABLE dump_test.test_table ENABLE ROW LEVEL SECURITY;\E\n.+
+ \QCREATE POLICY test_table ON dump_test.test_table USING (true);\E
+ /xms,
+ like => {
+ %full_runs,
+ %dump_test_schema_runs,
+ only_dump_test_table => 1,
+ section_post_data => 1,
+ },
+ unlike => {
+ exclude_dump_test_schema => 1,
+ exclude_test_table => 1,
+ no_policies => 1,
+ no_policies_restore => 1,
+ only_dump_measurement => 1,
+ },
+ },
+
'CREATE PUBLICATION pub1' => {
create_order => 50,
create_sql => 'CREATE PUBLICATION pub1;',
--
2.34.1
^ permalink raw reply [nested|flat] 8+ messages in thread
end of thread, other threads:[~2026-10-02 07:25 UTC | newest]
Thread overview: 8+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2026-08-20 10:20 pg_dump: assert failure sorting casts/transforms Alexander Kukushkin <cyberdemn@gmail.com>
2026-08-20 11:03 ` Nitin Jadhav <nitinjadhavpostgres@gmail.com>
2026-08-20 11:40 ` Alexander Kukushkin <cyberdemn@gmail.com>
2026-08-21 07:40 ` Nitin Jadhav <nitinjadhavpostgres@gmail.com>
2026-08-21 09:59 ` Alexander Kukushkin <cyberdemn@gmail.com>
2026-08-22 13:59 ` Nitin Jadhav <nitinjadhavpostgres@gmail.com>
2026-09-09 00:03 ` Noah Misch <noah@leadboat.com>
2026-10-02 07:25 ` Alexander Kukushkin <cyberdemn@gmail.com>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox