agora inbox for pgsql-hackers@postgresql.org
help / color / mirror / Atom feedFrom: Noah Misch <noah@leadboat.com>
To: Alexander Kukushkin <cyberdemn@gmail.com>
Cc: Nitin Jadhav <nitinjadhavpostgres@gmail.com>
Cc: Pg Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: pg_dump: assert failure sorting casts/transforms
Date: Tue, 8 Sep 2026 17:03:57 -0700
Message-ID: <20260909000357.b2.noahmisch@microsoft.com> (raw)
In-Reply-To: <CAFh8B=n-Dgffwdr68Or8x-psfGB1G0bMVR4n-NTiwJb8ro=oaA@mail.gmail.com>
References: <CAFh8B=nb2KLugfF5pFgOLY2Q0db8KOqp=46kxvWNzgvTQeP8oQ@mail.gmail.com>
<CAMm1aWYWzP4RpqmgOshyv3UhKDNRLFYFq0EXxQrqJGyrB-7FxA@mail.gmail.com>
<CAFh8B=nowWf0sDk01G7B=sa-TpGTObFEK=NoQMJQLzU_H6af=Q@mail.gmail.com>
<CAMm1aWb9N+t25eVH++SDzxK-usiS2tNA12X7_bYENqkU8P74fw@mail.gmail.com>
<CAFh8B=n-Dgffwdr68Or8x-psfGB1G0bMVR4n-NTiwJb8ro=oaA@mail.gmail.com>
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
view thread (8+ messages) latest in thread
Message-ID: <20260909000357.b2.noahmisch@microsoft.com>
Permalink: ../20260909000357.b2.noahmisch@microsoft.com/
Also on: postgresql.org/message-id/20260909000357.b2.noahmisch@microsoft.com
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: noah@leadboat.com, cyberdemn@gmail.com, nitinjadhavpostgres@gmail.com, pgsql-hackers@lists.postgresql.org
Subject: Re: pg_dump: assert failure sorting casts/transforms
In-Reply-To: <20260909000357.b2.noahmisch@microsoft.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox