Re: [HACKERS] Extra functionality to createuser
On Wed, Dec 11, 2013 at 7:53 AM, Robert Haas robertmh...@gmail.com wrote: On Tue, Dec 10, 2013 at 9:55 AM, Amit Kapila amit.kapil...@gmail.com wrote: On Tue, Dec 10, 2013 at 12:20 AM, Robert Haas robertmh...@gmail.com wrote: On Sat, Dec 7, 2013 at 11:39 PM, Amit Kapila amit.kapil...@gmail.com wrote: On Fri, Dec 6, 2013 at 10:31 AM, Peter Eisentraut pete...@gmx.net wrote: How about only one role name per -g option, but allowing the -g option to be repeated? I think that might simplify the problem and patch, but do you think it is okay to have inconsistency for usage of options between Create User statement and this utility? Yes. In general, command-line utilities use a very different syntax for options-passing that SQL commands. Trying to make them consistent feels unnecessary or perhaps even counterproductive. And the proposed syntax is certainly a convention common to many other command-line utilities, so I think it's fine. Okay, the new way for syntax suggested by Peter has simplified the problem. Please find the updated patch and docs for multiple -g options. Committed. Looks good, thanks! -- When confronted by a difficult problem, solve it by reducing it to the question, How would the Lone Ranger handle this? -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Extra functionality to createuser
On Tue, Dec 10, 2013 at 9:55 AM, Amit Kapila amit.kapil...@gmail.com wrote: On Tue, Dec 10, 2013 at 12:20 AM, Robert Haas robertmh...@gmail.com wrote: On Sat, Dec 7, 2013 at 11:39 PM, Amit Kapila amit.kapil...@gmail.com wrote: On Fri, Dec 6, 2013 at 10:31 AM, Peter Eisentraut pete...@gmx.net wrote: How about only one role name per -g option, but allowing the -g option to be repeated? I think that might simplify the problem and patch, but do you think it is okay to have inconsistency for usage of options between Create User statement and this utility? Yes. In general, command-line utilities use a very different syntax for options-passing that SQL commands. Trying to make them consistent feels unnecessary or perhaps even counterproductive. And the proposed syntax is certainly a convention common to many other command-line utilities, so I think it's fine. Okay, the new way for syntax suggested by Peter has simplified the problem. Please find the updated patch and docs for multiple -g options. Committed. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Extra functionality to createuser
On Wed, Dec 11, 2013 at 6:23 PM, Robert Haas robertmh...@gmail.com wrote: On Tue, Dec 10, 2013 at 9:55 AM, Amit Kapila amit.kapil...@gmail.com wrote: Okay, the new way for syntax suggested by Peter has simplified the problem. Please find the updated patch and docs for multiple -g options. Committed. Thank you. With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Extra functionality to createuser
On Tue, Dec 10, 2013 at 12:20 AM, Robert Haas robertmh...@gmail.com wrote:
On Sat, Dec 7, 2013 at 11:39 PM, Amit Kapila amit.kapil...@gmail.com wrote:
On Fri, Dec 6, 2013 at 10:31 AM, Peter Eisentraut pete...@gmx.net wrote:
How about only one role name per -g option, but allowing the -g option
to be repeated?
I think that might simplify the problem and patch, but do you think
it is okay to have inconsistency
for usage of options between Create User statement and this utility?
Yes. In general, command-line utilities use a very different syntax
for options-passing that SQL commands. Trying to make them consistent
feels unnecessary or perhaps even counterproductive. And the proposed
syntax is certainly a convention common to many other command-line
utilities, so I think it's fine.
Okay, the new way for syntax suggested by Peter has simplified the problem.
Please find the updated patch and docs for multiple -g options.
If there are no objections, then I will mark this patch as Ready For Committer.
Christopher, please check once, if you have any comments/objections
for modifications.
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com
diff --git a/doc/src/sgml/ref/createuser.sgml b/doc/src/sgml/ref/createuser.sgml
index 2f1ea2f..63d4c6c 100644
--- a/doc/src/sgml/ref/createuser.sgml
+++ b/doc/src/sgml/ref/createuser.sgml
@@ -131,6 +131,19 @@ PostgreSQL documentation
/varlistentry
varlistentry
+ termoption-g replaceable
class=parameterrole/replaceable//term
+ termoption--role=replaceable
class=parameterrole/replaceable//term
+ listitem
+ para
+ Indicates role to which this role will be added immediately as a new
+ member. Multiple roles to which this role will be added as a member
+ can be specified by writing multiple
+ option-g/ switches.
+ /para
+ /listitem
+ /varlistentry
+
+ varlistentry
termoption-i//term
termoption--inherit//term
listitem
diff --git a/src/bin/scripts/createuser.c b/src/bin/scripts/createuser.c
index 83623ea..d98b420 100644
--- a/src/bin/scripts/createuser.c
+++ b/src/bin/scripts/createuser.c
@@ -24,6 +24,7 @@ main(int argc, char *argv[])
{host, required_argument, NULL, 'h'},
{port, required_argument, NULL, 'p'},
{username, required_argument, NULL, 'U'},
+ {role, required_argument, NULL, 'g'},
{no-password, no_argument, NULL, 'w'},
{password, no_argument, NULL, 'W'},
{echo, no_argument, NULL, 'e'},
@@ -57,6 +58,7 @@ main(int argc, char *argv[])
char *host = NULL;
char *port = NULL;
char *username = NULL;
+ SimpleStringList roles = {NULL, NULL};
enum trivalue prompt_password = TRI_DEFAULT;
boolecho = false;
boolinteractive = false;
@@ -83,7 +85,7 @@ main(int argc, char *argv[])
handle_help_version_opts(argc, argv, createuser, help);
- while ((c = getopt_long(argc, argv, h:p:U:wWedDsSaArRiIlLc:PEN,
+ while ((c = getopt_long(argc, argv, h:p:U:g:wWedDsSaArRiIlLc:PEN,
long_options,
optindex)) != -1)
{
switch (c)
@@ -97,6 +99,9 @@ main(int argc, char *argv[])
case 'U':
username = pg_strdup(optarg);
break;
+ case 'g':
+ simple_string_list_append(roles, optarg);
+ break;
case 'w':
prompt_password = TRI_NO;
break;
@@ -302,6 +307,19 @@ main(int argc, char *argv[])
appendPQExpBufferStr(sql, NOREPLICATION);
if (conn_limit != NULL)
appendPQExpBuffer(sql, CONNECTION LIMIT %s, conn_limit);
+ if (roles.head != NULL)
+ {
+ SimpleStringListCell *cell;
+ appendPQExpBufferStr(sql, IN ROLE );
+
+ for (cell = roles.head; cell; cell = cell-next)
+ {
+ if (cell-next)
+ appendPQExpBuffer(sql, %s,,
fmtId(cell-val));
+ else
+ appendPQExpBuffer(sql, %s, fmtId(cell-val));
+ }
+ }
appendPQExpBufferStr(sql, ;\n);
if (echo)
@@ -334,6 +352,7 @@ help(const char *progname)
printf(_( -D, --no-createdb role cannot create databases
(default)\n));
printf(_( -e, --echoshow the commands being sent to
the server\n));
printf(_( -E, --encrypted encrypt stored password\n));
+ printf(_( -g, --role=ROLE role(s) to associate with this
new role\n));
printf(_( -i, --inherit role
Re: [HACKERS] Extra functionality to createuser
On Sat, Dec 7, 2013 at 11:39 PM, Amit Kapila amit.kapil...@gmail.com wrote: On Fri, Dec 6, 2013 at 10:31 AM, Peter Eisentraut pete...@gmx.net wrote: On Wed, 2013-11-20 at 11:23 -0500, Christopher Browne wrote: I note that similar (with not quite identical behaviour) issues apply to the user name. Perhaps the resolution to this is to leave quoting issues to the administrator. That simplifies the problem away. How about only one role name per -g option, but allowing the -g option to be repeated? I think that might simplify the problem and patch, but do you think it is okay to have inconsistency for usage of options between Create User statement and this utility? Yes. In general, command-line utilities use a very different syntax for options-passing that SQL commands. Trying to make them consistent feels unnecessary or perhaps even counterproductive. And the proposed syntax is certainly a convention common to many other command-line utilities, so I think it's fine. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Extra functionality to createuser
On Fri, Dec 6, 2013 at 10:31 AM, Peter Eisentraut pete...@gmx.net wrote: On Wed, 2013-11-20 at 11:23 -0500, Christopher Browne wrote: I note that similar (with not quite identical behaviour) issues apply to the user name. Perhaps the resolution to this is to leave quoting issues to the administrator. That simplifies the problem away. How about only one role name per -g option, but allowing the -g option to be repeated? I think that might simplify the problem and patch, but do you think it is okay to have inconsistency for usage of options between Create User statement and this utility? With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Extra functionality to createuser
On Wed, 2013-11-20 at 11:23 -0500, Christopher Browne wrote: I note that similar (with not quite identical behaviour) issues apply to the user name. Perhaps the resolution to this is to leave quoting issues to the administrator. That simplifies the problem away. How about only one role name per -g option, but allowing the -g option to be repeated? -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Extra functionality to createuser
On Tue, Nov 19, 2013 at 11:54 PM, Amit Kapila amit.kapil...@gmail.com wrote:
On further tests, I found inconsistency in behavior when some special
characters are used in role names.
1. Test for role name containing quotes
a. In psql, create a role containing quotes in role name.
create role amitk in role test_ro'le_3;
b. Now if we try to make a new role member of this role using
createuser utility, it gives error
try-1
createuser.exe -g test_ro'le_3 -p 5446 amitk_2
createuser: creation of new role failed: ERROR: unterminated quoted
string at or near 'le_3;
LINE 1: ... NOCREATEDB NOCREATEROLE INHERIT LOGIN IN ROLE test_ro'le_3;
try-2
createuser.exe -g test_ro'le_3 -p 5446 amitk
createuser: creation of new role failed: ERROR: unterminated quoted
string at or near 'le_3;
LINE 1: ... NOCREATEDB NOCREATEROLE INHERIT LOGIN IN ROLE test_ro'le_3;
c. If I try quoted string in new role to be created, it works fine.
createuser.exe -p 5446 am'itk_2
As quoted strings work well for role names, I think it should work
with -g option as well.
2. Test for role name containing special character ';' (semicolon)
a. create role test;_1;
b. Now if we try to make a new role member of this role using
createuser utility, it gives error
try-1
createuser.exe -g test;_1 -p 5446 amitk_4
createuser: creation of new role failed: ERROR: syntax error at or near _1
LINE 1: ...RUSER NOCREATEDB NOCREATEROLE INHERIT LOGIN IN ROLE test;_1;
try-2^
createuser.exe -g test;_1 -p 5446 amitk_4
createuser: creation of new role failed: ERROR: syntax error at or near _1
LINE 1: ...RUSER NOCREATEDB NOCREATEROLE INHERIT LOGIN IN ROLE test;_1;
^
try-3
createuser.exe -g 'test;_1' -p 5446 amitk_4
createuser: creation of new role failed: ERROR: syntax error at or
near 'test;_1'
LINE 1: ...SER NOCREATEDB NOCREATEROLE INHERIT LOGIN IN ROLE 'test;_1';
c. If I try semicolon in new role to be created, it works fine.
createuser.exe -p 5446 amit;k_3
As semicolon work well for role names, I think it should work with -g
option as well.
I was not unconscious of there being the potential for issue here; there is an
easy answer of double quoting the string, thus:
diff --git a/src/bin/scripts/createuser.c b/src/bin/scripts/createuser.c
index 88b8f2a..04ec324 100644
--- a/src/bin/scripts/createuser.c
+++ b/src/bin/scripts/createuser.c
@@ -308,7 +308,7 @@ main(int argc, char *argv[])
if (conn_limit != NULL)
appendPQExpBuffer(sql, CONNECTION LIMIT %s, conn_limit);
if (roles != NULL)
- appendPQExpBuffer(sql, IN ROLE %s, roles);
+ appendPQExpBuffer(sql, IN ROLE \%s\, roles);
appendPQExpBufferStr(sql, ;\n);
if (echo)
(END)
I was conscious of not quoting it. Note that other parameters are not quoted
either, so I imagined I was being consistent with that.
I have added the above change, as well as rebasing, per Peter's recommendation.
--
When confronted by a difficult problem, solve it by reducing it to the
question, How would the Lone Ranger handle this?
diff --git a/doc/src/sgml/ref/createuser.sgml b/doc/src/sgml/ref/createuser.sgml
index 2f1ea2f..5a38d2e 100644
--- a/doc/src/sgml/ref/createuser.sgml
+++ b/doc/src/sgml/ref/createuser.sgml
@@ -131,6 +131,16 @@ PostgreSQL documentation
/varlistentry
varlistentry
+ termoption-g replaceable
class=parameterroles/replaceable//term
+ termoption--roles=replaceable
class=parameterroles/replaceable//term
+ listitem
+ para
+Indicates roles to which this role will be added immediately as a new
member.
+ /para
+ /listitem
+ /varlistentry
+
+ varlistentry
termoption-i//term
termoption--inherit//term
listitem
diff --git a/src/bin/scripts/createuser.c b/src/bin/scripts/createuser.c
index 83623ea..04ec324 100644
--- a/src/bin/scripts/createuser.c
+++ b/src/bin/scripts/createuser.c
@@ -24,6 +24,7 @@ main(int argc, char *argv[])
{host, required_argument, NULL, 'h'},
{port, required_argument, NULL, 'p'},
{username, required_argument, NULL, 'U'},
+ {roles, required_argument, NULL, 'g'},
{no-password, no_argument, NULL, 'w'},
{password, no_argument, NULL, 'W'},
{echo, no_argument, NULL, 'e'},
@@ -57,6 +58,7 @@ main(int argc, char *argv[])
char *host = NULL;
char *port = NULL;
char *username = NULL;
+ char *roles = NULL;
enum trivalue prompt_password = TRI_DEFAULT;
boolecho = false;
boolinteractive = false;
@@ -83,7 +85,7 @@ main(int argc, char *argv[])
handle_help_version_opts(argc, argv, createuser, help);
- while ((c =
Re: [HACKERS] Extra functionality to createuser
Wait, that doesn't work if more than one role is added, as they get merged together by the quoting. A somewhat ugly amount of quoting can be done at the shell level to induce double quotes. $ createuser -g \test_rol'e_3\ usequoted3 I note that similar (with not quite identical behaviour) issues apply to the user name. Perhaps the resolution to this is to leave quoting issues to the administrator. That simplifies the problem away. I suspect that the apparatus needed to do a thorough solution (e.g. - parse the string, and do something smarter) may be larger than is worth getting into. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Extra functionality to createuser
On Wed, Nov 20, 2013 at 9:53 PM, Christopher Browne cbbro...@gmail.com wrote: Wait, that doesn't work if more than one role is added, as they get merged together by the quoting. A somewhat ugly amount of quoting can be done at the shell level to induce double quotes. $ createuser -g \test_rol'e_3\ usequoted3 I note that similar (with not quite identical behaviour) issues apply to the user name. Perhaps the resolution to this is to leave quoting issues to the administrator. That simplifies the problem away. We are already doing something similar for username, refer below line: printfPQExpBuffer(sql, CREATE ROLE %s, fmtId(newuser)); Here fmtId() is doing handling for quotes. Now for new syntax IN ROLE, the difference is that it can have multiple strings which needs additional handling. I think some similar handling should be there in server, pg_dump as well. I suspect that the apparatus needed to do a thorough solution (e.g. - parse the string, and do something smarter) may be larger than is worth getting into. I think if it needs some bigger solution then we can leave it by having small note in documentation, but if it's just a matter of calling some existing functions or mimic some handling done at other place, then it is worth trying. With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Extra functionality to createuser
On Mon, Nov 18, 2013 at 1:01 AM, Amit Kapila amit.kapil...@gmail.com wrote:
On Sat, Nov 16, 2013 at 4:57 AM, Christopher Browne cbbro...@gmail.com
wrote:
Few comments:
1.
+ termoption-g//term
+ termoption--roles//term
All other options which require argument are of form:
termoption-c replaceable
class=parameternumber/replaceable//term
termoption--connection-limit=replaceable
class=parameternumber/replaceable//term
So I think it is better to have this new option which require argument
in similar form.
Sounds good, done.
2.
+Indicates roles to associate with this role.
I think word associate is not very clear, wouldn't it be better to
mention that this new role will be member of roles specified.
For example:
Indicates roles to which the new role will be immediately added as a new
member.
With a switch of immediately and added, done. That does better
describe the behaviour.
3.
+ case 'g':
+ roles = pg_strdup(optarg);
+ break;
If we see most of other options in case handling are ordered as per
their order in long_options array. For example
static struct option long_options[] = {
{host, required_argument, NULL, 'h'},
{port, required_argument, NULL, 'p'},
..
Now the order of handling for both is same in switch case or while get
opt_long() function call. I think this makes code easy to understand
and modify.
However there is no functionality issue here, so you can keep the code
as per your existing patch as well, this is just a suggestion.
That is easy enough to change, and yes, indeed, having the new code
look just like what it is near seems an improvement.
I picked the location of the 'g:' in the opt_long() call basically arbitrarily;
if there is any reason for it to go in a different spot, I'd be happy to
shift it.
--
When confronted by a difficult problem, solve it by reducing it to the
question, How would the Lone Ranger handle this?
diff --git a/doc/src/sgml/ref/createuser.sgml b/doc/src/sgml/ref/createuser.sgml
index 2f1ea2f..5a38d2e 100644
--- a/doc/src/sgml/ref/createuser.sgml
+++ b/doc/src/sgml/ref/createuser.sgml
@@ -131,6 +131,16 @@ PostgreSQL documentation
/varlistentry
varlistentry
+ termoption-g replaceable
class=parameterroles/replaceable//term
+ termoption--roles=replaceable
class=parameterroles/replaceable//term
+ listitem
+ para
+Indicates roles to which this role will be added immediately as a new
member.
+ /para
+ /listitem
+ /varlistentry
+
+ varlistentry
termoption-i//term
termoption--inherit//term
listitem
diff --git a/src/bin/scripts/createuser.c b/src/bin/scripts/createuser.c
index d1542d9..ecbb21c 100644
--- a/src/bin/scripts/createuser.c
+++ b/src/bin/scripts/createuser.c
@@ -24,6 +24,7 @@ main(int argc, char *argv[])
{host, required_argument, NULL, 'h'},
{port, required_argument, NULL, 'p'},
{username, required_argument, NULL, 'U'},
+ {roles, required_argument, NULL, 'g'},
{no-password, no_argument, NULL, 'w'},
{password, no_argument, NULL, 'W'},
{echo, no_argument, NULL, 'e'},
@@ -57,6 +58,7 @@ main(int argc, char *argv[])
char *host = NULL;
char *port = NULL;
char *username = NULL;
+ char *roles = NULL;
enum trivalue prompt_password = TRI_DEFAULT;
boolecho = false;
boolinteractive = false;
@@ -83,7 +85,7 @@ main(int argc, char *argv[])
handle_help_version_opts(argc, argv, createuser, help);
- while ((c = getopt_long(argc, argv, h:p:U:wWedDsSaArRiIlLc:PEN,
+ while ((c = getopt_long(argc, argv, h:p:U:g:wWedDsSaArRiIlLc:PEN,
long_options,
optindex)) != -1)
{
switch (c)
@@ -97,6 +99,9 @@ main(int argc, char *argv[])
case 'U':
username = pg_strdup(optarg);
break;
+ case 'g':
+ roles = pg_strdup(optarg);
+ break;
case 'w':
prompt_password = TRI_NO;
break;
@@ -302,6 +307,8 @@ main(int argc, char *argv[])
appendPQExpBuffer(sql, NOREPLICATION);
if (conn_limit != NULL)
appendPQExpBuffer(sql, CONNECTION LIMIT %s, conn_limit);
+ if (roles != NULL)
+ appendPQExpBuffer(sql, IN ROLE %s, roles);
appendPQExpBuffer(sql, ;\n);
if (echo)
@@ -334,6 +341,7 @@ help(const char *progname)
printf(_( -D, --no-createdb role cannot create databases
(default)\n));
printf(_( -e, --echoshow the commands being sent to
the server\n));
printf(_( -E,
Re: [HACKERS] Extra functionality to createuser
Patch needs to be rebased again. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Extra functionality to createuser
On Wed, Nov 20, 2013 at 2:05 AM, Christopher Browne cbbro...@gmail.com wrote: On Mon, Nov 18, 2013 at 1:01 AM, Amit Kapila amit.kapil...@gmail.com wrote: On Sat, Nov 16, 2013 at 4:57 AM, Christopher Browne cbbro...@gmail.com wrote: I picked the location of the 'g:' in the opt_long() call basically arbitrarily; I think this is okay, the main point was to maintain consistency with surrounding code. if there is any reason for it to go in a different spot, I'd be happy to shift it. The only other option could be to add it at end which is generally better unless we get any benefit by adding it in middle. On further tests, I found inconsistency in behavior when some special characters are used in role names. 1. Test for role name containing quotes a. In psql, create a role containing quotes in role name. create role amitk in role test_ro'le_3; b. Now if we try to make a new role member of this role using createuser utility, it gives error try-1 createuser.exe -g test_ro'le_3 -p 5446 amitk_2 createuser: creation of new role failed: ERROR: unterminated quoted string at or near 'le_3; LINE 1: ... NOCREATEDB NOCREATEROLE INHERIT LOGIN IN ROLE test_ro'le_3; try-2 createuser.exe -g test_ro'le_3 -p 5446 amitk createuser: creation of new role failed: ERROR: unterminated quoted string at or near 'le_3; LINE 1: ... NOCREATEDB NOCREATEROLE INHERIT LOGIN IN ROLE test_ro'le_3; c. If I try quoted string in new role to be created, it works fine. createuser.exe -p 5446 am'itk_2 As quoted strings work well for role names, I think it should work with -g option as well. 2. Test for role name containing special character ';' (semicolon) a. create role test;_1; b. Now if we try to make a new role member of this role using createuser utility, it gives error try-1 createuser.exe -g test;_1 -p 5446 amitk_4 createuser: creation of new role failed: ERROR: syntax error at or near _1 LINE 1: ...RUSER NOCREATEDB NOCREATEROLE INHERIT LOGIN IN ROLE test;_1; try-2^ createuser.exe -g test;_1 -p 5446 amitk_4 createuser: creation of new role failed: ERROR: syntax error at or near _1 LINE 1: ...RUSER NOCREATEDB NOCREATEROLE INHERIT LOGIN IN ROLE test;_1; ^ try-3 createuser.exe -g 'test;_1' -p 5446 amitk_4 createuser: creation of new role failed: ERROR: syntax error at or near 'test;_1' LINE 1: ...SER NOCREATEDB NOCREATEROLE INHERIT LOGIN IN ROLE 'test;_1'; c. If I try semicolon in new role to be created, it works fine. createuser.exe -p 5446 amit;k_3 As semicolon work well for role names, I think it should work with -g option as well. With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Extra functionality to createuser
On Sat, Nov 16, 2013 at 4:57 AM, Christopher Browne cbbro...@gmail.com wrote:
On Fri, Nov 15, 2013 at 3:14 PM, Peter Eisentraut pete...@gmx.net wrote:
On 11/14/13, 4:35 PM, Christopher Browne wrote: On Thu, Nov 14, 2013 at
5:41 AM, Sameer Thakur samthaku...@gmail.com wrote:
So i think -g option is failing
Right you are.
This patch adds useful option '-g' to createuser utility which will
allow user to make new roles as member of existing roles and the same
is already possible by Create Role/User syntax.
Few comments:
1.
+ termoption-g//term
+ termoption--roles//term
All other options which require argument are of form:
termoption-c replaceable class=parameternumber/replaceable//term
termoption--connection-limit=replaceable
class=parameternumber/replaceable//term
So I think it is better to have this new option which require argument
in similar form.
2.
+Indicates roles to associate with this role.
I think word associate is not very clear, wouldn't it be better to
mention that this new role will be member of roles specified.
For example:
Indicates roles to which the new role will be immediately added as a new member.
3.
+ case 'g':
+ roles = pg_strdup(optarg);
+ break;
If we see most of other options in case handling are ordered as per
their order in long_options array. For example
static struct option long_options[] = {
{host, required_argument, NULL, 'h'},
{port, required_argument, NULL, 'p'},
..
Now the order of handling for both is same in switch case or while get
opt_long() function call. I think this makes code easy to understand
and modify.
However there is no functionality issue here, so you can keep the code
as per your existing patch as well, this is just a suggestion.
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Extra functionality to createuser
On 11/14/13, 4:35 PM, Christopher Browne wrote: On Thu, Nov 14, 2013 at 5:41 AM, Sameer Thakur samthaku...@gmail.com wrote: So i think -g option is failing Right you are. I was missing a g: in the getopt_long() call. Attached is a revised patch that handles that. src/bin/scripts/createuser.c:117: indent with spaces. + case 'g': src/bin/scripts/createuser.c:118: indent with spaces. + roles = pg_strdup(optarg); -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Extra functionality to createuser
On Fri, Nov 15, 2013 at 3:14 PM, Peter Eisentraut pete...@gmx.net wrote:
On 11/14/13, 4:35 PM, Christopher Browne wrote: On Thu, Nov 14, 2013 at
5:41 AM, Sameer Thakur samthaku...@gmail.com wrote:
So i think -g option is failing
Right you are.
I was missing a g: in the getopt_long() call.
Attached is a revised patch that handles that.
src/bin/scripts/createuser.c:117: indent with spaces.
+ case 'g':
src/bin/scripts/createuser.c:118: indent with spaces.
+ roles = pg_strdup(optarg);
OK, I ran pgindent on createuser.c, which leads to the Next Patch...
--
When confronted by a difficult problem, solve it by reducing it to the
question, How would the Lone Ranger handle this?
diff --git a/doc/src/sgml/ref/createuser.sgml b/doc/src/sgml/ref/createuser.sgml
index 2f1ea2f..5fedc80 100644
--- a/doc/src/sgml/ref/createuser.sgml
+++ b/doc/src/sgml/ref/createuser.sgml
@@ -131,6 +131,16 @@ PostgreSQL documentation
/varlistentry
varlistentry
+ termoption-g//term
+ termoption--roles//term
+ listitem
+ para
+Indicates roles to associate with this role.
+ /para
+ /listitem
+ /varlistentry
+
+ varlistentry
termoption-i//term
termoption--inherit//term
listitem
diff --git a/src/bin/scripts/createuser.c b/src/bin/scripts/createuser.c
index d1542d9..e2e1134 100644
--- a/src/bin/scripts/createuser.c
+++ b/src/bin/scripts/createuser.c
@@ -47,6 +47,7 @@ main(int argc, char *argv[])
{pwprompt, no_argument, NULL, 'P'},
{encrypted, no_argument, NULL, 'E'},
{unencrypted, no_argument, NULL, 'N'},
+ {roles, required_argument, NULL, 'g'},
{NULL, 0, NULL, 0}
};
@@ -57,6 +58,7 @@ main(int argc, char *argv[])
char *host = NULL;
char *port = NULL;
char *username = NULL;
+ char *roles = NULL;
enum trivalue prompt_password = TRI_DEFAULT;
boolecho = false;
boolinteractive = false;
@@ -83,7 +85,7 @@ main(int argc, char *argv[])
handle_help_version_opts(argc, argv, createuser, help);
- while ((c = getopt_long(argc, argv, h:p:U:wWedDsSaArRiIlLc:PEN,
+ while ((c = getopt_long(argc, argv, h:p:U:g:wWedDsSaArRiIlLc:PEN,
long_options,
optindex)) != -1)
{
switch (c)
@@ -112,6 +114,9 @@ main(int argc, char *argv[])
case 'D':
createdb = TRI_NO;
break;
+ case 'g':
+ roles = pg_strdup(optarg);
+ break;
case 's':
case 'a':
superuser = TRI_YES;
@@ -302,6 +307,8 @@ main(int argc, char *argv[])
appendPQExpBuffer(sql, NOREPLICATION);
if (conn_limit != NULL)
appendPQExpBuffer(sql, CONNECTION LIMIT %s, conn_limit);
+ if (roles != NULL)
+ appendPQExpBuffer(sql, IN ROLE %s, roles);
appendPQExpBuffer(sql, ;\n);
if (echo)
@@ -334,6 +341,7 @@ help(const char *progname)
printf(_( -D, --no-createdb role cannot create databases
(default)\n));
printf(_( -e, --echoshow the commands being sent to
the server\n));
printf(_( -E, --encrypted encrypt stored password\n));
+ printf(_( -g, --roles roles to associate with this new
role\n));
printf(_( -i, --inherit role inherits privileges of roles
it is a\n
member of (default)\n));
printf(_( -I, --no-inherit role does not inherit
privileges\n));
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Extra functionality to createuser
Hello,
Tried to test this patch. Did the following
1. cloned from https://github.com/samthakur74/postgres
2. Applied patch and make install
3. created rolesapp_readonly_role,app2_writer_role
4. Tried createuser -D -S -l -g app_readonly_role,app2_writer_role
test_user got error: createuser: invalid option -- 'g'
5. Tried createuser -D -S -l --roles
app_readonly_role,app2_writer_role test_user. This does not give
error.
6. Confirmed that test_user is created using \du and it has
postgres=# \du
List of roles
Role name | Attributes |
Member of
---++---
---
Sameer| Superuser, Create role, Create DB, Replication | {}
app2_writer_role | Cannot login | {}
app_readonly_role | Cannot login | {}
my_new_user || {app_reado
nly_role,app2_writer_role}
test_user || {app_reado
nly_role,app2_writer_role}
7. createuser --help does show -g, --roles roles to
associate with this new role
So i think -g option is failing
regards
Sameer
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Extra functionality to createuser
1. cloned from https://github.com/samthakur74/postgres Sorry. cloned from https://github.com/postgres/postgres regards Sameer -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Extra functionality to createuser
On Thu, Nov 14, 2013 at 5:41 AM, Sameer Thakur samthaku...@gmail.com wrote:
So i think -g option is failing
Right you are.
I was missing a g: in the getopt_long() call.
Attached is a revised patch that handles that.
And it behaves better:
postgres@cbbrowne ~/p/s/b/scripts ./createuser -g purge_role -U
postgres newuser4
postgres@cbbrowne ~/p/s/b/scripts pg_dumpall -g | grep newuser4
CREATE ROLE newuser4;
ALTER ROLE newuser4 WITH NOSUPERUSER INHERIT NOCREATEROLE NOCREATEDB
LOGIN NOREPLICATION;
GRANT purge_role TO newuser4 GRANTED BY postgres;
--
When confronted by a difficult problem, solve it by reducing it to the
question, How would the Lone Ranger handle this?
diff --git a/doc/src/sgml/ref/createuser.sgml b/doc/src/sgml/ref/createuser.sgml
index 2f1ea2f..5fedc80 100644
--- a/doc/src/sgml/ref/createuser.sgml
+++ b/doc/src/sgml/ref/createuser.sgml
@@ -131,6 +131,16 @@ PostgreSQL documentation
/varlistentry
varlistentry
+ termoption-g//term
+ termoption--roles//term
+ listitem
+ para
+Indicates roles to associate with this role.
+ /para
+ /listitem
+ /varlistentry
+
+ varlistentry
termoption-i//term
termoption--inherit//term
listitem
diff --git a/src/bin/scripts/createuser.c b/src/bin/scripts/createuser.c
index d1542d9..c469b52 100644
--- a/src/bin/scripts/createuser.c
+++ b/src/bin/scripts/createuser.c
@@ -47,6 +47,7 @@ main(int argc, char *argv[])
{pwprompt, no_argument, NULL, 'P'},
{encrypted, no_argument, NULL, 'E'},
{unencrypted, no_argument, NULL, 'N'},
+ {roles, required_argument, NULL, 'g'},
{NULL, 0, NULL, 0}
};
@@ -57,6 +58,7 @@ main(int argc, char *argv[])
char *host = NULL;
char *port = NULL;
char *username = NULL;
+ char *roles = NULL;
enum trivalue prompt_password = TRI_DEFAULT;
boolecho = false;
boolinteractive = false;
@@ -83,7 +85,7 @@ main(int argc, char *argv[])
handle_help_version_opts(argc, argv, createuser, help);
- while ((c = getopt_long(argc, argv, h:p:U:wWedDsSaArRiIlLc:PEN,
+ while ((c = getopt_long(argc, argv, h:p:U:g:wWedDsSaArRiIlLc:PEN,
long_options,
optindex)) != -1)
{
switch (c)
@@ -112,6 +114,9 @@ main(int argc, char *argv[])
case 'D':
createdb = TRI_NO;
break;
+ case 'g':
+ roles = pg_strdup(optarg);
+ break;
case 's':
case 'a':
superuser = TRI_YES;
@@ -302,6 +307,8 @@ main(int argc, char *argv[])
appendPQExpBuffer(sql, NOREPLICATION);
if (conn_limit != NULL)
appendPQExpBuffer(sql, CONNECTION LIMIT %s, conn_limit);
+ if (roles != NULL)
+ appendPQExpBuffer(sql, IN ROLE %s, roles);
appendPQExpBuffer(sql, ;\n);
if (echo)
@@ -334,6 +341,7 @@ help(const char *progname)
printf(_( -D, --no-createdb role cannot create databases
(default)\n));
printf(_( -e, --echoshow the commands being sent to
the server\n));
printf(_( -E, --encrypted encrypt stored password\n));
+ printf(_( -g, --roles roles to associate with this new
role\n));
printf(_( -i, --inherit role inherits privileges of roles
it is a\n
member of (default)\n));
printf(_( -I, --no-inherit role does not inherit
privileges\n));
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Extra functionality to createuser
On Thu, Sep 26, 2013 at 1:04 PM, Christopher Browne cbbro...@gmail.com wrote: Sitting on my todo list for a while has been to consider the idea of adding a bit of additional functionality to createuser. One of the functions of CREATE ROLE is to associate the role with other roles, thus... create role my_new_user nosuperuser nocreatedb login IN ROLE app_readonly_role, app2_writer_role; That isn't something that I can do using createuser; to do that, I would need to submit two requests separately: PGUSER=postgres createuser -D -S -l my_new_user PGUSER=postgres psql -c grant app_readonly_role, app2_writer_role to my_new_user; I could certainly change over to using psql to do all the work, but it would be rather nice if createuser had (say) a -g option which allowed specifying the set of roles that should be assigned. Thus, the above commands might be replaced by: PGUSER=postgres createuser -D -S -l -g app_readonly_role,app2_writer_role my_new_user Would this be worth adding to the ToDo list? I'd be inclined to favor a patch implementing this. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Extra functionality to createuser
On 9/27/13 6:01 AM, Robert Haas wrote: Thus, the above commands might be replaced by: PGUSER=postgres createuser -D -S -l -g app_readonly_role,app2_writer_role my_new_user Would this be worth adding to the ToDo list? I'd be inclined to favor a patch implementing this. +1 -- Jim C. Nasby, Data Architect j...@nasby.net 512.569.9461 (cell) http://jim.nasby.net -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Extra functionality to createuser
Attached is a patch implementing the -g / --roles option for createuser. I'll be attaching it to the open CommitFest shortly. createuser.diff Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Extra functionality to createuser
Sitting on my todo list for a while has been to consider the idea of adding a bit of additional functionality to createuser. One of the functions of CREATE ROLE is to associate the role with other roles, thus... create role my_new_user nosuperuser nocreatedb login IN ROLE app_readonly_role, app2_writer_role; That isn't something that I can do using createuser; to do that, I would need to submit two requests separately: PGUSER=postgres createuser -D -S -l my_new_user PGUSER=postgres psql -c grant app_readonly_role, app2_writer_role to my_new_user; I could certainly change over to using psql to do all the work, but it would be rather nice if createuser had (say) a -g option which allowed specifying the set of roles that should be assigned. Thus, the above commands might be replaced by: PGUSER=postgres createuser -D -S -l -g app_readonly_role,app2_writer_role my_new_user Would this be worth adding to the ToDo list? -- When confronted by a difficult problem, solve it by reducing it to the question, How would the Lone Ranger handle this? -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
