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, 25 insertions, 12 deletions

diff --git a/src/bin/scripts/t/090_reindexdb.pl b/src/bin/scripts/t/090_reindexdb.pl
index b5fff5a9cf1..e706d686e39 100644
--- a/src/bin/scripts/t/090_reindexdb.pl
+++ b/src/bin/scripts/t/090_reindexdb.pl
@@ -40,12 +40,12 @@ my $toast_index = $node->safe_psql('postgres',
 # REINDEX operations.  A set of relfilenodes is saved from the catalogs
 # and then compared with pg_class.
 $node->safe_psql('postgres',
-	'CREATE TABLE index_relfilenodes (parent regclass, indname regclass, relfilenode oid);'
+	'CREATE TABLE index_relfilenodes (parent regclass, indname text, indoid oid, relfilenode oid);'
 );
 # Save the relfilenode of a set of toast indexes, one from the catalog
 # pg_constraint and one from the test table.
 my $fetch_toast_relfilenodes =
-  qq{SELECT b.oid::regclass, c.oid::regclass, c.relfilenode
+  qq{SELECT b.oid::regclass, c.oid::regclass::text, c.oid, c.relfilenode
   FROM pg_class a
     JOIN pg_class b ON (a.oid = b.reltoastrelid)
     JOIN pg_index i on (a.oid = i.indrelid)
@@ -53,7 +53,7 @@ my $fetch_toast_relfilenodes =
   WHERE b.oid IN ('pg_constraint'::regclass, 'test1'::regclass)};
 # Same for relfilenodes of normal indexes.  This saves the relfilenode
 # from an index of pg_constraint, and from the index of the test table.
-my $fetch_index_relfilenodes = qq{SELECT i.indrelid, a.oid, a.relfilenode
+my $fetch_index_relfilenodes = qq{SELECT i.indrelid, a.oid::regclass::text, a.oid, a.relfilenode
   FROM pg_class a
     JOIN pg_index i ON (i.indexrelid = a.oid)
   WHERE a.relname IN ('pg_constraint_oid_index', 'test1x')};
@@ -69,6 +69,8 @@ my $save_relfilenodes =
 # parent table is included to provide more context.
 my $compare_relfilenodes = qq(SELECT b.parent::regclass,
   regexp_replace(b.indname::text, '(pg_toast.pg_toast_)\\d+(_index)', '\\1<oid>\\2'),
+  CASE WHEN a.oid = b.indoid THEN 'OID is unchanged'
+    ELSE 'OID has changed' END,
   CASE WHEN a.relfilenode = b.relfilenode THEN 'relfilenode is unchanged'
     ELSE 'relfilenode has changed' END
   FROM index_relfilenodes b
@@ -83,10 +85,10 @@ $node->issues_sql_like(
 	'SQL REINDEX run');
 my $relnode_info = $node->safe_psql('postgres', $compare_relfilenodes);
 is( $relnode_info,
-	qq(pg_constraint|pg_constraint_oid_index|relfilenode is unchanged
-pg_constraint|pg_toast.pg_toast_<oid>_index|relfilenode is unchanged
-test1|pg_toast.pg_toast_<oid>_index|relfilenode has changed
-test1|test1x|relfilenode has changed),
+	qq(pg_constraint|pg_constraint_oid_index|OID is unchanged|relfilenode is unchanged
+pg_constraint|pg_toast.pg_toast_<oid>_index|OID is unchanged|relfilenode is unchanged
+test1|pg_toast.pg_toast_<oid>_index|OID is unchanged|relfilenode has changed
+test1|test1x|OID is unchanged|relfilenode has changed),
 	'relfilenode change after REINDEX DATABASE');
 
 # Re-save and run the second one.
@@ -98,10 +100,10 @@ $node->issues_sql_like(
 	'reindex system tables');
 $relnode_info = $node->safe_psql('postgres', $compare_relfilenodes);
 is( $relnode_info,
-	qq(pg_constraint|pg_constraint_oid_index|relfilenode has changed
-pg_constraint|pg_toast.pg_toast_<oid>_index|relfilenode has changed
-test1|pg_toast.pg_toast_<oid>_index|relfilenode is unchanged
-test1|test1x|relfilenode is unchanged),
+	qq(pg_constraint|pg_constraint_oid_index|OID is unchanged|relfilenode has changed
+pg_constraint|pg_toast.pg_toast_<oid>_index|OID is unchanged|relfilenode has changed
+test1|pg_toast.pg_toast_<oid>_index|OID is unchanged|relfilenode is unchanged
+test1|test1x|OID is unchanged|relfilenode is unchanged),
 	'relfilenode change after REINDEX SYSTEM');
 
 $node->issues_sql_like(
@@ -132,11 +134,22 @@ $node->issues_sql_like(
 	qr/statement: REINDEX \(VERBOSE, TABLESPACE $tbspace_name\) TABLE public\.test1;/,
 	'reindex with verbose output and tablespace');
 
-# the same with --concurrently
+# Same with --concurrently.
+# Save the state of the relations and compare them after the DATABASE
+# rebuild.
+$node->safe_psql('postgres',
+	"TRUNCATE index_relfilenodes; $save_relfilenodes");
 $node->issues_sql_like(
 	[ 'reindexdb', '--concurrently', 'postgres' ],
 	qr/statement: REINDEX DATABASE CONCURRENTLY postgres;/,
 	'SQL REINDEX CONCURRENTLY run');
+$relnode_info = $node->safe_psql('postgres', $compare_relfilenodes);
+is( $relnode_info,
+	qq(pg_constraint|pg_constraint_oid_index|OID is unchanged|relfilenode is unchanged
+pg_constraint|pg_toast.pg_toast_<oid>_index|OID is unchanged|relfilenode is unchanged
+test1|pg_toast.pg_toast_<oid>_index|OID has changed|relfilenode has changed
+test1|test1x|OID has changed|relfilenode has changed),
+	'OID change after REINDEX DATABASE CONCURRENTLY');
 
 $node->issues_sql_like(
 	[ 'reindexdb', '--concurrently', '-t', 'test1', 'postgres' ],