Fix a few issues with REINDEX grammar

This addresses a couple of bugs in the REINDEX grammar, introduced by
83011ce:
- A name was never specified for DATABASE/SYSTEM, even if the query
included one.  This caused such REINDEX queries to always work with any
object name, but we should complain if the object name specified does
not match the name of the database we are connected to.  A test is added
for this case in the main regression test suite, provided by Álvaro.
- REINDEX SYSTEM CONCURRENTLY [name] was getting rejected in the
parser.  Concurrent rebuilds are not supported for catalogs but the
error provided at execution time is more helpful for the user, and
allowing this flavor results in a simplification of the parsing logic.
- REINDEX DATABASE CONCURRENTLY was rebuilding the index in a
non-concurrent way, as the option was not being appended correctly in
the list of DefElems in ReindexStmt (REINDEX (CONCURRENTLY) DATABASE was
working fine.  A test is added in the TAP tests of reindexdb for this
case, where we already have a REINDEX DATABASE CONCURRENTLY query
running on a small-ish instance.  This relies on the work done in
2cbc3c1 for SYSTEM, but here we check if the OIDs of the index relations
match or not after the concurrent rebuild.  Note that in order to get
this part to work, I had to tweak the tests so as the index OID and
names are saved separately.  This change not affect the reliability or
of the coverage of the existing tests.

While on it, I have implemented a tweak in the grammar to reduce the
parsing by one branch, simplifying things even more.

Author: Michael Paquier, Álvaro Herrera
Discussion: https://postgr.es/m/YttqI6O64wDxGn0K@paquier.xyz

1 files changed, 17 insertions, 19 deletions

diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index ace4fb5c778..f9037761f96 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -564,7 +564,7 @@ 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
+%type <ival>	reindex_target_relation reindex_target_all
 %type <list>	opt_reindex_option_list
 
 %type <node>	copy_generic_opt_arg copy_generic_opt_arg_list_item
@@ -9092,13 +9092,12 @@ DropTransformStmt: DROP TRANSFORM opt_if_exists FOR Typename LANGUAGE name opt_d
  *
  *		QUERY:
  *
- *		REINDEX [ (options) ] {TABLE | INDEX | SCHEMA} [CONCURRENTLY] <name>
- *		REINDEX [ (options) ] DATABASE [CONCURRENTLY] [<name>]
- *		REINDEX [ (options) ] SYSTEM [<name>]
+ *		REINDEX [ (options) ] {INDEX | TABLE | SCHEMA} [CONCURRENTLY] <name>
+ *		REINDEX [ (options) ] {DATABASE | SYSTEM} [CONCURRENTLY] [<name>]
  *****************************************************************************/
 
 ReindexStmt:
-			REINDEX opt_reindex_option_list reindex_target_type opt_concurrently qualified_name
+			REINDEX opt_reindex_option_list reindex_target_relation opt_concurrently qualified_name
 				{
 					ReindexStmt *n = makeNode(ReindexStmt);
 
@@ -9116,37 +9115,36 @@ ReindexStmt:
 					ReindexStmt *n = makeNode(ReindexStmt);
 
 					n->kind = REINDEX_OBJECT_SCHEMA;
-					n->name = $5;
 					n->relation = NULL;
+					n->name = $5;
 					n->params = $2;
 					if ($4)
 						n->params = lappend(n->params,
 											makeDefElem("concurrently", NULL, @4));
 					$$ = (Node *) n;
 				}
-			| REINDEX opt_reindex_option_list DATABASE opt_concurrently opt_single_name
-				{
-					ReindexStmt *n = makeNode(ReindexStmt);
-					n->kind = REINDEX_OBJECT_DATABASE;
-					n->name = NULL;
-					n->relation = NULL;
-					n->params = $2;
-					$$ = (Node *) n;
-				}
-			| REINDEX opt_reindex_option_list SYSTEM_P opt_single_name
+			| REINDEX opt_reindex_option_list reindex_target_all opt_concurrently opt_single_name
 				{
 					ReindexStmt *n = makeNode(ReindexStmt);
-					n->kind = REINDEX_OBJECT_SYSTEM;
-					n->name = NULL;
+
+					n->kind = $3;
 					n->relation = NULL;
+					n->name = $5;
 					n->params = $2;
+					if ($4)
+						n->params = lappend(n->params,
+											makeDefElem("concurrently", NULL, @4));
 					$$ = (Node *) n;
 				}
 		;
-reindex_target_type:
+reindex_target_relation:
 			INDEX					{ $$ = REINDEX_OBJECT_INDEX; }
 			| TABLE					{ $$ = REINDEX_OBJECT_TABLE; }
 		;
+reindex_target_all:
+			SYSTEM_P				{ $$ = REINDEX_OBJECT_SYSTEM; }
+			| DATABASE				{ $$ = REINDEX_OBJECT_DATABASE; }
+		;
 opt_reindex_option_list:
 			'(' utility_option_list ')'				{ $$ = $2; }
 			| /* EMPTY */							{ $$ = NULL; }