On Mon, Aug 25, 2025 at 10:30:32AM +0900, Yugo Nagata wrote:
> The documentation fix looks good to me. However, it’s not very user-friendly that,
> when the user lacks the required privileges, an error from the internal query is
> raised. Instead, how about checking whether the user has the necessary privileges
> and printing an appropriate message if any privilege is missing?
Thanks for taking a look. I appreciate your suggestion, but I'm finding
myself leaning -1 for a couple of reasons:
* There's no precedent for this in vacuumdb. In fact, if the user
specifies a nonexistent table, we return the error from the internal
query, and if the user lacks privileges to VACUUM or ANALYZE a table, we
let the command emit a WARNING and skip it. Intentionally or not, we
seem to let the server do most of the work when it comes to this sort of
thing. vacuumdb is just responsible for building and sending the
commands.
* I'm a little worried that warning about SELECT privileges on these
catalogs will encourage people to grant privileges on them, which we
probably don't want them to do. My proposed documentation note already
carries some risk of this, but it at least specifically calls out that
superusers should have the necessary privileges.
* While probably rare, checking the privileges beforehand introduces race
conditions that would cause the internal query error, anyway. I think
there are already a few such race conditions already (e.g., if the table
is dropped between the catalog query and the VACUUM or ANALYZE command),
which AFAICT we just live with, but I'm wary of pressing our luck too
much in this area.
I'd be grateful for others' thoughts on this. I'm hoping to resolve this
open item within the next couple of days, given 18rc1 is planned for next
week.
--
nathan
