Re: Major Version Upgrade failure due to orphan roles entries in catalog

On Wed, Feb 25, 2026 at 11:50 AM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> I don't entirely agree that it's "business as usual": somebody screwed
> up to get the database into that state. I'm not here to apportion
> blame for that between the user and the database, but I do think it's
> appropriate for pg_dumpall to complain that it's facing invalid data.
>
> If you're good with pg_dumpall producing a warning and then emitting
> the GRANT with no grantor clause, I will go make that happen.

Well, I guess I don't really see why there should be a warning. I
mean, if you're not here to apportion blame between the user and
database, then allow me to step in: in v15-, it's entirely the
database's fault. It does absolutely zilch to keep that field valid.
As I showed in the earlier example, all the user has to do is create a
role, do something, and drop the role, and that field is no longer
valid. That's about the simplest scenario imaginable, and I don't
think any user would expect it to produce a corrupted database, and I
think the status quo ante prior to this commit was that we did not
consider that to be a corrupted database.

If that feels like the wrong answer to you, I suggest that the reason
is that you're not used to PostgreSQL code being as horrifically bad
as this code was prior to v16. I don't know of anywhere else in the
database where we store an OID and do absolutely nothing to ensure its
validity. Sure, we've found some holes over the years where we didn't
do enough, especially around concurrency, and we've probably all seen
examples of TOAST corruption where a value can't be de-TOASTed due to
some inconsistency. But those are examples of where we had had some
guards in place and they weren't as thorough as they needed to be, or
where the user voided the warranty by doing something stupid, or where
the storage manhandled the data. But before
6566133c5f52771198aca07ed18f84519fac1be7, we didn't just have
inadequate machinery in this area: we had none. I remember being
absolutely gobsmacked at this discovery that it had been this way for
years and apparently nobody was too fussed about it. It seemed to me
then and it seems to me now that such handling was way below our usual
quality standards, even for older code when we weren't as strict as we
are today. But that's nevertheless how it was.

Now, if you go and do as you propose here, and adjust the code so that
the grant is dumped but a warning is produced, my fear is that someone
upgrading from v15- to v16+ will see that warning and think that there
is a problem with their database that needs fixing. But, except for
the fact that they should really upgrade to a newer and better
version, that's not really true. They're just running a version where
no care was put into making that field valid, and so sometimes it
isn't. I suppose that is OK in the end: when such a user files a
support ticket with $EMPLOYER or any other company, somebody can
simply tell that user that the warning requires no corrective action
and can be safely ignored. But of course, they'll have to know that
this is the case, and not everyone will have read this discussion.
Moreover, we'll emit essentially the same warning for the member case,
where the warning does point to a problem that someone might want to
think about correcting, and exactly the same warning against a v16+
database where it indicates that something has actually gone wrong. I
don't know why we would want to do that instead of what I proposed,
but if you're dead-set on it, I can live with it: it will at least
mean people can upgrade without having perfectly good role grants
disappear into the ether.

--
Robert Haas
EDB: http://www.enterprisedb.com