On Fri, Mar 6, 2015 at 11:07 AM, Jim Nasby <jim.na...@bluetreble.com> wrote:
> On 3/2/15 10:58 AM, Sawada Masahiko wrote:
>>
>> On Wed, Feb 25, 2015 at 4:58 PM, Jim Nasby <jim.na...@bluetreble.com>
>> wrote:
>>>
>>> On 2/24/15 8:28 AM, Sawada Masahiko wrote:
>>>>>
>>>>>
>>>>> According to the above discussion, VACUUM and REINDEX should have
>>>>> trailing options. Tom seems (to me) suggesting that SQL-style
>>>>> (bare word preceded by WITH) options and Jim suggesting '()'
>>>>> style options? (Anyway VACUUM gets the*third additional* option
>>>>> sytle, but it is the different discussion from this)
>>>
>>>
>>>
>>> Well, almost everything does a trailing WITH. We need to either stick
>>> with
>>> that for consistency, or add leading () as an option to those WITH
>>> commands.
>>>
>>> Does anyone know why those are WITH? Is it ANSI?
>>>
>>> As a refresher, current commands are:
>>>
>>> VACUUM (ANALYZE, VERBOSE) table1 (col1);
>>> REINDEX INDEX index1 FORCE;
>>> COPY table1 FROM 'file.txt' WITH (FORMAT csv);
>>> CREATE MATERIALIZED VIEW mv1 WITH (storageparam, ...) AS qry WITH DATA;
>>> CREATE EXTENSION ext1 WITH SCHEMA s1 VERSION v1 FROM over;
>>> CREATE ROLE role WITH LOGIN;
>>> GRANT .... WITH GRANT OPTION;
>>> CREATE VIEW v1 AS qry WITH CASCADED CHECK OPTION;
>>> ALTER DATABASE db1 WITH CONNECTION LIMIT 50;
>>> DECLARE c1 INSENSITIVE SCROLL CURSOR WITH HOLD;
>
>
> BTW, I'm fine with Tom's bare-word with WITH idea. That seems to be the most
> consistent with everything else. Is there a problem with doing that? I know
> getting syntax is one of the hard parts of new features, but it seems like
> we reached consensus here...
Attached is latest version patch based on Tom's idea as follows.
REINDEX { INDEX | ... } name WITH ( options [, ...] )
>
>> We have discussed about this option including FORCE option, but I
>> think there are not user who want to use both FORCE and VERBOSE option
>> at same time.
>
>
> I find that very hard to believe... I would expect a primary use case for
> VERBOSE to be "I ran REINDEX, but it doesn't seem to have done anything...
> what's going on?" and that's exactly when you might want to use FORCE.
>
In currently code, nothing happens even if FORCE option is specified.
This option completely exist for backward compatibility.
But this patch add new syntax including FORCE option for now.
Todo
- tab completion
- reindexdb command
Regards,
-------
Sawada Masahiko
diff --git a/doc/src/sgml/ref/reindex.sgml b/doc/src/sgml/ref/reindex.sgml
index 0a4c7d4..a4109aa 100644
--- a/doc/src/sgml/ref/reindex.sgml
+++ b/doc/src/sgml/ref/reindex.sgml
@@ -22,6 +22,12 @@ PostgreSQL documentation
<refsynopsisdiv>
<synopsis>
REINDEX { INDEX | TABLE | SCHEMA | DATABASE | SYSTEM } <replaceable class="PARAMETER">name</replaceable> [ FORCE ]
+REINDEX { INDEX | TABLE | SCHEMA | DATABASE | SYSTEM } <replaceable class="PARAMETER">name</replaceable> WITH ( <replaceable class="PARAMETER">options</replaceable> [, ...] )
+
+<phrase>where <replaceable class="PARAMETER">option</replaceable> can be one of:</phrase>
+
+ FORCE [ <replaceable class="PARAMETER">boolean</replaceable> ]
+ VERBOSE [ <replaceable class="PARAMETER">boolean</replaceable> ]
</synopsis>
</refsynopsisdiv>
@@ -159,6 +165,29 @@ REINDEX { INDEX | TABLE | SCHEMA | DATABASE | SYSTEM } <replaceable class="PARAM
</para>
</listitem>
</varlistentry>
+
+ <varlistentry>
+ <term><literal>VERBOSE</literal></term>
+ <listitem>
+ <para>
+ Prints a progress report as each index is reindexed.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
+ <term><replaceable class="parameter">boolean</replaceable></term>
+ <listitem>
+ <para>
+ Specifies whether the selected option should be turned on or off.
+ You can write <literal>TRUE</literal>, <literal>ON</>, or
+ <literal>1</literal> to enable the option, and <literal>FALSE</literal>,
+ <literal>OFF</>, or <literal>0</literal> to disable it. The
+ <replaceable class="parameter">boolean</replaceable> value can also
+ be omitted, in which case <literal>TRUE</literal> is assumed.
+ </para>
+ </listitem>
+ </varlistentry>
</variablelist>
</refsect1>
diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c
index f85ed93..786f173 100644
--- a/src/backend/catalog/index.c
+++ b/src/backend/catalog/index.c
@@ -63,6 +63,7 @@
#include "utils/inval.h"
#include "utils/lsyscache.h"
#include "utils/memutils.h"
+#include "utils/pg_rusage.h"
#include "utils/syscache.h"
#include "utils/tuplesort.h"
#include "utils/snapmgr.h"
@@ -3130,13 +3131,18 @@ IndexGetRelation(Oid indexId, bool missing_ok)
* reindex_index - This routine is used to recreate a single index
*/
void
-reindex_index(Oid indexId, bool skip_constraint_checks, char persistence)
+reindex_index(Oid indexId, bool skip_constraint_checks, char persistence,
+ bool verbose)
{
Relation iRel,
heapRelation;
Oid heapId;
IndexInfo *indexInfo;
volatile bool skipped_constraint = false;
+ int elevel = verbose ? INFO : DEBUG2;
+ PGRUsage ru0;
+
+ pg_rusage_init(&ru0);
/*
* Open and lock the parent heap relation. ShareLock is sufficient since
@@ -3280,6 +3286,13 @@ reindex_index(Oid indexId, bool skip_constraint_checks, char persistence)
heap_close(pg_index, RowExclusiveLock);
}
+ /* Log what we did */
+ ereport(elevel,
+ (errmsg("index \"%s\" was reindexed.",
+ get_rel_name(indexId)),
+ errdetail("%s.",
+ pg_rusage_show(&ru0))));
+
/* Close rels, but keep locks */
index_close(iRel, NoLock);
heap_close(heapRelation, NoLock);
@@ -3321,7 +3334,7 @@ reindex_index(Oid indexId, bool skip_constraint_checks, char persistence)
* index rebuild.
*/
bool
-reindex_relation(Oid relid, int flags)
+reindex_relation(Oid relid, int flags, bool verbose)
{
Relation rel;
Oid toast_relid;
@@ -3412,7 +3425,7 @@ reindex_relation(Oid relid, int flags)
RelationSetIndexList(rel, doneIndexes, InvalidOid);
reindex_index(indexOid, !(flags & REINDEX_REL_CHECK_CONSTRAINTS),
- persistence);
+ persistence, verbose);
CommandCounterIncrement();
@@ -3447,7 +3460,7 @@ reindex_relation(Oid relid, int flags)
* still hold the lock on the master table.
*/
if ((flags & REINDEX_REL_PROCESS_TOAST) && OidIsValid(toast_relid))
- result |= reindex_relation(toast_relid, flags);
+ result |= reindex_relation(toast_relid, flags, verbose);
return result;
}
diff --git a/src/backend/commands/cluster.c b/src/backend/commands/cluster.c
index 3febdd5..34ffaba 100644
--- a/src/backend/commands/cluster.c
+++ b/src/backend/commands/cluster.c
@@ -1532,7 +1532,7 @@ finish_heap_swap(Oid OIDOldHeap, Oid OIDNewHeap,
else if (newrelpersistence == RELPERSISTENCE_PERMANENT)
reindex_flags |= REINDEX_REL_FORCE_INDEXES_PERMANENT;
- reindex_relation(OIDOldHeap, reindex_flags);
+ reindex_relation(OIDOldHeap, reindex_flags, false);
/*
* If the relation being rebuild is pg_class, swap_relation_files()
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index 1c1d0da..24ec705 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -1681,10 +1681,26 @@ ChooseIndexColumnNames(List *indexElems)
* Recreate a specific index.
*/
Oid
-ReindexIndex(RangeVar *indexRelation)
+ReindexIndex(RangeVar *indexRelation, List *options)
{
Oid indOid;
Oid heapOid = InvalidOid;
+ bool verbose = false;
+ ListCell *lc;
+
+ /* Parse list of options */
+ foreach(lc, options)
+ {
+ DefElem *opt = (DefElem *) lfirst(lc);
+ if (strcmp(opt->defname, "verbose") == 0)
+ verbose = defGetBoolean(opt);
+ else if (strcmp(opt->defname, "force") == 0)
+ { /* Nothing to do */ }
+ else
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("unrecognized REINDEX option \"%s\"", opt->defname)));
+ }
/* lock level used here should match index lock reindex_index() */
indOid = RangeVarGetRelidExtended(indexRelation, AccessExclusiveLock,
@@ -1692,7 +1708,7 @@ ReindexIndex(RangeVar *indexRelation)
RangeVarCallbackForReindexIndex,
(void *) &heapOid);
- reindex_index(indOid, false, indexRelation->relpersistence);
+ reindex_index(indOid, false, indexRelation->relpersistence, verbose);
return indOid;
}
@@ -1761,9 +1777,25 @@ RangeVarCallbackForReindexIndex(const RangeVar *relation,
* Recreate all indexes of a table (and of its toast table, if any)
*/
Oid
-ReindexTable(RangeVar *relation)
+ReindexTable(RangeVar *relation, List *options)
{
Oid heapOid;
+ bool verbose = false;
+ ListCell *lc;
+
+ /* Parse list of options */
+ foreach(lc, options)
+ {
+ DefElem *opt = (DefElem *) lfirst(lc);
+ if (strcmp(opt->defname, "verbose") == 0)
+ verbose = defGetBoolean(opt);
+ else if (strcmp(opt->defname, "force") == 0)
+ { /* Nothing to do */ }
+ else
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("unrecognized REINDEX option \"%s\"", opt->defname)));
+ }
/* The lock level used here should match reindex_relation(). */
heapOid = RangeVarGetRelidExtended(relation, ShareLock, false, false,
@@ -1771,7 +1803,8 @@ ReindexTable(RangeVar *relation)
if (!reindex_relation(heapOid,
REINDEX_REL_PROCESS_TOAST |
- REINDEX_REL_CHECK_CONSTRAINTS))
+ REINDEX_REL_CHECK_CONSTRAINTS,
+ verbose))
ereport(NOTICE,
(errmsg("table \"%s\" has no indexes",
relation->relname)));
@@ -1788,7 +1821,7 @@ ReindexTable(RangeVar *relation)
* That means this must not be called within a user transaction block!
*/
void
-ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind)
+ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind, List *options)
{
Oid objectOid;
Relation relationRelation;
@@ -1800,12 +1833,31 @@ ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind)
List *relids = NIL;
ListCell *l;
int num_keys;
+ bool verbose;
+ int elevel = DEBUG2;
AssertArg(objectName);
Assert(objectKind == REINDEX_OBJECT_SCHEMA ||
objectKind == REINDEX_OBJECT_SYSTEM ||
objectKind == REINDEX_OBJECT_DATABASE);
+ /* Parse list of options */
+ foreach(l, options)
+ {
+ DefElem *opt = (DefElem *) lfirst(l);
+ if (strcmp(opt->defname, "verbose") == 0)
+ {
+ verbose = defGetBoolean(opt);
+ elevel = INFO;
+ }
+ else if (strcmp(opt->defname, "force") == 0)
+ { /* Nothing to do */ }
+ else
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("unrecognized REINDEX option \"%s\"", opt->defname)));
+ }
+
/*
* Get OID of object to reindex, being the database currently being used
* by session for a database or for system catalogs, or the schema defined
@@ -1924,9 +1976,10 @@ ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind)
PushActiveSnapshot(GetTransactionSnapshot());
if (reindex_relation(relid,
REINDEX_REL_PROCESS_TOAST |
- REINDEX_REL_CHECK_CONSTRAINTS))
- ereport(DEBUG1,
- (errmsg("table \"%s.%s\" was reindexed",
+ REINDEX_REL_CHECK_CONSTRAINTS,
+ verbose))
+ ereport(elevel,
+ (errmsg("indexes of whole table \"%s.%s\" were reindexed",
get_namespace_name(get_rel_namespace(relid)),
get_rel_name(relid))));
PopActiveSnapshot();
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 6536778..792fc00 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -1225,7 +1225,7 @@ ExecuteTruncate(TruncateStmt *stmt)
/*
* Reconstruct the indexes to match, and we're done.
*/
- reindex_relation(heap_relid, REINDEX_REL_PROCESS_TOAST);
+ reindex_relation(heap_relid, REINDEX_REL_PROCESS_TOAST, false);
}
pgstat_count_truncate(rel);
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index 581f7a1..1c8970f 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -456,6 +456,14 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query);
%type <node> explain_option_arg
%type <defelt> explain_option_elem
%type <list> explain_option_list
+
+%type <str> reindex_option_name
+%type <node> reindex_option_arg
+%type <defelt> reindex_option_elem
+%type <ival> reindex_target_type
+%type <ival> reindex_target_multitable
+%type <list> reindex_option_list
+
%type <node> copy_generic_opt_arg copy_generic_opt_arg_list_item
%type <defelt> copy_generic_opt_elem
%type <list> copy_generic_opt_list copy_generic_opt_arg_list
@@ -7316,57 +7324,87 @@ opt_if_exists: IF_P EXISTS { $$ = TRUE; }
* QUERY:
*
* REINDEX type <name> [FORCE]
+ * REINDEX type <name> WITH ( options )
*
* FORCE no longer does anything, but we accept it for backwards compatibility
*****************************************************************************/
ReindexStmt:
- REINDEX INDEX qualified_name opt_force
+ REINDEX reindex_target_type qualified_name opt_force
{
ReindexStmt *n = makeNode(ReindexStmt);
- n->kind = REINDEX_OBJECT_INDEX;
+ n->kind = $2;
n->relation = $3;
n->name = NULL;
+ n->options = NULL;
$$ = (Node *)n;
}
- | REINDEX TABLE qualified_name opt_force
+ | REINDEX reindex_target_type qualified_name WITH '(' reindex_option_list ')'
{
ReindexStmt *n = makeNode(ReindexStmt);
- n->kind = REINDEX_OBJECT_TABLE;
+ n->kind = $2;
n->relation = $3;
n->name = NULL;
+ n->options = $6;
$$ = (Node *)n;
}
- | REINDEX SCHEMA name opt_force
+ | REINDEX reindex_target_multitable name opt_force
{
ReindexStmt *n = makeNode(ReindexStmt);
- n->kind = REINDEX_OBJECT_SCHEMA;
+ n->kind = $2;
n->name = $3;
n->relation = NULL;
+ n->options = NULL;
$$ = (Node *)n;
}
- | REINDEX SYSTEM_P name opt_force
+ | REINDEX reindex_target_multitable name WITH '(' reindex_option_list ')'
{
ReindexStmt *n = makeNode(ReindexStmt);
- n->kind = REINDEX_OBJECT_SYSTEM;
+ n->kind = $2;
n->name = $3;
n->relation = NULL;
+ n->options = $6;
$$ = (Node *)n;
}
- | REINDEX DATABASE name opt_force
+ ;
+reindex_target_type:
+ INDEX { $$ = REINDEX_OBJECT_INDEX; }
+ | TABLE { $$ = REINDEX_OBJECT_TABLE; }
+;
+reindex_target_multitable:
+ SCHEMA { $$ = REINDEX_OBJECT_SCHEMA; }
+ | SYSTEM_P { $$ = REINDEX_OBJECT_SYSTEM; }
+ | DATABASE { $$ = REINDEX_OBJECT_DATABASE; }
+;
+opt_force:
+ FORCE { $$ = TRUE; }
+ | /* EMPTY */ { $$ = FALSE; }
+;
+reindex_option_list:
+ reindex_option_elem
{
- ReindexStmt *n = makeNode(ReindexStmt);
- n->kind = REINDEX_OBJECT_DATABASE;
- n->name = $3;
- n->relation = NULL;
- $$ = (Node *)n;
+ $$ = list_make1($1);
}
- ;
-
-opt_force: FORCE { $$ = TRUE; }
- | /* EMPTY */ { $$ = FALSE; }
- ;
-
+ | reindex_option_list ',' reindex_option_elem
+ {
+ $$ = lappend($1, $3);
+ }
+;
+reindex_option_elem:
+ reindex_option_name reindex_option_arg
+ {
+ $$ = makeDefElem($1, $2);
+ }
+;
+reindex_option_name:
+ VERBOSE { $$ = "verbose"; }
+ | FORCE { $$ = "force"; }
+;
+reindex_option_arg:
+ opt_boolean_or_string { $$ = (Node *) makeString($1); }
+ | NumericOnly { $$ = (Node *) $1; }
+ | /* EMPTY */ { $$ = NULL; }
+;
/*****************************************************************************
*
diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c
index 126e38d..0b41701 100644
--- a/src/backend/tcop/utility.c
+++ b/src/backend/tcop/utility.c
@@ -737,10 +737,10 @@ standard_ProcessUtility(Node *parsetree,
switch (stmt->kind)
{
case REINDEX_OBJECT_INDEX:
- ReindexIndex(stmt->relation);
+ ReindexIndex(stmt->relation, stmt->options);
break;
case REINDEX_OBJECT_TABLE:
- ReindexTable(stmt->relation);
+ ReindexTable(stmt->relation, stmt->options);
break;
case REINDEX_OBJECT_SCHEMA:
case REINDEX_OBJECT_SYSTEM:
@@ -756,7 +756,7 @@ standard_ProcessUtility(Node *parsetree,
(stmt->kind == REINDEX_OBJECT_SCHEMA) ? "REINDEX SCHEMA" :
(stmt->kind == REINDEX_OBJECT_SYSTEM) ? "REINDEX SYSTEM" :
"REINDEX DATABASE");
- ReindexMultipleTables(stmt->name, stmt->kind);
+ ReindexMultipleTables(stmt->name, stmt->kind, stmt->options);
break;
default:
elog(ERROR, "unrecognized object type: %d",
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index e39a07c..d3835c8 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -3342,7 +3342,7 @@ psql_completion(const char *text, int start, int end)
COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_tm, NULL);
else if (pg_strcasecmp(prev_wd, "INDEX") == 0)
COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_indexes, NULL);
- else if (pg_strcasecmp(prev_wd, "SCHEMA") == 0 )
+ else if (pg_strcasecmp(prev_wd, "SCHEMA") == 0)
COMPLETE_WITH_QUERY(Query_for_list_of_schemas);
else if (pg_strcasecmp(prev_wd, "SYSTEM") == 0 ||
pg_strcasecmp(prev_wd, "DATABASE") == 0)
diff --git a/src/include/catalog/index.h b/src/include/catalog/index.h
index e7cc7a0..48f8660 100644
--- a/src/include/catalog/index.h
+++ b/src/include/catalog/index.h
@@ -112,7 +112,7 @@ extern void validate_index(Oid heapId, Oid indexId, Snapshot snapshot);
extern void index_set_state_flags(Oid indexId, IndexStateFlagsAction action);
extern void reindex_index(Oid indexId, bool skip_constraint_checks,
- char relpersistence);
+ char relpersistence, bool verbose);
/* Flag bits for reindex_relation(): */
#define REINDEX_REL_PROCESS_TOAST 0x01
@@ -121,7 +121,7 @@ extern void reindex_index(Oid indexId, bool skip_constraint_checks,
#define REINDEX_REL_FORCE_INDEXES_UNLOGGED 0x08
#define REINDEX_REL_FORCE_INDEXES_PERMANENT 0x10
-extern bool reindex_relation(Oid relid, int flags);
+extern bool reindex_relation(Oid relid, int flags, bool verbose);
extern bool ReindexIsProcessingHeap(Oid heapOid);
extern bool ReindexIsProcessingIndex(Oid indexOid);
diff --git a/src/include/commands/defrem.h b/src/include/commands/defrem.h
index a9c6783..0b5d023 100644
--- a/src/include/commands/defrem.h
+++ b/src/include/commands/defrem.h
@@ -29,9 +29,9 @@ extern ObjectAddress DefineIndex(Oid relationId,
bool check_rights,
bool skip_build,
bool quiet);
-extern Oid ReindexIndex(RangeVar *indexRelation);
-extern Oid ReindexTable(RangeVar *relation);
-extern void ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind);
+extern Oid ReindexIndex(RangeVar *indexRelation, List *options);
+extern Oid ReindexTable(RangeVar *relation, List *optionsc);
+extern void ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind, List *options);
extern char *makeObjectName(const char *name1, const char *name2,
const char *label);
extern char *ChooseRelationName(const char *name1, const char *name2,
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index ac13302..fb98fa7 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -2747,6 +2747,7 @@ typedef struct ReindexStmt
ReindexObjectType kind; /* REINDEX_OBJECT_INDEX, REINDEX_OBJECT_TABLE, etc. */
RangeVar *relation; /* Table or index to reindex */
const char *name; /* name of database to reindex */
+ List *options; /* list of options */
} ReindexStmt;
/* ----------------------
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
