pg.ddx.io pgsql-hackers@postgresql.org mailing list archive help / color / mirror / Atom feed From: Alvaro Herrera <alvherre@alvh.no-ip.org>
To: Michael Paquier <michael@paquier.xyz>
Cc: Dean Rasheed <dean.a.rasheed@gmail.com>
Cc: Matthias van de Meent <boekewurm+postgres@gmail.com>
Cc: Simon Riggs <simon.riggs@enterprisedb.com>
Cc: PostgreSQL Hackers <pgsql-hackers@postgresql.org>
Subject: Re: Make name optional in CREATE STATISTICS
Date: Fri, 22 Jul 2022 15:06:46 +0200
Message-ID: <20220722130646.3da5oh7tgz5qgooi@alvherre.pgsql> (raw )
In-Reply-To: <YtnzFv3/jyVjRrkP@paquier.xyz >
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)
Attachments:
v2-0001-Rework-grammar-for-REINDEX.patch (text/x-diff, 7.3K) download
From 1c5f90b475b94f623a91fe10514e69cd8ffcc38a Mon Sep 17 00:00:00 2001
From: Alvaro Herrera <alvherre@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
view thread (22+ messages) latest in thread
Message-ID: <20220722130646.3da5oh7tgz5qgooi@alvherre.pgsql>
Permalink: ../20220722130646.3da5oh7tgz5qgooi@alvherre.pgsql/
Also on: postgresql.org/message-id/20220722130646.3da5oh7tgz5qgooi@alvherre.pgsql
copy link · copy postgr.es · copy git am
reply Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: alvherre@alvh.no-ip.org, michael@paquier.xyz, dean.a.rasheed@gmail.com, boekewurm+postgres@gmail.com, simon.riggs@enterprisedb.com
Subject: Re: Make name optional in CREATE STATISTICS
In-Reply-To: <20220722130646.3da5oh7tgz5qgooi@alvherre.pgsql>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox