On 14.01.25 10:10, Alvaro Herrera wrote:
> 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.
> + */
Committed with that change. Thanks.
