On Tue, 2021-11-02 at 14:26 -0400, Stephen Frost wrote:
> Think you meant 'Stephen' there. ;)
Yes ;-)
> > The approach in the patch looks alright to me, but another one
> > could
> > be to build a SelectStmt when parsing CHECKPOINT. I think that'd
> > simplify the standard_ProcessUtility() changes.
Nathan, if I understand correctly, that would mean no CheckPointStmt at
all. So it would either lack the right command tag, or we would have to
hack it in somewhere. The utility changes in the existing patch are
fairly minor, so I'll stick with that approach unless I'm missing
something.
> For my 2c, at least, I'm not really partial to either approach,
> though
> I'd want to see what error messages end up looking like. Seems like
> we
> might want to exercise a bit more control than we'd be able to if we
> transformed it directly into a SelectStmt (that is, we might add a
> HINT:
> roles with execute rights on pg_checkpoint() can run this command, or
> something; maybe not too tho).
I changed the error message to:
ERROR: permission denied for command CHECKPOINT
HINT: The CHECKPOINT command requires the EXECUTE privilege
on the function pg_checkpoint().
New version attached.
Andres suggested that I also consider a new form of the GRANT clause
that works on the CHECKPOINT command directly. I looked into that
briefly, but in every other case it seems that GRANT works on an object
(like a function). It doesn't feel like grating on a command is the
right solution.
The approach of using a function's ACL to represent the ACL of a
higher-level command (as in this patch) does feel right to me. It feels
like something we might extend to similar situations in the future; and
even if we don't, it seems like a clean solution in isolation.
Regards,
Jeff Davis