On 07/30/2014 07:46 PM, Tom Lane wrote:
> Bruce Momjian <br...@momjian.us> writes:
>> On Wed, Jul 30, 2014 at 01:29:31PM -0400, Tom Lane wrote:
>>> I don't find it all that odd. We should not be encouraging routine
>>> database-wide reindexes.
>
>> Uh, do we encourage database-wide VACUUM FULL or CLUSTER, as we use them
>> there with no parameter. Is there a reason REINDEX should be harder,
>> and require a dummy argument to run?
>
> I believe that REINDEX on system catalogs carries a risk of deadlock
> failures against other processes --- there was a recent example of that
> in the mailing lists. VACUUM FULL has such risks too, but that's been
> pretty well deprecated for many years. (I think CLUSTER is probably
> relatively safe on this score because it's not going to think any system
> catalogs are clustered.)
>
> If there were a variant of REINDEX that only hit user tables, I'd be fine
> with making that easy to invoke.
Here are two patches for this.
The first one, reindex_user_tables.v1.patch, implements the variant that
only hits user tables, as suggested by you.
The second one, reindex_no_dbname.v1.patch, allows the three
database-wide variants to omit the database name (voted for by Daniel
Migowski, Bruce, and myself; voted against by you). This patch is to be
applied on top of the first one.
--
Vik
*** a/doc/src/sgml/ref/reindex.sgml
--- b/doc/src/sgml/ref/reindex.sgml
***************
*** 21,27 **** PostgreSQL documentation
<refsynopsisdiv>
<synopsis>
! REINDEX { INDEX | TABLE | DATABASE | SYSTEM } <replaceable class="PARAMETER">name</replaceable> [ FORCE ]
</synopsis>
</refsynopsisdiv>
--- 21,27 ----
<refsynopsisdiv>
<synopsis>
! REINDEX { INDEX | TABLE | DATABASE | SYSTEM | USER TABLES } <replaceable class="PARAMETER">name</replaceable> [ FORCE ]
</synopsis>
</refsynopsisdiv>
***************
*** 126,139 **** REINDEX { INDEX | TABLE | DATABASE | SYSTEM } <replaceable class="PARAMETER">nam
</varlistentry>
<varlistentry>
<term><replaceable class="PARAMETER">name</replaceable></term>
<listitem>
<para>
The name of the specific index, table, or database to be
reindexed. Index and table names can be schema-qualified.
! Presently, <command>REINDEX DATABASE</> and <command>REINDEX SYSTEM</>
! can only reindex the current database, so their parameter must match
! the current database's name.
</para>
</listitem>
</varlistentry>
--- 126,151 ----
</varlistentry>
<varlistentry>
+ <term><literal>USER TABLES</literal></term>
+ <listitem>
+ <para>
+ Recreate all indexes on user tables within the current database.
+ Indexes on system catalogs are not processed.
+ This form of <command>REINDEX</command> cannot be executed inside a
+ transaction block.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
<term><replaceable class="PARAMETER">name</replaceable></term>
<listitem>
<para>
The name of the specific index, table, or database to be
reindexed. Index and table names can be schema-qualified.
! Presently, <command>REINDEX DATABASE</>, <command>REINDEX SYSTEM</>,
! and <command>REINDEX USER TABLES</> can only reindex the current
! database, so their parameter must match the current database's name.
</para>
</listitem>
</varlistentry>
*** a/doc/src/sgml/ref/reindexdb.sgml
--- b/doc/src/sgml/ref/reindexdb.sgml
***************
*** 65,70 **** PostgreSQL documentation
--- 65,80 ----
</group>
<arg choice="opt"><replaceable>dbname</replaceable></arg>
</cmdsynopsis>
+
+ <cmdsynopsis>
+ <command>reindexdb</command>
+ <arg rep="repeat"><replaceable>connection-option</replaceable></arg>
+ <group choice="plain">
+ <arg choice="plain"><option>--usertables</option></arg>
+ <arg choice="plain"><option>-u</option></arg>
+ </group>
+ <arg choice="opt"><replaceable>dbname</replaceable></arg>
+ </cmdsynopsis>
</refsynopsisdiv>
***************
*** 173,178 **** PostgreSQL documentation
--- 183,198 ----
</listitem>
</varlistentry>
+ <varlistentry>
+ <term><option>-u</></term>
+ <term><option>--usertables</></term>
+ <listitem>
+ <para>
+ Reindex database's user tables.
+ </para>
+ </listitem>
+ </varlistentry>
+
<varlistentry>
<term><option>-V</></term>
<term><option>--version</></term>
*** a/src/backend/parser/gram.y
--- b/src/backend/parser/gram.y
***************
*** 6984,6989 **** ReindexStmt:
--- 6984,6999 ----
n->do_user = true;
$$ = (Node *)n;
}
+ | REINDEX USER TABLES name opt_force
+ {
+ ReindexStmt *n = makeNode(ReindexStmt);
+ n->kind = OBJECT_DATABASE;
+ n->name = $4;
+ n->relation = NULL;
+ n->do_system = false;
+ n->do_user = true;
+ $$ = (Node *)n;
+ }
;
reindex_type:
*** a/src/bin/psql/tab-complete.c
--- b/src/bin/psql/tab-complete.c
***************
*** 3145,3151 **** psql_completion(const char *text, int start, int end)
else if (pg_strcasecmp(prev_wd, "REINDEX") == 0)
{
static const char *const list_REINDEX[] =
! {"TABLE", "INDEX", "SYSTEM", "DATABASE", NULL};
COMPLETE_WITH_LIST(list_REINDEX);
}
--- 3145,3151 ----
else if (pg_strcasecmp(prev_wd, "REINDEX") == 0)
{
static const char *const list_REINDEX[] =
! {"TABLE", "INDEX", "SYSTEM", "DATABASE", "USER TABLES", NULL};
COMPLETE_WITH_LIST(list_REINDEX);
}
***************
*** 3158,3163 **** psql_completion(const char *text, int start, int end)
--- 3158,3171 ----
else if (pg_strcasecmp(prev_wd, "SYSTEM") == 0 ||
pg_strcasecmp(prev_wd, "DATABASE") == 0)
COMPLETE_WITH_QUERY(Query_for_list_of_databases);
+ else if (pg_strcasecmp(prev_wd, "USER") == 0)
+ COMPLETE_WITH_CONST("TABLES");
+ }
+ else if (pg_strcasecmp(prev3_wd, "REINDEX") == 0)
+ {
+ if (pg_strcasecmp(prev2_wd, "USER") == 0 &&
+ pg_strcasecmp(prev_wd, "TABLES") == 0)
+ COMPLETE_WITH_QUERY(Query_for_list_of_databases);
}
/* SECURITY LABEL */
*** a/src/bin/scripts/reindexdb.c
--- b/src/bin/scripts/reindexdb.c
***************
*** 28,35 **** static void reindex_system_catalogs(const char *dbname,
--- 28,41 ----
const char *host, const char *port,
const char *username, enum trivalue prompt_password,
const char *progname, bool echo);
+ static void reindex_user_tables(const char *dbname,
+ const char *host, const char *port,
+ const char *username, enum trivalue prompt_password,
+ const char *progname, bool echo);
static void help(const char *progname);
+ #define exit_nicely(code) exit(code)
+
int
main(int argc, char *argv[])
{
***************
*** 44,49 **** main(int argc, char *argv[])
--- 50,56 ----
{"dbname", required_argument, NULL, 'd'},
{"all", no_argument, NULL, 'a'},
{"system", no_argument, NULL, 's'},
+ {"usertables", no_argument, NULL, 'u'},
{"table", required_argument, NULL, 't'},
{"index", required_argument, NULL, 'i'},
{"maintenance-db", required_argument, NULL, 2},
***************
*** 61,66 **** main(int argc, char *argv[])
--- 68,74 ----
const char *username = NULL;
enum trivalue prompt_password = TRI_DEFAULT;
bool syscatalog = false;
+ bool usertables = false;
bool alldb = false;
bool echo = false;
bool quiet = false;
***************
*** 73,79 **** 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:wWeqd:ast:i:", long_options, &optindex)) != -1)
{
switch (c)
{
--- 81,87 ----
handle_help_version_opts(argc, argv, "reindexdb", help);
/* process command-line options */
! while ((c = getopt_long(argc, argv, "h:p:U:wWeqd:asut:i:", long_options, &optindex)) != -1)
{
switch (c)
{
***************
*** 107,112 **** main(int argc, char *argv[])
--- 115,123 ----
case 's':
syscatalog = true;
break;
+ case 'u':
+ usertables = true;
+ break;
case 't':
simple_string_list_append(&tables, optarg);
break;
***************
*** 194,199 **** main(int argc, char *argv[])
--- 205,236 ----
reindex_system_catalogs(dbname, host, port, username, prompt_password,
progname, echo);
}
+ else if (usertables)
+ {
+ if (tables.head != NULL)
+ {
+ fprintf(stderr, _("%s: cannot reindex specific table(s) and all user tables at the same time\n"), progname);
+ exit(1);
+ }
+ if (indexes.head != NULL)
+ {
+ fprintf(stderr, _("%s: cannot reindex specific index(es) and all user tables at the same time\n"), progname);
+ exit(1);
+ }
+
+ if (dbname == NULL)
+ {
+ if (getenv("PGDATABASE"))
+ dbname = getenv("PGDATABASE");
+ else if (getenv("PGUSER"))
+ dbname = getenv("PGUSER");
+ else
+ dbname = get_user_name_or_exit(progname);
+ }
+
+ reindex_user_tables(dbname, host, port, username, prompt_password,
+ progname, echo);
+ }
else
{
if (dbname == NULL)
***************
*** 336,341 **** reindex_system_catalogs(const char *dbname, const char *host, const char *port,
--- 373,413 ----
}
static void
+ reindex_user_tables(const char *dbname, const char *host, const char *port,
+ const char *username, enum trivalue prompt_password,
+ const char *progname, bool echo)
+ {
+ PQExpBufferData sql;
+
+ PGconn *conn;
+
+ initPQExpBuffer(&sql);
+
+ appendPQExpBuffer(&sql, "REINDEX USER TABLES %s;", dbname);
+
+ conn = connectDatabase(dbname, host, port, username, prompt_password,
+ progname, false);
+
+ /* This feature is only available starting in 9.5 */
+ if (PQserverVersion(conn) < 90500)
+ {
+ fprintf(stderr, _("%s: REINDEX USER TABLES is only available in server versions 9.5 and newer\n"),
+ progname);
+ exit_nicely(1);
+ }
+
+ if (!executeMaintenanceCommand(conn, sql.data, echo))
+ {
+ fprintf(stderr, _("%s: reindexing of user tables failed: %s"),
+ progname, PQerrorMessage(conn));
+ PQfinish(conn);
+ exit(1);
+ }
+ PQfinish(conn);
+ termPQExpBuffer(&sql);
+ }
+
+ static void
help(const char *progname)
{
printf(_("%s reindexes a PostgreSQL database.\n\n"), progname);
***************
*** 348,353 **** help(const char *progname)
--- 420,426 ----
printf(_(" -i, --index=INDEX recreate specific index(es) only\n"));
printf(_(" -q, --quiet don't write any messages\n"));
printf(_(" -s, --system reindex system catalogs\n"));
+ printf(_(" -u, --usertables reindex all user tables\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"));
*** a/doc/src/sgml/ref/reindex.sgml
--- b/doc/src/sgml/ref/reindex.sgml
***************
*** 22,27 **** PostgreSQL documentation
--- 22,28 ----
<refsynopsisdiv>
<synopsis>
REINDEX { INDEX | TABLE | DATABASE | SYSTEM | USER TABLES } <replaceable class="PARAMETER">name</replaceable> [ FORCE ]
+ REINDEX { DATABASE | SYSTEM | USER TABLES }
</synopsis>
</refsynopsisdiv>
***************
*** 145,151 **** REINDEX { INDEX | TABLE | DATABASE | SYSTEM | USER TABLES } <replaceable class="
reindexed. Index and table names can be schema-qualified.
Presently, <command>REINDEX DATABASE</>, <command>REINDEX SYSTEM</>,
and <command>REINDEX USER TABLES</> can only reindex the current
! database, so their parameter must match the current database's name.
</para>
</listitem>
</varlistentry>
--- 146,153 ----
reindexed. Index and table names can be schema-qualified.
Presently, <command>REINDEX DATABASE</>, <command>REINDEX SYSTEM</>,
and <command>REINDEX USER TABLES</> can only reindex the current
! database, so their parameter must match the current database's name
! if provided.
</para>
</listitem>
</varlistentry>
*** a/src/backend/commands/indexcmds.c
--- b/src/backend/commands/indexcmds.c
***************
*** 1784,1799 **** ReindexDatabase(const char *databaseName, bool do_system, bool do_user)
List *relids = NIL;
ListCell *l;
! AssertArg(databaseName);
!
! if (strcmp(databaseName, get_database_name(MyDatabaseId)) != 0)
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
errmsg("can only reindex the currently open database")));
if (!pg_database_ownercheck(MyDatabaseId, GetUserId()))
aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_DATABASE,
! databaseName);
/*
* Create a memory context that will survive forced transaction commits we
--- 1784,1797 ----
List *relids = NIL;
ListCell *l;
! if (databaseName != NULL && strcmp(databaseName, get_database_name(MyDatabaseId)) != 0)
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
errmsg("can only reindex the currently open database")));
if (!pg_database_ownercheck(MyDatabaseId, GetUserId()))
aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_DATABASE,
! get_database_name(MyDatabaseId));
/*
* Create a memory context that will survive forced transaction commits we
*** a/src/backend/parser/gram.y
--- b/src/backend/parser/gram.y
***************
*** 6974,6979 **** ReindexStmt:
--- 6974,6989 ----
n->do_user = false;
$$ = (Node *)n;
}
+ | REINDEX SYSTEM_P
+ {
+ ReindexStmt *n = makeNode(ReindexStmt);
+ n->kind = OBJECT_DATABASE;
+ n->name = NULL;
+ n->relation = NULL;
+ n->do_system = true;
+ n->do_user = false;
+ $$ = (Node *)n;
+ }
| REINDEX DATABASE name opt_force
{
ReindexStmt *n = makeNode(ReindexStmt);
***************
*** 6984,6989 **** ReindexStmt:
--- 6994,7009 ----
n->do_user = true;
$$ = (Node *)n;
}
+ | REINDEX DATABASE
+ {
+ ReindexStmt *n = makeNode(ReindexStmt);
+ n->kind = OBJECT_DATABASE;
+ n->name = NULL;
+ n->relation = NULL;
+ n->do_system = true;
+ n->do_user = true;
+ $$ = (Node *)n;
+ }
| REINDEX USER TABLES name opt_force
{
ReindexStmt *n = makeNode(ReindexStmt);
***************
*** 6994,6999 **** ReindexStmt:
--- 7014,7029 ----
n->do_user = true;
$$ = (Node *)n;
}
+ | REINDEX USER TABLES
+ {
+ ReindexStmt *n = makeNode(ReindexStmt);
+ n->kind = OBJECT_DATABASE;
+ n->name = NULL;
+ n->relation = NULL;
+ n->do_system = false;
+ n->do_user = true;
+ $$ = (Node *)n;
+ }
;
reindex_type:
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
