On 2022-Jul-22, Michael Paquier wrote:
> So this indeed has as effect to make possible the use of CONCURRENTLY
> for DATABASE and SYSTEM only within the parenthesized grammar. Seeing
> the simplifications this creates, I'd agree with dropping this part of
> the grammar.
Actually, looking at the grammar again I realized that the '('options')'
part could be refactored; and with that, keeping an extra production for
REINDEX DATABASE CONCURRENTLY is short enough. It is removed from
REINDEX SYSTEM, but that's OK because that doesn't work anyway.
I added the new test lines you proposed and amended the docs; the result
is attached.
Initially I wanted to use the "optional list of options" for all
utilities that have similar constructions, (VACUUM, ANALYZE, CLUSTER,
EXPLAIN) but it is not possible because their alternative productions
accept different keywords, so it doesn't look possible.
--
Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/
"If you have nothing to say, maybe you need just the right tool to help you
not say it." (New York Times, about Microsoft PowerPoint)
>From 1c5f90b475b94f623a91fe10514e69cd8ffcc38a Mon Sep 17 00:00:00 2001
From: Alvaro Herrera <alvhe...@alvh.no-ip.org>
Date: Thu, 21 Jul 2022 16:48:51 +0200
Subject: [PATCH v2] Rework grammar for REINDEX
---
doc/src/sgml/ref/reindex.sgml | 3 +-
src/backend/parser/gram.y | 87 ++++++++--------------
src/test/regress/expected/create_index.out | 6 ++
src/test/regress/sql/create_index.sql | 2 +
4 files changed, 41 insertions(+), 57 deletions(-)
diff --git a/doc/src/sgml/ref/reindex.sgml b/doc/src/sgml/ref/reindex.sgml
index fcbda88149..6750eb6e47 100644
--- a/doc/src/sgml/ref/reindex.sgml
+++ b/doc/src/sgml/ref/reindex.sgml
@@ -22,7 +22,8 @@ PostgreSQL documentation
<refsynopsisdiv>
<synopsis>
REINDEX [ ( <replaceable class="parameter">option</replaceable> [, ...] ) ] { INDEX | TABLE | SCHEMA } [ CONCURRENTLY ] <replaceable class="parameter">name</replaceable>
-REINDEX [ ( <replaceable class="parameter">option</replaceable> [, ...] ) ] { DATABASE | SYSTEM } [ CONCURRENTLY ] [ <replaceable class="parameter">name</replaceable> ]
+REINDEX [ ( <replaceable class="parameter">option</replaceable> [, ...] ) ] DATABASE [ CONCURRENTLY ] [ <replaceable class="parameter">name</replaceable> ]
+REINDEX [ ( <replaceable class="parameter">option</replaceable> [, ...] ) ] SYSTEM [ <replaceable class="parameter">name</replaceable> ]
<phrase>where <replaceable class="parameter">option</replaceable> can be one of:</phrase>
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index b761a5b5d2..c6f74bf24e 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -564,7 +564,8 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query);
%type <defelt> generic_option_elem alter_generic_option_elem
%type <list> generic_option_list alter_generic_option_list
-%type <ival> reindex_target_type reindex_target_multitable reindex_name_optional
+%type <ival> reindex_target_type
+%type <list> opt_reindex_option_list
%type <node> copy_generic_opt_arg copy_generic_opt_arg_list_item
%type <defelt> copy_generic_opt_elem
@@ -9091,95 +9092,69 @@ DropTransformStmt: DROP TRANSFORM opt_if_exists FOR Typename LANGUAGE name opt_d
*
* QUERY:
*
- * REINDEX [ (options) ] type [CONCURRENTLY] <name>
+ * REINDEX [ (options) ] {TABLE | INDEX | SCHEMA} [CONCURRENTLY] <name>
+ * REINDEX [ (options) ] DATABASE [CONCURRENTLY] [<name>]
+ * REINDEX [ (options) ] SYSTEM [<name>]
*****************************************************************************/
ReindexStmt:
- REINDEX reindex_target_type opt_concurrently qualified_name
+ REINDEX opt_reindex_option_list reindex_target_type opt_concurrently qualified_name
{
ReindexStmt *n = makeNode(ReindexStmt);
- n->kind = $2;
- n->relation = $4;
+ n->kind = $3;
+ n->relation = $5;
n->name = NULL;
- n->params = NIL;
- if ($3)
+ n->params = $2;
+ if ($4)
n->params = lappend(n->params,
- makeDefElem("concurrently", NULL, @3));
+ makeDefElem("concurrently", NULL, @4));
$$ = (Node *) n;
}
- | REINDEX reindex_target_multitable opt_concurrently name
+ | REINDEX opt_reindex_option_list SCHEMA opt_concurrently name
{
ReindexStmt *n = makeNode(ReindexStmt);
- n->kind = $2;
- n->name = $4;
+ n->kind = REINDEX_OBJECT_SCHEMA;
+ n->name = $5;
n->relation = NULL;
- n->params = NIL;
- if ($3)
+ n->params = $2;
+ if ($4)
n->params = lappend(n->params,
- makeDefElem("concurrently", NULL, @3));
+ makeDefElem("concurrently", NULL, @4));
$$ = (Node *) n;
}
- | REINDEX reindex_name_optional
+ | REINDEX opt_reindex_option_list DATABASE opt_concurrently opt_single_name
{
ReindexStmt *n = makeNode(ReindexStmt);
- n->kind = $2;
+ n->kind = REINDEX_OBJECT_DATABASE;
n->name = NULL;
n->relation = NULL;
- n->params = NIL;
+ n->params = $2;
$$ = (Node *)n;
}
- | REINDEX '(' utility_option_list ')' reindex_name_optional
+ | REINDEX opt_reindex_option_list SYSTEM_P opt_single_name
{
ReindexStmt *n = makeNode(ReindexStmt);
- n->kind = $5;
+ n->kind = REINDEX_OBJECT_SYSTEM;
n->name = NULL;
n->relation = NULL;
- n->params = $3;
+ n->params = $2;
$$ = (Node *)n;
}
- | REINDEX '(' utility_option_list ')' reindex_target_type opt_concurrently qualified_name
- {
- ReindexStmt *n = makeNode(ReindexStmt);
-
- n->kind = $5;
- n->relation = $7;
- n->name = NULL;
- n->params = $3;
- if ($6)
- n->params = lappend(n->params,
- makeDefElem("concurrently", NULL, @6));
- $$ = (Node *) n;
- }
- | REINDEX '(' utility_option_list ')' reindex_target_multitable opt_concurrently name
- {
- ReindexStmt *n = makeNode(ReindexStmt);
-
- n->kind = $5;
- n->name = $7;
- n->relation = NULL;
- n->params = $3;
- if ($6)
- n->params = lappend(n->params,
- makeDefElem("concurrently", NULL, @6));
- $$ = (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; }
- ;
-/* For these options the name is optional */
-reindex_name_optional:
- SYSTEM_P { $$ = REINDEX_OBJECT_SYSTEM; }
- | DATABASE { $$ = REINDEX_OBJECT_DATABASE; }
- ;
+opt_reindex_option_list:
+ '(' utility_option_list ')'
+ {
+ $$ = $2;
+ }
+ | /* EMPTY */
+ { $$ = NULL; }
+ ;
/*****************************************************************************
*
diff --git a/src/test/regress/expected/create_index.out b/src/test/regress/expected/create_index.out
index d55aec3a1d..a913a1846f 100644
--- a/src/test/regress/expected/create_index.out
+++ b/src/test/regress/expected/create_index.out
@@ -2521,6 +2521,12 @@ ERROR: cannot reindex system catalogs concurrently
REINDEX INDEX CONCURRENTLY pg_toast.pg_toast_1260_index; -- no catalog toast index
ERROR: cannot reindex system catalogs concurrently
REINDEX SYSTEM CONCURRENTLY postgres; -- not allowed for SYSTEM
+ERROR: syntax error at or near "CONCURRENTLY"
+LINE 1: REINDEX SYSTEM CONCURRENTLY postgres;
+ ^
+REINDEX (CONCURRENTLY) SYSTEM postgres; -- ditto
+ERROR: cannot reindex system catalogs concurrently
+REINDEX (CONCURRENTLY) SYSTEM; -- ditto
ERROR: cannot reindex system catalogs concurrently
-- Warns about catalog relations
REINDEX SCHEMA CONCURRENTLY pg_catalog;
diff --git a/src/test/regress/sql/create_index.sql b/src/test/regress/sql/create_index.sql
index d8fded3d93..4b75790e47 100644
--- a/src/test/regress/sql/create_index.sql
+++ b/src/test/regress/sql/create_index.sql
@@ -1072,6 +1072,8 @@ REINDEX INDEX CONCURRENTLY pg_class_oid_index; -- no catalog index
REINDEX TABLE CONCURRENTLY pg_toast.pg_toast_1260; -- no catalog toast table
REINDEX INDEX CONCURRENTLY pg_toast.pg_toast_1260_index; -- no catalog toast index
REINDEX SYSTEM CONCURRENTLY postgres; -- not allowed for SYSTEM
+REINDEX (CONCURRENTLY) SYSTEM postgres; -- ditto
+REINDEX (CONCURRENTLY) SYSTEM; -- ditto
-- Warns about catalog relations
REINDEX SCHEMA CONCURRENTLY pg_catalog;
--
2.30.2
