Date: Sun, 10 Aug 2025 09:36:59 -0700
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>
Subject: Re: Test instability when pg_dump orders by OID
Message-ID: <20250810163659.71.nmisch@google.com>
References: <20250707192654.9e.nmisch@google.com>
 <CA+TgmoYnAiypU5sYstJ0nCqr+VVubf5wczANg=KqEeOYmdVeAw@mail.gmail.com>
 <20250718191702.45.nmisch@google.com>
 <CA+Tgmoa8FumzaUUSnjK0x=6_oXDXggQPpzgPfX-KE=PVrTyamg@mail.gmail.com>
 <20250725022740.30.nmisch@google.com>
 <CA+TgmoZXmAZmB9OZaXTiy-HG2_1d7UiYrFKXf17C_yLfPa5pQg@mail.gmail.com>
 <20250804000321.8b.nmisch@google.com>
 <d32aaa8d-df7c-4f94-bcb3-4c85f02bea21@gmail.com>
 <CALdSSPhZyLua3Vz2Ug8VOASbhP41h-7784MT8s9_+90eE06Aaw@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: 
 <CALdSSPhZyLua3Vz2Ug8VOASbhP41h-7784MT8s9_+90eE06Aaw@mail.gmail.com>
User-Agent: Mutt/2.2.12 (2023-09-09)
Archived-At: 
 <https://www.postgresql.org/message-id/20250810163659.71.nmisch%40google.com>
Precedence: bulk

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.)