On Mon, Nov 18, 2013 at 1:01 AM, Amit Kapila <[email protected]> wrote:
> On Sat, Nov 16, 2013 at 4:57 AM, Christopher Browne <[email protected]>
> wrote:
> Few comments:
>
> 1.
> + <term><option>-g</></term>
> + <term><option>--roles</></term>
>
> All other options which require argument are of form:
> <term><option>-c <replaceable
> class="parameter">number</replaceable></></term>
> <term><option>--connection-limit=<replaceable
> class="parameter">number</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>
+ <term><option>-g <replaceable
class="parameter">roles</replaceable></></term>
+ <term><option>--roles=<replaceable
class="parameter">roles</replaceable></></term>
+ <listitem>
+ <para>
+ Indicates roles to which this role will be added immediately as a new
member.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
<term><option>-i</></term>
<term><option>--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;
bool echo = false;
bool interactive = 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, --echo show 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 ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
