Re: Test instability when pg_dump orders by OID

On Mon, Jul 21, 2025 at 09:40:02AM -0400, Robert Haas wrote:
> On Fri, Jul 18, 2025 at 3:17 PM Noah Misch <noah(at)leadboat(dot)com> wrote:
> > + * Sort by encoding, per pg_collation_name_enc_nsp_index. Wherever
> > + * this changes dump order, restoring the dump fails anyway. CREATE
> > + * COLLATION can't create a tie for this to break, because it imposes
> > + * restrictions to make (nspname, collname) uniquely identify a
> > + * collation within a given DatabaseEncoding. While
> > + * pg_import_system_collations() can create a tie, pg_dump+restore
> > + * fails after pg_import_system_collations('my_schema') does so.
> > + * There's little to gain by ignoring one natural key column on the
> > + * basis of those limitations elsewhere, so respect the full natural
> > + * key like we do for other object types.
>
> This is also good. I suggest s/Wherever/Technically, this is not
> necessary, because wherever/ and s/There's/However, there's/.

I used that. I started to prepare the back-branch versions, but that revealed
three problems affecting the master patch:

(1) Sorting constraints segfaulted if either of a pair of equal-name
constraints was a domain constraint. Fortunately, commit da71717 added a test
case for that between when I mailed patch v1 and when I went to commit. One
can reproduce it by dumping a database containing:

CREATE DOMAIN d1 AS int CONSTRAINT dc CHECK (value > 0);
CREATE DOMAIN d2 AS int CONSTRAINT dc CHECK (value > 0);

I made pg_dump sort domain constraints of a given name before table
constraints of that name, for consistency with our decision to sort CREATE
DOMAIN before CREATE TABLE. The main alternative was to sort by parent object
name irrespective of parent object type, i.e. DOMAIN a < TABLE b < DOMAIN c.
That alternative lacked a relevant precedent. I've now audited the natural
keys of catalogs for which I'm changing sort order, and I think that was the
only one I missed.

(2) Sorting opclasses failed a new assertion when dumping a v9.2 source (and
likely 9.[345]), because getAccessMethods() doesn't read pg_am when dumping
from a version predating CREATE ACCESS METHOD. findAccessMethodByOid() found
no access methods, since pg_dump had read none. I've changed
getAccessMethods() to always read pg_am. (For pre-v9.6 sources, I've kept the
function's behavior of never marking an access method for dumping.) pg_am is
small enough for this read to incur negligible cost. The main alternative
was, for pre-v9.6, sorting access methods by pg_am.oid. That would have been
less code, but dump order would have differed between pre-v9.6 and v9.6+.

(3) pgTypeNameCompare() implied postfix operators don't exist, but master
pg_dump will support reading from pre-v14 clusters for several more years.
The code behaved fine, but I've updated the comment.

I regret missing those in v1. I've attached v2, including branch-specific
patches. I'll first need to back-patch 350e6b8, which fixed sorting of CREATE
RULE, to v17 and earlier. Since 350e6b8 is conflict-free all the way back to
v13, I'm not attaching it.

Thanks,
nm