public inbox for [email protected]
help / color / mirror / Atom feed[PATCH v25 1/6] Change REINDEX/CLUSTER to accept an option list..
3+ messages / 3 participants
[nested] [flat]
* [PATCH v25 1/6] Change REINDEX/CLUSTER to accept an option list..
@ 2020-03-27 22:50 Justin Pryzby <[email protected]>
0 siblings, 0 replies; 3+ 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 | 27 +++++++++++++++++---
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 | 33 +++++++++++++-----------
src/backend/tcop/utility.c | 36 +++++++++++++++++++++++---
src/bin/psql/tab-complete.c | 22 ++++++++++++----
src/include/commands/cluster.h | 3 ++-
src/include/commands/defrem.h | 7 +++---
src/include/nodes/parsenodes.h | 2 ++
12 files changed, 175 insertions(+), 66 deletions(-)
diff --git a/doc/src/sgml/ref/cluster.sgml b/doc/src/sgml/ref/cluster.sgml
index 4da60d8d56..e6ebce27e6 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,15 @@ CLUSTER [VERBOSE]
<title>Parameters</title>
<variablelist>
+ <varlistentry>
+ <term><literal>VERBOSE</literal></term>
+ <listitem>
+ <para>
+ Prints a progress report as each table is clustered.
+ </para>
+ </listitem>
+ </varlistentry>
+
<varlistentry>
<term><replaceable class="parameter">table_name</replaceable></term>
<listitem>
@@ -100,10 +114,15 @@ 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>
diff --git a/doc/src/sgml/ref/reindex.sgml b/doc/src/sgml/ref/reindex.sgml
index c16f223e4e..a32e192a87 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 0d647e912c..a42dfe98f4 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"
@@ -102,8 +103,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 b3a92381f9..e8383feea7 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)
@@ -2774,10 +2777,10 @@ ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind,
continue;
}
- if (concurrent && get_rel_persistence(relid) != RELPERSISTENCE_TEMP)
+ if (stmt->concurrent && get_rel_persistence(relid) != RELPERSISTENCE_TEMP)
{
(void) ReindexRelationConcurrently(relid,
- options |
+ stmt->options |
REINDEXOPT_MISSING_OK);
/* ReindexRelationConcurrently() does the verbose output */
}
@@ -2788,11 +2791,11 @@ ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind,
result = reindex_relation(relid,
REINDEX_REL_PROCESS_TOAST |
REINDEX_REL_CHECK_CONSTRAINTS,
- options |
+ stmt->options |
REINDEXOPT_REPORT_PROGRESS |
REINDEXOPT_MISSING_OK);
- 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..c5be94af45 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,16 @@ ClusterStmt:
n->options = 0;
if ($2)
n->options |= CLUOPT_VERBOSE;
+ n->params = NIL;
+ $$ = (Node*)n;
+ }
+
+ | CLUSTER '(' vac_analyze_option_list ')' qualified_name cluster_index_specification
+ {
+ ClusterStmt *n = makeNode(ClusterStmt);
+ n->relation = $5;
+ n->indexname = $6;
+ n->params = $3;
$$ = (Node*)n;
}
| CLUSTER opt_verbose
@@ -10404,6 +10407,7 @@ ClusterStmt:
n->options = 0;
if ($2)
n->options |= CLUOPT_VERBOSE;
+ n->params = NIL;
$$ = (Node*)n;
}
/* kept for pre-8.3 compatibility */
@@ -10415,6 +10419,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 6154d2c8c6..0e074d7745 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..245919aead 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -2259,21 +2259,33 @@ 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", "(*)"))
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 +3482,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 1eb144204b..c761b9575a 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,
bool isTopLevel);
extern void check_index_is_clusterable(Relation OldHeap, Oid indexOid,
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 d52c563305..4e3188ff87 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -3211,6 +3211,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;
/* ----------------------
@@ -3371,6 +3372,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
--l76fUT7nc3MelDdI
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v25-0002-Deprecate-ReindexStmt-concurrent-and-options.patch"
^ permalink raw reply [nested|flat] 3+ messages in thread
* Re: Let's make PostgreSQL multi-threaded
@ 2023-06-08 15:55 Matthias van de Meent <[email protected]>
2023-06-08 18:48 ` Re: Let's make PostgreSQL multi-threaded Andres Freund <[email protected]>
0 siblings, 1 reply; 3+ messages in thread
From: Matthias van de Meent @ 2023-06-08 15:55 UTC (permalink / raw)
To: Hannu Krosing <[email protected]>; +Cc: Robert Haas <[email protected]>; Andres Freund <[email protected]>; Jonathan S. Katz <[email protected]>; Heikki Linnakangas <[email protected]>; Tom Lane <[email protected]>; pgsql-hackers
On Thu, 8 Jun 2023 at 17:02, Hannu Krosing <[email protected]> wrote:
>
> On Thu, Jun 8, 2023 at 4:56 PM Robert Haas <[email protected]> wrote:
> >
> > On Thu, Jun 8, 2023 at 8:44 AM Hannu Krosing <[email protected]> wrote:
> > > > That sounds like a bad idea, dynamic shared memory is more expensive
> > > > to maintain than our static shared memory systems, not in the least
> > > > because DSM is not guaranteed to share the same addresses in each
> > > > process' address space.
> > >
> > > Then this too needs to be fixed
> >
> > Honestly, I'm struggling to respond to this non-sarcastically. I mean,
> > I was the one who implemented DSM. Do you think it works the way that
> > it works because I considered doing something smart and decided to do
> > something dumb instead?
>
> No, I meant that this needs to be fixed at OS level, by being able to
> use the same mapping.
>
> We should not shy away from asking the OS people for adding the useful
> features still missing.
While I agree that "sharing page tables across processes" is useful,
it looks like it'd be much more effort to correctly implement for e.g.
DSM than implementing threading.
Konstantin's diff is "only" 20.1k lines [0] added and/or modified,
which is a lot, but it's manageable (13k+ of which are from files that
were auto-generated and then committed, likely accidentally).
> It was mentioned in the Unconference Kernel Hacker AMA talk and said
> kernel hacker works for Oracle, andf they also seemed to be needing
> this :)
Though these new kernel features allowing for better performance
(mostly in kernel memory usage, probably) would be nice to have, we
wouldn't get performance benefits for older kernels, benefits which we
would get if we were to implement threading.
I'm not on board with a policy of us twiddling thumbs and waiting for
the OS to fix our architectural performance issues. Sure, the kernel
could optimize for our usage pattern, but I think that's not something
we can (or should) rely on for performance ^1.
Kind regards,
Matthias van de Meent
[0] https://github.com/postgrespro/postgresql.pthreads/compare/801386af...d5933309?w=1
^1 OT: I think the same about us (ab)using the OS page cache, but
that's a tale for a different time and thread.
^ permalink raw reply [nested|flat] 3+ messages in thread
* Re: Let's make PostgreSQL multi-threaded
2023-06-08 15:55 Re: Let's make PostgreSQL multi-threaded Matthias van de Meent <[email protected]>
@ 2023-06-08 18:48 ` Andres Freund <[email protected]>
0 siblings, 0 replies; 3+ messages in thread
From: Andres Freund @ 2023-06-08 18:48 UTC (permalink / raw)
To: Matthias van de Meent <[email protected]>; +Cc: Hannu Krosing <[email protected]>; Robert Haas <[email protected]>; Jonathan S. Katz <[email protected]>; Heikki Linnakangas <[email protected]>; Tom Lane <[email protected]>; pgsql-hackers
Hi,
On 2023-06-08 17:55:57 +0200, Matthias van de Meent wrote:
> While I agree that "sharing page tables across processes" is useful,
> it looks like it'd be much more effort to correctly implement for e.g.
> DSM than implementing threading.
> Konstantin's diff is "only" 20.1k lines [0] added and/or modified,
> which is a lot, but it's manageable (13k+ of which are from files that
> were auto-generated and then committed, likely accidentally).
Honestly, I don't think this patch is in a good enough state to allow a
realistic estimation of the overall work. Making global variables TLS is the
*easy* part. Redesigning postmaster, definining how to deal with extension
libraries, extension compatibility, developing tools to make developing a
threaded postgres feasible, dealing with freeing session lifetime memory
allocations that previously were freed via process exit, making the change
realistically reviewable, portability are all much harder.
Greetings,
Andres Freund
^ permalink raw reply [nested|flat] 3+ messages in thread
end of thread, other threads:[~2023-06-08 18:48 UTC | newest]
Thread overview: 3+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2020-03-27 22:50 [PATCH v25 1/6] Change REINDEX/CLUSTER to accept an option list.. Justin Pryzby <[email protected]>
2023-06-08 15:55 Re: Let's make PostgreSQL multi-threaded Matthias van de Meent <[email protected]>
2023-06-08 18:48 ` Re: Let's make PostgreSQL multi-threaded Andres Freund <[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