On Fri, Apr 10, 2015 at 2:52 AM, Sawada Masahiko <sawada.m...@gmail.com> wrote: > On Thu, Apr 9, 2015 at 1:14 PM, Fujii Masao <masao.fu...@gmail.com> wrote: >> On Wed, Apr 8, 2015 at 10:53 PM, Sawada Masahiko <sawada.m...@gmail.com> >> wrote: >>> On Wed, Apr 8, 2015 at 1:09 PM, Fujii Masao <masao.fu...@gmail.com> wrote: >>>> On Wed, Apr 8, 2015 at 1:57 AM, Sawada Masahiko <sawada.m...@gmail.com> >>>> wrote: >>>>> On Wed, Apr 8, 2015 at 1:11 AM, Fabrízio de Royes Mello >>>>> <fabriziome...@gmail.com> wrote: >>>>>> >>>>>> On Tue, Apr 7, 2015 at 1:04 PM, Sawada Masahiko <sawada.m...@gmail.com> >>>>>> wrote: >>>>>>> >>>>>>> On Tue, Apr 7, 2015 at 10:12 PM, Fabrízio de Royes Mello >>>>>>> <fabriziome...@gmail.com> wrote: >>>>>>> > >>>>>>> > On Tue, Apr 7, 2015 at 7:22 AM, Sawada Masahiko >>>>>>> > <sawada.m...@gmail.com> >>>>>>> > wrote: >>>>>>> >> >>>>>>> >> Thank you for your reviewing. >>>>>>> >> I modified the patch and attached latest version patch(v7). >>>>>>> >> Please have a look it. >>>>>>> >> >>>>>>> > >>>>>>> > Looks good to me. Attached patch (v8) just fix a tab indentation in >>>>>>> > gram.y. >>>>>>> > >>>>>>> >>>>>>> I had forgotten fix a tab indentation, sorry. >>>>>>> Thank you for reviewing! >>>>>>> It looks good to me too. >>>>>>> Can this patch be marked as "Ready for Committer"? >>>>>>> >>>>>> >>>>>> +1 >>>>>> >>>>> >>>>> Changed status to "Ready for Committer". >>>> >>>> The patch adds new syntax like "REINDEX ... WITH VERBOSE", i.e., () is not >>>> added after WITH clause. Did we reach the consensus about this syntax? >>>> The last email from Robert just makes me think that () should be added >>>> into the syntax. >>>> >>> >>> Thank you for taking time for this patch! >> >> I removed the FORCE option from REINDEX, so you would need to update the >> patch. > > Thanks. > I will change the patch with this change. > >>> This was quite complicated issue since we already have a lot of style >>> command currently. >>> We can think many of style from various perspective: kind of DDL, new >>> or old command, maintenance command. And each command has each its >>> story. >>> I believe we have reached the consensus with this style at least once >>> (please see previous discussion), but we might needs to discuss >>> more... >> >> Okay, another question is that; WITH must be required whenever the options >> are specified? Or should it be abbreviatable? > > In previous discussion, the WITH clause is always required by VERBOSE > option. I don't think to enable us to abbreviate WITH clause for now. > Also, at this time that FORCE option is removed, we could bring back > idea is to put VERBOSE at after object name like CLUSTER. (INDEX, > TABLE, etc.) >
Attached v10 patch is latest version patch. The syntax is, REINDEX { INDEX | ... } name [ WITH ] [ VERBOSE ] That is, WITH clause is optional. Regards, ------- Sawada Masahiko
diff --git a/doc/src/sgml/ref/reindex.sgml b/doc/src/sgml/ref/reindex.sgml index 998340c..2e8db5a 100644 --- a/doc/src/sgml/ref/reindex.sgml +++ b/doc/src/sgml/ref/reindex.sgml @@ -21,7 +21,7 @@ PostgreSQL documentation <refsynopsisdiv> <synopsis> -REINDEX { INDEX | TABLE | SCHEMA | DATABASE | SYSTEM } <replaceable class="PARAMETER">name</replaceable> +REINDEX { INDEX | TABLE | SCHEMA | DATABASE | SYSTEM } <replaceable class="PARAMETER">name</replaceable> [ WITH ] [ VERBOSE ] </synopsis> </refsynopsisdiv> @@ -150,6 +150,15 @@ 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> </variablelist> </refsect1> diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c index ac3b785..27364ec 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" @@ -3133,13 +3134,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 @@ -3283,6 +3289,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); @@ -3324,7 +3337,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; @@ -3415,7 +3428,7 @@ reindex_relation(Oid relid, int flags) RelationSetIndexList(rel, doneIndexes, InvalidOid); reindex_index(indexOid, !(flags & REINDEX_REL_CHECK_CONSTRAINTS), - persistence); + persistence, verbose); CommandCounterIncrement(); @@ -3450,7 +3463,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 351d48e..5d44bda 100644 --- a/src/backend/commands/indexcmds.c +++ b/src/backend/commands/indexcmds.c @@ -1681,7 +1681,7 @@ ChooseIndexColumnNames(List *indexElems) * Recreate a specific index. */ Oid -ReindexIndex(RangeVar *indexRelation) +ReindexIndex(RangeVar *indexRelation, bool verbose) { Oid indOid; Oid heapOid = InvalidOid; @@ -1706,7 +1706,7 @@ ReindexIndex(RangeVar *indexRelation) persistence = irel->rd_rel->relpersistence; index_close(irel, NoLock); - reindex_index(indOid, false, persistence); + reindex_index(indOid, false, persistence, verbose); return indOid; } @@ -1775,7 +1775,7 @@ RangeVarCallbackForReindexIndex(const RangeVar *relation, * Recreate all indexes of a table (and of its toast table, if any) */ Oid -ReindexTable(RangeVar *relation) +ReindexTable(RangeVar *relation, bool verbose) { Oid heapOid; @@ -1785,7 +1785,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))); @@ -1802,7 +1803,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, bool verbose) { Oid objectOid; Relation relationRelation; @@ -1814,6 +1815,7 @@ ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind) List *relids = NIL; ListCell *l; int num_keys; + int elevel = verbose ? INFO : DEBUG2; AssertArg(objectName); Assert(objectKind == REINDEX_OBJECT_SCHEMA || @@ -1938,9 +1940,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 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 5d84285..7ef9f51 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -1234,7 +1234,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/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c index 1685efe..6b1d119 100644 --- a/src/backend/nodes/copyfuncs.c +++ b/src/backend/nodes/copyfuncs.c @@ -3778,6 +3778,7 @@ _copyReindexStmt(const ReindexStmt *from) COPY_SCALAR_FIELD(kind); COPY_NODE_FIELD(relation); COPY_STRING_FIELD(name); + COPY_SCALAR_FIELD(verbose); return newnode; } diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c index 578ead5..ef816cc 100644 --- a/src/backend/nodes/equalfuncs.c +++ b/src/backend/nodes/equalfuncs.c @@ -1908,6 +1908,7 @@ _equalReindexStmt(const ReindexStmt *a, const ReindexStmt *b) COMPARE_SCALAR_FIELD(kind); COMPARE_NODE_FIELD(relation); COMPARE_STRING_FIELD(name); + COMPARE_SCALAR_FIELD(verbose); return true; } diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y index 962a69d..27974b8 100644 --- a/src/backend/parser/gram.y +++ b/src/backend/parser/gram.y @@ -459,6 +459,10 @@ 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 <ival> reindex_target_type +%type <ival> reindex_target_multitable + %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 @@ -7383,51 +7387,56 @@ DropTransformStmt: DROP TRANSFORM opt_if_exists FOR Typename LANGUAGE name opt_d * * QUERY: * - * REINDEX type <name> + * REINDEX type <name> [WITH] [VERBOSE] *****************************************************************************/ ReindexStmt: - REINDEX INDEX qualified_name + REINDEX reindex_target_type qualified_name WITH opt_verbose { ReindexStmt *n = makeNode(ReindexStmt); - n->kind = REINDEX_OBJECT_INDEX; + n->kind = $2; n->relation = $3; n->name = NULL; + n->verbose = $5; $$ = (Node *)n; } - | REINDEX TABLE qualified_name + | REINDEX reindex_target_type qualified_name opt_verbose { ReindexStmt *n = makeNode(ReindexStmt); - n->kind = REINDEX_OBJECT_TABLE; + n->kind = $2; n->relation = $3; n->name = NULL; + n->verbose = $4; $$ = (Node *)n; } - | REINDEX SCHEMA name - { - ReindexStmt *n = makeNode(ReindexStmt); - n->kind = REINDEX_OBJECT_SCHEMA; - n->name = $3; - n->relation = NULL; - $$ = (Node *)n; - } - | REINDEX SYSTEM_P name + | REINDEX reindex_target_multitable name WITH opt_verbose { ReindexStmt *n = makeNode(ReindexStmt); - n->kind = REINDEX_OBJECT_SYSTEM; + n->kind = $2; n->name = $3; n->relation = NULL; + n->verbose = $5; $$ = (Node *)n; } - | REINDEX DATABASE name + | REINDEX reindex_target_multitable name opt_verbose { ReindexStmt *n = makeNode(ReindexStmt); - n->kind = REINDEX_OBJECT_DATABASE; + n->kind = $2; n->name = $3; n->relation = NULL; + n->verbose = $4; $$ = (Node *)n; } ; +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; } +; /***************************************************************************** diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c index 31e9d4c..98c3b29 100644 --- a/src/backend/tcop/utility.c +++ b/src/backend/tcop/utility.c @@ -738,10 +738,10 @@ standard_ProcessUtility(Node *parsetree, switch (stmt->kind) { case REINDEX_OBJECT_INDEX: - ReindexIndex(stmt->relation); + ReindexIndex(stmt->relation, stmt->verbose); break; case REINDEX_OBJECT_TABLE: - ReindexTable(stmt->relation); + ReindexTable(stmt->relation, stmt->verbose); break; case REINDEX_OBJECT_SCHEMA: case REINDEX_OBJECT_SYSTEM: @@ -757,7 +757,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->verbose); 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 750e29d..d46d04d 100644 --- a/src/bin/psql/tab-complete.c +++ b/src/bin/psql/tab-complete.c @@ -3346,12 +3346,36 @@ 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) COMPLETE_WITH_QUERY(Query_for_list_of_databases); } + else if (pg_strcasecmp(prev3_wd, "REINDEX") == 0) + { + if (pg_strcasecmp(prev2_wd, "TABLE") == 0 || + pg_strcasecmp(prev2_wd, "INDEX") == 0 || + pg_strcasecmp(prev2_wd, "SCHEMA") == 0 || + pg_strcasecmp(prev2_wd, "SYSTEM") == 0 || + pg_strcasecmp(prev2_wd, "DATABASE") == 0) + { + static const char *const list_REINDEX_option[] = + {"WITH", "VERBOSE", NULL}; + COMPLETE_WITH_LIST(list_REINDEX_option); + } + } + else if (pg_strcasecmp(prev4_wd, "REINDEX") == 0) + { + if ((pg_strcasecmp(prev3_wd, "TABLE") == 0 || + pg_strcasecmp(prev3_wd, "INDEX") == 0 || + pg_strcasecmp(prev3_wd, "SCHEMA") == 0 || + pg_strcasecmp(prev3_wd, "SYSTEM") == 0 || + pg_strcasecmp(prev3_wd, "DATABASE") == 0) && + pg_strcasecmp(prev_wd, "WITH") == 0) + COMPLETE_WITH_CONST("VERBOSE"); + } + /* SECURITY LABEL */ else if (pg_strcasecmp(prev_wd, "SECURITY") == 0) diff --git a/src/include/catalog/index.h b/src/include/catalog/index.h index a04def9..0662f46 100644 --- a/src/include/catalog/index.h +++ b/src/include/catalog/index.h @@ -113,7 +113,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 @@ -122,7 +122,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 335f09c..a1d334f 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, bool verbose); +extern Oid ReindexTable(RangeVar *relation, bool verbose ); +extern void ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind, bool verbose); 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 852eb4f..3db9c6c 100644 --- a/src/include/nodes/parsenodes.h +++ b/src/include/nodes/parsenodes.h @@ -2760,6 +2760,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 */ + bool verbose; /* print progress info */ } ReindexStmt; /* ---------------------- diff --git a/src/test/regress/expected/create_index.out b/src/test/regress/expected/create_index.out index abe64e5..d062148 100644 --- a/src/test/regress/expected/create_index.out +++ b/src/test/regress/expected/create_index.out @@ -2832,6 +2832,16 @@ explain (costs off) (2 rows) -- +-- REINDEX [ WITH ] VERBOSE +-- +CREATE TABLE reindex_verbose(id integer primary key); +\set VERBOSITY terse +REINDEX TABLE reindex_verbose WITH VERBOSE; +INFO: index "reindex_verbose_pkey" was reindexed. +REINDEX TABLE reindex_verbose VERBOSE; +INFO: index "reindex_verbose_pkey" was reindexed. +DROP TABLE reindex_verbose; +-- -- REINDEX SCHEMA -- REINDEX SCHEMA schema_to_reindex; -- failure, schema does not exist diff --git a/src/test/regress/sql/create_index.sql b/src/test/regress/sql/create_index.sql index f779fa0..ae103bf 100644 --- a/src/test/regress/sql/create_index.sql +++ b/src/test/regress/sql/create_index.sql @@ -966,6 +966,15 @@ explain (costs off) select * from tenk1 where (thousand, tenthous) in ((1,1001), (null,null)); -- +-- REINDEX [ WITH ] VERBOSE +-- +CREATE TABLE reindex_verbose(id integer primary key); +\set VERBOSITY terse +REINDEX TABLE reindex_verbose WITH VERBOSE; +REINDEX TABLE reindex_verbose VERBOSE; +DROP TABLE reindex_verbose; + +-- -- REINDEX SCHEMA -- REINDEX SCHEMA schema_to_reindex; -- failure, schema does not exist
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers