Horton

Thread

Commits

  1. Rename pg_checkpointer predefined role to pg_checkpoint.

  1. pg_checkpointer is not a verb or verb phrase

    Robert Haas <robertmhaas@gmail.com> — 2022-06-30T12:48:32Z

    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

  2. Re: pg_checkpointer is not a verb or verb phrase

    Isaac Morland <isaac.morland@gmail.com> — 2022-06-30T12:57:04Z

    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.

  3. Re: pg_checkpointer is not a verb or verb phrase

    Michael Paquier <michael@paquier.xyz> — 2022-07-01T01:22:16Z

    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

  4. Re: pg_checkpointer is not a verb or verb phrase

    Isaac Morland <isaac.morland@gmail.com> — 2022-07-01T02:58:51Z

    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.

  5. Re: pg_checkpointer is not a verb or verb phrase

    Justin Pryzby <pryzby@telsasoft.com> — 2022-07-01T03:03:34Z

    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

  6. Re: pg_checkpointer is not a verb or verb phrase

    Robert Haas <robertmhaas@gmail.com> — 2022-07-01T13:18:12Z

    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

  7. Re: pg_checkpointer is not a verb or verb phrase

    Magnus Hagander <magnus@hagander.net> — 2022-07-01T13:36:48Z

    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...

-- 
 Magnus Hagander
 Me: https://www.hagander.net/ <http://www.hagander.net/>
 Work: https://www.redpill-linpro.com/ <http://www.redpill-linpro.com/>

  8. Re: pg_checkpointer is not a verb or verb phrase

    Nathan Bossart <nathandbossart@gmail.com> — 2022-07-01T21:50:54Z

    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

  9. Re: pg_checkpointer is not a verb or verb phrase

    Robert Haas <robertmhaas@gmail.com> — 2022-07-05T17:38:43Z

    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

  10. Re: pg_checkpointer is not a verb or verb phrase

    Nathan Bossart <nathandbossart@gmail.com> — 2022-07-05T19:42:13Z

    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

  11. Re: pg_checkpointer is not a verb or verb phrase

    Robert Haas <robertmhaas@gmail.com> — 2022-07-05T21:09:04Z

    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

  12. Re: pg_checkpointer is not a verb or verb phrase

    Tom Lane <tgl@sss.pgh.pa.us> — 2022-07-05T22:27:39Z

    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