public inbox for [email protected]
help / color / mirror / Atom feed[PATCH v22 1/5] Change REINDEX/CLUSTER to accept an option list..
4+ messages / 3 participants
[nested] [flat]
* [PATCH v22 1/5] Change REINDEX/CLUSTER to accept an option list..
@ 2020-03-27 22:50 Justin Pryzby <[email protected]>
0 siblings, 0 replies; 4+ messages in thread
From: Justin Pryzby @ 2020-03-27 22:50 UTC (permalink / raw)
..like EXPLAIN (..), VACUUM (..), and ANALYZE (..).
Change docs in the style of VACUUM. See also: 52dcfda48778d16683c64ca4372299a099a15b96
---
doc/src/sgml/ref/cluster.sgml | 29 ++++++++++++++++++---
doc/src/sgml/ref/reindex.sgml | 43 +++++++++++++++++++++-----------
src/backend/commands/cluster.c | 23 ++++++++++++++++-
src/backend/commands/indexcmds.c | 41 ++++++++++++++++--------------
src/backend/nodes/copyfuncs.c | 2 ++
src/backend/nodes/equalfuncs.c | 2 ++
src/backend/parser/gram.y | 35 +++++++++++++++-----------
src/backend/tcop/utility.c | 36 +++++++++++++++++++++++---
src/bin/psql/tab-complete.c | 23 +++++++++++++----
src/include/commands/cluster.h | 3 ++-
src/include/commands/defrem.h | 7 +++---
src/include/nodes/parsenodes.h | 2 ++
12 files changed, 180 insertions(+), 66 deletions(-)
diff --git a/doc/src/sgml/ref/cluster.sgml b/doc/src/sgml/ref/cluster.sgml
index 4da60d8d56..a6df8a3d81 100644
--- a/doc/src/sgml/ref/cluster.sgml
+++ b/doc/src/sgml/ref/cluster.sgml
@@ -21,8 +21,13 @@ PostgreSQL documentation
<refsynopsisdiv>
<synopsis>
-CLUSTER [VERBOSE] <replaceable class="parameter">table_name</replaceable> [ USING <replaceable class="parameter">index_name</replaceable> ]
-CLUSTER [VERBOSE]
+CLUSTER [VERBOSE] [ ( <replaceable class="parameter">option</replaceable> [, ...] ) ] <replaceable class="parameter">table_name</replaceable> [ USING <replaceable class="parameter">index_name</replaceable> ]
+CLUSTER [VERBOSE] [ ( <replaceable class="parameter">option</replaceable> [, ...] ) ]
+
+<phrase>where <replaceable class="parameter">option</replaceable> can be one of:</phrase>
+
+ VERBOSE [ <replaceable class="parameter">boolean</replaceable> ]
+
</synopsis>
</refsynopsisdiv>
@@ -81,6 +86,16 @@ CLUSTER [VERBOSE]
<title>Parameters</title>
<variablelist>
+ <varlistentry>
+ <term><literal>VERBOSE</literal></term>
+ <listitem>
+ <para>
+ Prints a progress report as each table is clustered.
+<!-- When specified within parenthesis, <literal>VERBOSE</literal> may be followed by a boolean ...-->
+ </para>
+ </listitem>
+ </varlistentry>
+
<varlistentry>
<term><replaceable class="parameter">table_name</replaceable></term>
<listitem>
@@ -100,13 +115,19 @@ CLUSTER [VERBOSE]
</varlistentry>
<varlistentry>
- <term><literal>VERBOSE</literal></term>
+ <term><replaceable class="parameter">boolean</replaceable></term>
<listitem>
<para>
- Prints a progress report as each table is clustered.
+ Specifies whether the selected option should be turned on or off.
+ You can write <literal>TRUE</literal>, <literal>ON</literal>, or
+ <literal>1</literal> to enable the option, and <literal>FALSE</literal>,
+ <literal>OFF</literal>, 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/doc/src/sgml/ref/reindex.sgml b/doc/src/sgml/ref/reindex.sgml
index aac5d5be23..ff369a7313 100644
--- a/doc/src/sgml/ref/reindex.sgml
+++ b/doc/src/sgml/ref/reindex.sgml
@@ -25,7 +25,7 @@ REINDEX [ ( <replaceable class="parameter">option</replaceable> [, ...] ) ] { IN
<phrase>where <replaceable class="parameter">option</replaceable> can be one of:</phrase>
- VERBOSE
+ VERBOSE [ <replaceable class="parameter">boolean</replaceable> ]
</synopsis>
</refsynopsisdiv>
@@ -141,19 +141,6 @@ REINDEX [ ( <replaceable class="parameter">option</replaceable> [, ...] ) ] { IN
</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> and <command>REINDEX SYSTEM</command>
- can only reindex the current database, so their parameter must match
- the current database's name.
- </para>
- </listitem>
- </varlistentry>
-
<varlistentry>
<term><literal>CONCURRENTLY</literal></term>
<listitem>
@@ -181,6 +168,34 @@ REINDEX [ ( <replaceable class="parameter">option</replaceable> [, ...] ) ] { IN
</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> and <command>REINDEX SYSTEM</command>
+ can only reindex the current database, so their parameter must match
+ the current database's name.
+ </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</literal>, or
+ <literal>1</literal> to enable the option, and <literal>FALSE</literal>,
+ <literal>OFF</literal>, 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/commands/cluster.c b/src/backend/commands/cluster.c
index 04d12a7ece..1817b8829a 100644
--- a/src/backend/commands/cluster.c
+++ b/src/backend/commands/cluster.c
@@ -35,6 +35,7 @@
#include "catalog/pg_am.h"
#include "catalog/toasting.h"
#include "commands/cluster.h"
+#include "commands/defrem.h"
#include "commands/progress.h"
#include "commands/tablecmds.h"
#include "commands/vacuum.h"
@@ -99,8 +100,28 @@ static List *get_tables_to_cluster(MemoryContext cluster_context);
*---------------------------------------------------------------------------
*/
void
-cluster(ClusterStmt *stmt, bool isTopLevel)
+cluster(ParseState *pstate, ClusterStmt *stmt, bool isTopLevel)
{
+ ListCell *lc;
+
+ /* Parse list of generic parameters not handled by the parser */
+ foreach(lc, stmt->params)
+ {
+ DefElem *opt = (DefElem *) lfirst(lc);
+
+ if (strcmp(opt->defname, "verbose") == 0)
+ if (defGetBoolean(opt))
+ stmt->options |= CLUOPT_VERBOSE;
+ else
+ stmt->options &= ~CLUOPT_VERBOSE;
+ else
+ ereport(ERROR,
+ (errcode(ERRCODE_SYNTAX_ERROR),
+ errmsg("unrecognized CLUSTER option \"%s\"",
+ opt->defname),
+ parser_errposition(pstate, opt->location)));
+ }
+
if (stmt->relation != NULL)
{
/* This is the single-relation case. */
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index 7819266a63..a22b32fc74 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -2420,8 +2420,9 @@ ChooseIndexColumnNames(List *indexElems)
* Recreate a specific index.
*/
void
-ReindexIndex(RangeVar *indexRelation, int options, bool concurrent)
+ReindexIndex(ReindexStmt *stmt)
{
+ RangeVar *indexRelation = stmt->relation;
struct ReindexIndexCallbackState state;
Oid indOid;
Relation irel;
@@ -2437,10 +2438,10 @@ ReindexIndex(RangeVar *indexRelation, int options, bool concurrent)
* upgrade the lock, but that's OK, because other sessions can't hold
* locks on our temporary table.
*/
- state.concurrent = concurrent;
+ state.concurrent = stmt->concurrent;
state.locked_table_oid = InvalidOid;
indOid = RangeVarGetRelidExtended(indexRelation,
- concurrent ? ShareUpdateExclusiveLock : AccessExclusiveLock,
+ stmt->concurrent ? ShareUpdateExclusiveLock : AccessExclusiveLock,
0,
RangeVarCallbackForReindexIndex,
&state);
@@ -2460,11 +2461,11 @@ ReindexIndex(RangeVar *indexRelation, int options, bool concurrent)
persistence = irel->rd_rel->relpersistence;
index_close(irel, NoLock);
- if (concurrent && persistence != RELPERSISTENCE_TEMP)
- ReindexRelationConcurrently(indOid, options);
+ if (stmt->concurrent && persistence != RELPERSISTENCE_TEMP)
+ ReindexRelationConcurrently(indOid, stmt->options);
else
reindex_index(indOid, false, persistence,
- options | REINDEXOPT_REPORT_PROGRESS);
+ stmt->options | REINDEXOPT_REPORT_PROGRESS);
}
/*
@@ -2542,8 +2543,9 @@ RangeVarCallbackForReindexIndex(const RangeVar *relation,
* Recreate all indexes of a table (and of its toast table, if any)
*/
Oid
-ReindexTable(RangeVar *relation, int options, bool concurrent)
+ReindexTable(ReindexStmt *stmt)
{
+ RangeVar *relation = stmt->relation;
Oid heapOid;
bool result;
@@ -2556,13 +2558,13 @@ ReindexTable(RangeVar *relation, int options, bool concurrent)
* locks on our temporary table.
*/
heapOid = RangeVarGetRelidExtended(relation,
- concurrent ? ShareUpdateExclusiveLock : ShareLock,
+ stmt->concurrent ? ShareUpdateExclusiveLock : ShareLock,
0,
RangeVarCallbackOwnsTable, NULL);
- if (concurrent && get_rel_persistence(heapOid) != RELPERSISTENCE_TEMP)
+ if (stmt->concurrent && get_rel_persistence(heapOid) != RELPERSISTENCE_TEMP)
{
- result = ReindexRelationConcurrently(heapOid, options);
+ result = ReindexRelationConcurrently(heapOid, stmt->options);
if (!result)
ereport(NOTICE,
@@ -2574,7 +2576,7 @@ ReindexTable(RangeVar *relation, int options, bool concurrent)
result = reindex_relation(heapOid,
REINDEX_REL_PROCESS_TOAST |
REINDEX_REL_CHECK_CONSTRAINTS,
- options | REINDEXOPT_REPORT_PROGRESS);
+ stmt->options | REINDEXOPT_REPORT_PROGRESS);
if (!result)
ereport(NOTICE,
(errmsg("table \"%s\" has no indexes to reindex",
@@ -2593,9 +2595,10 @@ ReindexTable(RangeVar *relation, int options, bool concurrent)
* That means this must not be called within a user transaction block!
*/
void
-ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind,
- int options, bool concurrent)
+ReindexMultipleTables(ReindexStmt *stmt)
{
+ const char *objectName = stmt->name;
+ ReindexObjectType objectKind = stmt->kind;
Oid objectOid;
Relation relationRelation;
TableScanDesc scan;
@@ -2613,7 +2616,7 @@ ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind,
objectKind == REINDEX_OBJECT_SYSTEM ||
objectKind == REINDEX_OBJECT_DATABASE);
- if (objectKind == REINDEX_OBJECT_SYSTEM && concurrent)
+ if (objectKind == REINDEX_OBJECT_SYSTEM && stmt->concurrent)
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
errmsg("cannot reindex system catalogs concurrently")));
@@ -2724,7 +2727,7 @@ ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind,
* Skip system tables, since index_create() would reject indexing them
* concurrently (and it would likely fail if we tried).
*/
- if (concurrent &&
+ if (stmt->concurrent &&
IsCatalogRelationOid(relid))
{
if (!concurrent_warning)
@@ -2766,9 +2769,9 @@ ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind,
/* functions in indexes may want a snapshot set */
PushActiveSnapshot(GetTransactionSnapshot());
- if (concurrent && get_rel_persistence(relid) != RELPERSISTENCE_TEMP)
+ if (stmt->concurrent && get_rel_persistence(relid) != RELPERSISTENCE_TEMP)
{
- (void) ReindexRelationConcurrently(relid, options);
+ (void) ReindexRelationConcurrently(relid, stmt->options);
/* ReindexRelationConcurrently() does the verbose output */
}
else
@@ -2778,9 +2781,9 @@ ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind,
result = reindex_relation(relid,
REINDEX_REL_PROCESS_TOAST |
REINDEX_REL_CHECK_CONSTRAINTS,
- options | REINDEXOPT_REPORT_PROGRESS);
+ stmt->options | REINDEXOPT_REPORT_PROGRESS);
- if (result && (options & REINDEXOPT_VERBOSE))
+ if (result && (stmt->options & REINDEXOPT_VERBOSE))
ereport(INFO,
(errmsg("table \"%s.%s\" was reindexed",
get_namespace_name(get_rel_namespace(relid)),
diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c
index 89c409de66..71548acc0c 100644
--- a/src/backend/nodes/copyfuncs.c
+++ b/src/backend/nodes/copyfuncs.c
@@ -3358,6 +3358,7 @@ _copyClusterStmt(const ClusterStmt *from)
COPY_NODE_FIELD(relation);
COPY_STRING_FIELD(indexname);
COPY_SCALAR_FIELD(options);
+ COPY_NODE_FIELD(params);
return newnode;
}
@@ -4450,6 +4451,7 @@ _copyReindexStmt(const ReindexStmt *from)
COPY_NODE_FIELD(relation);
COPY_STRING_FIELD(name);
COPY_SCALAR_FIELD(options);
+ COPY_NODE_FIELD(params);
COPY_SCALAR_FIELD(concurrent);
return newnode;
diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c
index e3f33c40be..de48a42cdd 100644
--- a/src/backend/nodes/equalfuncs.c
+++ b/src/backend/nodes/equalfuncs.c
@@ -1217,6 +1217,7 @@ _equalClusterStmt(const ClusterStmt *a, const ClusterStmt *b)
COMPARE_NODE_FIELD(relation);
COMPARE_STRING_FIELD(indexname);
COMPARE_SCALAR_FIELD(options);
+ COMPARE_NODE_FIELD(params);
return true;
}
@@ -2135,6 +2136,7 @@ _equalReindexStmt(const ReindexStmt *a, const ReindexStmt *b)
COMPARE_NODE_FIELD(relation);
COMPARE_STRING_FIELD(name);
COMPARE_SCALAR_FIELD(options);
+ COMPARE_NODE_FIELD(params);
COMPARE_SCALAR_FIELD(concurrent);
return true;
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index dbb47d4982..b909b161a6 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -519,7 +519,6 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query);
%type <list> explain_option_list
%type <ival> reindex_target_type reindex_target_multitable
-%type <ival> reindex_option_list reindex_option_elem
%type <node> copy_generic_opt_arg copy_generic_opt_arg_list_item
%type <defelt> copy_generic_opt_elem
@@ -8180,7 +8179,7 @@ ReindexStmt:
n->concurrent = $3;
n->relation = $4;
n->name = NULL;
- n->options = 0;
+ n->params = NIL;
$$ = (Node *)n;
}
| REINDEX reindex_target_multitable opt_concurrently name
@@ -8190,27 +8189,27 @@ ReindexStmt:
n->concurrent = $3;
n->name = $4;
n->relation = NULL;
- n->options = 0;
+ n->params = NIL;
$$ = (Node *)n;
}
- | REINDEX '(' reindex_option_list ')' reindex_target_type opt_concurrently qualified_name
+ | REINDEX '(' vac_analyze_option_list ')' reindex_target_type opt_concurrently qualified_name
{
ReindexStmt *n = makeNode(ReindexStmt);
n->kind = $5;
n->concurrent = $6;
n->relation = $7;
n->name = NULL;
- n->options = $3;
+ n->params = $3;
$$ = (Node *)n;
}
- | REINDEX '(' reindex_option_list ')' reindex_target_multitable opt_concurrently name
+ | REINDEX '(' vac_analyze_option_list ')' reindex_target_multitable opt_concurrently name
{
ReindexStmt *n = makeNode(ReindexStmt);
n->kind = $5;
n->concurrent = $6;
n->name = $7;
n->relation = NULL;
- n->options = $3;
+ n->params = $3;
$$ = (Node *)n;
}
;
@@ -8223,13 +8222,6 @@ reindex_target_multitable:
| SYSTEM_P { $$ = REINDEX_OBJECT_SYSTEM; }
| DATABASE { $$ = REINDEX_OBJECT_DATABASE; }
;
-reindex_option_list:
- reindex_option_elem { $$ = $1; }
- | reindex_option_list ',' reindex_option_elem { $$ = $1 | $3; }
- ;
-reindex_option_elem:
- VERBOSE { $$ = REINDEXOPT_VERBOSE; }
- ;
/*****************************************************************************
*
@@ -10380,6 +10372,7 @@ CreateConversionStmt:
*
* QUERY:
* CLUSTER [VERBOSE] <qualified_name> [ USING <index_name> ]
+ * CLUSTER [VERBOSE] [(options)] <qualified_name> [ USING <index_name> ]
* CLUSTER [VERBOSE]
* CLUSTER [VERBOSE] <index_name> ON <qualified_name> (for pre-8.3)
*
@@ -10394,6 +10387,18 @@ ClusterStmt:
n->options = 0;
if ($2)
n->options |= CLUOPT_VERBOSE;
+ n->params = NIL;
+ $$ = (Node*)n;
+ }
+
+ | CLUSTER opt_verbose '(' vac_analyze_option_list ')' qualified_name cluster_index_specification
+ {
+ ClusterStmt *n = makeNode(ClusterStmt);
+ n->relation = $6;
+ n->indexname = $7;
+ if ($2)
+ n->options |= CLUOPT_VERBOSE;
+ n->params = $4;
$$ = (Node*)n;
}
| CLUSTER opt_verbose
@@ -10404,6 +10409,7 @@ ClusterStmt:
n->options = 0;
if ($2)
n->options |= CLUOPT_VERBOSE;
+ n->params = NIL;
$$ = (Node*)n;
}
/* kept for pre-8.3 compatibility */
@@ -10415,6 +10421,7 @@ ClusterStmt:
n->options = 0;
if ($2)
n->options |= CLUOPT_VERBOSE;
+ n->params = NIL;
$$ = (Node*)n;
}
;
diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c
index 9b0c376c8c..c48983a5de 100644
--- a/src/backend/tcop/utility.c
+++ b/src/backend/tcop/utility.c
@@ -526,6 +526,33 @@ ProcessUtility(PlannedStmt *pstmt,
dest, qc);
}
+/* Parse params not parsed by the grammar */
+static
+void parse_reindex_params(ParseState *pstate, ReindexStmt *stmt)
+{
+ ListCell *lc;
+ foreach(lc, stmt->params)
+ {
+ DefElem *opt = (DefElem *) lfirst(lc);
+
+ if (strcmp(opt->defname, "verbose") == 0)
+ {
+ if (defGetBoolean(opt))
+ stmt->options |= REINDEXOPT_VERBOSE;
+ else
+ stmt->options &= ~REINDEXOPT_VERBOSE;
+ }
+ else if (strcmp(opt->defname, "concurrently") == 0)
+ stmt->concurrent = true;
+ else
+ ereport(ERROR,
+ (errcode(ERRCODE_SYNTAX_ERROR),
+ errmsg("unrecognized REINDEX option \"%s\"",
+ opt->defname),
+ parser_errposition(pstate, opt->location)));
+ }
+}
+
/*
* standard_ProcessUtility itself deals only with utility commands for
* which we do not provide event trigger support. Commands that do have
@@ -818,7 +845,7 @@ standard_ProcessUtility(PlannedStmt *pstmt,
break;
case T_ClusterStmt:
- cluster((ClusterStmt *) parsetree, isTopLevel);
+ cluster(pstate, (ClusterStmt *) parsetree, isTopLevel);
break;
case T_VacuumStmt:
@@ -923,13 +950,14 @@ standard_ProcessUtility(PlannedStmt *pstmt,
PreventInTransactionBlock(isTopLevel,
"REINDEX CONCURRENTLY");
+ parse_reindex_params(pstate, stmt);
switch (stmt->kind)
{
case REINDEX_OBJECT_INDEX:
- ReindexIndex(stmt->relation, stmt->options, stmt->concurrent);
+ ReindexIndex(stmt);
break;
case REINDEX_OBJECT_TABLE:
- ReindexTable(stmt->relation, stmt->options, stmt->concurrent);
+ ReindexTable(stmt);
break;
case REINDEX_OBJECT_SCHEMA:
case REINDEX_OBJECT_SYSTEM:
@@ -945,7 +973,7 @@ standard_ProcessUtility(PlannedStmt *pstmt,
(stmt->kind == REINDEX_OBJECT_SCHEMA) ? "REINDEX SCHEMA" :
(stmt->kind == REINDEX_OBJECT_SYSTEM) ? "REINDEX SYSTEM" :
"REINDEX DATABASE");
- ReindexMultipleTables(stmt->name, stmt->kind, stmt->options, stmt->concurrent);
+ ReindexMultipleTables(stmt);
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 f41785f11c..de85772ff9 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -2259,21 +2259,34 @@ psql_completion(const char *text, int start, int end)
/* CLUSTER */
else if (Matches("CLUSTER"))
COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_clusterables, "UNION SELECT 'VERBOSE'");
- else if (Matches("CLUSTER", "VERBOSE"))
+ else if (Matches("CLUSTER", "VERBOSE") ||
+ Matches("CLUSTER", "VERBOSE", "(*)") ||
+ Matches("CLUSTER", "(*)"))
COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_clusterables, NULL);
/* If we have CLUSTER <sth>, then add "USING" */
- else if (Matches("CLUSTER", MatchAnyExcept("VERBOSE|ON")))
+ else if (Matches("CLUSTER", MatchAnyExcept("VERBOSE|ON|(|(*)")))
COMPLETE_WITH("USING");
/* If we have CLUSTER VERBOSE <sth>, then add "USING" */
- else if (Matches("CLUSTER", "VERBOSE", MatchAny))
+ else if (Matches("CLUSTER", "VERBOSE|(*)", MatchAny))
COMPLETE_WITH("USING");
/* If we have CLUSTER <sth> USING, then add the index as well */
else if (Matches("CLUSTER", MatchAny, "USING") ||
- Matches("CLUSTER", "VERBOSE", MatchAny, "USING"))
+ Matches("CLUSTER", "VERBOSE|(*)", MatchAny, "USING"))
{
completion_info_charp = prev2_wd;
COMPLETE_WITH_QUERY(Query_for_index_of_table);
}
+ else if (HeadMatches("CLUSTER", "(*") &&
+ !HeadMatches("CLUSTER", "(*)"))
+ {
+ /*
+ * This fires if we're in an unfinished parenthesized option list.
+ * get_previous_words treats a completed parenthesized option list as
+ * one word, so the above test is correct.
+ */
+ if (ends_with(prev_wd, '(') || ends_with(prev_wd, ','))
+ COMPLETE_WITH("VERBOSE");
+ }
/* COMMENT */
else if (Matches("COMMENT"))
@@ -3470,7 +3483,7 @@ psql_completion(const char *text, int start, int end)
* one word, so the above test is correct.
*/
if (ends_with(prev_wd, '(') || ends_with(prev_wd, ','))
- COMPLETE_WITH("VERBOSE");
+ COMPLETE_WITH("CONCURRENTLY", "VERBOSE");
}
/* SECURITY LABEL */
diff --git a/src/include/commands/cluster.h b/src/include/commands/cluster.h
index e05884781b..674cdcd0cd 100644
--- a/src/include/commands/cluster.h
+++ b/src/include/commands/cluster.h
@@ -14,11 +14,12 @@
#define CLUSTER_H
#include "nodes/parsenodes.h"
+#include "parser/parse_node.h"
#include "storage/lock.h"
#include "utils/relcache.h"
-extern void cluster(ClusterStmt *stmt, bool isTopLevel);
+extern void cluster(ParseState *pstate, ClusterStmt *stmt, bool isTopLevel);
extern void cluster_rel(Oid tableOid, Oid indexOid, int options);
extern void check_index_is_clusterable(Relation OldHeap, Oid indexOid,
bool recheck, LOCKMODE lockmode);
diff --git a/src/include/commands/defrem.h b/src/include/commands/defrem.h
index c26a102b17..a97c00a02e 100644
--- a/src/include/commands/defrem.h
+++ b/src/include/commands/defrem.h
@@ -34,10 +34,9 @@ extern ObjectAddress DefineIndex(Oid relationId,
bool check_not_in_use,
bool skip_build,
bool quiet);
-extern void ReindexIndex(RangeVar *indexRelation, int options, bool concurrent);
-extern Oid ReindexTable(RangeVar *relation, int options, bool concurrent);
-extern void ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind,
- int options, bool concurrent);
+extern void ReindexIndex(ReindexStmt *stmt);
+extern Oid ReindexTable(ReindexStmt *stmt);
+extern void ReindexMultipleTables(ReindexStmt *stmt);
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 151bcdb7ef..295c2802a4 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -3210,6 +3210,7 @@ typedef struct ClusterStmt
RangeVar *relation; /* relation being indexed, or NULL if all */
char *indexname; /* original index defined */
int options; /* OR of ClusterOption flags */
+ List *params; /* Params not further parsed by the grammar */
} ClusterStmt;
/* ----------------------
@@ -3369,6 +3370,7 @@ typedef struct ReindexStmt
RangeVar *relation; /* Table or index to reindex */
const char *name; /* name of database to reindex */
int options; /* Reindex options flags */
+ List *params; /* Params not further parsed by the grammer */
bool concurrent; /* reindex concurrently? */
} ReindexStmt;
--
2.17.0
--g7w8+K/95kPelPD2
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v22-0002-Deprecate-ReindexStmt-concurrent.patch"
^ permalink raw reply [nested|flat] 4+ messages in thread
* Re: ALTER TABLE SET ACCESS METHOD on partitioned tables
@ 2024-04-17 08:13 Michael Paquier <[email protected]>
2024-04-17 08:31 ` Re: ALTER TABLE SET ACCESS METHOD on partitioned tables Alvaro Herrera <[email protected]>
0 siblings, 1 reply; 4+ messages in thread
From: Michael Paquier @ 2024-04-17 08:13 UTC (permalink / raw)
To: Alvaro Herrera <[email protected]>; +Cc: Tom Lane <[email protected]>; Justin Pryzby <[email protected]>; Alexander Lakhin <[email protected]>; Peter Eisentraut <[email protected]>; Soumyadeep Chakraborty <[email protected]>; Zhihong Yu <[email protected]>; pgsql-hackers; Ashwin Agrawal <[email protected]>; [email protected]
On Wed, Apr 17, 2024 at 09:50:02AM +0200, Alvaro Herrera wrote:
> I think it's easy enough to add a "bool ispartitioned" to TableInfo and
> use an ALTER TABLE or rely on the GUC depending on that -- seems easy
> enough.
Yeah, that would be easy enough to track but I was wondering about
adding the relkind instead. Still, one thing that I found confusing
is the dump generated in this case, as it would mix the SET and the
ALTER TABLE commands so one reading the dumps may wonder why the SET
has no effect for a CREATE TABLE PARTITION OF without USING. Perhaps
that's fine and I just worry too much ;)
The extra ALTER commands need to be generated after the object
definitions, so we'd need a new subroutine similar to
_selectTableAccessMethod() like a _selectTableAccessMethodNoStorage().
Or grouping both together is just simpler?
--
Michael
Attachments:
[application/pgp-signature] signature.asc (833B, ../../[email protected]/2-signature.asc)
download
^ permalink raw reply [nested|flat] 4+ messages in thread
* Re: ALTER TABLE SET ACCESS METHOD on partitioned tables
2024-04-17 08:13 Re: ALTER TABLE SET ACCESS METHOD on partitioned tables Michael Paquier <[email protected]>
@ 2024-04-17 08:31 ` Alvaro Herrera <[email protected]>
2024-04-18 00:42 ` Re: ALTER TABLE SET ACCESS METHOD on partitioned tables Michael Paquier <[email protected]>
0 siblings, 1 reply; 4+ messages in thread
From: Alvaro Herrera @ 2024-04-17 08:31 UTC (permalink / raw)
To: Michael Paquier <[email protected]>; +Cc: Tom Lane <[email protected]>; Justin Pryzby <[email protected]>; Alexander Lakhin <[email protected]>; Peter Eisentraut <[email protected]>; Soumyadeep Chakraborty <[email protected]>; Zhihong Yu <[email protected]>; pgsql-hackers; Ashwin Agrawal <[email protected]>; [email protected]
On 2024-Apr-17, Michael Paquier wrote:
> Yeah, that would be easy enough to track but I was wondering about
> adding the relkind instead. Still, one thing that I found confusing
> is the dump generated in this case, as it would mix the SET and the
> ALTER TABLE commands so one reading the dumps may wonder why the SET
> has no effect for a CREATE TABLE PARTITION OF without USING. Perhaps
> that's fine and I just worry too much ;)
Hmm, maybe we should do a RESET of default_table_access_method before
printing the CREATE TABLE to avoid the confusion.
> The extra ALTER commands need to be generated after the object
> definitions, so we'd need a new subroutine similar to
> _selectTableAccessMethod() like a _selectTableAccessMethodNoStorage().
> Or grouping both together is just simpler?
I think there should be two routines, since _select* routines just print
a SET command; maybe the new one would be _printAlterTableAM() or
something like that. Having _select() print an ALTER TABLE command
depending on relkind (or the boolean flag) would be confusing, I think.
--
Álvaro Herrera Breisgau, Deutschland — https://www.EnterpriseDB.com/
^ permalink raw reply [nested|flat] 4+ messages in thread
* Re: ALTER TABLE SET ACCESS METHOD on partitioned tables
2024-04-17 08:13 Re: ALTER TABLE SET ACCESS METHOD on partitioned tables Michael Paquier <[email protected]>
2024-04-17 08:31 ` Re: ALTER TABLE SET ACCESS METHOD on partitioned tables Alvaro Herrera <[email protected]>
@ 2024-04-18 00:42 ` Michael Paquier <[email protected]>
0 siblings, 0 replies; 4+ messages in thread
From: Michael Paquier @ 2024-04-18 00:42 UTC (permalink / raw)
To: Alvaro Herrera <[email protected]>; +Cc: Tom Lane <[email protected]>; Justin Pryzby <[email protected]>; Alexander Lakhin <[email protected]>; Peter Eisentraut <[email protected]>; Soumyadeep Chakraborty <[email protected]>; Zhihong Yu <[email protected]>; pgsql-hackers; Ashwin Agrawal <[email protected]>; [email protected]
On Wed, Apr 17, 2024 at 10:31:52AM +0200, Alvaro Herrera wrote:
> On 2024-Apr-17, Michael Paquier wrote:
>> Yeah, that would be easy enough to track but I was wondering about
>> adding the relkind instead. Still, one thing that I found confusing
>> is the dump generated in this case, as it would mix the SET and the
>> ALTER TABLE commands so one reading the dumps may wonder why the SET
>> has no effect for a CREATE TABLE PARTITION OF without USING. Perhaps
>> that's fine and I just worry too much ;)
>
> Hmm, maybe we should do a RESET of default_table_access_method before
> printing the CREATE TABLE to avoid the confusion.
A hard reset would make the business around currTableAM that decides
when to generate the SET default_table_access_method queries slightly
more complicated, while increasing the number of queries run on the
server.
>> The extra ALTER commands need to be generated after the object
>> definitions, so we'd need a new subroutine similar to
>> _selectTableAccessMethod() like a _selectTableAccessMethodNoStorage().
>> Or grouping both together is just simpler?
>
> I think there should be two routines, since _select* routines just print
> a SET command; maybe the new one would be _printAlterTableAM() or
> something like that. Having _select() print an ALTER TABLE command
> depending on relkind (or the boolean flag) would be confusing, I think.
Fine by me to use two routines to generate the two different commands.
I am finishing with the attached for now, making dumps, restores and
upgrades work happily as far as I've tested.
I was also worrying about a need to dump the protocol version to be
able to track the relkind in the toc entries, but a45c78e3284b has
already done one. The difference in AM handling between relations
without storage and relations with storage pushes the relkind logic
more within the internals of pg_backup_archiver.c.
What do you think?
--
Michael
Attachments:
[text/x-diff] v2-0001-Set-properly-table-AMs-of-partitioned-tables-in-p.patch (7.3K, ../../[email protected]/2-v2-0001-Set-properly-table-AMs-of-partitioned-tables-in-p.patch)
download | inline diff:
From ca3d0c1828336275d86a5c51ed975c0a43ca6af9 Mon Sep 17 00:00:00 2001
From: Michael Paquier <[email protected]>
Date: Thu, 18 Apr 2024 09:37:20 +0900
Subject: [PATCH v2] Set properly table AMs of partitioned tables in pg_dump
---
src/bin/pg_dump/pg_backup_archiver.c | 70 +++++++++++++++++++++++++++-
src/bin/pg_dump/pg_backup_archiver.h | 5 +-
src/bin/pg_dump/pg_dump.c | 1 +
src/bin/pg_dump/t/002_pg_dump.pl | 6 +--
4 files changed, 75 insertions(+), 7 deletions(-)
diff --git a/src/bin/pg_dump/pg_backup_archiver.c b/src/bin/pg_dump/pg_backup_archiver.c
index c7a6c918a6..f69f72d731 100644
--- a/src/bin/pg_dump/pg_backup_archiver.c
+++ b/src/bin/pg_dump/pg_backup_archiver.c
@@ -30,6 +30,7 @@
#include <io.h>
#endif
+#include "catalog/pg_class_d.h"
#include "common/string.h"
#include "compress_io.h"
#include "dumputils.h"
@@ -62,6 +63,8 @@ static void _becomeOwner(ArchiveHandle *AH, TocEntry *te);
static void _selectOutputSchema(ArchiveHandle *AH, const char *schemaName);
static void _selectTablespace(ArchiveHandle *AH, const char *tablespace);
static void _selectTableAccessMethod(ArchiveHandle *AH, const char *tableam);
+static void _printTableAccessMethodNoStorage(ArchiveHandle *AH,
+ TocEntry *te);
static void processEncodingEntry(ArchiveHandle *AH, TocEntry *te);
static void processStdStringsEntry(ArchiveHandle *AH, TocEntry *te);
static void processSearchPathEntry(ArchiveHandle *AH, TocEntry *te);
@@ -1222,6 +1225,7 @@ ArchiveEntry(Archive *AHX, CatalogId catalogId, DumpId dumpId,
newToc->namespace = opts->namespace ? pg_strdup(opts->namespace) : NULL;
newToc->tablespace = opts->tablespace ? pg_strdup(opts->tablespace) : NULL;
newToc->tableam = opts->tableam ? pg_strdup(opts->tableam) : NULL;
+ newToc->relkind = opts->relkind;
newToc->owner = opts->owner ? pg_strdup(opts->owner) : NULL;
newToc->desc = pg_strdup(opts->description);
newToc->defn = opts->createStmt ? pg_strdup(opts->createStmt) : NULL;
@@ -2602,6 +2606,7 @@ WriteToc(ArchiveHandle *AH)
WriteStr(AH, te->namespace);
WriteStr(AH, te->tablespace);
WriteStr(AH, te->tableam);
+ WriteInt(AH, te->relkind);
WriteStr(AH, te->owner);
WriteStr(AH, "false");
@@ -2707,6 +2712,9 @@ ReadToc(ArchiveHandle *AH)
if (AH->version >= K_VERS_1_14)
te->tableam = ReadStr(AH);
+ if (AH->version >= K_VERS_1_16)
+ te->relkind = ReadInt(AH);
+
te->owner = ReadStr(AH);
is_supported = true;
if (AH->version < K_VERS_1_9)
@@ -3567,6 +3575,51 @@ _selectTableAccessMethod(ArchiveHandle *AH, const char *tableam)
AH->currTableAm = pg_strdup(want);
}
+/*
+ * Set the proper default table access method for a table without storage,
+ * like a partitioned one where a table access method may be set.
+ */
+static void
+_printTableAccessMethodNoStorage(ArchiveHandle *AH, TocEntry *te)
+{
+ RestoreOptions *ropt = AH->public.ropt;
+ const char *tableam = te->tableam;
+ PQExpBuffer cmd;
+
+ /* do nothing in --no-table-access-method mode */
+ if (ropt->noTableAm)
+ return;
+
+ if (!tableam)
+ return;
+
+ Assert(te->relkind == RELKIND_PARTITIONED_TABLE);
+
+ cmd = createPQExpBuffer();
+
+ appendPQExpBufferStr(cmd, "ALTER TABLE ");
+ appendPQExpBuffer(cmd, "%s ", fmtQualifiedId(te->namespace, te->tag));
+ appendPQExpBuffer(cmd, "SET ACCESS METHOD %s;",
+ fmtId(tableam));
+
+ if (RestoringToDB(AH))
+ {
+ PGresult *res;
+
+ res = PQexec(AH->connection, cmd->data);
+
+ if (!res || PQresultStatus(res) != PGRES_COMMAND_OK)
+ warn_or_exit_horribly(AH,
+ "could not alter table access method: %s",
+ PQerrorMessage(AH->connection));
+ PQclear(res);
+ }
+ else
+ ahprintf(AH, "%s\n\n", cmd->data);
+
+ destroyPQExpBuffer(cmd);
+}
+
/*
* Extract an object description for a TOC entry, and append it to buf.
*
@@ -3673,11 +3726,17 @@ _printTocEntry(ArchiveHandle *AH, TocEntry *te, bool isData)
{
RestoreOptions *ropt = AH->public.ropt;
- /* Select owner, schema, tablespace and default AM as necessary */
+ /*
+ * Select owner, schema, tablespace and default AM as necessary.
+ * The default access method for partitioned tables is handled
+ * after generating the object definition, as it requires an ALTER
+ * command rather than SET.
+ */
_becomeOwner(AH, te);
_selectOutputSchema(AH, te->namespace);
_selectTablespace(AH, te->tablespace);
- _selectTableAccessMethod(AH, te->tableam);
+ if (te->relkind != RELKIND_PARTITIONED_TABLE)
+ _selectTableAccessMethod(AH, te->tableam);
/* Emit header comment for item */
if (!AH->noTocComments)
@@ -3812,6 +3871,13 @@ _printTocEntry(ArchiveHandle *AH, TocEntry *te, bool isData)
}
}
+ /*
+ * Select a partitioned table's default AM, once the table definition
+ * has been generated.
+ */
+ if (te->relkind == RELKIND_PARTITIONED_TABLE)
+ _printTableAccessMethodNoStorage(AH, te);
+
/*
* If it's an ACL entry, it might contain SET SESSION AUTHORIZATION
* commands, so we can no longer assume we know the current auth setting.
diff --git a/src/bin/pg_dump/pg_backup_archiver.h b/src/bin/pg_dump/pg_backup_archiver.h
index d6104a7196..ce5ed1dd39 100644
--- a/src/bin/pg_dump/pg_backup_archiver.h
+++ b/src/bin/pg_dump/pg_backup_archiver.h
@@ -69,7 +69,8 @@
* compression_algorithm
* in header */
#define K_VERS_1_16 MAKE_ARCHIVE_VERSION(1, 16, 0) /* BLOB METADATA entries
- * and multiple BLOBS */
+ * and multiple BLOBS,
+ * relkind */
/* Current archive version number (the format we can output) */
#define K_VERS_MAJOR 1
@@ -353,6 +354,7 @@ struct _tocEntry
char *tablespace; /* null if not in a tablespace; empty string
* means use database default */
char *tableam; /* table access method, only for TABLE tags */
+ char relkind; /* relation kind, only for TABLE tags */
char *owner;
char *desc;
char *defn;
@@ -393,6 +395,7 @@ typedef struct _archiveOpts
const char *namespace;
const char *tablespace;
const char *tableam;
+ char relkind;
const char *owner;
const char *description;
teSection section;
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index c52e961b30..6e167dda03 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -16741,6 +16741,7 @@ dumpTableSchema(Archive *fout, const TableInfo *tbinfo)
.namespace = tbinfo->dobj.namespace->dobj.name,
.tablespace = tablespace,
.tableam = tableam,
+ .relkind = tbinfo->relkind,
.owner = tbinfo->rolname,
.description = reltypename,
.section = tbinfo->postponed_def ?
diff --git a/src/bin/pg_dump/t/002_pg_dump.pl b/src/bin/pg_dump/t/002_pg_dump.pl
index 0c057fef94..25b4082273 100644
--- a/src/bin/pg_dump/t/002_pg_dump.pl
+++ b/src/bin/pg_dump/t/002_pg_dump.pl
@@ -4591,11 +4591,9 @@ my %tests = (
CREATE TABLE dump_test.regress_pg_dump_table_am_child_2
PARTITION OF dump_test.regress_pg_dump_table_am_parent FOR VALUES IN (2);',
regexp => qr/^
- \QSET default_table_access_method = regress_table_am;\E
- (\n(?!SET[^;]+;)[^\n]*)*
- \n\QCREATE TABLE dump_test.regress_pg_dump_table_am_parent (\E
- (.*\n)*
\QSET default_table_access_method = heap;\E
+ (.*\n)*
+ \QALTER TABLE dump_test.regress_pg_dump_table_am_parent SET ACCESS METHOD regress_table_am;\E
(\n(?!SET[^;]+;)[^\n]*)*
\n\QCREATE TABLE dump_test.regress_pg_dump_table_am_child_1 (\E
(.*\n)*
--
2.43.0
[application/pgp-signature] signature.asc (833B, ../../[email protected]/3-signature.asc)
download
^ permalink raw reply [nested|flat] 4+ messages in thread
end of thread, other threads:[~2024-04-18 00:42 UTC | newest]
Thread overview: 4+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2020-03-27 22:50 [PATCH v22 1/5] Change REINDEX/CLUSTER to accept an option list.. Justin Pryzby <[email protected]>
2024-04-17 08:13 Re: ALTER TABLE SET ACCESS METHOD on partitioned tables Michael Paquier <[email protected]>
2024-04-17 08:31 ` Re: ALTER TABLE SET ACCESS METHOD on partitioned tables Alvaro Herrera <[email protected]>
2024-04-18 00:42 ` Re: ALTER TABLE SET ACCESS METHOD on partitioned tables Michael Paquier <[email protected]>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox