On 11/2/21, 11:27 AM, "Stephen Frost" <sfr...@snowman.net> wrote: > * Bossart, Nathan (bossa...@amazon.com) wrote: >> 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. > > 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 don't feel strongly one way or the other as well, but you have a good point about extra control over the error messages. The latest patch just does a standard aclcheck_error(), so you'd probably see "permission denied for function" if you didn't have privileges for CHECKPOINT. That could be confusing. Nathan
