Thread: pg_checkpointer is not a verb or verb phrase
Hi,
I was just looking at the list of predefined roles that we have, and
pg_checkpointer is conspicuously not like the others:
rhaas=# select rolname from pg_authid where oid!=10;
rolname
---------------------------
pg_database_owner
pg_read_all_data
pg_write_all_data
pg_monitor
pg_read_all_settings
pg_read_all_stats
pg_stat_scan_tables
pg_read_server_files
pg_write_server_files
pg_execute_server_program
pg_signal_backend
pg_checkpointer
(12 rows)
Almost all of these are verbs or verb phrases: having this role gives
you the ability to read all data, or write all data, or read all
settings, or whatever. But you can't say that having this role gives
you the ability to checkpointer. It gives you the ability to
checkpoint, or to perform a checkpoint.
So I think the name is inconsistent with our usual convention, and we
ought to fix it.
--
Robert Haas
EDB: http://www.enterprisedb.com
On Thu, 30 Jun 2022 at 08:48, Robert Haas <robertmhaas@gmail.com> wrote:
Almost all of these are verbs or verb phrases: having this role gives
you the ability to read all data, or write all data, or read all
settings, or whatever. But you can't say that having this role gives
you the ability to checkpointer. It gives you the ability to
checkpoint, or to perform a checkpoint.
So I think the name is inconsistent with our usual convention, and we
ought to fix it.
I was going to point out that pg_database_owner is the same way, but it is fundamentally different in that it has no special allowed access and is meant to be the target of permission grants rather than being granted to other roles.
+1 to rename it to pg_checkpoint or to some similar name.
On Thu, Jun 30, 2022 at 08:57:04AM -0400, Isaac Morland wrote: > I was going to point out that pg_database_owner is the same way, but it is > fundamentally different in that it has no special allowed access and is > meant to be the target of permission grants rather than being granted to > other roles. > > +1 to rename it to pg_checkpoint or to some similar name. We are still in beta, so, FWIW, I am fine to adjust this name even if it means an extra catversion bump. "checkpoint" is not a verb (right?), so would something like "pg_perform_checkpoint" rather than "pg_checkpoint" fit better in the larger picture? -- Michael
Attachment
On Thu, 30 Jun 2022 at 21:22, Michael Paquier <michael@paquier.xyz> wrote:
On Thu, Jun 30, 2022 at 08:57:04AM -0400, Isaac Morland wrote:
> I was going to point out that pg_database_owner is the same way, but it is
> fundamentally different in that it has no special allowed access and is
> meant to be the target of permission grants rather than being granted to
> other roles.
>
> +1 to rename it to pg_checkpoint or to some similar name.
We are still in beta, so, FWIW, I am fine to adjust this name even if
it means an extra catversion bump.
"checkpoint" is not a verb (right?), so would something like
"pg_perform_checkpoint" rather than "pg_checkpoint" fit better in the
larger picture?
I would argue it’s OK. In the Postgres context, I can imagine someone saying they’re going to checkpoint the database, and the actual command is just CHECKPOINT. Changing from checkpointer to checkpoint means that we’re describing the action rather than what a role member is.
If we are going to put a more standard verb in there, I would use execute rather than perform, because that is what the documentation says members of this role can do — “Allow executing the CHECKPOINT command”. Zooming out a little, I think we normally talk about executing commands rather than performing them, so this is consistent with those other uses; otherwise we should reconsider what the documentation itself says to match other commands that we talk about running.
OK, I think I’ve bikeshedded enough. I’m just happy to have all these new roles to avoid handing out full superuser access routinely.
On Fri, Jul 01, 2022 at 10:22:16AM +0900, Michael Paquier wrote: > On Thu, Jun 30, 2022 at 08:57:04AM -0400, Isaac Morland wrote: > > I was going to point out that pg_database_owner is the same way, but it is > > fundamentally different in that it has no special allowed access and is > > meant to be the target of permission grants rather than being granted to > > other roles. > > > > +1 to rename it to pg_checkpoint or to some similar name. > > We are still in beta, so, FWIW, I am fine to adjust this name even if > it means an extra catversion bump. > > "checkpoint" is not a verb (right?), so would something like Why not ? There's a *command* called "checkpoint". It is also a noun. -- Justin
On Thu, Jun 30, 2022 at 9:22 PM Michael Paquier <michael@paquier.xyz> wrote: > "checkpoint" is not a verb (right?), so would something like > "pg_perform_checkpoint" rather than "pg_checkpoint" fit better in the > larger picture? It's true that the dictionary describes checkpoint as a noun, but I think in a database context it is often used as a verb. One example is the CHECKPOINT command itself: command names are all verbs, and CHECKPOINT is a command name, so we're using it as a verb. Similarly I think you can talk about needing to checkpoint the database. Therefore I think pg_checkpoint is OK, and it has the advantage of being short. -- Robert Haas EDB: http://www.enterprisedb.com
On Fri, Jul 1, 2022 at 3:18 PM Robert Haas <robertmhaas@gmail.com> wrote:
On Thu, Jun 30, 2022 at 9:22 PM Michael Paquier <michael@paquier.xyz> wrote:
> "checkpoint" is not a verb (right?), so would something like
> "pg_perform_checkpoint" rather than "pg_checkpoint" fit better in the
> larger picture?
It's true that the dictionary describes checkpoint as a noun, but I
think in a database context it is often used as a verb. One example is
the CHECKPOINT command itself: command names are all verbs, and
CHECKPOINT is a command name, so we're using it as a verb. Similarly I
think you can talk about needing to checkpoint the database. Therefore
I think pg_checkpoint is OK, and it has the advantage of being short.
+1 for pg_checkpoint on that -- let's not make it longer than necessary.
And yes, +1 for actually changing it. It's a lot cheaper to change it now than it will be in the future. Yes, it would've been even cheaper to have already changed it, but we can't go back in time...
On Fri, Jul 01, 2022 at 03:36:48PM +0200, Magnus Hagander wrote: > +1 for pg_checkpoint on that -- let's not make it longer than necessary. > > And yes, +1 for actually changing it. It's a lot cheaper to change it now > than it will be in the future. Yes, it would've been even cheaper to have > already changed it, but we can't go back in time... Yeah, pg_checkpoint seems like the obvious alternative to pg_checkpointer. I didn't see a patch in this thread yet, so I've put one together. I did not include the catversion bump. -- Nathan Bossart Amazon Web Services: https://aws.amazon.com
Attachment
On Fri, Jul 1, 2022 at 5:50 PM Nathan Bossart <nathandbossart@gmail.com> wrote: > On Fri, Jul 01, 2022 at 03:36:48PM +0200, Magnus Hagander wrote: > > +1 for pg_checkpoint on that -- let's not make it longer than necessary. > > > > And yes, +1 for actually changing it. It's a lot cheaper to change it now > > than it will be in the future. Yes, it would've been even cheaper to have > > already changed it, but we can't go back in time... > > Yeah, pg_checkpoint seems like the obvious alternative to pg_checkpointer. > I didn't see a patch in this thread yet, so I've put one together. I did > not include the catversion bump. Hearing several votes in favor and none opposed, committed and back-patched to v15. I added the catversion bump, but left out the .po file changes, figuring it was better to let those files get updated via the normal process. -- Robert Haas EDB: http://www.enterprisedb.com
On Tue, Jul 05, 2022 at 01:38:43PM -0400, Robert Haas wrote: > Hearing several votes in favor and none opposed, committed and > back-patched to v15. Thanks. > I added the catversion bump, but left out the .po > file changes, figuring it was better to let those files get updated > via the normal process. I'll keep this in mind for future patches. The changes looked pretty obvious, so I wasn't sure whether to include it. -- Nathan Bossart Amazon Web Services: https://aws.amazon.com
On Tue, Jul 5, 2022 at 3:42 PM Nathan Bossart <nathandbossart@gmail.com> wrote: > On Tue, Jul 05, 2022 at 01:38:43PM -0400, Robert Haas wrote: > > Hearing several votes in favor and none opposed, committed and > > back-patched to v15. > > Thanks. > > > I added the catversion bump, but left out the .po > > file changes, figuring it was better to let those files get updated > > via the normal process. > > I'll keep this in mind for future patches. The changes looked pretty > obvious, so I wasn't sure whether to include it. I believe Peter periodically runs a script that bulk copies everything over from the translation repository. -- Robert Haas EDB: http://www.enterprisedb.com
Robert Haas <robertmhaas@gmail.com> writes:
> On Tue, Jul 5, 2022 at 3:42 PM Nathan Bossart <nathandbossart@gmail.com> wrote:
>> On Tue, Jul 05, 2022 at 01:38:43PM -0400, Robert Haas wrote:
>>> I added the catversion bump, but left out the .po
>>> file changes, figuring it was better to let those files get updated
>>> via the normal process.
>> I'll keep this in mind for future patches. The changes looked pretty
>> obvious, so I wasn't sure whether to include it.
> I believe Peter periodically runs a script that bulk copies everything
> over from the translation repository.
Indeed. If we did commit anything, it would just be wiped out in the
next bulk update. The authoritative versions of the .po files are in
the pgtranslation repo, not gitmaster.
regards, tom lane