Feedback-ID: ia2694551:Fastmail
Date: Mon, 25 Jul 2022 11:49:50 +0200
From: Alvaro Herrera <alvherre@alvh.no-ip.org>
To: Michael Paquier <michael@paquier.xyz>
Cc: Tom Lane <tgl@sss.pgh.pa.us>, Dean Rasheed <dean.a.rasheed@gmail.com>,
	Matthias van de Meent <boekewurm+postgres@gmail.com>,
	Simon Riggs <simon.riggs@enterprisedb.com>,
	PostgreSQL Hackers <pgsql-hackers@postgresql.org>
Subject: Re: Make name optional in CREATE STATISTICS
Message-ID: <20220725094950.sifjxu2olmptbuz3@alvherre.pgsql>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="anhvcjyfzjkrlz2x"
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <YtuKmAr7DKn5w/MQ@paquier.xyz>
Archived-At: 
 <https://www.postgresql.org/message-id/20220725094950.sifjxu2olmptbuz3%40alvherre.pgsql>
Precedence: bulk


--anhvcjyfzjkrlz2x
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit

On 2022-Jul-23, Michael Paquier wrote:

> As the problem comes down to the fact that INDEX/TABLE, SCHEMA and
> DATABASE/SYSTEM need to handle names for different object types each,
> I think that we could do something like the attached, removing one
> block on the way at the cost of an extra parser node.

Yeah, looks good.  I propose to also test the error for reindexing a
different database, which is currently uncovered, as attached.

> By the way, it seems that 83011ce also broke the case of "REINDEX
> DATABASE CONCURRENTLY", where the parser missed the addition of a
> DefElem for "concurrently" in this case.

Wow.

-- 
Álvaro Herrera         PostgreSQL Developer  —  https://www.EnterpriseDB.com/
"Escucha y olvidarás; ve y recordarás; haz y entenderás" (Confucio)

--anhvcjyfzjkrlz2x
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment; filename="tweak-reindex.patch"

diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index dfb0d85d66..f9037761f9 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -9115,8 +9115,8 @@ 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,
@@ -9126,9 +9126,10 @@ ReindexStmt:
 			| REINDEX opt_reindex_option_list reindex_target_all opt_concurrently opt_single_name
 				{
 					ReindexStmt *n = makeNode(ReindexStmt);
+
 					n->kind = $3;
-					n->name = $5;
 					n->relation = NULL;
+					n->name = $5;
 					n->params = $2;
 					if ($4)
 						n->params = lappend(n->params,
diff --git a/src/test/regress/expected/create_index.out b/src/test/regress/expected/create_index.out
index 801b396398..6cd57e3eaa 100644
--- a/src/test/regress/expected/create_index.out
+++ b/src/test/regress/expected/create_index.out
@@ -2529,6 +2529,9 @@ ERROR:  cannot reindex system catalogs concurrently
 -- Warns about catalog relations
 REINDEX SCHEMA CONCURRENTLY pg_catalog;
 WARNING:  cannot reindex system catalogs concurrently, skipping all
+-- Not the current database
+REINDEX DATABASE not_current_database;
+ERROR:  can only reindex the currently open database
 -- Check the relation status, there should not be invalid indexes
 \d concur_reindex_tab
          Table "public.concur_reindex_tab"
diff --git a/src/test/regress/sql/create_index.sql b/src/test/regress/sql/create_index.sql
index 4b75790e47..a3738833b2 100644
--- a/src/test/regress/sql/create_index.sql
+++ b/src/test/regress/sql/create_index.sql
@@ -1076,6 +1076,8 @@ REINDEX (CONCURRENTLY) SYSTEM postgres; -- ditto
 REINDEX (CONCURRENTLY) SYSTEM;  -- ditto
 -- Warns about catalog relations
 REINDEX SCHEMA CONCURRENTLY pg_catalog;
+-- Not the current database
+REINDEX DATABASE not_current_database;
 
 -- Check the relation status, there should not be invalid indexes
 \d concur_reindex_tab

--anhvcjyfzjkrlz2x--