On Sun, May 10, 2015 at 2:23 AM, Sawada Masahiko <sawada.m...@gmail.com> wrote:
> On Sat, May 9, 2015 at 4:26 AM, Fabrízio de Royes Mello
> <fabriziome...@gmail.com> wrote:
>>
>>
>> On Fri, May 8, 2015 at 4:23 PM, Fabrízio de Royes Mello
>> <fabriziome...@gmail.com> wrote:
>>>
>>>
>>> On Thu, May 7, 2015 at 7:55 PM, Sawada Masahiko <sawada.m...@gmail.com>
>>> wrote:
>>> >
>>> > On 5/7/15, Sawada Masahiko <sawada.m...@gmail.com> wrote:
>>> > > On Wed, May 6, 2015 at 5:42 AM, Robert Haas <robertmh...@gmail.com
>>> > > <javascript:;>> wrote:
>>> > >> On Tue, May 5, 2015 at 11:10 AM, Sawada Masahiko
>>> > >> <sawada.m...@gmail.com
>>> > > <javascript:;>> wrote:
>>> > >>> On Fri, May 1, 2015 at 9:04 PM, Robert Haas <robertmh...@gmail.com
>>> > > <javascript:;>> wrote:
>>> > >>>> On Thu, Apr 30, 2015 at 11:05 PM, Sawada Masahiko
>>> > >>>> <sawada.m...@gmail.com
>>> > > <javascript:;>> wrote:
>>> > >>>>> VACUUM has both syntax: with parentheses and without parentheses.
>>> > >>>>> I think we should have both syntax for REINDEX like VACUUM does
>>> > >>>>> because it would be pain to put parentheses whenever we want to do
>>> > >>>>> REINDEX.
>>> > >>>>> Are the parentheses optional in REINDEX command?
>>> > >>>>
>>> > >>>> No. The unparenthesized VACUUM syntax was added back before we
>>> > >>>> realized that that kind of syntax is a terrible idea. It requires
>>> > >>>> every option to be a keyword, and those keywords have to be in a
>>> > >>>> fixed
>>> > >>>> order. I believe the intention is to keep the old VACUUM syntax
>>> > >>>> around for backward-compatibility, but not to extend it. Same for
>>> > >>>> EXPLAIN and COPY.
>>> > >>>
>>> > >>> REINDEX will have only one option VERBOSE for now.
>>> > >>> Even we're in a situation like that it's not clear to be added newly
>>> > >>> additional option to REINDEX now, we should need to put parenthesis?
>>> > >>
>>> > >> In my opinion, yes. The whole point of a flexible options syntax is
>>> > >> that we can add new options without changing the grammar. That
>>> > >> involves some compromise on the syntax, which doesn't bother me a
>>> > >> bit.
>>> > >> Our previous experiments with this for EXPLAIN and COPY and VACUUM
>>> > >> have worked out quite well, and I see no reason for pessimism here.
>>> > >
>>> > > I agree that flexible option syntax does not need to change grammar
>>> > > whenever we add new options.
>>> > > Attached patch is changed based on your suggestion.
>>> > > And the patch for reindexdb is also attached.
>>> > > Please feedbacks.
>>> > >
>>> > >>> Also I'm not sure that both implementation and documentation
>>> > >>> regarding
>>> > >>> VERBOSE option should be optional.
>>> > >>
>>> > >> I don't know what this means.
>>> > >>
>>> > >
>>> > > Sorry for confusing you.
>>> > > Please ignore this.
>>> > >
>>> >
>>> > Sorry, I forgot attach files.
>>> >
>>>
>>> I applied the two patches to master and I got some errors when compile:
>>>
>>> tab-complete.c: In function ‘psql_completion’:
>>> tab-complete.c:3338:12: warning: left-hand operand of comma expression has
>>> no effect [-Wunused-value]
>>> {"TABLE", "INDEX", "SYSTEM", "SCHEMA", "DATABASE", NULL};
>>> ^
>>> tab-complete.c:3338:21: warning: left-hand operand of comma expression has
>>> no effect [-Wunused-value]
>>> {"TABLE", "INDEX", "SYSTEM", "SCHEMA", "DATABASE", NULL};
>>> ^
>>> tab-complete.c:3338:31: warning: left-hand operand of comma expression has
>>> no effect [-Wunused-value]
>>> {"TABLE", "INDEX", "SYSTEM", "SCHEMA", "DATABASE", NULL};
>>> ^
>>> tab-complete.c:3338:41: warning: left-hand operand of comma expression has
>>> no effect [-Wunused-value]
>>> {"TABLE", "INDEX", "SYSTEM", "SCHEMA", "DATABASE", NULL};
>>> ^
>>> tab-complete.c:3338:53: warning: left-hand operand of comma expression has
>>> no effect [-Wunused-value]
>>> {"TABLE", "INDEX", "SYSTEM", "SCHEMA", "DATABASE", NULL};
>>> ^
>>> tab-complete.c:3338:5: warning: statement with no effect [-Wunused-value]
>>> {"TABLE", "INDEX", "SYSTEM", "SCHEMA", "DATABASE", NULL};
>>> ^
>>> tab-complete.c:3338:59: error: expected ‘;’ before ‘}’ token
>>> {"TABLE", "INDEX", "SYSTEM", "SCHEMA", "DATABASE", NULL};
>>> ^
>>> tab-complete.c:3340:22: error: ‘list_REINDEX’ undeclared (first use in
>>> this function)
>>> COMPLETE_WITH_LIST(list_REINDEX);
>>> ^
>>> tab-complete.c:169:22: note: in definition of macro ‘COMPLETE_WITH_LIST’
>>> completion_charpp = list; \
>>> ^
>>> tab-complete.c:3340:22: note: each undeclared identifier is reported only
>>> once for each function it appears in
>>> COMPLETE_WITH_LIST(list_REINDEX);
>>> ^
>>> tab-complete.c:169:22: note: in definition of macro ‘COMPLETE_WITH_LIST’
>>> completion_charpp = list; \
>>> ^
>>> make[3]: *** [tab-complete.o] Error 1
>>> make[3]: *** Waiting for unfinished jobs....
>>> make[2]: *** [install-psql-recurse] Error 2
>>> make[2]: *** Waiting for unfinished jobs....
>>> make[1]: *** [install-bin-recurse] Error 2
>>> make: *** [install-src-recurse] Error 2
>>>
>>>
>>> Looking at the code I think you remove one line accidentally from
>>> tab-complete.c:
>>>
>>> $ git diff src/bin/psql/tab-complete.c
>>> diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
>>> index 750e29d..55b0df5 100644
>>> --- a/src/bin/psql/tab-complete.c
>>> +++ b/src/bin/psql/tab-complete.c
>>> @@ -3335,7 +3335,6 @@ psql_completion(const char *text, int start, int
>>> end)
>>> /* REINDEX */
>>> else if (pg_strcasecmp(prev_wd, "REINDEX") == 0)
>>> {
>>> - static const char *const list_REINDEX[] =
>>> {"TABLE", "INDEX", "SYSTEM", "SCHEMA", "DATABASE", NULL};
>>>
>>> COMPLETE_WITH_LIST(list_REINDEX);
>>>
>>>
>>> The attached fix it and now seems good to me.
>> Just one last note. IMHO we should add a regression to
>> src/bin/scripts/090_reindexdb.pl.
>>
>
> Thank you for your patch!
> (Sorry for attaching the patch still has compile error..)
>
> - 000_reindex_verbose_v13.patch
> Looks good to me.
>
> - 001_reindexdb_verbose_option_v1.patch
> I noticed a bug in reindexdb patch, so fixed version is attached.
> The regression test for reindexdb is added as well.
I forgot to add documentation patch.
The latest version patch is attached.
(the latest version of REINDEX VERBOSE patch is v13 Fabrizio attached)
Regards,
-------
Sawada Masahiko
diff --git a/doc/src/sgml/ref/reindexdb.sgml b/doc/src/sgml/ref/reindexdb.sgml
index b5b449c..951dbbb 100644
--- a/doc/src/sgml/ref/reindexdb.sgml
+++ b/doc/src/sgml/ref/reindexdb.sgml
@@ -24,6 +24,13 @@ PostgreSQL documentation
<command>reindexdb</command>
<arg rep="repeat"><replaceable>connection-option</replaceable></arg>
+ <arg choice="opt">
+ <group choice="plain">
+ <arg choice="plain"><option>--verbose</option></arg>
+ <arg choice="plain"><option>-v</option></arg>
+ </group>
+ </arg>
+
<arg choice="plain" rep="repeat">
<arg choice="opt">
<group choice="plain">
@@ -60,6 +67,14 @@ PostgreSQL documentation
<cmdsynopsis>
<command>reindexdb</command>
<arg rep="repeat"><replaceable>connection-option</replaceable></arg>
+
+ <arg choice="opt">
+ <group choice="plain">
+ <arg choice="plain"><option>--verbose</option></arg>
+ <arg choice="plain"><option>-v</option></arg>
+ </group>
+ </arg>
+
<group choice="plain">
<arg choice="plain"><option>--all</option></arg>
<arg choice="plain"><option>-a</option></arg>
@@ -69,6 +84,14 @@ PostgreSQL documentation
<cmdsynopsis>
<command>reindexdb</command>
<arg rep="repeat"><replaceable>connection-option</replaceable></arg>
+
+ <arg choice="opt">
+ <group choice="plain">
+ <arg choice="plain"><option>--verbose</option></arg>
+ <arg choice="plain"><option>-v</option></arg>
+ </group>
+ </arg>
+
<group choice="plain">
<arg choice="plain"><option>--system</option></arg>
<arg choice="plain"><option>-s</option></arg>
@@ -196,6 +219,16 @@ PostgreSQL documentation
</varlistentry>
<varlistentry>
+ <term><option>-v</></term>
+ <term><option>--verbose</></term>
+ <listitem>
+ <para>
+ Print detailed information during processing.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
<term><option>-V</></term>
<term><option>--version</></term>
<listitem>
diff --git a/src/bin/scripts/reindexdb.c b/src/bin/scripts/reindexdb.c
index 778d72a..547d0f8 100644
--- a/src/bin/scripts/reindexdb.c
+++ b/src/bin/scripts/reindexdb.c
@@ -18,16 +18,16 @@ static void reindex_one_database(const char *name, const char *dbname,
const char *type, const char *host,
const char *port, const char *username,
enum trivalue prompt_password, const char *progname,
- bool echo);
+ bool echo, bool verbose);
static void reindex_all_databases(const char *maintenance_db,
const char *host, const char *port,
const char *username, enum trivalue prompt_password,
const char *progname, bool echo,
- bool quiet);
+ bool quiet, bool verbose);
static void reindex_system_catalogs(const char *dbname,
const char *host, const char *port,
const char *username, enum trivalue prompt_password,
- const char *progname, bool echo);
+ const char *progname, bool echo, bool verbose);
static void help(const char *progname);
int
@@ -47,6 +47,7 @@ main(int argc, char *argv[])
{"system", no_argument, NULL, 's'},
{"table", required_argument, NULL, 't'},
{"index", required_argument, NULL, 'i'},
+ {"verbose", no_argument, NULL, 'v'},
{"maintenance-db", required_argument, NULL, 2},
{NULL, 0, NULL, 0}
};
@@ -65,6 +66,7 @@ main(int argc, char *argv[])
bool alldb = false;
bool echo = false;
bool quiet = false;
+ bool verbose = false;
SimpleStringList indexes = {NULL, NULL};
SimpleStringList tables = {NULL, NULL};
SimpleStringList schemas = {NULL, NULL};
@@ -75,7 +77,7 @@ main(int argc, char *argv[])
handle_help_version_opts(argc, argv, "reindexdb", help);
/* process command-line options */
- while ((c = getopt_long(argc, argv, "h:p:U:wWeqS:d:ast:i:", long_options, &optindex)) != -1)
+ while ((c = getopt_long(argc, argv, "h:p:U:wWeqS:d:ast:i:v", long_options, &optindex)) != -1)
{
switch (c)
{
@@ -118,6 +120,9 @@ main(int argc, char *argv[])
case 'i':
simple_string_list_append(&indexes, optarg);
break;
+ case 'v':
+ verbose = true;
+ break;
case 2:
maintenance_db = pg_strdup(optarg);
break;
@@ -176,7 +181,7 @@ main(int argc, char *argv[])
}
reindex_all_databases(maintenance_db, host, port, username,
- prompt_password, progname, echo, quiet);
+ prompt_password, progname, echo, quiet, verbose);
}
else if (syscatalog)
{
@@ -207,7 +212,7 @@ main(int argc, char *argv[])
}
reindex_system_catalogs(dbname, host, port, username, prompt_password,
- progname, echo);
+ progname, echo, verbose);
}
else
{
@@ -228,7 +233,7 @@ main(int argc, char *argv[])
for (cell = schemas.head; cell; cell = cell->next)
{
reindex_one_database(cell->val, dbname, "SCHEMA", host, port,
- username, prompt_password, progname, echo);
+ username, prompt_password, progname, echo, verbose);
}
}
@@ -239,7 +244,7 @@ main(int argc, char *argv[])
for (cell = indexes.head; cell; cell = cell->next)
{
reindex_one_database(cell->val, dbname, "INDEX", host, port,
- username, prompt_password, progname, echo);
+ username, prompt_password, progname, echo, verbose);
}
}
if (tables.head != NULL)
@@ -249,13 +254,13 @@ main(int argc, char *argv[])
for (cell = tables.head; cell; cell = cell->next)
{
reindex_one_database(cell->val, dbname, "TABLE", host, port,
- username, prompt_password, progname, echo);
+ username, prompt_password, progname, echo, verbose);
}
}
/* reindex database only if neither index nor table nor schema is specified */
if (indexes.head == NULL && tables.head == NULL && schemas.head == NULL)
reindex_one_database(dbname, dbname, "DATABASE", host, port,
- username, prompt_password, progname, echo);
+ username, prompt_password, progname, echo, verbose);
}
exit(0);
@@ -264,7 +269,8 @@ main(int argc, char *argv[])
static void
reindex_one_database(const char *name, const char *dbname, const char *type,
const char *host, const char *port, const char *username,
- enum trivalue prompt_password, const char *progname, bool echo)
+ enum trivalue prompt_password, const char *progname, bool echo,
+ bool verbose)
{
PQExpBufferData sql;
@@ -273,6 +279,10 @@ reindex_one_database(const char *name, const char *dbname, const char *type,
initPQExpBuffer(&sql);
appendPQExpBufferStr(&sql, "REINDEX");
+
+ if (verbose)
+ appendPQExpBufferStr(&sql, " (VERBOSE)");
+
if (strcmp(type, "TABLE") == 0)
appendPQExpBuffer(&sql, " TABLE %s", name);
else if (strcmp(type, "INDEX") == 0)
@@ -312,7 +322,7 @@ static void
reindex_all_databases(const char *maintenance_db,
const char *host, const char *port,
const char *username, enum trivalue prompt_password,
- const char *progname, bool echo, bool quiet)
+ const char *progname, bool echo, bool quiet, bool verbose)
{
PGconn *conn;
PGresult *result;
@@ -334,7 +344,7 @@ reindex_all_databases(const char *maintenance_db,
}
reindex_one_database(dbname, dbname, "DATABASE", host, port, username,
- prompt_password, progname, echo);
+ prompt_password, progname, echo, verbose);
}
PQclear(result);
@@ -343,7 +353,7 @@ reindex_all_databases(const char *maintenance_db,
static void
reindex_system_catalogs(const char *dbname, const char *host, const char *port,
const char *username, enum trivalue prompt_password,
- const char *progname, bool echo)
+ const char *progname, bool echo, bool verbose)
{
PQExpBufferData sql;
@@ -351,7 +361,12 @@ reindex_system_catalogs(const char *dbname, const char *host, const char *port,
initPQExpBuffer(&sql);
- appendPQExpBuffer(&sql, "REINDEX SYSTEM %s;", dbname);
+ appendPQExpBuffer(&sql, "REINDEX");
+
+ if (verbose)
+ appendPQExpBuffer(&sql, " (VERBOSE)");
+
+ appendPQExpBuffer(&sql, " SYSTEM %s;", dbname);
conn = connectDatabase(dbname, host, port, username, prompt_password,
progname, false);
@@ -380,6 +395,7 @@ help(const char *progname)
printf(_(" -q, --quiet don't write any messages\n"));
printf(_(" -s, --system reindex system catalogs\n"));
printf(_(" -S, --schema=SCHEMA recreate specific schema(s) only\n"));
+ printf(_(" -v, --verbose write a lot of output\n"));
printf(_(" -t, --table=TABLE reindex specific table(s) only\n"));
printf(_(" -V, --version output version information, then exit\n"));
printf(_(" -?, --help show this help, then exit\n"));
diff --git a/src/bin/scripts/t/090_reindexdb.pl b/src/bin/scripts/t/090_reindexdb.pl
index d5bce43..42628c2 100644
--- a/src/bin/scripts/t/090_reindexdb.pl
+++ b/src/bin/scripts/t/090_reindexdb.pl
@@ -1,7 +1,7 @@
use strict;
use warnings;
use TestLib;
-use Test::More tests => 18;
+use Test::More tests => 20;
program_help_ok('reindexdb');
program_version_ok('reindexdb');
@@ -35,3 +35,7 @@ issues_sql_like(
[ 'reindexdb', '-s', 'postgres' ],
qr/statement: REINDEX SYSTEM postgres;/,
'reindex system tables');
+issues_sql_like(
+ [ 'reindexdb', '-v', '-t', 'test1', 'postgres' ],
+ qr/statement: REINDEX \(VERBOSE\) TABLE test1;/,
+ 'reindex with verbose output');
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
