Re: downgrade some aclchk.c errors to internal

On 2024-Dec-20, Peter Eisentraut wrote:

> On 20.12.24 12:47, Peter Eisentraut wrote:
> > In aclchk.c, there are a few error messages that use ereport() but it
> > seems like they should be internal error messages.  Moreover, they are
> > using get_object_class_descr(), which is only meant for internal errors.
> >  (For example, it does not have translation support.)  I dug through
> > this and it seems like these errors are indeed not or no longer user-
> > facing, so we can downgrade them to internal.  See commit messages in
> > the attached patches for further explanations.
>
> There was a mistake in the second patch, I had missed some other callers
> that I have fixed up here. Amended patch set attached.

LGTM.

> Subject: [PATCH v2 1/2] Downgrade errors in object_ownercheck() to internal
[...]

> For the has_xxx_privilege functions, the error has not been
> user-facing since commit 403ac226ddd. The remaining users are
> pg_database_size() and pg_tablespace_size(). The call stack here is
> pretty deep and this dependency is not obvious. Here we can put in an
> explicit existence check with a bespoke error message early in the

I'd add a comment on why we have that, otherwise it seems a bit random:

+ /*
+ * Not needed for correctness, but avoid non-user-facing error later
+ * message if the [database,whatever] doesn't exist.
+ */

(AFAIU it can still happen that the database disappears just after this
check and before the ACL check, so it's possible, just unlikely, to get
the later error. That seems OK.)

--
Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/
"World domination is proceeding according to plan" (Andrew Morton)