Horton

Re: Test instability when pg_dump orders by OID

Noah Misch <noah@leadboat.com>

View thread
From: Noah Misch <noah@leadboat.com>
To: Kirill Reshke <reshkekirill@gmail.com>
Cc: Alexander Lakhin <exclusion@gmail.com>, pgsql-hackers@postgresql.org, Robert Haas <robertmhaas@gmail.com>
Date: 2025-08-10T16:36:59Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Rewrite previous commit's test for TestUpgradeXversion compatibility.

  2. Sort DO_DEFAULT_ACL dump objects independent of OIDs.

  3. Remove, from stable branches, the new assertion of no pg_dump OID sort.

  4. Sort dump objects independent of OIDs, for the 7 holdout object types.

  5. pg_dump: provide a stable sort order for rules.

  6. pg_dump: include comments on not-null constraints on domains, too

  7. Verify roundtrip dump/restore of regression database

  8. In pg_dump, use simplehash.h to look up dumpable objects by OID.

  9. Tweak pg_dumpall to add GRANT CONNECT ON DATABASE ... TO PUBLIC when dumping

  10. Restructure operator classes to allow improved handling of cross-data-type

  11. First phase of project to use fixed OIDs for all system catalogs and

  12. Tweak pg_dump to say GRANT ALL when appropriate, rather than enumerating

On Sun, Aug 10, 2025 at 04:41:20PM +0500, Kirill Reshke wrote:
> On Sun, 10 Aug 2025 at 12:00, Alexander Lakhin <exclusion@gmail.com> wrote:
> > 04.08.2025 03:03, Noah Misch wrote:
> > > Pushed as 0decd5e.  ...
> >
> > Please look at a new anomaly introduced with that commit. The following
> > script:
> > createdb regression
> >
> > echo "
> > CREATE USER u1;
> > ALTER DEFAULT PRIVILEGES FOR ROLE u1 REVOKE INSERT ON TABLES FROM u1;
> >
> > CREATE USER u2;
> > ALTER DEFAULT PRIVILEGES FOR ROLE u2 REVOKE INSERT ON TABLES FROM u2;
> > " | psql regression
> >
> > pg_dump regression
> >
> > triggers:
> > pg_dump: pg_dump_sort.c:454: DOTypeNameCompare: Assertion `0' failed.
> 
> I reproduced this. Indeed, in case of default acl we happen to use OID sort.

Thanks.  Given the current state of freeze for tomorrow's release wrap, the
decision is less obvious than usual.  I'm seeing these options:

1. Remove the new assertion in v13-v18.
2. Push your proposed fix.
3. Change nothing.  (This would be the choice if one is maximally concerned
   about deviating from the freeze and unconcerned about --enable-cassert
   builds of releases.)

I am inclined to make today's change be (1).  A fresh audit of catalog PRIMARY
KEY and UNIQUE constraints didn't find any more missed cases, but (1) still
feels like the right level of cautiousness.  If there are no objections in the
next 3hr, I'll proceed with (1).

> PFA resolves this issue. I simply added DEFAULT ACL case-specific
> tiebreaker that resolves object order.

Thanks.  Could you make src/test/regress create regression database objects so
the code addition has coverage?  Using pg_signal_backend and
pg_read_all_settings as the default ACL role names should avoid that suite's
limitations.  (The suite must run under any role name and must drop any roles
it creates, so it can't assume any particular non-system role name survives
the suite.)