On 2018/08/21 12:57, Tatsuro Yamada wrote:
On 2018/08/21 12:40, Michael Paquier wrote:On Tue, Aug 21, 2018 at 12:26:15PM +0900, Tatsuro Yamada wrote:BTW, can I add the patch to the Commitfest September?You should.Thanks, I'll do that. I'll send 2 patches in this week, probably.
I revised the patch and created new tap tests.
fix_oid2name_wip3.patch:
bug fix
- Remove "-P password" option from the document
improvements
- Divide "Options section" into "Options" and "Connection Options" sections
in
the help message (--help)
- Add long options
- Add "-H host" and "-h host" options to the help message and the document
- Add environment variable (PGHOST, PGPORT and PGUSER) to the document
fix_vacuumlo_wip3.patch:
improvements
- Add long options
- Add environment variable (PGHOST, PGPORT and PGUSER) to the document
Regarding to Long options,
I read the following both codes and I understand meanings of short options, so I
leave all long options in patches not only for connection options.
oid2name.c
/* these are the opts structures for command line params */
struct options
{
eary *tables;
eary *oids;
eary *filenodes;
bool quiet;
bool systables;
bool indexes;
bool nodb;
bool extended;
bool tablespaces;
char *dbname;
char *hostname;
char *port;
char *username;
const char *progname;
};
vacuumlo.c
struct _param
{
char *pg_user;
enum trivalue pg_prompt;
char *pg_port;
char *pg_host;
const char *progname;
int verbose;
int dry_run;
long transaction_limit;
};
Following TAP tests is for checking command-line options but I coudn't add some
test cases such as it is required argument and mixed varaiety of options.
I'm not sure about naming rule of tap test, so I added 300 and 301 to names as
temporarily. Please rename these files to more suitable names.
300_oid2name.pl
301_vacuumlo.pl
Please find attached files.
Thanks,
Tatsuro Yamada
diff --git a/contrib/oid2name/oid2name.c b/contrib/oid2name/oid2name.c
index 63e360c4c5..2779af78e8 100644
--- a/contrib/oid2name/oid2name.c
+++ b/contrib/oid2name/oid2name.c
@@ -60,8 +60,26 @@ void sql_exec_dumpalltbspc(PGconn *, struct options *);
void
get_opts(int argc, char **argv, struct options *my_opts)
{
+ static struct option long_options[] = {
+ {"host", required_argument, NULL, 'h'},
+ {"host", required_argument, NULL, 'H'}, /* deprecated */
+ {"port", required_argument, NULL, 'p'},
+ {"username", required_argument, NULL, 'U'},
+ {"dbname", required_argument, NULL, 'd'},
+ {"table", required_argument, NULL, 't'},
+ {"oid", required_argument, NULL, 'o'},
+ {"filenode", required_argument, NULL, 'f'},
+ {"quiet", no_argument, NULL, 'q'},
+ {"systable", no_argument, NULL, 'S'},
+ {"extended", no_argument, NULL, 'x'},
+ {"index", no_argument, NULL, 'i'},
+ {"tablespace", no_argument, NULL, 's'},
+ {NULL, 0, NULL, 0}
+ };
+
int c;
const char *progname;
+ int optindex;
progname = get_progname(argv[0]);
@@ -93,10 +111,30 @@ get_opts(int argc, char **argv, struct options *my_opts)
}
/* get opts */
- while ((c = getopt(argc, argv, "H:p:U:d:t:o:f:qSxish")) != -1)
+ while ((c = getopt_long(argc, argv, "h:H:p:U:d:t:o:f:qSxis", long_options, &optindex)) != -1)
{
switch (c)
{
+ /* host to connect to */
+ case 'h':
+ my_opts->hostname = pg_strdup(optarg);
+ break;
+
+ /* (deprecated) host to connect to */
+ case 'H':
+ my_opts->hostname = pg_strdup(optarg);
+ break;
+
+ /* port to connect to on remote host */
+ case 'p':
+ my_opts->port = pg_strdup(optarg);
+ break;
+
+ /* username */
+ case 'U':
+ my_opts->username = pg_strdup(optarg);
+ break;
+
/* specify the database */
case 'd':
my_opts->dbname = pg_strdup(optarg);
@@ -122,46 +160,26 @@ get_opts(int argc, char **argv, struct options *my_opts)
my_opts->quiet = true;
break;
- /* host to connect to */
- case 'H':
- my_opts->hostname = pg_strdup(optarg);
- break;
-
- /* port to connect to on remote host */
- case 'p':
- my_opts->port = pg_strdup(optarg);
- break;
-
- /* username */
- case 'U':
- my_opts->username = pg_strdup(optarg);
- break;
-
/* display system tables */
case 'S':
my_opts->systables = true;
break;
- /* also display indexes */
- case 'i':
- my_opts->indexes = true;
- break;
-
/* display extra columns */
case 'x':
my_opts->extended = true;
break;
+ /* also display indexes */
+ case 'i':
+ my_opts->indexes = true;
+ break;
+
/* dump tablespaces only */
case 's':
my_opts->tablespaces = true;
break;
- case 'h':
- help(progname);
- exit(0);
- break;
-
default:
fprintf(stderr, _("Try \"%s --help\" for more information.\n"), progname);
exit(1);
@@ -176,20 +194,22 @@ help(const char *progname)
"Usage:\n"
" %s [OPTION]...\n"
"\nOptions:\n"
- " -d DBNAME database to connect to\n"
- " -f FILENODE show info for table with given file node\n"
- " -H HOSTNAME database server host or socket directory\n"
- " -i show indexes and sequences too\n"
- " -o OID show info for table with given OID\n"
- " -p PORT database server port number\n"
- " -q quiet (don't show headers)\n"
- " -s show all tablespaces\n"
- " -S show system objects too\n"
- " -t TABLE show info for named table\n"
- " -U NAME connect as specified database user\n"
- " -V, --version output version information, then exit\n"
- " -x extended (show additional columns)\n"
- " -?, --help show this help, then exit\n"
+ " -f, --filenode FILENODE show info for table with given file node\n"
+ " -i, --index show indexes and sequences too\n"
+ " -o, --oid=OID show info for table with given OID\n"
+ " -q, --quiet quiet (don't show headers)\n"
+ " -s, --tablespace show all tablespaces\n"
+ " -S, --systable show system objects too\n"
+ " -t, --table=TABLE show info for named table\n"
+ " -V, --version output version information, then exit\n"
+ " -x, --extended extended (show additional columns)\n"
+ " -?, --help show this help, then exit\n"
+ "\nConnection options:\n"
+ " -d, --dbname=DBNAME database to connect to\n"
+ " -h, --host=HOSTNAME database server host or socket directory\n"
+ " -H is same as -h but deprecated\n"
+ " -p, --port=PORT database server port number\n"
+ " -U, --username=USERNAME connect as specified database user\n"
"\nThe default action is to show all database OIDs.\n\n"
"Report bugs to <pgsql-b...@postgresql.org>.\n",
progname, progname);
diff --git a/doc/src/sgml/oid2name.sgml b/doc/src/sgml/oid2name.sgml
index dd875281c8..ae68828abb 100644
--- a/doc/src/sgml/oid2name.sgml
+++ b/doc/src/sgml/oid2name.sgml
@@ -60,45 +60,52 @@
<variablelist>
<varlistentry>
- <term><option>-f</option> <replaceable>filenode</replaceable></term>
- <listitem><para>show info for table with filenode <replaceable>filenode</replaceable></para></listitem>
+ <term><option>-f <replaceable class="parameter">filenode</replaceable></option></term>
+ <term><option>--filenode=<replaceable class="parameter">filenode</replaceable></option></term>
+ <listitem><para>show info for table with filenode <replaceable class="parameter">filenode</replaceable>.</para></listitem>
</varlistentry>
<varlistentry>
- <term><option>-i</option></term>
- <listitem><para>include indexes and sequences in the listing</para></listitem>
+ <term><option>-i </option></term>
+ <term><option>--index</option></term>
+ <listitem><para>include indexes and sequences in the listing.</para></listitem>
</varlistentry>
<varlistentry>
- <term><option>-o</option> <replaceable>oid</replaceable></term>
- <listitem><para>show info for table with OID <replaceable>oid</replaceable></para></listitem>
+ <term><option>-o <replaceable class="parameter">oid</replaceable></option></term>
+ <term><option>--oid=<replaceable class="parameter">oid</replaceable></option></term>
+ <listitem><para>show info for table with OID <replaceable class="parameter">oid</replaceable>.</para></listitem>
</varlistentry>
<varlistentry>
- <term><option>-q</option></term>
- <listitem><para>omit headers (useful for scripting)</para></listitem>
+ <term><option>-q </option></term>
+ <term><option>--quiet</option></term>
+ <listitem><para>omit headers (useful for scripting).</para></listitem>
</varlistentry>
<varlistentry>
- <term><option>-s</option></term>
- <listitem><para>show tablespace OIDs</para></listitem>
+ <term><option>-s </option></term>
+ <term><option>--tablespac</option></term>
+ <listitem><para>show tablespace OIDs.</para></listitem>
</varlistentry>
<varlistentry>
- <term><option>-S</option></term>
+ <term><option>-S </option></term>
+ <term><option>--system</option></term>
<listitem><para>include system objects (those in
<option>information_schema</option>, <option>pg_toast</option>
- and <option>pg_catalog</option> schemas)
+ and <option>pg_catalog</option> schemas).
</para></listitem>
</varlistentry>
<varlistentry>
- <term><option>-t</option> <replaceable>tablename_pattern</replaceable></term>
- <listitem><para>show info for table(s) matching <replaceable>tablename_pattern</replaceable></para></listitem>
+ <term><option>-t <replaceable class="parameter">tablename_pattern</replaceable></option></term>
+ <term><option>--tablename=<replaceable class="parameter">tablename_pattern</replaceable></option></term>
+ <listitem><para>show info for table(s) matching <replaceable class="parameter">tablename_pattern</replaceable>.</para></listitem>
</varlistentry>
<varlistentry>
- <term><option>-V</option></term>
+ <term><option>-V </option></term>
<term><option>--version</option></term>
<listitem>
<para>
@@ -108,14 +115,15 @@
</varlistentry>
<varlistentry>
- <term><option>-x</option></term>
+ <term><option>-x </option></term>
+ <term><option>--extended</option></term>
<listitem><para>display more information about each object shown: tablespace name,
- schema name, and OID
+ schema name, and OID.
</para></listitem>
</varlistentry>
<varlistentry>
- <term><option>-?</option></term>
+ <term><option>-? </option></term>
<term><option>--help</option></term>
<listitem>
<para>
@@ -133,29 +141,27 @@
<variablelist>
<varlistentry>
- <term><option>-d</option> <replaceable>database</replaceable></term>
- <listitem><para>database to connect to</para></listitem>
+ <term><option>-d <replaceable>database</replaceable></option></term>
+ <term><option>--dbname=<replaceable class="parameter">database</replaceable></option></term>
+ <listitem><para>database to connect to.</para></listitem>
</varlistentry>
<varlistentry>
- <term><option>-H</option> <replaceable>host</replaceable></term>
- <listitem><para>database server's host</para></listitem>
+ <term><option>-h <replaceable class="parameter">host</replaceable></option></term>
+ <term><option>--host=<replaceable class="parameter">host</replaceable></option></term>
+ <listitem><para>database server's host. -H is also able to use instead -h, however it's a deprecated option.</para></listitem>
</varlistentry>
<varlistentry>
- <term><option>-p</option> <replaceable>port</replaceable></term>
- <listitem><para>database server's port</para></listitem>
+ <term><option>-p <replaceable>port</replaceable></option></term>
+ <term><option>--port=<replaceable class="parameter">port</replaceable></option></term>
+ <listitem><para>database server's port.</para></listitem>
</varlistentry>
<varlistentry>
- <term><option>-U</option> <replaceable>username</replaceable></term>
- <listitem><para>user name to connect as</para></listitem>
- </varlistentry>
-
- <varlistentry>
- <term><option>-P</option> <replaceable>password</replaceable></term>
- <listitem><para>password (deprecated — putting this on the command line
- is a security hazard)</para></listitem>
+ <term><option>-U <replaceable>username</replaceable></option></term>
+ <term><option>--username=<replaceable class="parameter">username</replaceable></option></term>
+ <listitem><para>user name to connect as.</para></listitem>
</varlistentry>
</variablelist>
@@ -188,6 +194,30 @@
</para>
</refsect1>
+ <refsect1>
+ <title>Environment</title>
+
+ <variablelist>
+ <varlistentry>
+ <term><envar>PGHOST</envar></term>
+ <term><envar>PGPORT</envar></term>
+ <term><envar>PGUSER</envar></term>
+
+ <listitem>
+ <para>
+ Default connection parameters
+ </para>
+ </listitem>
+ </varlistentry>
+ </variablelist>
+
+ <para>
+ This utility, like most other <productname>PostgreSQL</productname> utilities,
+ also uses the environment variables supported by <application>libpq</application>
+ (see <xref linkend="libpq-envars"/>).
+ </para>
+ </refsect1>
+
<refsect1>
<title>Notes</title>
300_oid2name.pl
Description: Perl program
301_vacuumlo.pl
Description: Perl program
diff --git a/contrib/vacuumlo/vacuumlo.c b/contrib/vacuumlo/vacuumlo.c
index 7eb474ca3e..bcd7c1c90b 100644
--- a/contrib/vacuumlo/vacuumlo.c
+++ b/contrib/vacuumlo/vacuumlo.c
@@ -434,17 +434,17 @@ usage(const char *progname)
printf("%s removes unreferenced large objects from databases.\n\n", progname);
printf("Usage:\n %s [OPTION]... DBNAME...\n\n", progname);
printf("Options:\n");
- printf(" -l LIMIT commit after removing each LIMIT large objects\n");
- printf(" -n don't remove large objects, just show what would be done\n");
- printf(" -v write a lot of progress messages\n");
- printf(" -V, --version output version information, then exit\n");
- printf(" -?, --help show this help, then exit\n");
+ printf(" -l, --limit=LIMIT commit after removing each LIMIT large objects\n");
+ printf(" -n, --dry-run don't remove large objects, just show what would be done\n");
+ printf(" -v, --verbose write a lot of progress messages\n");
+ printf(" -V, --version output version information, then exit\n");
+ printf(" -?, --help show this help, then exit\n");
printf("\nConnection options:\n");
- printf(" -h HOSTNAME database server host or socket directory\n");
- printf(" -p PORT database server port\n");
- printf(" -U USERNAME user name to connect as\n");
- printf(" -w never prompt for password\n");
- printf(" -W force password prompt\n");
+ printf(" -h, --host=HOSTNAME database server host or socket directory\n");
+ printf(" -p, --port=PORT database server port\n");
+ printf(" -U, --username=USERNAME user name to connect as\n");
+ printf(" -w, --no-password never prompt for password\n");
+ printf(" -W, --password force password prompt\n");
printf("\n");
printf("Report bugs to <pgsql-b...@postgresql.org>.\n");
}
@@ -453,11 +453,24 @@ usage(const char *progname)
int
main(int argc, char **argv)
{
+ static struct option long_options[] = {
+ {"host", required_argument, NULL, 'h'},
+ {"limit", required_argument, NULL, 'l'},
+ {"username", required_argument, NULL, 'U'},
+ {"port", required_argument, NULL, 'p'},
+ {"verbose", no_argument, NULL, 'v'},
+ {"dry-run", no_argument, NULL, 'n'},
+ {"no-password", no_argument, NULL, 'w'},
+ {"password", no_argument, NULL, 'W'},
+ {NULL, 0, NULL, 0}
+ };
+
int rc = 0;
struct _param param;
int c;
int port;
const char *progname;
+ int optindex;
progname = get_progname(argv[0]);
@@ -486,25 +499,14 @@ main(int argc, char **argv)
}
}
- while (1)
+ while((c = getopt_long(argc, argv, "h:l:U:p:vnwW", long_options, &optindex)) != -1)
{
- c = getopt(argc, argv, "h:l:U:p:vnwW");
- if (c == -1)
- break;
-
switch (c)
{
- case '?':
- fprintf(stderr, _("Try \"%s --help\" for more information.\n"), progname);
- exit(1);
case ':':
exit(1);
- case 'v':
- param.verbose = 1;
- break;
- case 'n':
- param.dry_run = 1;
- param.verbose = 1;
+ case 'h':
+ param.pg_host = pg_strdup(optarg);
break;
case 'l':
param.transaction_limit = strtol(optarg, NULL, 10);
@@ -519,12 +521,6 @@ main(int argc, char **argv)
case 'U':
param.pg_user = pg_strdup(optarg);
break;
- case 'w':
- param.pg_prompt = TRI_NO;
- break;
- case 'W':
- param.pg_prompt = TRI_YES;
- break;
case 'p':
port = strtol(optarg, NULL, 10);
if ((port < 1) || (port > 65535))
@@ -534,8 +530,18 @@ main(int argc, char **argv)
}
param.pg_port = pg_strdup(optarg);
break;
- case 'h':
- param.pg_host = pg_strdup(optarg);
+ case 'v':
+ param.verbose = 1;
+ break;
+ case 'n':
+ param.dry_run = 1;
+ param.verbose = 1;
+ break;
+ case 'w':
+ param.pg_prompt = TRI_NO;
+ break;
+ case 'W':
+ param.pg_prompt = TRI_YES;
break;
}
}
diff --git a/doc/src/sgml/vacuumlo.sgml b/doc/src/sgml/vacuumlo.sgml
index 0b4dfc2b17..e46c27030b 100644
--- a/doc/src/sgml/vacuumlo.sgml
+++ b/doc/src/sgml/vacuumlo.sgml
@@ -55,10 +55,11 @@
<variablelist>
<varlistentry>
- <term><option>-l</option> <replaceable>limit</replaceable></term>
+ <term><option>-l <replaceable class="parameter">limit</replaceable></option></term>
+ <term><option>--limit=<replaceable class="parameter">limit</replaceable></option></term>
<listitem>
<para>
- Remove no more than <replaceable>limit</replaceable> large objects per
+ Remove no more than <replaceable class="parameter">limit</replaceable> large objects per
transaction (default 1000). Since the server acquires a lock per LO
removed, removing too many LOs in one transaction risks exceeding
<xref linkend="guc-max-locks-per-transaction"/>. Set the limit to
@@ -68,21 +69,23 @@
</varlistentry>
<varlistentry>
- <term><option>-n</option></term>
+ <term><option>-n </option></term>
+ <term><option>--dry-run</option></term>
<listitem>
<para>Don't remove anything, just show what would be done.</para>
</listitem>
</varlistentry>
<varlistentry>
- <term><option>-v</option></term>
+ <term><option>-v </option></term>
+ <term><option>--verbose</option></term>
<listitem>
<para>Write a lot of progress messages.</para>
</listitem>
</varlistentry>
<varlistentry>
- <term><option>-V</option></term>
+ <term><option>-V </option></term>
<term><option>--version</option></term>
<listitem>
<para>
@@ -92,7 +95,7 @@
</varlistentry>
<varlistentry>
- <term><option>-?</option></term>
+ <term><option>-? </option></term>
<term><option>--help</option></term>
<listitem>
<para>
@@ -110,28 +113,31 @@
<variablelist>
<varlistentry>
- <term><option>-h</option> <replaceable>hostname</replaceable></term>
+ <term><option>-h <replaceable class="parameter">host</replaceable></option></term>
+ <term><option>--host=<replaceable class="parameter">host</replaceable></option></term>
<listitem>
<para>Database server's host.</para>
</listitem>
</varlistentry>
<varlistentry>
- <term><option>-p</option> <replaceable>port</replaceable></term>
+ <term><option>-p <replaceable>port</replaceable></option></term>
+ <term><option>--port=<replaceable class="parameter">port</replaceable></option></term>
<listitem>
<para>Database server's port.</para>
</listitem>
</varlistentry>
<varlistentry>
- <term><option>-U</option> <replaceable>username</replaceable></term>
+ <term><option>-U <replaceable>username</replaceable></option></term>
+ <term><option>--username=<replaceable class="parameter">username</replaceable></option></term>
<listitem>
<para>User name to connect as.</para>
</listitem>
</varlistentry>
<varlistentry>
- <term><option>-w</option></term>
+ <term><option>-w </option></term>
<term><option>--no-password</option></term>
<listitem>
<para>
@@ -145,7 +151,8 @@
</varlistentry>
<varlistentry>
- <term><option>-W</option></term>
+ <term><option>-W </option></term>
+ <term><option>--passowrd</option></term>
<listitem>
<para>
Force <application>vacuumlo</application> to prompt for a
@@ -167,6 +174,30 @@
</para>
</refsect1>
+ <refsect1>
+ <title>Environment</title>
+
+ <variablelist>
+ <varlistentry>
+ <term><envar>PGHOST</envar></term>
+ <term><envar>PGPORT</envar></term>
+ <term><envar>PGUSER</envar></term>
+
+ <listitem>
+ <para>
+ Default connection parameters
+ </para>
+ </listitem>
+ </varlistentry>
+ </variablelist>
+
+ <para>
+ This utility, like most other <productname>PostgreSQL</productname> utilities,
+ also uses the environment variables supported by <application>libpq</application>
+ (see <xref linkend="libpq-envars"/>).
+ </para>
+ </refsect1>
+
<refsect1>
<title>Notes</title>
