createuser --memeber and PG 16

On 10 May 2023, at 19:33, Bruce Momjian <bruce@momjian.us> wrote:

I frankly don't
know why --role matching CREATE ROLE ... ROLE IN was not already
confusing in pre-PG 16. Any new ideas for improvement?

IIRC there were a number of ideas presented in that thread but backwards
compatibility with --role already "taken" made it complicated, so --role and
--member were the least bad options.

At a minium I would like to apply the attached doc patch to PG 16 to
improve awkward wording in CREATE ROLE and createuser.

No objection.

+ role. (This in effect makes the new role a <quote>group</quote>.)
While not introduced here, isn't the latter part interesting enough to warrant
not being inside parenthesis?

--
Daniel Gustafsson

On Thu, May 11, 2023 at 02:21:22PM +0200, Daniel Gustafsson wrote:

On 10 May 2023, at 19:33, Bruce Momjian <bruce@momjian.us> wrote:

I frankly don't
know why --role matching CREATE ROLE ... ROLE IN was not already
confusing in pre-PG 16. Any new ideas for improvement?

IIRC there were a number of ideas presented in that thread but backwards
compatibility with --role already "taken" made it complicated, so --role and
--member were the least bad options.

At a minimum I would like to apply the attached doc patch to PG 16 to
improve awkward wording in CREATE ROLE and createuser.

No objection.

+ role. (This in effect makes the new role a <quote>group</quote>.)
While not introduced here, isn't the latter part interesting enough to warrant
not being inside parenthesis?

The concept of group itself is deprecated, which I think is why the
parenthesis are used.

--
Bruce Momjian <bruce@momjian.us> https://momjian.us
EDB https://enterprisedb.com

Embrace your flaws. They make you human, rather than perfect,
which you will never be.

On Wed, May 10, 2023 at 1:33 PM Bruce Momjian <bruce@momjian.us> wrote:

This seems like it will be forever confusing to people. I frankly don't
know why --role matching CREATE ROLE ... ROLE IN was not already
confusing in pre-PG 16. Any new ideas for improvement?

Yeah, it's a bad situation. I think --role is basically misnamed.
Something like --add-to-group would have been clearer, but that also
has the problem of being inconsistent with the SQL command. The whole
ROLE vs. IN ROLE thing is inherently quite confusing, I think. It's
very easy to get confused about which direction the membership arrows
are pointing.

--
Robert Haas
EDB: http://www.enterprisedb.com

On 11.05.23 16:07, Robert Haas wrote:

On Wed, May 10, 2023 at 1:33 PM Bruce Momjian <bruce@momjian.us> wrote:

This seems like it will be forever confusing to people. I frankly don't
know why --role matching CREATE ROLE ... ROLE IN was not already
confusing in pre-PG 16. Any new ideas for improvement?

Yeah, it's a bad situation. I think --role is basically misnamed.
Something like --add-to-group would have been clearer, but that also
has the problem of being inconsistent with the SQL command. The whole
ROLE vs. IN ROLE thing is inherently quite confusing, I think. It's
very easy to get confused about which direction the membership arrows
are pointing.

It's hard to tell that for the --member option as well. For

createuser foo --member bar

it's not intuitive whether foo becomes a member of bar or bar becomes a
member of foo. Maybe something more verbose like --member-of would help?

On Fri, May 12, 2023 at 04:35:34PM +0200, Peter Eisentraut wrote:

it's not intuitive whether foo becomes a member of bar or bar becomes a
member of foo. Maybe something more verbose like --member-of would help?

Indeed, presented like that it could be confusing, and --member-of
sounds like it could be a good idea instead of --member.
--
Michael

On Thu, May 11, 2023 at 09:34:42AM -0400, Bruce Momjian wrote:

On Thu, May 11, 2023 at 02:21:22PM +0200, Daniel Gustafsson wrote:

IIRC there were a number of ideas presented in that thread but backwards
compatibility with --role already "taken" made it complicated, so --role and
--member were the least bad options.

At a minimum I would like to apply the attached doc patch to PG 16 to
improve awkward wording in CREATE ROLE and createuser.

No objection.

None from here as well.

+ role. (This in effect makes the new role a <quote>group</quote>.)
While not introduced here, isn't the latter part interesting enough to warrant
not being inside parenthesis?

The concept of group itself is deprecated, which I think is why the
parenthesis are used.

Not sure on this one. The original docs come from 58d214e, and this
sentence was already in there.
--
Michael

On Mon, May 15, 2023 at 04:33:27PM +0900, Michael Paquier wrote:

On Thu, May 11, 2023 at 09:34:42AM -0400, Bruce Momjian wrote:

On Thu, May 11, 2023 at 02:21:22PM +0200, Daniel Gustafsson wrote:

IIRC there were a number of ideas presented in that thread but backwards
compatibility with --role already "taken" made it complicated, so --role and
--member were the least bad options.

At a minimum I would like to apply the attached doc patch to PG 16 to
improve awkward wording in CREATE ROLE and createuser.

No objection.

None from here as well.

+ role. (This in effect makes the new role a <quote>group</quote>.)
While not introduced here, isn't the latter part interesting enough to warrant
not being inside parenthesis?

The concept of group itself is deprecated, which I think is why the
parenthesis are used.

Not sure on this one. The original docs come from 58d214e, and this
sentence was already in there.

True. I have removed the parenthesis in this updated patch.

--
Bruce Momjian <bruce@momjian.us> https://momjian.us
EDB https://enterprisedb.com

Embrace your flaws. They make you human, rather than perfect,
which you will never be.

On Mon, May 15, 2023 at 04:27:04PM +0900, Michael Paquier wrote:

On Fri, May 12, 2023 at 04:35:34PM +0200, Peter Eisentraut wrote:

it's not intuitive whether foo becomes a member of bar or bar becomes a
member of foo. Maybe something more verbose like --member-of would help?

Indeed, presented like that it could be confusing, and --member-of
sounds like it could be a good idea instead of --member.

--member specifieѕ an existing role that will be given membership to the
new role (i.e., GRANT newrole TO existingrole). IMO --member-of sounds
like the new role will be given membership to the specified existing role
(i.e., GRANT existingrole TO newrole). IOW a command like

createuser newrole --member-of existingrole

would make existingrole a "member of" newrole according to \du. Perhaps
--role should be --member-of because it makes the new role a member of the
existing role.

--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com

On Wed, May 10, 2023 at 01:33:26PM -0400, Bruce Momjian wrote:

In writing the PG 16 release notes, I came upon an oddity in our new
createuser syntax, specifically --role and --member. It turns out that
--role matches CREATE ROLE ... ROLE IN (and has prior to PG 16) while
the new --member option matches CREATE ROLE ... ROLE. The PG 16 feature
discussion thread is here:

/messages/by-id/69a9851035cf0f0477bcc5d742b031a3@oss.nttdata.com

This seems like it will be forever confusing to people. I frankly don't
know why --role matching CREATE ROLE ... ROLE IN was not already
confusing in pre-PG 16. Any new ideas for improvement?

At a minium I would like to apply the attached doc patch to PG 16 to
improve awkward wording in CREATE ROLE and createuser.

Patch applied.

--
Bruce Momjian <bruce@momjian.us> https://momjian.us
EDB https://enterprisedb.com

Only you can decide what is important to you.

On Thu, May 18, 2023 at 10:22:32PM -0400, Bruce Momjian wrote:

Patch applied.

Thanks, Bruce.
--
Michael

On 15.05.23 22:11, Nathan Bossart wrote:

On Mon, May 15, 2023 at 04:27:04PM +0900, Michael Paquier wrote:

On Fri, May 12, 2023 at 04:35:34PM +0200, Peter Eisentraut wrote:

it's not intuitive whether foo becomes a member of bar or bar becomes a
member of foo. Maybe something more verbose like --member-of would help?

Indeed, presented like that it could be confusing, and --member-of
sounds like it could be a good idea instead of --member.

--member specifieѕ an existing role that will be given membership to the
new role (i.e., GRANT newrole TO existingrole). IMO --member-of sounds
like the new role will be given membership to the specified existing role
(i.e., GRANT existingrole TO newrole). IOW a command like

createuser newrole --member-of existingrole

would make existingrole a "member of" newrole according to \du. Perhaps
--role should be --member-of because it makes the new role a member of the
existing role.

Yeah, that's exactly my confusion.

Maybe

createuser --with-members

and

createuser --member-of

would be clearer.

On Sun, May 21, 2023 at 08:00:15AM +0200, Peter Eisentraut wrote:

Maybe

createuser --with-members

and

createuser --member-of

would be clearer.

Those seem like reasonable choices to me. I suspect we'll want to keep
--role around for backward compatibility.

--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com

On Sun, May 21, 2023 at 07:44:49AM -0700, Nathan Bossart wrote:

On Sun, May 21, 2023 at 08:00:15AM +0200, Peter Eisentraut wrote:

Maybe

createuser --with-members

and

createuser --member-of

would be clearer.

Those seem like reasonable choices to me. I suspect we'll want to keep
--role around for backward compatibility.

I've attached a draft patch for this. I also changed --admin to
--with-admin.

--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com

On Sun, May 21, 2023 at 08:22:05AM -0700, Nathan Bossart wrote:

On Sun, May 21, 2023 at 07:44:49AM -0700, Nathan Bossart wrote:

On Sun, May 21, 2023 at 08:00:15AM +0200, Peter Eisentraut wrote:

Maybe

createuser --with-members

and

createuser --member-of

would be clearer.

Those seem like reasonable choices to me. I suspect we'll want to keep
--role around for backward compatibility.

I've attached a draft patch for this. I also changed --admin to
--with-admin.

If we want to go forward with this, the big question is whether we want
to get this in before beta1. FYI, the release notes don't mention the
option names.

--
Bruce Momjian <bruce@momjian.us> https://momjian.us
EDB https://enterprisedb.com

Only you can decide what is important to you.

Bruce Momjian <bruce@momjian.us> writes:

On Sun, May 21, 2023 at 08:22:05AM -0700, Nathan Bossart wrote:

I've attached a draft patch for this. I also changed --admin to
--with-admin.

If we want to go forward with this, the big question is whether we want
to get this in before beta1. FYI, the release notes don't mention the
option names.

+1 for doing it before beta1.

A few comments on the patch:

Indicates an existing role that will be automatically added as a member of the new

"Specifies" would be clearer than "indicates" (not your fault, but
let's avoid the passive construction while we are here). Likewise
nearby.

+ {"member-of", required_argument, NULL, 6},

Why didn't you just translate this as 'g' instead of inventing
a new switch case?

-	printf(_("  -a, --admin=ROLE          this role will be a member of new role with admin\n"
+	printf(_("  -a, --with-admin=ROLE     this role will be a member of new role with admin\n"

I think clearer would be

+ printf(_(" -a, --with-admin=ROLE ROLE will be a member of new role with admin\n"

Likewise

+ printf(_(" -g, --member-of=ROLE new role will be a member of ROLE\n"));

(I assume that's what this should say, it's backwards ATM)
and

+ printf(_(" -m, --with-member=ROLE ROLE will be a member of new role\n"));

regards, tom lane

On Sun, May 21, 2023 at 11:45:24AM -0400, Tom Lane wrote:

A few comments on the patch:

Thanks for taking a look.

Indicates an existing role that will be automatically added as a member of the new

"Specifies" would be clearer than "indicates" (not your fault, but
let's avoid the passive construction while we are here). Likewise
nearby.

Fixed.

+ {"member-of", required_argument, NULL, 6},

Why didn't you just translate this as 'g' instead of inventing
a new switch case?

Fixed. *facepalm*

I think clearer would be

+ printf(_(" -a, --with-admin=ROLE ROLE will be a member of new role with admin\n"

Likewise

+ printf(_(" -g, --member-of=ROLE new role will be a member of ROLE\n"));

(I assume that's what this should say, it's backwards ATM)
and

+ printf(_(" -m, --with-member=ROLE ROLE will be a member of new role\n"));

Fixed.

How do folks feel about keeping --role undocumented? Should we give it a
mention in the docs for --member-of?

--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com

Nathan Bossart <nathandbossart@gmail.com> writes:

Fixed.

v2 looks good to me, except the documentation wording for --with-role
is needlessly inconsistent with --with-admin. The --with-admin
wording looks better, so I suggest

-        Indicates the specified existing role should be automatically
+        Specifies an existing role that will be automatically
         added as a member of the new role. Multiple existing roles can

How do folks feel about keeping --role undocumented? Should we give it a
mention in the docs for --member-of?

I'm okay with leaving it undocumented, but I won't fight about it
if somebody wants to argue for the other.

regards, tom lane

On Sun, May 21, 2023 at 01:20:01PM -0400, Tom Lane wrote:

Nathan Bossart <nathandbossart@gmail.com> writes:

Fixed.

v2 looks good to me, except the documentation wording for --with-role
is needlessly inconsistent with --with-admin. The --with-admin
wording looks better, so I suggest

-        Indicates the specified existing role should be automatically
+        Specifies an existing role that will be automatically
added as a member of the new role. Multiple existing roles can

Will do.

How do folks feel about keeping --role undocumented? Should we give it a
mention in the docs for --member-of?

I'm okay with leaving it undocumented, but I won't fight about it
if somebody wants to argue for the other.

Alright. Barring any additional feedback, I'll commit this tonight.

--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com

On Sun, May 21, 2023 at 12:16:58PM -0700, Nathan Bossart wrote:

On Sun, May 21, 2023 at 01:20:01PM -0400, Tom Lane wrote:

Nathan Bossart <nathandbossart@gmail.com> writes:

How do folks feel about keeping --role undocumented? Should we give it a
mention in the docs for --member-of?

I'm okay with leaving it undocumented, but I won't fight about it
if somebody wants to argue for the other.

Alright. Barring any additional feedback, I'll commit this tonight.

v2 passes the eye test, and I am not spotting any references to the
past option names. Thanks!

+$node->issues_sql_like(
+   [ 'createuser', 'regress_user11', '--role', 'regress_user1' ],
+   qr/statement: CREATE ROLE regress_user11 NOSUPERUSER NOCREATEDB NOCREATEROLE INHERIT LOGIN NOREPLICATION NOBYPASSRLS IN ROLE regress_user1;/,
+   '--role (for backward compatibility)');

Not sure I would have kept this test, still that's cheap enough to
test.
--
Michael