PATCH: Add REINDEX tag to event triggers

Started by Garrett Thornburgover 2 years ago27 messages
#1Garrett Thornburg
film42@gmail.com
1 attachment(s)

Added my v1 patch to add REINDEX to event triggers.

I originally built this against pg15 but rebased to master for the patch
to hopefully make it easier for maintainers to merge. The rebase was
automatic so it should be easy to include into any recent version. I'd love
for this to land in pg16 if possible.

I added regression tests and they are passing. I also formatted the code
using the project tools. Docs are included too.

A few notes:
1. I tried to not touch the function parameters too much and opted to
create a convenience function that makes it easy to attach index or table
OIDs to the current event trigger list.
2. I made sure both concurrent and regular reindex of index, table,
database work as expected.
3. The ddl end command will make available all indexes that were modified
by the reindex command. This is a large list when you run "reindex
database" but works really well. I debated returning records of the table
or DB that were reindexed but that doesn't really make sense. Returning
each index fits my use case of building an audit record around the index
lifecycle (hence the motivation for the patch).

Thanks for your time,

Garrett Thornburg

Attachments:

v1-0001-Add-REINDEX-tag-to-event-triggers.patchapplication/octet-stream; name=v1-0001-Add-REINDEX-tag-to-event-triggers.patchDownload
From 19f380263cb81a977b3ba8c6fb5fd2b32af4d8ed Mon Sep 17 00:00:00 2001
From: Garrett Thornburg <garrett.thornburg@hey.com>
Date: Thu, 20 Jul 2023 20:35:26 -0600
Subject: [PATCH v1] Add REINDEX tag to event triggers

This turns on the event tag REINDEX. This will now return a record for
each index that was modified during as part of start/end ddl commands.

For concurrent reindex, this will return the OID for the new index. This
includes concurrently reindexing tables and databases. It will only
report the new index records and not the ones destroyed by the
concurrent reindex process.
---
 doc/src/sgml/event-trigger.sgml             |  8 ++
 src/backend/commands/indexcmds.c            | 83 +++++++++++++++++++++
 src/backend/tcop/utility.c                  | 12 ++-
 src/include/tcop/cmdtaglist.h               |  2 +-
 src/test/regress/expected/event_trigger.out | 35 +++++++++
 src/test/regress/sql/event_trigger.sql      | 33 ++++++++
 6 files changed, 170 insertions(+), 3 deletions(-)

diff --git a/doc/src/sgml/event-trigger.sgml b/doc/src/sgml/event-trigger.sgml
index 3b6a5361b3..14d3fac29d 100644
--- a/doc/src/sgml/event-trigger.sgml
+++ b/doc/src/sgml/event-trigger.sgml
@@ -1013,6 +1013,14 @@
         <entry align="center"><literal>-</literal></entry>
         <entry align="left"></entry>
        </row>
+       <row>
+        <entry align="left"><literal>REINDEX</literal></entry>
+        <entry align="center"><literal>-</literal></entry>
+        <entry align="center"><literal>X</literal></entry>
+        <entry align="center"><literal>-</literal></entry>
+        <entry align="center"><literal>-</literal></entry>
+        <entry align="left"></entry>
+       </row>
        <row>
         <entry align="left"><literal>REVOKE</literal></entry>
         <entry align="center"><literal>X</literal></entry>
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index baf3e6e57a..2dc74225ed 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -94,6 +94,8 @@ static char *ChooseIndexNameAddition(List *colnames);
 static List *ChooseIndexColumnNames(List *indexElems);
 static void ReindexIndex(RangeVar *indexRelation, ReindexParams *params,
 						 bool isTopLevel);
+static void reindex_event_trigger_collect(Oid oid);
+static void reindex_event_trigger_collect_relation(Oid oid);
 static void RangeVarCallbackForReindexIndex(const RangeVar *relation,
 											Oid relId, Oid oldRelId, void *arg);
 static Oid	ReindexTable(RangeVar *relation, ReindexParams *params,
@@ -110,6 +112,8 @@ static bool ReindexRelationConcurrently(Oid relationOid,
 static void update_relispartition(Oid relationId, bool newval);
 static inline void set_indexsafe_procflags(void);
 
+static ReindexStmt *currentReindexStatement = NULL;
+
 /*
  * callback argument type for RangeVarCallbackForReindexIndex()
  */
@@ -2699,6 +2703,12 @@ ExecReindex(ParseState *pstate, ReindexStmt *stmt, bool isTopLevel)
 	bool		verbose = false;
 	char	   *tablespacename = NULL;
 
+	/*
+	 * Make current stmt available for event triggers without directly passing
+	 * the context to every subsequent call.
+	 */
+	currentReindexStatement = stmt;
+
 	/* Parse option list */
 	foreach(lc, stmt->params)
 	{
@@ -2779,6 +2789,12 @@ ExecReindex(ParseState *pstate, ReindexStmt *stmt, bool isTopLevel)
 				 (int) stmt->kind);
 			break;
 	}
+
+	/*
+	 * Clear the reindex stmt global reference now that triggers should have
+	 * completed
+	 */
+	currentReindexStatement = NULL;
 }
 
 /*
@@ -2830,6 +2846,8 @@ ReindexIndex(RangeVar *indexRelation, ReindexParams *params, bool isTopLevel)
 
 		newparams.options |= REINDEXOPT_REPORT_PROGRESS;
 		reindex_index(indOid, false, persistence, &newparams);
+		/* Add the index to event trigger */
+		reindex_event_trigger_collect(indOid);
 	}
 }
 
@@ -2953,6 +2971,9 @@ ReindexTable(RangeVar *relation, ReindexParams *params, bool isTopLevel)
 			ereport(NOTICE,
 					(errmsg("table \"%s\" has no indexes to reindex",
 							relation->relname)));
+
+		/* Create even for the indexes being modified */
+		reindex_event_trigger_collect_relation(heapOid);
 	}
 
 	return heapOid;
@@ -3369,6 +3390,8 @@ ReindexMultipleInternal(List *relids, ReindexParams *params)
 			newparams.options |=
 				REINDEXOPT_REPORT_PROGRESS | REINDEXOPT_MISSING_OK;
 			reindex_index(relid, false, relpersistence, &newparams);
+			/* Add the index to event trigger */
+			reindex_event_trigger_collect(relid);
 			PopActiveSnapshot();
 			/* reindex_index() does the verbose output */
 		}
@@ -3390,6 +3413,9 @@ ReindexMultipleInternal(List *relids, ReindexParams *params)
 								get_namespace_name(get_rel_namespace(relid)),
 								get_rel_name(relid))));
 
+			/* Create even for the indexes being modified */
+			reindex_event_trigger_collect_relation(relid);
+
 			PopActiveSnapshot();
 		}
 
@@ -3837,6 +3863,9 @@ ReindexRelationConcurrently(Oid relationOid, ReindexParams *params)
 
 		newIndexIds = lappend(newIndexIds, newidx);
 
+		/* Add the index to event trigger */
+		reindex_event_trigger_collect(newIndexId);
+
 		/*
 		 * Save lockrelid to protect each relation from drop then close
 		 * relations. The lockrelid on parent relation is not taken here to
@@ -4441,3 +4470,57 @@ set_indexsafe_procflags(void)
 	ProcGlobal->statusFlags[MyProc->pgxactoff] = MyProc->statusFlags;
 	LWLockRelease(ProcArrayLock);
 }
+
+static void
+reindex_event_trigger_collect(Oid oid)
+{
+	ObjectAddress address;
+
+	if (currentReindexStatement == NULL)
+		return;
+
+	address.classId = RelationRelationId;
+	address.objectId = oid;
+	address.objectSubId = 0;
+
+	EventTriggerCollectSimpleCommand(address,
+									 InvalidObjectAddress, (Node *) currentReindexStatement);
+}
+
+static void
+reindex_event_trigger_collect_relation(Oid relid)
+{
+	Relation	rel;
+	List	   *indexIds = NULL;
+	ListCell   *indexId = NULL;
+
+	/*
+	 * Open and lock the relation.  ShareLock is sufficient since we only need
+	 * to prevent schema and data changes in it.  The lock level used here
+	 * should match catalog's reindex_relation().
+	 */
+	rel = try_table_open(relid, ShareLock);
+
+	/* if relation is gone, leave */
+	if (!rel)
+		return;
+
+	/*
+	 * Get the list of index OIDs for this relation.  (We trust to the
+	 * relcache to get this with a sequential scan if ignoring system
+	 * indexes.)
+	 */
+	indexIds = RelationGetIndexList(rel);
+
+	/*
+	 * Get the list of index OIDs for this relation.
+	 */
+	foreach(indexId, indexIds)
+	{
+		Oid			indexOid = lfirst_oid(indexId);
+
+		reindex_event_trigger_collect(indexOid);
+	}
+
+	table_close(rel, ShareLock);
+}
diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c
index e3ccf6c7f7..a269b5b6dc 100644
--- a/src/backend/tcop/utility.c
+++ b/src/backend/tcop/utility.c
@@ -961,7 +961,9 @@ standard_ProcessUtility(PlannedStmt *pstmt,
 			break;
 
 		case T_ReindexStmt:
-			ExecReindex(pstate, (ReindexStmt *) parsetree, isTopLevel);
+			ProcessUtilitySlow(pstate, pstmt, queryString,
+							   context, params, queryEnv,
+							   dest, qc);
 			break;
 
 			/*
@@ -1574,6 +1576,12 @@ ProcessUtilitySlow(ParseState *pstate,
 				}
 				break;
 
+			case T_ReindexStmt:
+				ExecReindex(pstate, (ReindexStmt *) parsetree, isTopLevel);
+				/* no commands stashed for REINDEX */
+				commandCollected = true;
+				break;
+
 			case T_CreateExtensionStmt:
 				address = CreateExtension(pstate, (CreateExtensionStmt *) parsetree);
 				break;
@@ -3620,7 +3628,7 @@ GetCommandLogLevel(Node *parsetree)
 			break;
 
 		case T_ReindexStmt:
-			lev = LOGSTMT_ALL;	/* should this be DDL? */
+			lev = LOGSTMT_DDL;
 			break;
 
 		case T_CreateConversionStmt:
diff --git a/src/include/tcop/cmdtaglist.h b/src/include/tcop/cmdtaglist.h
index e738ac1c09..d0e0b18609 100644
--- a/src/include/tcop/cmdtaglist.h
+++ b/src/include/tcop/cmdtaglist.h
@@ -193,7 +193,7 @@ PG_CMDTAG(CMDTAG_PREPARE, "PREPARE", false, false, false)
 PG_CMDTAG(CMDTAG_PREPARE_TRANSACTION, "PREPARE TRANSACTION", false, false, false)
 PG_CMDTAG(CMDTAG_REASSIGN_OWNED, "REASSIGN OWNED", false, false, false)
 PG_CMDTAG(CMDTAG_REFRESH_MATERIALIZED_VIEW, "REFRESH MATERIALIZED VIEW", true, false, false)
-PG_CMDTAG(CMDTAG_REINDEX, "REINDEX", false, false, false)
+PG_CMDTAG(CMDTAG_REINDEX, "REINDEX", true, false, false)
 PG_CMDTAG(CMDTAG_RELEASE, "RELEASE", false, false, false)
 PG_CMDTAG(CMDTAG_RESET, "RESET", false, false, false)
 PG_CMDTAG(CMDTAG_REVOKE, "REVOKE", true, false, false)
diff --git a/src/test/regress/expected/event_trigger.out b/src/test/regress/expected/event_trigger.out
index 5a10958df5..986787bf53 100644
--- a/src/test/regress/expected/event_trigger.out
+++ b/src/test/regress/expected/event_trigger.out
@@ -554,6 +554,41 @@ ERROR:  cannot alter type "rewritetype" because column "rewritemetoo3.a" uses it
 drop table rewriteme;
 drop event trigger no_rewrite_allowed;
 drop function test_evtrig_no_rewrite();
+-- test Reindex Event Trigger
+CREATE SCHEMA reindex_test;
+CREATE TABLE reindex_test.reindex_tester1 (a int);
+CREATE INDEX reindex_test1_idx1 ON reindex_test.reindex_tester1 (a);
+CREATE INDEX reindex_test1_idx2 ON reindex_test.reindex_tester1 (a);
+CREATE TABLE reindex_test.reindex_tester2 (a int);
+CREATE INDEX reindex_test2_idx1 ON reindex_test.reindex_tester2 (a);
+CREATE OR REPLACE FUNCTION reindex_end_command()
+RETURNS event_trigger AS $$
+DECLARE
+  obj record;
+BEGIN
+  FOR obj IN SELECT * FROM pg_event_trigger_ddl_commands()
+    LOOP
+      RAISE NOTICE 'ddl_end_command -- REINDEX: %', pg_get_indexdef(obj.objid);
+    END LOOP;
+END;
+$$ LANGUAGE plpgsql;
+CREATE EVENT TRIGGER end_reindex_command ON ddl_command_end
+    WHEN TAG IN ('REINDEX') EXECUTE PROCEDURE reindex_end_command();
+REINDEX INDEX reindex_test.reindex_test1_idx1;
+NOTICE:  ddl_end_command -- REINDEX: CREATE INDEX reindex_test1_idx1 ON reindex_test.reindex_tester1 USING btree (a)
+REINDEX TABLE reindex_test.reindex_tester1;
+NOTICE:  ddl_end_command -- REINDEX: CREATE INDEX reindex_test1_idx1 ON reindex_test.reindex_tester1 USING btree (a)
+NOTICE:  ddl_end_command -- REINDEX: CREATE INDEX reindex_test1_idx2 ON reindex_test.reindex_tester1 USING btree (a)
+REINDEX INDEX CONCURRENTLY reindex_test.reindex_test2_idx1;
+NOTICE:  ddl_end_command -- REINDEX: CREATE INDEX reindex_test2_idx1 ON reindex_test.reindex_tester2 USING btree (a)
+REINDEX TABLE CONCURRENTLY reindex_test.reindex_tester2;
+NOTICE:  ddl_end_command -- REINDEX: CREATE INDEX reindex_test2_idx1 ON reindex_test.reindex_tester2 USING btree (a)
+DROP EVENT TRIGGER end_reindex_command;
+DROP SCHEMA reindex_test CASCADE;
+NOTICE:  drop cascades to 2 other objects
+DETAIL:  drop cascades to table reindex_test.reindex_tester1
+drop cascades to table reindex_test.reindex_tester2
+DROP FUNCTION reindex_end_command;
 -- test Row Security Event Trigger
 RESET SESSION AUTHORIZATION;
 CREATE TABLE event_trigger_test (a integer, b text);
diff --git a/src/test/regress/sql/event_trigger.sql b/src/test/regress/sql/event_trigger.sql
index 1aeaddbe71..53d0aff398 100644
--- a/src/test/regress/sql/event_trigger.sql
+++ b/src/test/regress/sql/event_trigger.sql
@@ -418,6 +418,39 @@ drop table rewriteme;
 drop event trigger no_rewrite_allowed;
 drop function test_evtrig_no_rewrite();
 
+-- test Reindex Event Trigger
+CREATE SCHEMA reindex_test;
+CREATE TABLE reindex_test.reindex_tester1 (a int);
+CREATE INDEX reindex_test1_idx1 ON reindex_test.reindex_tester1 (a);
+CREATE INDEX reindex_test1_idx2 ON reindex_test.reindex_tester1 (a);
+CREATE TABLE reindex_test.reindex_tester2 (a int);
+CREATE INDEX reindex_test2_idx1 ON reindex_test.reindex_tester2 (a);
+
+CREATE OR REPLACE FUNCTION reindex_end_command()
+RETURNS event_trigger AS $$
+DECLARE
+  obj record;
+BEGIN
+  FOR obj IN SELECT * FROM pg_event_trigger_ddl_commands()
+    LOOP
+      RAISE NOTICE 'ddl_end_command -- REINDEX: %', pg_get_indexdef(obj.objid);
+    END LOOP;
+END;
+$$ LANGUAGE plpgsql;
+
+CREATE EVENT TRIGGER end_reindex_command ON ddl_command_end
+    WHEN TAG IN ('REINDEX') EXECUTE PROCEDURE reindex_end_command();
+
+REINDEX INDEX reindex_test.reindex_test1_idx1;
+REINDEX TABLE reindex_test.reindex_tester1;
+REINDEX INDEX CONCURRENTLY reindex_test.reindex_test2_idx1;
+REINDEX TABLE CONCURRENTLY reindex_test.reindex_tester2;
+
+DROP EVENT TRIGGER end_reindex_command;
+DROP SCHEMA reindex_test CASCADE;
+DROP FUNCTION reindex_start_command;
+DROP FUNCTION reindex_end_command;
+
 -- test Row Security Event Trigger
 RESET SESSION AUTHORIZATION;
 CREATE TABLE event_trigger_test (a integer, b text);
-- 
2.38.1

#2Michael Paquier
michael@paquier.xyz
In reply to: Garrett Thornburg (#1)
Re: PATCH: Add REINDEX tag to event triggers

On Thu, Jul 20, 2023 at 10:47:00PM -0600, Garrett Thornburg wrote:

Added my v1 patch to add REINDEX to event triggers.

I originally built this against pg15 but rebased to master for the patch
to hopefully make it easier for maintainers to merge. The rebase was
automatic so it should be easy to include into any recent version. I'd love
for this to land in pg16 if possible.

The development of Postgres 16 has finished last April and this
version is in a stabilization period. We are currently running the
first commit fest of PostgreSQL 17, though. I would recommend to
register your patch here so as it is not lost:
https://commitfest.postgresql.org/44/

I added regression tests and they are passing. I also formatted the code
using the project tools. Docs are included too.

Hmm. What are the use cases you have in mind where you need to know
the list of indexes listed by an event trigger like this? Just
monitoring to know which indexes have been rebuilt? We have a
table_rewrite which is equivalent to check if a relfilenode has
changed, so why not, I guess..

Assuming that ddl_command_start is not supported is incorrect. For
example, with your patch applied, if I do the following setup:
create event trigger regress_event_trigger on ddl_command_start
execute procedure test_event_trigger();
create function test_event_trigger() returns event_trigger as $$
BEGIN
RAISE NOTICE 'test_event_trigger: % %', tg_event, tg_tag;
END
$$ language plpgsql;

Then a REINDEX gives me that:
reindex table pg_class;
NOTICE: 00000: test_event_trigger: ddl_command_start REINDEX

The first paragraphs of event-trigger.sgml describe the following:
The <literal>ddl_command_start</literal> event occurs just before the
execution of a <literal>CREATE</literal>, <literal>ALTER</literal>, <literal>DROP</literal>,
<literal>SECURITY LABEL</literal>,
<literal>COMMENT</literal>, <literal>GRANT</literal> or <literal>REVOKE</literal>
[...]
The <literal>ddl_command_end</literal> event occurs just after the execution of
this same set of commands. To obtain more details on the <acronym>DDL</acronym>

So it would need a refresh to list REINDEX.

        case T_ReindexStmt:
-           lev = LOGSTMT_ALL;  /* should this be DDL? */
+           lev = LOGSTMT_DDL;

This, unfortunately, may not be as right as it is simple because
REINDEX is not a DDL as far as I know (it is not in the SQL
standard).
--
Michael

#3jian he
jian.universality@gmail.com
In reply to: Garrett Thornburg (#1)
Re: PATCH: Add REINDEX tag to event triggers

On Fri, Jul 21, 2023 at 12:47 PM Garrett Thornburg <film42@gmail.com> wrote:

Added my v1 patch to add REINDEX to event triggers.

I originally built this against pg15 but rebased to master for the patch to hopefully make it easier for maintainers to merge. The rebase was automatic so it should be easy to include into any recent version. I'd love for this to land in pg16 if possible.

I added regression tests and they are passing. I also formatted the code using the project tools. Docs are included too.

A few notes:
1. I tried to not touch the function parameters too much and opted to create a convenience function that makes it easy to attach index or table OIDs to the current event trigger list.
2. I made sure both concurrent and regular reindex of index, table, database work as expected.
3. The ddl end command will make available all indexes that were modified by the reindex command. This is a large list when you run "reindex database" but works really well. I debated returning records of the table or DB that were reindexed but that doesn't really make sense. Returning each index fits my use case of building an audit record around the index lifecycle (hence the motivation for the patch).

Thanks for your time,

Garrett Thornburg

main/postgres/src/backend/tcop/utility.c
535: /*
536: * standard_ProcessUtility itself deals only with utility commands for
537: * which we do not provide event trigger support. Commands that do have
538: * such support are passed down to ProcessUtilitySlow, which contains the
539: * necessary infrastructure for such triggers.
540: *
541: * This division is not just for performance: it's critical that the
542: * event trigger code not be invoked when doing START TRANSACTION for
543: * example, because we might need to refresh the event trigger cache,
544: * which requires being in a valid transaction.
545: */

so T_ReindexStmt should only be in ProcessUtilitySlow, if you want
to create an event trigger on reindex?

regression tests work fine. I even play with partitions.

#4Michael Paquier
michael@paquier.xyz
In reply to: jian he (#3)
Re: PATCH: Add REINDEX tag to event triggers

On Tue, Jul 25, 2023 at 04:34:47PM +0800, jian he wrote:

so T_ReindexStmt should only be in ProcessUtilitySlow, if you want
to create an event trigger on reindex?

regression tests work fine. I even play with partitions.

It would be an idea to have some regression tests for partitions,
actually, so as some patterns around ReindexMultipleInternal() are
checked. We could have a REINDEX DATABASE in a TAP test with an event
trigger, as well, but I don't feel strongly about the need to do that
much extra work in 090_reindexdb.pl or 091_reindexdb_all.pl if
partitions cover the multi-table case.
--
Michael

#5jian he
jian.universality@gmail.com
In reply to: Michael Paquier (#4)
Re: PATCH: Add REINDEX tag to event triggers

On Wed, Jul 26, 2023 at 7:51 AM Michael Paquier <michael@paquier.xyz> wrote:

On Tue, Jul 25, 2023 at 04:34:47PM +0800, jian he wrote:

so T_ReindexStmt should only be in ProcessUtilitySlow, if you want
to create an event trigger on reindex?

regression tests work fine. I even play with partitions.

It would be an idea to have some regression tests for partitions,
actually, so as some patterns around ReindexMultipleInternal() are
checked. We could have a REINDEX DATABASE in a TAP test with an event
trigger, as well, but I don't feel strongly about the need to do that
much extra work in 090_reindexdb.pl or 091_reindexdb_all.pl if
partitions cover the multi-table case.
--
Michael

quite verbose, copied from partition-info.sql. meet the expectation:
partitioned index will do nothing, partition index will trigger event
trigger.
------------------------------------------------
DROP EVENT TRIGGER IF EXISTS end_reindex_command CASCADE;
DROP EVENT TRIGGER IF EXISTS start_reindex_command CASCADE;

BEGIN;
CREATE OR REPLACE FUNCTION reindex_end_command()
RETURNS event_trigger AS $$
DECLARE
obj record;
BEGIN
raise notice 'begin of reindex_end_command';

FOR obj IN SELECT * FROM pg_event_trigger_ddl_commands()
LOOP
RAISE NOTICE
'obj.command_tag:% obj.object_type:% obj.schema_name:%
obj.object_identity:%'
,obj.command_tag, obj.object_type,obj.schema_name,obj.object_identity;
RAISE NOTICE 'ddl_end_command -- REINDEX: %', pg_get_indexdef(obj.objid);
END LOOP;
END;
$$ LANGUAGE plpgsql;

CREATE OR REPLACE FUNCTION start_reindex_command()
RETURNS event_trigger AS $$
DECLARE
obj record;
BEGIN
FOR obj IN SELECT * FROM pg_event_trigger_ddl_commands()
LOOP
RAISE NOTICE
'obj.command_tag:% obj.object_type:% obj.schema_name:%
obj.object_identity:%'
, obj.command_tag, obj.object_type,obj.schema_name,obj.object_identity;
RAISE NOTICE 'ddl_start_command -- REINDEX: %',
pg_get_indexdef(obj.objid);
END LOOP;
raise notice 'end of start_reindex_command';
END;
$$ LANGUAGE plpgsql;

BEGIN;
CREATE EVENT TRIGGER end_reindex_command ON ddl_command_end
WHEN TAG IN ('REINDEX') EXECUTE PROCEDURE reindex_end_command();
CREATE EVENT TRIGGER start_reindex_command ON ddl_command_start
WHEN TAG IN ('REINDEX') EXECUTE PROCEDURE start_reindex_command();
COMMIT;

-- test Reindex Event Trigger
BEGIN;
drop table if EXISTS ptif_test CASCADE;
CREATE TABLE ptif_test (a int, b int) PARTITION BY range (a);
CREATE TABLE ptif_test0 PARTITION OF ptif_test
FOR VALUES FROM (minvalue) TO (0) PARTITION BY list (b);
CREATE TABLE ptif_test01 PARTITION OF ptif_test0 FOR VALUES IN (1);
CREATE TABLE ptif_test1 PARTITION OF ptif_test
FOR VALUES FROM (0) TO (100) PARTITION BY list (b);
CREATE TABLE ptif_test11 PARTITION OF ptif_test1 FOR VALUES IN (1);

CREATE TABLE ptif_test2 PARTITION OF ptif_test
FOR VALUES FROM (100) TO (200);
-- This partitioned table should remain with no partitions.
CREATE TABLE ptif_test3 PARTITION OF ptif_test
FOR VALUES FROM (200) TO (maxvalue) PARTITION BY list (b);

-- Test index partition tree
CREATE INDEX ptif_test_index ON ONLY ptif_test (a);

CREATE INDEX ptif_test0_index ON ONLY ptif_test0 (a);
ALTER INDEX ptif_test_index ATTACH PARTITION ptif_test0_index;

CREATE INDEX ptif_test01_index ON ptif_test01 (a);
ALTER INDEX ptif_test0_index ATTACH PARTITION ptif_test01_index;

CREATE INDEX ptif_test1_index ON ONLY ptif_test1 (a);
ALTER INDEX ptif_test_index ATTACH PARTITION ptif_test1_index;

CREATE INDEX ptif_test11_index ON ptif_test11 (a);
ALTER INDEX ptif_test1_index ATTACH PARTITION ptif_test11_index;

CREATE INDEX ptif_test2_index ON ptif_test2 (a);
ALTER INDEX ptif_test_index ATTACH PARTITION ptif_test2_index;

CREATE INDEX ptif_test3_index ON ptif_test3 (a);
ALTER INDEX ptif_test_index ATTACH PARTITION ptif_test3_index;
COMMIT;

--top level partitioned index. will recurse to each partition index.
REINDEX INDEX CONCURRENTLY public.ptif_test_index;

--ptif_test0 is partitioned table. it will index partition: ptif_test01_index
-- event trigger will log ptif_test01_index
REINDEX INDEX CONCURRENTLY public.ptif_test0_index;

--ptif_test1_index is partitioned index. it will index partition:
ptif_test11_index
-- event trigger will effect on partion index:ptif_test11_index
REINDEX INDEX CONCURRENTLY public.ptif_test1_index;

--ptif_test2 is a partition. event trigger will log ptif_test2_index
REINDEX INDEX CONCURRENTLY public.ptif_test2_index;

--no partitions. event trigger won't do anything.
REINDEX INDEX CONCURRENTLY public.ptif_test3_index;

reindex table ptif_test; --top level. will recurse to each partition index.
reindex table ptif_test0; -- will direct to ptif_test01
reindex table ptif_test01; -- will index it's associtaed index
reindex table ptif_test11; -- will index it's associtaed index
reindex table ptif_test2; -- will index it's associtaed index
reindex table ptif_test3; -- no partion, index won't do anything.

DROP EVENT TRIGGER IF EXISTS end_reindex_command CASCADE;
DROP EVENT TRIGGER IF EXISTS start_reindex_command CASCADE;
DROP FUNCTION IF EXISTS reindex_start_command;
DROP FUNCTION IF EXISTS reindex_end_command;
DROP TABLE if EXISTS ptif_test CASCADE;
-----------------------

#6Garrett Thornburg
film42@gmail.com
In reply to: Michael Paquier (#2)
Re: PATCH: Add REINDEX tag to event triggers

Thanks! I added the patch to the commitfest. TIL.

Hmm. What are the use cases you have in mind where you need to know the

list of indexes listed by an event trigger like this?

Direct use case is general index health and rebuild. We had a recent
upgrade go poorly (our fault) that left a bunch of indexes in a corrupted
state. We ran a mass rebuild a few times. Would have been great to be able
to observe changes happening in scripts. That left us wishing there was a
way to monitor all changes around indexes. Figured I'd add support since my
first thread seemed somewhat supportive of the idea.

We have a table_rewrite which is equivalent to check if a relfilenode has

changed, so why not, I guess..

Yeah. I was actually debating if we need to add a new reindex event instead
of building on top of the DDL start/end. You are right that the
documentation does not include nonsql commands like reindex. Maybe I should
consider going that direction again?

Assuming that ddl_command_start is not supported is incorrect.

Also great callout. Using `SELECT * FROM pg_event_trigger_ddl_commands()`
in the event does not yield anything. Bad assumption there. I'll add a test
with my next patch version.

Re: LOGSTMT_ALL

Agree here. I think this is more for reference than anything else so I'm
fine to call it LOGSTMT_ALL, but that goes back to the previous question
around ddl events vs a new event similar to table_rewrite.

P.S. Sorry for the double post. Sent from the wrong email so the mailing
list didn't get the message.

On Tue, Jul 25, 2023 at 12:55 AM Michael Paquier <michael@paquier.xyz>
wrote:

Show quoted text

On Thu, Jul 20, 2023 at 10:47:00PM -0600, Garrett Thornburg wrote:

Added my v1 patch to add REINDEX to event triggers.

I originally built this against pg15 but rebased to master for the patch
to hopefully make it easier for maintainers to merge. The rebase was
automatic so it should be easy to include into any recent version. I'd

love

for this to land in pg16 if possible.

The development of Postgres 16 has finished last April and this
version is in a stabilization period. We are currently running the
first commit fest of PostgreSQL 17, though. I would recommend to
register your patch here so as it is not lost:
https://commitfest.postgresql.org/44/

I added regression tests and they are passing. I also formatted the code
using the project tools. Docs are included too.

Hmm. What are the use cases you have in mind where you need to know
the list of indexes listed by an event trigger like this? Just
monitoring to know which indexes have been rebuilt? We have a
table_rewrite which is equivalent to check if a relfilenode has
changed, so why not, I guess..

Assuming that ddl_command_start is not supported is incorrect. For
example, with your patch applied, if I do the following setup:
create event trigger regress_event_trigger on ddl_command_start
execute procedure test_event_trigger();
create function test_event_trigger() returns event_trigger as $$
BEGIN
RAISE NOTICE 'test_event_trigger: % %', tg_event, tg_tag;
END
$$ language plpgsql;

Then a REINDEX gives me that:
reindex table pg_class;
NOTICE: 00000: test_event_trigger: ddl_command_start REINDEX

The first paragraphs of event-trigger.sgml describe the following:
The <literal>ddl_command_start</literal> event occurs just before the
execution of a <literal>CREATE</literal>, <literal>ALTER</literal>,
<literal>DROP</literal>,
<literal>SECURITY LABEL</literal>,
<literal>COMMENT</literal>, <literal>GRANT</literal> or
<literal>REVOKE</literal>
[...]
The <literal>ddl_command_end</literal> event occurs just after the
execution of
this same set of commands. To obtain more details on the
<acronym>DDL</acronym>

So it would need a refresh to list REINDEX.

case T_ReindexStmt:
-           lev = LOGSTMT_ALL;  /* should this be DDL? */
+           lev = LOGSTMT_DDL;

This, unfortunately, may not be as right as it is simple because
REINDEX is not a DDL as far as I know (it is not in the SQL
standard).
--
Michael

#7Garrett Thornburg
film42@gmail.com
In reply to: jian he (#5)
Re: PATCH: Add REINDEX tag to event triggers

Thank you for this, jian he! I will include it in the next patch version.

P.S. Sorry for the double post. Sent from the wrong email address so I'm
resending so the mailing list gets the email. My apologies!

On Wed, Jul 26, 2023 at 4:30 AM jian he <jian.universality@gmail.com> wrote:

Show quoted text

On Wed, Jul 26, 2023 at 7:51 AM Michael Paquier <michael@paquier.xyz>
wrote:

On Tue, Jul 25, 2023 at 04:34:47PM +0800, jian he wrote:

so T_ReindexStmt should only be in ProcessUtilitySlow, if you want
to create an event trigger on reindex?

regression tests work fine. I even play with partitions.

It would be an idea to have some regression tests for partitions,
actually, so as some patterns around ReindexMultipleInternal() are
checked. We could have a REINDEX DATABASE in a TAP test with an event
trigger, as well, but I don't feel strongly about the need to do that
much extra work in 090_reindexdb.pl or 091_reindexdb_all.pl if
partitions cover the multi-table case.
--
Michael

quite verbose, copied from partition-info.sql. meet the expectation:
partitioned index will do nothing, partition index will trigger event
trigger.
------------------------------------------------
DROP EVENT TRIGGER IF EXISTS end_reindex_command CASCADE;
DROP EVENT TRIGGER IF EXISTS start_reindex_command CASCADE;

BEGIN;
CREATE OR REPLACE FUNCTION reindex_end_command()
RETURNS event_trigger AS $$
DECLARE
obj record;
BEGIN
raise notice 'begin of reindex_end_command';

FOR obj IN SELECT * FROM pg_event_trigger_ddl_commands()
LOOP
RAISE NOTICE
'obj.command_tag:% obj.object_type:% obj.schema_name:%
obj.object_identity:%'
,obj.command_tag,
obj.object_type,obj.schema_name,obj.object_identity;
RAISE NOTICE 'ddl_end_command -- REINDEX: %',
pg_get_indexdef(obj.objid);
END LOOP;
END;
$$ LANGUAGE plpgsql;

CREATE OR REPLACE FUNCTION start_reindex_command()
RETURNS event_trigger AS $$
DECLARE
obj record;
BEGIN
FOR obj IN SELECT * FROM pg_event_trigger_ddl_commands()
LOOP
RAISE NOTICE
'obj.command_tag:% obj.object_type:% obj.schema_name:%
obj.object_identity:%'
, obj.command_tag,
obj.object_type,obj.schema_name,obj.object_identity;
RAISE NOTICE 'ddl_start_command -- REINDEX: %',
pg_get_indexdef(obj.objid);
END LOOP;
raise notice 'end of start_reindex_command';
END;
$$ LANGUAGE plpgsql;

BEGIN;
CREATE EVENT TRIGGER end_reindex_command ON ddl_command_end
WHEN TAG IN ('REINDEX') EXECUTE PROCEDURE reindex_end_command();
CREATE EVENT TRIGGER start_reindex_command ON ddl_command_start
WHEN TAG IN ('REINDEX') EXECUTE PROCEDURE start_reindex_command();
COMMIT;

-- test Reindex Event Trigger
BEGIN;
drop table if EXISTS ptif_test CASCADE;
CREATE TABLE ptif_test (a int, b int) PARTITION BY range (a);
CREATE TABLE ptif_test0 PARTITION OF ptif_test
FOR VALUES FROM (minvalue) TO (0) PARTITION BY list (b);
CREATE TABLE ptif_test01 PARTITION OF ptif_test0 FOR VALUES IN (1);
CREATE TABLE ptif_test1 PARTITION OF ptif_test
FOR VALUES FROM (0) TO (100) PARTITION BY list (b);
CREATE TABLE ptif_test11 PARTITION OF ptif_test1 FOR VALUES IN (1);

CREATE TABLE ptif_test2 PARTITION OF ptif_test
FOR VALUES FROM (100) TO (200);
-- This partitioned table should remain with no partitions.
CREATE TABLE ptif_test3 PARTITION OF ptif_test
FOR VALUES FROM (200) TO (maxvalue) PARTITION BY list (b);

-- Test index partition tree
CREATE INDEX ptif_test_index ON ONLY ptif_test (a);

CREATE INDEX ptif_test0_index ON ONLY ptif_test0 (a);
ALTER INDEX ptif_test_index ATTACH PARTITION ptif_test0_index;

CREATE INDEX ptif_test01_index ON ptif_test01 (a);
ALTER INDEX ptif_test0_index ATTACH PARTITION ptif_test01_index;

CREATE INDEX ptif_test1_index ON ONLY ptif_test1 (a);
ALTER INDEX ptif_test_index ATTACH PARTITION ptif_test1_index;

CREATE INDEX ptif_test11_index ON ptif_test11 (a);
ALTER INDEX ptif_test1_index ATTACH PARTITION ptif_test11_index;

CREATE INDEX ptif_test2_index ON ptif_test2 (a);
ALTER INDEX ptif_test_index ATTACH PARTITION ptif_test2_index;

CREATE INDEX ptif_test3_index ON ptif_test3 (a);
ALTER INDEX ptif_test_index ATTACH PARTITION ptif_test3_index;
COMMIT;

--top level partitioned index. will recurse to each partition index.
REINDEX INDEX CONCURRENTLY public.ptif_test_index;

--ptif_test0 is partitioned table. it will index partition:
ptif_test01_index
-- event trigger will log ptif_test01_index
REINDEX INDEX CONCURRENTLY public.ptif_test0_index;

--ptif_test1_index is partitioned index. it will index partition:
ptif_test11_index
-- event trigger will effect on partion index:ptif_test11_index
REINDEX INDEX CONCURRENTLY public.ptif_test1_index;

--ptif_test2 is a partition. event trigger will log ptif_test2_index
REINDEX INDEX CONCURRENTLY public.ptif_test2_index;

--no partitions. event trigger won't do anything.
REINDEX INDEX CONCURRENTLY public.ptif_test3_index;

reindex table ptif_test; --top level. will recurse to each partition
index.
reindex table ptif_test0; -- will direct to ptif_test01
reindex table ptif_test01; -- will index it's associtaed index
reindex table ptif_test11; -- will index it's associtaed index
reindex table ptif_test2; -- will index it's associtaed index
reindex table ptif_test3; -- no partion, index won't do anything.

DROP EVENT TRIGGER IF EXISTS end_reindex_command CASCADE;
DROP EVENT TRIGGER IF EXISTS start_reindex_command CASCADE;
DROP FUNCTION IF EXISTS reindex_start_command;
DROP FUNCTION IF EXISTS reindex_end_command;
DROP TABLE if EXISTS ptif_test CASCADE;
-----------------------

#8Garrett Thornburg
film42@gmail.com
In reply to: Garrett Thornburg (#1)
1 attachment(s)
Re: PATCH: Add REINDEX tag to event triggers

I added a v2 patch for adding REINDEX to event triggers. The following has
changed:

1. I fixed the docs to note that ddl_command_start is supported for
REINDEX. Thanks, Michael!
2. I added Jian He's excellent partition table test and updated the
expectations to include that regression test.

What is still up for debate:

Michael pointed out that REINDEX probably isn't DDL because the docs only
specify commands in the standard like CREATE, ALTER, etc. It might be worth
creating a new event to trigger on (similar to what was done for
table_rewrite) to make a REINDEX trigger fit in a little nicer. Let me
know what you think here. Until then, I left the code as `lev =
LOGSTMT_DDL` for `T_ReindexStmt`.

Thanks,
Garrett

On Thu, Jul 20, 2023 at 10:47 PM Garrett Thornburg <film42@gmail.com> wrote:

Show quoted text

Added my v1 patch to add REINDEX to event triggers.

I originally built this against pg15 but rebased to master for the patch
to hopefully make it easier for maintainers to merge. The rebase was
automatic so it should be easy to include into any recent version. I'd love
for this to land in pg16 if possible.

I added regression tests and they are passing. I also formatted the code
using the project tools. Docs are included too.

A few notes:
1. I tried to not touch the function parameters too much and opted to
create a convenience function that makes it easy to attach index or table
OIDs to the current event trigger list.
2. I made sure both concurrent and regular reindex of index, table,
database work as expected.
3. The ddl end command will make available all indexes that were modified
by the reindex command. This is a large list when you run "reindex
database" but works really well. I debated returning records of the table
or DB that were reindexed but that doesn't really make sense. Returning
each index fits my use case of building an audit record around the index
lifecycle (hence the motivation for the patch).

Thanks for your time,

Garrett Thornburg

Attachments:

v2-0001-Add-REINDEX-tag-to-event-triggers.patchapplication/octet-stream; name=v2-0001-Add-REINDEX-tag-to-event-triggers.patchDownload
From ff0488f43db0febc1bcefb5236bbd79db64a506c Mon Sep 17 00:00:00 2001
From: Garrett Thornburg <garrett.thornburg@hey.com>
Date: Thu, 20 Jul 2023 20:35:26 -0600
Subject: [PATCH v2] Add REINDEX tag to event triggers

This turns on the event tag REINDEX. This will now return a record for
each index that was modified during as part of start/end ddl commands.

For concurrent reindex, this will return the OID for the new index. This
includes concurrently reindexing tables and databases. It will only
report the new index records and not the ones destroyed by the
concurrent reindex process.
---
 doc/src/sgml/event-trigger.sgml             |   8 ++
 src/backend/commands/indexcmds.c            |  83 +++++++++++++
 src/backend/tcop/utility.c                  |  12 +-
 src/include/tcop/cmdtaglist.h               |   2 +-
 src/test/regress/expected/event_trigger.out | 126 ++++++++++++++++++++
 src/test/regress/sql/event_trigger.sql      |  99 +++++++++++++++
 6 files changed, 327 insertions(+), 3 deletions(-)

diff --git a/doc/src/sgml/event-trigger.sgml b/doc/src/sgml/event-trigger.sgml
index 3b6a5361b3..aa1b359f35 100644
--- a/doc/src/sgml/event-trigger.sgml
+++ b/doc/src/sgml/event-trigger.sgml
@@ -1013,6 +1013,14 @@
         <entry align="center"><literal>-</literal></entry>
         <entry align="left"></entry>
        </row>
+       <row>
+        <entry align="left"><literal>REINDEX</literal></entry>
+        <entry align="center"><literal>X</literal></entry>
+        <entry align="center"><literal>X</literal></entry>
+        <entry align="center"><literal>-</literal></entry>
+        <entry align="center"><literal>-</literal></entry>
+        <entry align="left"></entry>
+       </row>
        <row>
         <entry align="left"><literal>REVOKE</literal></entry>
         <entry align="center"><literal>X</literal></entry>
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index baf3e6e57a..2dc74225ed 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -94,6 +94,8 @@ static char *ChooseIndexNameAddition(List *colnames);
 static List *ChooseIndexColumnNames(List *indexElems);
 static void ReindexIndex(RangeVar *indexRelation, ReindexParams *params,
 						 bool isTopLevel);
+static void reindex_event_trigger_collect(Oid oid);
+static void reindex_event_trigger_collect_relation(Oid oid);
 static void RangeVarCallbackForReindexIndex(const RangeVar *relation,
 											Oid relId, Oid oldRelId, void *arg);
 static Oid	ReindexTable(RangeVar *relation, ReindexParams *params,
@@ -110,6 +112,8 @@ static bool ReindexRelationConcurrently(Oid relationOid,
 static void update_relispartition(Oid relationId, bool newval);
 static inline void set_indexsafe_procflags(void);
 
+static ReindexStmt *currentReindexStatement = NULL;
+
 /*
  * callback argument type for RangeVarCallbackForReindexIndex()
  */
@@ -2699,6 +2703,12 @@ ExecReindex(ParseState *pstate, ReindexStmt *stmt, bool isTopLevel)
 	bool		verbose = false;
 	char	   *tablespacename = NULL;
 
+	/*
+	 * Make current stmt available for event triggers without directly passing
+	 * the context to every subsequent call.
+	 */
+	currentReindexStatement = stmt;
+
 	/* Parse option list */
 	foreach(lc, stmt->params)
 	{
@@ -2779,6 +2789,12 @@ ExecReindex(ParseState *pstate, ReindexStmt *stmt, bool isTopLevel)
 				 (int) stmt->kind);
 			break;
 	}
+
+	/*
+	 * Clear the reindex stmt global reference now that triggers should have
+	 * completed
+	 */
+	currentReindexStatement = NULL;
 }
 
 /*
@@ -2830,6 +2846,8 @@ ReindexIndex(RangeVar *indexRelation, ReindexParams *params, bool isTopLevel)
 
 		newparams.options |= REINDEXOPT_REPORT_PROGRESS;
 		reindex_index(indOid, false, persistence, &newparams);
+		/* Add the index to event trigger */
+		reindex_event_trigger_collect(indOid);
 	}
 }
 
@@ -2953,6 +2971,9 @@ ReindexTable(RangeVar *relation, ReindexParams *params, bool isTopLevel)
 			ereport(NOTICE,
 					(errmsg("table \"%s\" has no indexes to reindex",
 							relation->relname)));
+
+		/* Create even for the indexes being modified */
+		reindex_event_trigger_collect_relation(heapOid);
 	}
 
 	return heapOid;
@@ -3369,6 +3390,8 @@ ReindexMultipleInternal(List *relids, ReindexParams *params)
 			newparams.options |=
 				REINDEXOPT_REPORT_PROGRESS | REINDEXOPT_MISSING_OK;
 			reindex_index(relid, false, relpersistence, &newparams);
+			/* Add the index to event trigger */
+			reindex_event_trigger_collect(relid);
 			PopActiveSnapshot();
 			/* reindex_index() does the verbose output */
 		}
@@ -3390,6 +3413,9 @@ ReindexMultipleInternal(List *relids, ReindexParams *params)
 								get_namespace_name(get_rel_namespace(relid)),
 								get_rel_name(relid))));
 
+			/* Create even for the indexes being modified */
+			reindex_event_trigger_collect_relation(relid);
+
 			PopActiveSnapshot();
 		}
 
@@ -3837,6 +3863,9 @@ ReindexRelationConcurrently(Oid relationOid, ReindexParams *params)
 
 		newIndexIds = lappend(newIndexIds, newidx);
 
+		/* Add the index to event trigger */
+		reindex_event_trigger_collect(newIndexId);
+
 		/*
 		 * Save lockrelid to protect each relation from drop then close
 		 * relations. The lockrelid on parent relation is not taken here to
@@ -4441,3 +4470,57 @@ set_indexsafe_procflags(void)
 	ProcGlobal->statusFlags[MyProc->pgxactoff] = MyProc->statusFlags;
 	LWLockRelease(ProcArrayLock);
 }
+
+static void
+reindex_event_trigger_collect(Oid oid)
+{
+	ObjectAddress address;
+
+	if (currentReindexStatement == NULL)
+		return;
+
+	address.classId = RelationRelationId;
+	address.objectId = oid;
+	address.objectSubId = 0;
+
+	EventTriggerCollectSimpleCommand(address,
+									 InvalidObjectAddress, (Node *) currentReindexStatement);
+}
+
+static void
+reindex_event_trigger_collect_relation(Oid relid)
+{
+	Relation	rel;
+	List	   *indexIds = NULL;
+	ListCell   *indexId = NULL;
+
+	/*
+	 * Open and lock the relation.  ShareLock is sufficient since we only need
+	 * to prevent schema and data changes in it.  The lock level used here
+	 * should match catalog's reindex_relation().
+	 */
+	rel = try_table_open(relid, ShareLock);
+
+	/* if relation is gone, leave */
+	if (!rel)
+		return;
+
+	/*
+	 * Get the list of index OIDs for this relation.  (We trust to the
+	 * relcache to get this with a sequential scan if ignoring system
+	 * indexes.)
+	 */
+	indexIds = RelationGetIndexList(rel);
+
+	/*
+	 * Get the list of index OIDs for this relation.
+	 */
+	foreach(indexId, indexIds)
+	{
+		Oid			indexOid = lfirst_oid(indexId);
+
+		reindex_event_trigger_collect(indexOid);
+	}
+
+	table_close(rel, ShareLock);
+}
diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c
index e3ccf6c7f7..a269b5b6dc 100644
--- a/src/backend/tcop/utility.c
+++ b/src/backend/tcop/utility.c
@@ -961,7 +961,9 @@ standard_ProcessUtility(PlannedStmt *pstmt,
 			break;
 
 		case T_ReindexStmt:
-			ExecReindex(pstate, (ReindexStmt *) parsetree, isTopLevel);
+			ProcessUtilitySlow(pstate, pstmt, queryString,
+							   context, params, queryEnv,
+							   dest, qc);
 			break;
 
 			/*
@@ -1574,6 +1576,12 @@ ProcessUtilitySlow(ParseState *pstate,
 				}
 				break;
 
+			case T_ReindexStmt:
+				ExecReindex(pstate, (ReindexStmt *) parsetree, isTopLevel);
+				/* no commands stashed for REINDEX */
+				commandCollected = true;
+				break;
+
 			case T_CreateExtensionStmt:
 				address = CreateExtension(pstate, (CreateExtensionStmt *) parsetree);
 				break;
@@ -3620,7 +3628,7 @@ GetCommandLogLevel(Node *parsetree)
 			break;
 
 		case T_ReindexStmt:
-			lev = LOGSTMT_ALL;	/* should this be DDL? */
+			lev = LOGSTMT_DDL;
 			break;
 
 		case T_CreateConversionStmt:
diff --git a/src/include/tcop/cmdtaglist.h b/src/include/tcop/cmdtaglist.h
index e738ac1c09..d0e0b18609 100644
--- a/src/include/tcop/cmdtaglist.h
+++ b/src/include/tcop/cmdtaglist.h
@@ -193,7 +193,7 @@ PG_CMDTAG(CMDTAG_PREPARE, "PREPARE", false, false, false)
 PG_CMDTAG(CMDTAG_PREPARE_TRANSACTION, "PREPARE TRANSACTION", false, false, false)
 PG_CMDTAG(CMDTAG_REASSIGN_OWNED, "REASSIGN OWNED", false, false, false)
 PG_CMDTAG(CMDTAG_REFRESH_MATERIALIZED_VIEW, "REFRESH MATERIALIZED VIEW", true, false, false)
-PG_CMDTAG(CMDTAG_REINDEX, "REINDEX", false, false, false)
+PG_CMDTAG(CMDTAG_REINDEX, "REINDEX", true, false, false)
 PG_CMDTAG(CMDTAG_RELEASE, "RELEASE", false, false, false)
 PG_CMDTAG(CMDTAG_RESET, "RESET", false, false, false)
 PG_CMDTAG(CMDTAG_REVOKE, "REVOKE", true, false, false)
diff --git a/src/test/regress/expected/event_trigger.out b/src/test/regress/expected/event_trigger.out
index 5a10958df5..f9bad789c6 100644
--- a/src/test/regress/expected/event_trigger.out
+++ b/src/test/regress/expected/event_trigger.out
@@ -554,6 +554,132 @@ ERROR:  cannot alter type "rewritetype" because column "rewritemetoo3.a" uses it
 drop table rewriteme;
 drop event trigger no_rewrite_allowed;
 drop function test_evtrig_no_rewrite();
+-- test Reindex Event Trigger
+CREATE OR REPLACE FUNCTION reindex_start_command()
+RETURNS event_trigger AS $$
+BEGIN
+  RAISE NOTICE 'ddl_start_command -- REINDEX: % %', tg_event, tg_tag;
+END;
+$$ LANGUAGE plpgsql;
+CREATE EVENT TRIGGER start_reindex_command ON ddl_command_start
+    WHEN TAG IN ('REINDEX') EXECUTE PROCEDURE reindex_start_command();
+CREATE OR REPLACE FUNCTION reindex_end_command()
+RETURNS event_trigger AS $$
+DECLARE
+  obj record;
+BEGIN
+  FOR obj IN SELECT * FROM pg_event_trigger_ddl_commands()
+    LOOP
+      RAISE NOTICE 'ddl_end_command -- REINDEX: %', pg_get_indexdef(obj.objid);
+    END LOOP;
+END;
+$$ LANGUAGE plpgsql;
+CREATE EVENT TRIGGER end_reindex_command ON ddl_command_end
+    WHEN TAG IN ('REINDEX') EXECUTE PROCEDURE reindex_end_command();
+-- test REINDEX event for a standard table
+DROP SCHEMA IF EXISTS reindex_test CASCADE;
+NOTICE:  schema "reindex_test" does not exist, skipping
+CREATE SCHEMA reindex_test;
+CREATE TABLE reindex_test.reindex_tester1 (a int);
+CREATE INDEX reindex_test1_idx1 ON reindex_test.reindex_tester1 (a);
+CREATE INDEX reindex_test1_idx2 ON reindex_test.reindex_tester1 (a);
+CREATE TABLE reindex_test.reindex_tester2 (a int);
+CREATE INDEX reindex_test2_idx1 ON reindex_test.reindex_tester2 (a);
+REINDEX INDEX reindex_test.reindex_test1_idx1;
+NOTICE:  ddl_start_command -- REINDEX: ddl_command_start REINDEX
+NOTICE:  ddl_end_command -- REINDEX: CREATE INDEX reindex_test1_idx1 ON reindex_test.reindex_tester1 USING btree (a)
+REINDEX TABLE reindex_test.reindex_tester1;
+NOTICE:  ddl_start_command -- REINDEX: ddl_command_start REINDEX
+NOTICE:  ddl_end_command -- REINDEX: CREATE INDEX reindex_test1_idx1 ON reindex_test.reindex_tester1 USING btree (a)
+NOTICE:  ddl_end_command -- REINDEX: CREATE INDEX reindex_test1_idx2 ON reindex_test.reindex_tester1 USING btree (a)
+REINDEX INDEX CONCURRENTLY reindex_test.reindex_test2_idx1;
+NOTICE:  ddl_start_command -- REINDEX: ddl_command_start REINDEX
+NOTICE:  ddl_end_command -- REINDEX: CREATE INDEX reindex_test2_idx1 ON reindex_test.reindex_tester2 USING btree (a)
+REINDEX TABLE CONCURRENTLY reindex_test.reindex_tester2;
+NOTICE:  ddl_start_command -- REINDEX: ddl_command_start REINDEX
+NOTICE:  ddl_end_command -- REINDEX: CREATE INDEX reindex_test2_idx1 ON reindex_test.reindex_tester2 USING btree (a)
+-- test REINDEX event for a partitioned table
+CREATE TABLE reindex_test.ptif_test (a int, b int) PARTITION BY range (a);
+CREATE TABLE reindex_test.ptif_test0 PARTITION OF reindex_test.ptif_test
+  FOR VALUES FROM (minvalue) TO (0) PARTITION BY list (b);
+CREATE TABLE reindex_test.ptif_test01 PARTITION OF reindex_test.ptif_test0 FOR VALUES IN (1);
+CREATE TABLE reindex_test.ptif_test1 PARTITION OF reindex_test.ptif_test
+  FOR VALUES FROM (0) TO (100) PARTITION BY list (b);
+CREATE TABLE reindex_test.ptif_test11 PARTITION OF reindex_test.ptif_test1 FOR VALUES IN (1);
+CREATE TABLE reindex_test.ptif_test2 PARTITION OF reindex_test.ptif_test
+  FOR VALUES FROM (100) TO (200);
+-- This partitioned table should remain with no partitions.
+CREATE TABLE reindex_test.ptif_test3 PARTITION OF reindex_test.ptif_test
+  FOR VALUES FROM (200) TO (maxvalue) PARTITION BY list (b);
+CREATE INDEX ptif_test_index ON ONLY reindex_test.ptif_test (a);
+CREATE INDEX ptif_test0_index ON ONLY reindex_test.ptif_test0 (a);
+ALTER INDEX reindex_test.ptif_test_index ATTACH PARTITION reindex_test.ptif_test0_index;
+CREATE INDEX ptif_test01_index ON reindex_test.ptif_test01 (a);
+ALTER INDEX reindex_test.ptif_test0_index ATTACH PARTITION reindex_test.ptif_test01_index;
+CREATE INDEX ptif_test1_index ON ONLY reindex_test.ptif_test1 (a);
+ALTER INDEX reindex_test.ptif_test_index ATTACH PARTITION reindex_test.ptif_test1_index;
+CREATE INDEX ptif_test11_index ON reindex_test.ptif_test11 (a);
+ALTER INDEX reindex_test.ptif_test1_index ATTACH PARTITION reindex_test.ptif_test11_index;
+CREATE INDEX ptif_test2_index ON reindex_test.ptif_test2 (a);
+ALTER INDEX reindex_test.ptif_test_index ATTACH PARTITION reindex_test.ptif_test2_index;
+CREATE INDEX ptif_test3_index ON reindex_test.ptif_test3 (a);
+ALTER INDEX reindex_test.ptif_test_index ATTACH PARTITION reindex_test.ptif_test3_index;
+-- reindex at the top level table to recursively reindex each partition
+REINDEX INDEX CONCURRENTLY reindex_test.ptif_test_index;
+NOTICE:  ddl_start_command -- REINDEX: ddl_command_start REINDEX
+NOTICE:  ddl_end_command -- REINDEX: CREATE INDEX ptif_test2_index ON reindex_test.ptif_test2 USING btree (a)
+NOTICE:  ddl_end_command -- REINDEX: CREATE INDEX ptif_test01_index ON reindex_test.ptif_test01 USING btree (a)
+NOTICE:  ddl_end_command -- REINDEX: CREATE INDEX ptif_test11_index ON reindex_test.ptif_test11 USING btree (a)
+-- ptif_test0 is partitioned table so it will index partition: ptif_test01_index
+-- event trigger will log ptif_test01_index
+REINDEX INDEX CONCURRENTLY reindex_test.ptif_test0_index;
+NOTICE:  ddl_start_command -- REINDEX: ddl_command_start REINDEX
+NOTICE:  ddl_end_command -- REINDEX: CREATE INDEX ptif_test01_index ON reindex_test.ptif_test01 USING btree (a)
+-- ptif_test1_index is partitioned index so it will index partition: ptif_test11_index
+-- event trigger will effect on partion index:ptif_test11_index
+REINDEX INDEX CONCURRENTLY reindex_test.ptif_test1_index;
+NOTICE:  ddl_start_command -- REINDEX: ddl_command_start REINDEX
+NOTICE:  ddl_end_command -- REINDEX: CREATE INDEX ptif_test11_index ON reindex_test.ptif_test11 USING btree (a)
+-- ptif_test2 is a partition so event trigger will log ptif_test2_index
+REINDEX INDEX CONCURRENTLY reindex_test.ptif_test2_index;
+NOTICE:  ddl_start_command -- REINDEX: ddl_command_start REINDEX
+NOTICE:  ddl_end_command -- REINDEX: CREATE INDEX ptif_test2_index ON reindex_test.ptif_test2 USING btree (a)
+-- no partitions: event trigger won't do anything
+REINDEX INDEX CONCURRENTLY reindex_test.ptif_test3_index;
+NOTICE:  ddl_start_command -- REINDEX: ddl_command_start REINDEX
+-- reindex at the top level table to recursively reindex each partition
+REINDEX TABLE reindex_test.ptif_test;
+NOTICE:  ddl_start_command -- REINDEX: ddl_command_start REINDEX
+NOTICE:  ddl_end_command -- REINDEX: CREATE INDEX ptif_test2_index ON reindex_test.ptif_test2 USING btree (a)
+NOTICE:  ddl_end_command -- REINDEX: CREATE INDEX ptif_test01_index ON reindex_test.ptif_test01 USING btree (a)
+NOTICE:  ddl_end_command -- REINDEX: CREATE INDEX ptif_test11_index ON reindex_test.ptif_test11 USING btree (a)
+-- will direct to ptif_test01
+REINDEX TABLE reindex_test.ptif_test0;
+NOTICE:  ddl_start_command -- REINDEX: ddl_command_start REINDEX
+NOTICE:  ddl_end_command -- REINDEX: CREATE INDEX ptif_test01_index ON reindex_test.ptif_test01 USING btree (a)
+-- will index its associtaed index
+REINDEX TABLE reindex_test.ptif_test01;
+NOTICE:  ddl_start_command -- REINDEX: ddl_command_start REINDEX
+NOTICE:  ddl_end_command -- REINDEX: CREATE INDEX ptif_test01_index ON reindex_test.ptif_test01 USING btree (a)
+-- will index its associtaed index
+REINDEX TABLE reindex_test.ptif_test11;
+NOTICE:  ddl_start_command -- REINDEX: ddl_command_start REINDEX
+NOTICE:  ddl_end_command -- REINDEX: CREATE INDEX ptif_test11_index ON reindex_test.ptif_test11 USING btree (a)
+-- will index its associtaed index
+REINDEX TABLE reindex_test.ptif_test2;
+NOTICE:  ddl_start_command -- REINDEX: ddl_command_start REINDEX
+NOTICE:  ddl_end_command -- REINDEX: CREATE INDEX ptif_test2_index ON reindex_test.ptif_test2 USING btree (a)
+-- no partion, index won't do anything
+REINDEX TABLE reindex_test.ptif_test3;
+NOTICE:  ddl_start_command -- REINDEX: ddl_command_start REINDEX
+DROP EVENT TRIGGER start_reindex_command;
+DROP EVENT TRIGGER end_reindex_command;
+DROP SCHEMA reindex_test CASCADE;
+NOTICE:  drop cascades to 3 other objects
+DETAIL:  drop cascades to table reindex_test.reindex_tester1
+drop cascades to table reindex_test.reindex_tester2
+drop cascades to table reindex_test.ptif_test
+DROP FUNCTION reindex_end_command;
 -- test Row Security Event Trigger
 RESET SESSION AUTHORIZATION;
 CREATE TABLE event_trigger_test (a integer, b text);
diff --git a/src/test/regress/sql/event_trigger.sql b/src/test/regress/sql/event_trigger.sql
index 1aeaddbe71..d5d51bd9db 100644
--- a/src/test/regress/sql/event_trigger.sql
+++ b/src/test/regress/sql/event_trigger.sql
@@ -418,6 +418,105 @@ drop table rewriteme;
 drop event trigger no_rewrite_allowed;
 drop function test_evtrig_no_rewrite();
 
+-- test Reindex Event Trigger
+CREATE OR REPLACE FUNCTION reindex_start_command()
+RETURNS event_trigger AS $$
+BEGIN
+  RAISE NOTICE 'ddl_start_command -- REINDEX: % %', tg_event, tg_tag;
+END;
+$$ LANGUAGE plpgsql;
+
+CREATE EVENT TRIGGER start_reindex_command ON ddl_command_start
+    WHEN TAG IN ('REINDEX') EXECUTE PROCEDURE reindex_start_command();
+
+CREATE OR REPLACE FUNCTION reindex_end_command()
+RETURNS event_trigger AS $$
+DECLARE
+  obj record;
+BEGIN
+  FOR obj IN SELECT * FROM pg_event_trigger_ddl_commands()
+    LOOP
+      RAISE NOTICE 'ddl_end_command -- REINDEX: %', pg_get_indexdef(obj.objid);
+    END LOOP;
+END;
+$$ LANGUAGE plpgsql;
+
+CREATE EVENT TRIGGER end_reindex_command ON ddl_command_end
+    WHEN TAG IN ('REINDEX') EXECUTE PROCEDURE reindex_end_command();
+
+-- test REINDEX event for a standard table
+DROP SCHEMA IF EXISTS reindex_test CASCADE;
+CREATE SCHEMA reindex_test;
+CREATE TABLE reindex_test.reindex_tester1 (a int);
+CREATE INDEX reindex_test1_idx1 ON reindex_test.reindex_tester1 (a);
+CREATE INDEX reindex_test1_idx2 ON reindex_test.reindex_tester1 (a);
+CREATE TABLE reindex_test.reindex_tester2 (a int);
+CREATE INDEX reindex_test2_idx1 ON reindex_test.reindex_tester2 (a);
+
+REINDEX INDEX reindex_test.reindex_test1_idx1;
+REINDEX TABLE reindex_test.reindex_tester1;
+REINDEX INDEX CONCURRENTLY reindex_test.reindex_test2_idx1;
+REINDEX TABLE CONCURRENTLY reindex_test.reindex_tester2;
+
+-- test REINDEX event for a partitioned table
+CREATE TABLE reindex_test.ptif_test (a int, b int) PARTITION BY range (a);
+CREATE TABLE reindex_test.ptif_test0 PARTITION OF reindex_test.ptif_test
+  FOR VALUES FROM (minvalue) TO (0) PARTITION BY list (b);
+CREATE TABLE reindex_test.ptif_test01 PARTITION OF reindex_test.ptif_test0 FOR VALUES IN (1);
+CREATE TABLE reindex_test.ptif_test1 PARTITION OF reindex_test.ptif_test
+  FOR VALUES FROM (0) TO (100) PARTITION BY list (b);
+CREATE TABLE reindex_test.ptif_test11 PARTITION OF reindex_test.ptif_test1 FOR VALUES IN (1);
+CREATE TABLE reindex_test.ptif_test2 PARTITION OF reindex_test.ptif_test
+  FOR VALUES FROM (100) TO (200);
+-- This partitioned table should remain with no partitions.
+CREATE TABLE reindex_test.ptif_test3 PARTITION OF reindex_test.ptif_test
+  FOR VALUES FROM (200) TO (maxvalue) PARTITION BY list (b);
+
+CREATE INDEX ptif_test_index ON ONLY reindex_test.ptif_test (a);
+CREATE INDEX ptif_test0_index ON ONLY reindex_test.ptif_test0 (a);
+ALTER INDEX reindex_test.ptif_test_index ATTACH PARTITION reindex_test.ptif_test0_index;
+CREATE INDEX ptif_test01_index ON reindex_test.ptif_test01 (a);
+ALTER INDEX reindex_test.ptif_test0_index ATTACH PARTITION reindex_test.ptif_test01_index;
+CREATE INDEX ptif_test1_index ON ONLY reindex_test.ptif_test1 (a);
+ALTER INDEX reindex_test.ptif_test_index ATTACH PARTITION reindex_test.ptif_test1_index;
+CREATE INDEX ptif_test11_index ON reindex_test.ptif_test11 (a);
+ALTER INDEX reindex_test.ptif_test1_index ATTACH PARTITION reindex_test.ptif_test11_index;
+CREATE INDEX ptif_test2_index ON reindex_test.ptif_test2 (a);
+ALTER INDEX reindex_test.ptif_test_index ATTACH PARTITION reindex_test.ptif_test2_index;
+CREATE INDEX ptif_test3_index ON reindex_test.ptif_test3 (a);
+ALTER INDEX reindex_test.ptif_test_index ATTACH PARTITION reindex_test.ptif_test3_index;
+
+-- reindex at the top level table to recursively reindex each partition
+REINDEX INDEX CONCURRENTLY reindex_test.ptif_test_index;
+-- ptif_test0 is partitioned table so it will index partition: ptif_test01_index
+-- event trigger will log ptif_test01_index
+REINDEX INDEX CONCURRENTLY reindex_test.ptif_test0_index;
+-- ptif_test1_index is partitioned index so it will index partition: ptif_test11_index
+-- event trigger will effect on partion index:ptif_test11_index
+REINDEX INDEX CONCURRENTLY reindex_test.ptif_test1_index;
+-- ptif_test2 is a partition so event trigger will log ptif_test2_index
+REINDEX INDEX CONCURRENTLY reindex_test.ptif_test2_index;
+-- no partitions: event trigger won't do anything
+REINDEX INDEX CONCURRENTLY reindex_test.ptif_test3_index;
+
+-- reindex at the top level table to recursively reindex each partition
+REINDEX TABLE reindex_test.ptif_test;
+-- will direct to ptif_test01
+REINDEX TABLE reindex_test.ptif_test0;
+-- will index its associtaed index
+REINDEX TABLE reindex_test.ptif_test01;
+-- will index its associtaed index
+REINDEX TABLE reindex_test.ptif_test11;
+-- will index its associtaed index
+REINDEX TABLE reindex_test.ptif_test2;
+-- no partion, index won't do anything
+REINDEX TABLE reindex_test.ptif_test3;
+
+DROP EVENT TRIGGER start_reindex_command;
+DROP EVENT TRIGGER end_reindex_command;
+DROP SCHEMA reindex_test CASCADE;
+DROP FUNCTION reindex_end_command;
+
 -- test Row Security Event Trigger
 RESET SESSION AUTHORIZATION;
 CREATE TABLE event_trigger_test (a integer, b text);
-- 
2.38.1

#9Jim Jones
jim.jones@uni-muenster.de
In reply to: Garrett Thornburg (#8)
Re: PATCH: Add REINDEX tag to event triggers

Greetings

On 27.07.23 06:43, Garrett Thornburg wrote:

I added a v2 patch for adding REINDEX to event triggers. The following
has changed:

1. I fixed the docs to note that ddl_command_start is supported for
REINDEX. Thanks, Michael!
2. I added Jian He's excellent partition table test and updated the
expectations to include that regression test.

What is still up for debate:

Michael pointed out that REINDEX probably isn't DDL because the docs
only specify commands in the standard like CREATE, ALTER, etc. It
might be worth creating a new event to trigger on (similar to what was
done for table_rewrite) to make a REINDEX trigger fit in a little
nicer. Let me know what you think here. Until then, I left the code as
`lev = LOGSTMT_DDL` for `T_ReindexStmt`.

Thanks,
Garrett

Thanks for the patch, Garrett!

I was unable to apply it in 7ef5f5f
<https://github.com/postgres/postgres/commit/7ef5f5fb324096c7822c922ad59fd7fdd76f57b1&gt;

$ git apply -v v2-0001-Add-REINDEX-tag-to-event-triggers.patch
Checking patch doc/src/sgml/event-trigger.sgml...
Checking patch src/backend/commands/indexcmds.c...
error: while searching for:
static List *ChooseIndexColumnNames(List *indexElems);
static void ReindexIndex(RangeVar *indexRelation, ReindexParams *params,
                                                 bool isTopLevel);
static void RangeVarCallbackForReindexIndex(const RangeVar *relation,
Oid relId, Oid oldRelId, void *arg);
static Oid      ReindexTable(RangeVar *relation, ReindexParams *params,

error: patch failed: src/backend/commands/indexcmds.c:94
error: src/backend/commands/indexcmds.c: patch does not apply
Checking patch src/backend/tcop/utility.c...
Checking patch src/include/tcop/cmdtaglist.h...
Checking patch src/test/regress/expected/event_trigger.out...
Hunk #1 succeeded at 556 (offset 2 lines).
Checking patch src/test/regress/sql/event_trigger.sql...

Am I missing something?

Jim

#10jian he
jian.universality@gmail.com
In reply to: Jim Jones (#9)
1 attachment(s)
Re: PATCH: Add REINDEX tag to event triggers

On Wed, Aug 30, 2023 at 8:38 PM Jim Jones <jim.jones@uni-muenster.de> wrote:

Greetings

I was unable to apply it in 7ef5f5f

$ git apply -v v2-0001-Add-REINDEX-tag-to-event-triggers.patch
Checking patch doc/src/sgml/event-trigger.sgml...
Checking patch src/backend/commands/indexcmds.c...
error: while searching for:
static List *ChooseIndexColumnNames(List *indexElems);
static void ReindexIndex(RangeVar *indexRelation, ReindexParams *params,
bool isTopLevel);
static void RangeVarCallbackForReindexIndex(const RangeVar *relation,
Oid relId, Oid oldRelId, void *arg);
static Oid ReindexTable(RangeVar *relation, ReindexParams *params,

error: patch failed: src/backend/commands/indexcmds.c:94
error: src/backend/commands/indexcmds.c: patch does not apply
Checking patch src/backend/tcop/utility.c...
Checking patch src/include/tcop/cmdtaglist.h...
Checking patch src/test/regress/expected/event_trigger.out...
Hunk #1 succeeded at 556 (offset 2 lines).
Checking patch src/test/regress/sql/event_trigger.sql...

Am I missing something?

Jim

because the change made in here:
https://git.postgresql.org/cgit/postgresql.git/diff/src/backend/commands/indexcmds.c?id=11af63fb48d278b86aa948a5b57f136ef03c2bb7

I manually slight edited the patch content:
from
static List *ChooseIndexColumnNames(List *indexElems);
static void ReindexIndex(RangeVar *indexRelation, ReindexParams *params,
bool isTopLevel);
to
static List *ChooseIndexColumnNames(const List *indexElems);
static void ReindexIndex(const RangeVar *indexRelation, const
ReindexParams *params,
bool isTopLevel);

can you try the attached one.

Attachments:

v2-0002-Add-REINDEX-tag-to-event-triggers.patchtext/x-patch; charset=US-ASCII; name=v2-0002-Add-REINDEX-tag-to-event-triggers.patchDownload
diff --git a/doc/src/sgml/event-trigger.sgml b/doc/src/sgml/event-trigger.sgml
index 3b6a5361..aa1b359f 100644
--- a/doc/src/sgml/event-trigger.sgml
+++ b/doc/src/sgml/event-trigger.sgml
@@ -1013,6 +1013,14 @@
         <entry align="center"><literal>-</literal></entry>
         <entry align="left"></entry>
        </row>
+       <row>
+        <entry align="left"><literal>REINDEX</literal></entry>
+        <entry align="center"><literal>X</literal></entry>
+        <entry align="center"><literal>X</literal></entry>
+        <entry align="center"><literal>-</literal></entry>
+        <entry align="center"><literal>-</literal></entry>
+        <entry align="left"></entry>
+       </row>
        <row>
         <entry align="left"><literal>REVOKE</literal></entry>
         <entry align="center"><literal>X</literal></entry>
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index ab8b81b3..b78f1d87 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -95,6 +95,8 @@ static char *ChooseIndexNameAddition(const List *colnames);
 static List *ChooseIndexColumnNames(const List *indexElems);
 static void ReindexIndex(const RangeVar *indexRelation, const ReindexParams *params,
 						 bool isTopLevel);
+static void reindex_event_trigger_collect(Oid oid);
+static void reindex_event_trigger_collect_relation(Oid oid);
 static void RangeVarCallbackForReindexIndex(const RangeVar *relation,
 											Oid relId, Oid oldRelId, void *arg);
 static Oid	ReindexTable(const RangeVar *relation, const ReindexParams *params,
@@ -111,6 +113,8 @@ static bool ReindexRelationConcurrently(Oid relationOid,
 static void update_relispartition(Oid relationId, bool newval);
 static inline void set_indexsafe_procflags(void);
 
+static ReindexStmt *currentReindexStatement = NULL;
+
 /*
  * callback argument type for RangeVarCallbackForReindexIndex()
  */
@@ -2696,6 +2700,12 @@ ExecReindex(ParseState *pstate, const ReindexStmt *stmt, bool isTopLevel)
 	bool		verbose = false;
 	char	   *tablespacename = NULL;
 
+	/*
+	 * Make current stmt available for event triggers without directly passing
+	 * the context to every subsequent call.
+	 */
+	currentReindexStatement = stmt;
+
 	/* Parse option list */
 	foreach(lc, stmt->params)
 	{
@@ -2776,6 +2786,12 @@ ExecReindex(ParseState *pstate, const ReindexStmt *stmt, bool isTopLevel)
 				 (int) stmt->kind);
 			break;
 	}
+
+	/*
+	 * Clear the reindex stmt global reference now that triggers should have
+	 * completed
+	 */
+	currentReindexStatement = NULL;
 }
 
 /*
@@ -2827,6 +2843,8 @@ ReindexIndex(const RangeVar *indexRelation, const ReindexParams *params, bool is
 
 		newparams.options |= REINDEXOPT_REPORT_PROGRESS;
 		reindex_index(indOid, false, persistence, &newparams);
+		/* Add the index to event trigger */
+		reindex_event_trigger_collect(indOid);
 	}
 }
 
@@ -2950,6 +2968,9 @@ ReindexTable(const RangeVar *relation, const ReindexParams *params, bool isTopLe
 			ereport(NOTICE,
 					(errmsg("table \"%s\" has no indexes to reindex",
 							relation->relname)));
+
+		/* Create even for the indexes being modified */
+		reindex_event_trigger_collect_relation(heapOid);
 	}
 
 	return heapOid;
@@ -3366,6 +3387,8 @@ ReindexMultipleInternal(const List *relids, const ReindexParams *params)
 			newparams.options |=
 				REINDEXOPT_REPORT_PROGRESS | REINDEXOPT_MISSING_OK;
 			reindex_index(relid, false, relpersistence, &newparams);
+			/* Add the index to event trigger */
+			reindex_event_trigger_collect(relid);
 			PopActiveSnapshot();
 			/* reindex_index() does the verbose output */
 		}
@@ -3387,6 +3410,9 @@ ReindexMultipleInternal(const List *relids, const ReindexParams *params)
 								get_namespace_name(get_rel_namespace(relid)),
 								get_rel_name(relid))));
 
+			/* Create even for the indexes being modified */
+			reindex_event_trigger_collect_relation(relid);
+
 			PopActiveSnapshot();
 		}
 
@@ -3833,6 +3859,9 @@ ReindexRelationConcurrently(Oid relationOid, const ReindexParams *params)
 
 		newIndexIds = lappend(newIndexIds, newidx);
 
+		/* Add the index to event trigger */
+		reindex_event_trigger_collect(newIndexId);
+
 		/*
 		 * Save lockrelid to protect each relation from drop then close
 		 * relations. The lockrelid on parent relation is not taken here to
@@ -4436,3 +4465,57 @@ set_indexsafe_procflags(void)
 	ProcGlobal->statusFlags[MyProc->pgxactoff] = MyProc->statusFlags;
 	LWLockRelease(ProcArrayLock);
 }
+
+static void
+reindex_event_trigger_collect(Oid oid)
+{
+	ObjectAddress address;
+
+	if (currentReindexStatement == NULL)
+		return;
+
+	address.classId = RelationRelationId;
+	address.objectId = oid;
+	address.objectSubId = 0;
+
+	EventTriggerCollectSimpleCommand(address,
+									 InvalidObjectAddress, (Node *) currentReindexStatement);
+}
+
+static void
+reindex_event_trigger_collect_relation(Oid relid)
+{
+	Relation	rel;
+	List	   *indexIds = NULL;
+	ListCell   *indexId = NULL;
+
+	/*
+	 * Open and lock the relation.  ShareLock is sufficient since we only need
+	 * to prevent schema and data changes in it.  The lock level used here
+	 * should match catalog's reindex_relation().
+	 */
+	rel = try_table_open(relid, ShareLock);
+
+	/* if relation is gone, leave */
+	if (!rel)
+		return;
+
+	/*
+	 * Get the list of index OIDs for this relation.  (We trust to the
+	 * relcache to get this with a sequential scan if ignoring system
+	 * indexes.)
+	 */
+	indexIds = RelationGetIndexList(rel);
+
+	/*
+	 * Get the list of index OIDs for this relation.
+	 */
+	foreach(indexId, indexIds)
+	{
+		Oid			indexOid = lfirst_oid(indexId);
+
+		reindex_event_trigger_collect(indexOid);
+	}
+
+	table_close(rel, ShareLock);
+}
diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c
index e3ccf6c7..a269b5b6 100644
--- a/src/backend/tcop/utility.c
+++ b/src/backend/tcop/utility.c
@@ -961,7 +961,9 @@ standard_ProcessUtility(PlannedStmt *pstmt,
 			break;
 
 		case T_ReindexStmt:
-			ExecReindex(pstate, (ReindexStmt *) parsetree, isTopLevel);
+			ProcessUtilitySlow(pstate, pstmt, queryString,
+							   context, params, queryEnv,
+							   dest, qc);
 			break;
 
 			/*
@@ -1574,6 +1576,12 @@ ProcessUtilitySlow(ParseState *pstate,
 				}
 				break;
 
+			case T_ReindexStmt:
+				ExecReindex(pstate, (ReindexStmt *) parsetree, isTopLevel);
+				/* no commands stashed for REINDEX */
+				commandCollected = true;
+				break;
+
 			case T_CreateExtensionStmt:
 				address = CreateExtension(pstate, (CreateExtensionStmt *) parsetree);
 				break;
@@ -3620,7 +3628,7 @@ GetCommandLogLevel(Node *parsetree)
 			break;
 
 		case T_ReindexStmt:
-			lev = LOGSTMT_ALL;	/* should this be DDL? */
+			lev = LOGSTMT_DDL;
 			break;
 
 		case T_CreateConversionStmt:
diff --git a/src/include/tcop/cmdtaglist.h b/src/include/tcop/cmdtaglist.h
index e738ac1c..d0e0b186 100644
--- a/src/include/tcop/cmdtaglist.h
+++ b/src/include/tcop/cmdtaglist.h
@@ -193,7 +193,7 @@ PG_CMDTAG(CMDTAG_PREPARE, "PREPARE", false, false, false)
 PG_CMDTAG(CMDTAG_PREPARE_TRANSACTION, "PREPARE TRANSACTION", false, false, false)
 PG_CMDTAG(CMDTAG_REASSIGN_OWNED, "REASSIGN OWNED", false, false, false)
 PG_CMDTAG(CMDTAG_REFRESH_MATERIALIZED_VIEW, "REFRESH MATERIALIZED VIEW", true, false, false)
-PG_CMDTAG(CMDTAG_REINDEX, "REINDEX", false, false, false)
+PG_CMDTAG(CMDTAG_REINDEX, "REINDEX", true, false, false)
 PG_CMDTAG(CMDTAG_RELEASE, "RELEASE", false, false, false)
 PG_CMDTAG(CMDTAG_RESET, "RESET", false, false, false)
 PG_CMDTAG(CMDTAG_REVOKE, "REVOKE", true, false, false)
diff --git a/src/test/regress/expected/event_trigger.out b/src/test/regress/expected/event_trigger.out
index 2c8a6b22..874fba2a 100644
--- a/src/test/regress/expected/event_trigger.out
+++ b/src/test/regress/expected/event_trigger.out
@@ -556,6 +556,132 @@ ERROR:  cannot alter type "rewritetype" because column "rewritemetoo3.a" uses it
 drop table rewriteme;
 drop event trigger no_rewrite_allowed;
 drop function test_evtrig_no_rewrite();
+-- test Reindex Event Trigger
+CREATE OR REPLACE FUNCTION reindex_start_command()
+RETURNS event_trigger AS $$
+BEGIN
+  RAISE NOTICE 'ddl_start_command -- REINDEX: % %', tg_event, tg_tag;
+END;
+$$ LANGUAGE plpgsql;
+CREATE EVENT TRIGGER start_reindex_command ON ddl_command_start
+    WHEN TAG IN ('REINDEX') EXECUTE PROCEDURE reindex_start_command();
+CREATE OR REPLACE FUNCTION reindex_end_command()
+RETURNS event_trigger AS $$
+DECLARE
+  obj record;
+BEGIN
+  FOR obj IN SELECT * FROM pg_event_trigger_ddl_commands()
+    LOOP
+      RAISE NOTICE 'ddl_end_command -- REINDEX: %', pg_get_indexdef(obj.objid);
+    END LOOP;
+END;
+$$ LANGUAGE plpgsql;
+CREATE EVENT TRIGGER end_reindex_command ON ddl_command_end
+    WHEN TAG IN ('REINDEX') EXECUTE PROCEDURE reindex_end_command();
+-- test REINDEX event for a standard table
+DROP SCHEMA IF EXISTS reindex_test CASCADE;
+NOTICE:  schema "reindex_test" does not exist, skipping
+CREATE SCHEMA reindex_test;
+CREATE TABLE reindex_test.reindex_tester1 (a int);
+CREATE INDEX reindex_test1_idx1 ON reindex_test.reindex_tester1 (a);
+CREATE INDEX reindex_test1_idx2 ON reindex_test.reindex_tester1 (a);
+CREATE TABLE reindex_test.reindex_tester2 (a int);
+CREATE INDEX reindex_test2_idx1 ON reindex_test.reindex_tester2 (a);
+REINDEX INDEX reindex_test.reindex_test1_idx1;
+NOTICE:  ddl_start_command -- REINDEX: ddl_command_start REINDEX
+NOTICE:  ddl_end_command -- REINDEX: CREATE INDEX reindex_test1_idx1 ON reindex_test.reindex_tester1 USING btree (a)
+REINDEX TABLE reindex_test.reindex_tester1;
+NOTICE:  ddl_start_command -- REINDEX: ddl_command_start REINDEX
+NOTICE:  ddl_end_command -- REINDEX: CREATE INDEX reindex_test1_idx1 ON reindex_test.reindex_tester1 USING btree (a)
+NOTICE:  ddl_end_command -- REINDEX: CREATE INDEX reindex_test1_idx2 ON reindex_test.reindex_tester1 USING btree (a)
+REINDEX INDEX CONCURRENTLY reindex_test.reindex_test2_idx1;
+NOTICE:  ddl_start_command -- REINDEX: ddl_command_start REINDEX
+NOTICE:  ddl_end_command -- REINDEX: CREATE INDEX reindex_test2_idx1 ON reindex_test.reindex_tester2 USING btree (a)
+REINDEX TABLE CONCURRENTLY reindex_test.reindex_tester2;
+NOTICE:  ddl_start_command -- REINDEX: ddl_command_start REINDEX
+NOTICE:  ddl_end_command -- REINDEX: CREATE INDEX reindex_test2_idx1 ON reindex_test.reindex_tester2 USING btree (a)
+-- test REINDEX event for a partitioned table
+CREATE TABLE reindex_test.ptif_test (a int, b int) PARTITION BY range (a);
+CREATE TABLE reindex_test.ptif_test0 PARTITION OF reindex_test.ptif_test
+  FOR VALUES FROM (minvalue) TO (0) PARTITION BY list (b);
+CREATE TABLE reindex_test.ptif_test01 PARTITION OF reindex_test.ptif_test0 FOR VALUES IN (1);
+CREATE TABLE reindex_test.ptif_test1 PARTITION OF reindex_test.ptif_test
+  FOR VALUES FROM (0) TO (100) PARTITION BY list (b);
+CREATE TABLE reindex_test.ptif_test11 PARTITION OF reindex_test.ptif_test1 FOR VALUES IN (1);
+CREATE TABLE reindex_test.ptif_test2 PARTITION OF reindex_test.ptif_test
+  FOR VALUES FROM (100) TO (200);
+-- This partitioned table should remain with no partitions.
+CREATE TABLE reindex_test.ptif_test3 PARTITION OF reindex_test.ptif_test
+  FOR VALUES FROM (200) TO (maxvalue) PARTITION BY list (b);
+CREATE INDEX ptif_test_index ON ONLY reindex_test.ptif_test (a);
+CREATE INDEX ptif_test0_index ON ONLY reindex_test.ptif_test0 (a);
+ALTER INDEX reindex_test.ptif_test_index ATTACH PARTITION reindex_test.ptif_test0_index;
+CREATE INDEX ptif_test01_index ON reindex_test.ptif_test01 (a);
+ALTER INDEX reindex_test.ptif_test0_index ATTACH PARTITION reindex_test.ptif_test01_index;
+CREATE INDEX ptif_test1_index ON ONLY reindex_test.ptif_test1 (a);
+ALTER INDEX reindex_test.ptif_test_index ATTACH PARTITION reindex_test.ptif_test1_index;
+CREATE INDEX ptif_test11_index ON reindex_test.ptif_test11 (a);
+ALTER INDEX reindex_test.ptif_test1_index ATTACH PARTITION reindex_test.ptif_test11_index;
+CREATE INDEX ptif_test2_index ON reindex_test.ptif_test2 (a);
+ALTER INDEX reindex_test.ptif_test_index ATTACH PARTITION reindex_test.ptif_test2_index;
+CREATE INDEX ptif_test3_index ON reindex_test.ptif_test3 (a);
+ALTER INDEX reindex_test.ptif_test_index ATTACH PARTITION reindex_test.ptif_test3_index;
+-- reindex at the top level table to recursively reindex each partition
+REINDEX INDEX CONCURRENTLY reindex_test.ptif_test_index;
+NOTICE:  ddl_start_command -- REINDEX: ddl_command_start REINDEX
+NOTICE:  ddl_end_command -- REINDEX: CREATE INDEX ptif_test2_index ON reindex_test.ptif_test2 USING btree (a)
+NOTICE:  ddl_end_command -- REINDEX: CREATE INDEX ptif_test01_index ON reindex_test.ptif_test01 USING btree (a)
+NOTICE:  ddl_end_command -- REINDEX: CREATE INDEX ptif_test11_index ON reindex_test.ptif_test11 USING btree (a)
+-- ptif_test0 is partitioned table so it will index partition: ptif_test01_index
+-- event trigger will log ptif_test01_index
+REINDEX INDEX CONCURRENTLY reindex_test.ptif_test0_index;
+NOTICE:  ddl_start_command -- REINDEX: ddl_command_start REINDEX
+NOTICE:  ddl_end_command -- REINDEX: CREATE INDEX ptif_test01_index ON reindex_test.ptif_test01 USING btree (a)
+-- ptif_test1_index is partitioned index so it will index partition: ptif_test11_index
+-- event trigger will effect on partion index:ptif_test11_index
+REINDEX INDEX CONCURRENTLY reindex_test.ptif_test1_index;
+NOTICE:  ddl_start_command -- REINDEX: ddl_command_start REINDEX
+NOTICE:  ddl_end_command -- REINDEX: CREATE INDEX ptif_test11_index ON reindex_test.ptif_test11 USING btree (a)
+-- ptif_test2 is a partition so event trigger will log ptif_test2_index
+REINDEX INDEX CONCURRENTLY reindex_test.ptif_test2_index;
+NOTICE:  ddl_start_command -- REINDEX: ddl_command_start REINDEX
+NOTICE:  ddl_end_command -- REINDEX: CREATE INDEX ptif_test2_index ON reindex_test.ptif_test2 USING btree (a)
+-- no partitions: event trigger won't do anything
+REINDEX INDEX CONCURRENTLY reindex_test.ptif_test3_index;
+NOTICE:  ddl_start_command -- REINDEX: ddl_command_start REINDEX
+-- reindex at the top level table to recursively reindex each partition
+REINDEX TABLE reindex_test.ptif_test;
+NOTICE:  ddl_start_command -- REINDEX: ddl_command_start REINDEX
+NOTICE:  ddl_end_command -- REINDEX: CREATE INDEX ptif_test2_index ON reindex_test.ptif_test2 USING btree (a)
+NOTICE:  ddl_end_command -- REINDEX: CREATE INDEX ptif_test01_index ON reindex_test.ptif_test01 USING btree (a)
+NOTICE:  ddl_end_command -- REINDEX: CREATE INDEX ptif_test11_index ON reindex_test.ptif_test11 USING btree (a)
+-- will direct to ptif_test01
+REINDEX TABLE reindex_test.ptif_test0;
+NOTICE:  ddl_start_command -- REINDEX: ddl_command_start REINDEX
+NOTICE:  ddl_end_command -- REINDEX: CREATE INDEX ptif_test01_index ON reindex_test.ptif_test01 USING btree (a)
+-- will index its associtaed index
+REINDEX TABLE reindex_test.ptif_test01;
+NOTICE:  ddl_start_command -- REINDEX: ddl_command_start REINDEX
+NOTICE:  ddl_end_command -- REINDEX: CREATE INDEX ptif_test01_index ON reindex_test.ptif_test01 USING btree (a)
+-- will index its associtaed index
+REINDEX TABLE reindex_test.ptif_test11;
+NOTICE:  ddl_start_command -- REINDEX: ddl_command_start REINDEX
+NOTICE:  ddl_end_command -- REINDEX: CREATE INDEX ptif_test11_index ON reindex_test.ptif_test11 USING btree (a)
+-- will index its associtaed index
+REINDEX TABLE reindex_test.ptif_test2;
+NOTICE:  ddl_start_command -- REINDEX: ddl_command_start REINDEX
+NOTICE:  ddl_end_command -- REINDEX: CREATE INDEX ptif_test2_index ON reindex_test.ptif_test2 USING btree (a)
+-- no partion, index won't do anything
+REINDEX TABLE reindex_test.ptif_test3;
+NOTICE:  ddl_start_command -- REINDEX: ddl_command_start REINDEX
+DROP EVENT TRIGGER start_reindex_command;
+DROP EVENT TRIGGER end_reindex_command;
+DROP SCHEMA reindex_test CASCADE;
+NOTICE:  drop cascades to 3 other objects
+DETAIL:  drop cascades to table reindex_test.reindex_tester1
+drop cascades to table reindex_test.reindex_tester2
+drop cascades to table reindex_test.ptif_test
+DROP FUNCTION reindex_end_command;
 -- test Row Security Event Trigger
 RESET SESSION AUTHORIZATION;
 CREATE TABLE event_trigger_test (a integer, b text);
diff --git a/src/test/regress/sql/event_trigger.sql b/src/test/regress/sql/event_trigger.sql
index 1aeaddbe..d5d51bd9 100644
--- a/src/test/regress/sql/event_trigger.sql
+++ b/src/test/regress/sql/event_trigger.sql
@@ -418,6 +418,105 @@ drop table rewriteme;
 drop event trigger no_rewrite_allowed;
 drop function test_evtrig_no_rewrite();
 
+-- test Reindex Event Trigger
+CREATE OR REPLACE FUNCTION reindex_start_command()
+RETURNS event_trigger AS $$
+BEGIN
+  RAISE NOTICE 'ddl_start_command -- REINDEX: % %', tg_event, tg_tag;
+END;
+$$ LANGUAGE plpgsql;
+
+CREATE EVENT TRIGGER start_reindex_command ON ddl_command_start
+    WHEN TAG IN ('REINDEX') EXECUTE PROCEDURE reindex_start_command();
+
+CREATE OR REPLACE FUNCTION reindex_end_command()
+RETURNS event_trigger AS $$
+DECLARE
+  obj record;
+BEGIN
+  FOR obj IN SELECT * FROM pg_event_trigger_ddl_commands()
+    LOOP
+      RAISE NOTICE 'ddl_end_command -- REINDEX: %', pg_get_indexdef(obj.objid);
+    END LOOP;
+END;
+$$ LANGUAGE plpgsql;
+
+CREATE EVENT TRIGGER end_reindex_command ON ddl_command_end
+    WHEN TAG IN ('REINDEX') EXECUTE PROCEDURE reindex_end_command();
+
+-- test REINDEX event for a standard table
+DROP SCHEMA IF EXISTS reindex_test CASCADE;
+CREATE SCHEMA reindex_test;
+CREATE TABLE reindex_test.reindex_tester1 (a int);
+CREATE INDEX reindex_test1_idx1 ON reindex_test.reindex_tester1 (a);
+CREATE INDEX reindex_test1_idx2 ON reindex_test.reindex_tester1 (a);
+CREATE TABLE reindex_test.reindex_tester2 (a int);
+CREATE INDEX reindex_test2_idx1 ON reindex_test.reindex_tester2 (a);
+
+REINDEX INDEX reindex_test.reindex_test1_idx1;
+REINDEX TABLE reindex_test.reindex_tester1;
+REINDEX INDEX CONCURRENTLY reindex_test.reindex_test2_idx1;
+REINDEX TABLE CONCURRENTLY reindex_test.reindex_tester2;
+
+-- test REINDEX event for a partitioned table
+CREATE TABLE reindex_test.ptif_test (a int, b int) PARTITION BY range (a);
+CREATE TABLE reindex_test.ptif_test0 PARTITION OF reindex_test.ptif_test
+  FOR VALUES FROM (minvalue) TO (0) PARTITION BY list (b);
+CREATE TABLE reindex_test.ptif_test01 PARTITION OF reindex_test.ptif_test0 FOR VALUES IN (1);
+CREATE TABLE reindex_test.ptif_test1 PARTITION OF reindex_test.ptif_test
+  FOR VALUES FROM (0) TO (100) PARTITION BY list (b);
+CREATE TABLE reindex_test.ptif_test11 PARTITION OF reindex_test.ptif_test1 FOR VALUES IN (1);
+CREATE TABLE reindex_test.ptif_test2 PARTITION OF reindex_test.ptif_test
+  FOR VALUES FROM (100) TO (200);
+-- This partitioned table should remain with no partitions.
+CREATE TABLE reindex_test.ptif_test3 PARTITION OF reindex_test.ptif_test
+  FOR VALUES FROM (200) TO (maxvalue) PARTITION BY list (b);
+
+CREATE INDEX ptif_test_index ON ONLY reindex_test.ptif_test (a);
+CREATE INDEX ptif_test0_index ON ONLY reindex_test.ptif_test0 (a);
+ALTER INDEX reindex_test.ptif_test_index ATTACH PARTITION reindex_test.ptif_test0_index;
+CREATE INDEX ptif_test01_index ON reindex_test.ptif_test01 (a);
+ALTER INDEX reindex_test.ptif_test0_index ATTACH PARTITION reindex_test.ptif_test01_index;
+CREATE INDEX ptif_test1_index ON ONLY reindex_test.ptif_test1 (a);
+ALTER INDEX reindex_test.ptif_test_index ATTACH PARTITION reindex_test.ptif_test1_index;
+CREATE INDEX ptif_test11_index ON reindex_test.ptif_test11 (a);
+ALTER INDEX reindex_test.ptif_test1_index ATTACH PARTITION reindex_test.ptif_test11_index;
+CREATE INDEX ptif_test2_index ON reindex_test.ptif_test2 (a);
+ALTER INDEX reindex_test.ptif_test_index ATTACH PARTITION reindex_test.ptif_test2_index;
+CREATE INDEX ptif_test3_index ON reindex_test.ptif_test3 (a);
+ALTER INDEX reindex_test.ptif_test_index ATTACH PARTITION reindex_test.ptif_test3_index;
+
+-- reindex at the top level table to recursively reindex each partition
+REINDEX INDEX CONCURRENTLY reindex_test.ptif_test_index;
+-- ptif_test0 is partitioned table so it will index partition: ptif_test01_index
+-- event trigger will log ptif_test01_index
+REINDEX INDEX CONCURRENTLY reindex_test.ptif_test0_index;
+-- ptif_test1_index is partitioned index so it will index partition: ptif_test11_index
+-- event trigger will effect on partion index:ptif_test11_index
+REINDEX INDEX CONCURRENTLY reindex_test.ptif_test1_index;
+-- ptif_test2 is a partition so event trigger will log ptif_test2_index
+REINDEX INDEX CONCURRENTLY reindex_test.ptif_test2_index;
+-- no partitions: event trigger won't do anything
+REINDEX INDEX CONCURRENTLY reindex_test.ptif_test3_index;
+
+-- reindex at the top level table to recursively reindex each partition
+REINDEX TABLE reindex_test.ptif_test;
+-- will direct to ptif_test01
+REINDEX TABLE reindex_test.ptif_test0;
+-- will index its associtaed index
+REINDEX TABLE reindex_test.ptif_test01;
+-- will index its associtaed index
+REINDEX TABLE reindex_test.ptif_test11;
+-- will index its associtaed index
+REINDEX TABLE reindex_test.ptif_test2;
+-- no partion, index won't do anything
+REINDEX TABLE reindex_test.ptif_test3;
+
+DROP EVENT TRIGGER start_reindex_command;
+DROP EVENT TRIGGER end_reindex_command;
+DROP SCHEMA reindex_test CASCADE;
+DROP FUNCTION reindex_end_command;
+
 -- test Row Security Event Trigger
 RESET SESSION AUTHORIZATION;
 CREATE TABLE event_trigger_test (a integer, b text);
#11Jim Jones
jim.jones@uni-muenster.de
In reply to: jian he (#10)
Re: PATCH: Add REINDEX tag to event triggers

On 01.09.23 11:23, jian he wrote:

because the change made in here:
https://git.postgresql.org/cgit/postgresql.git/diff/src/backend/commands/indexcmds.c?id=11af63fb48d278b86aa948a5b57f136ef03c2bb7

I manually slight edited the patch content:
from
static List *ChooseIndexColumnNames(List *indexElems);
static void ReindexIndex(RangeVar *indexRelation, ReindexParams *params,
bool isTopLevel);
to
static List *ChooseIndexColumnNames(const List *indexElems);
static void ReindexIndex(const RangeVar *indexRelation, const
ReindexParams *params,
bool isTopLevel);

can you try the attached one.

The patch applies cleanly. However, now the compiler complains about a
qualifier mismatch

indexcmds.c:2707:33: warning: assignment discards ‘const’ qualifier from
pointer target type [-Wdiscarded-qualifiers]
 2707 |         currentReindexStatement = stmt;

Perhaps 'const ReindexStmt *currentReindexStatement'?

Tests also work just fine. I also tested it against unique and partial
indexes, and it worked as expected.

Jim

#12jian he
jian.universality@gmail.com
In reply to: Jim Jones (#11)
1 attachment(s)
Re: PATCH: Add REINDEX tag to event triggers

On Fri, Sep 1, 2023 at 6:40 PM Jim Jones <jim.jones@uni-muenster.de> wrote:

On 01.09.23 11:23, jian he wrote:

because the change made in here:
https://git.postgresql.org/cgit/postgresql.git/diff/src/backend/commands/indexcmds.c?id=11af63fb48d278b86aa948a5b57f136ef03c2bb7

I manually slight edited the patch content:
from
static List *ChooseIndexColumnNames(List *indexElems);
static void ReindexIndex(RangeVar *indexRelation, ReindexParams *params,
bool isTopLevel);
to
static List *ChooseIndexColumnNames(const List *indexElems);
static void ReindexIndex(const RangeVar *indexRelation, const
ReindexParams *params,
bool isTopLevel);

can you try the attached one.

The patch applies cleanly. However, now the compiler complains about a
qualifier mismatch

indexcmds.c:2707:33: warning: assignment discards ‘const’ qualifier from
pointer target type [-Wdiscarded-qualifiers]
2707 | c

Perhaps 'const ReindexStmt *currentReindexStatement'?

Tests also work just fine. I also tested it against unique and partial
indexes, and it worked as expected.

Jim

Thanks for pointing this out!
also due to commit
https://git.postgresql.org/cgit/postgresql.git/commit/?id=11af63fb48d278b86aa948a5b57f136ef03c2bb7
ExecReindex function input arguments also changed. so we need to
rebase this patch.

change to
currentReindexStatement = unconstify(ReindexStmt*, stmt);
seems to work for me. I tested it, no warning. But I am not 100% sure.

anyway I refactored the patch, making it git applyable
also change from "currentReindexStatement = stmt;" to
"currentReindexStatement = unconstify(ReindexStmt*, stmt);" due to
ExecReindex function changes.

Attachments:

v3-0001-Add-REINDEX-tag-to-event-triggers.patchtext/x-patch; charset=US-ASCII; name=v3-0001-Add-REINDEX-tag-to-event-triggers.patchDownload
From a3b6775362e07d53b367bcf303a3535d24f42cce Mon Sep 17 00:00:00 2001
From: pgaddict <jian.universality@gmail.com>
Date: Fri, 1 Sep 2023 23:51:46 +0800
Subject: [PATCH v3] Add REINDEX tag to event triggers
This turns on the event tag REINDEX. This will now
 return a record for each index that was modified during as part of start/end
 ddl commands.

For concurrent reindex, this will return the OID for the new index. This
includes concurrently reindexing tables and databases. It will only
report the new index records and not the ones destroyed by the
concurrent reindex process.

Author: Garrett Thornburg
---
 doc/src/sgml/event-trigger.sgml             |   8 ++
 src/backend/commands/indexcmds.c            |  83 +++++++++++++
 src/backend/tcop/utility.c                  |  12 +-
 src/include/tcop/cmdtaglist.h               |   2 +-
 src/test/regress/expected/event_trigger.out | 126 ++++++++++++++++++++
 src/test/regress/sql/event_trigger.sql      |  99 +++++++++++++++
 6 files changed, 327 insertions(+), 3 deletions(-)

diff --git a/doc/src/sgml/event-trigger.sgml b/doc/src/sgml/event-trigger.sgml
index 3b6a5361..aa1b359f 100644
--- a/doc/src/sgml/event-trigger.sgml
+++ b/doc/src/sgml/event-trigger.sgml
@@ -1013,6 +1013,14 @@
         <entry align="center"><literal>-</literal></entry>
         <entry align="left"></entry>
        </row>
+       <row>
+        <entry align="left"><literal>REINDEX</literal></entry>
+        <entry align="center"><literal>X</literal></entry>
+        <entry align="center"><literal>X</literal></entry>
+        <entry align="center"><literal>-</literal></entry>
+        <entry align="center"><literal>-</literal></entry>
+        <entry align="left"></entry>
+       </row>
        <row>
         <entry align="left"><literal>REVOKE</literal></entry>
         <entry align="center"><literal>X</literal></entry>
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index ab8b81b3..483b0665 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -95,6 +95,8 @@ static char *ChooseIndexNameAddition(const List *colnames);
 static List *ChooseIndexColumnNames(const List *indexElems);
 static void ReindexIndex(const RangeVar *indexRelation, const ReindexParams *params,
 						 bool isTopLevel);
+static void reindex_event_trigger_collect(Oid oid);
+static void reindex_event_trigger_collect_relation(Oid oid);
 static void RangeVarCallbackForReindexIndex(const RangeVar *relation,
 											Oid relId, Oid oldRelId, void *arg);
 static Oid	ReindexTable(const RangeVar *relation, const ReindexParams *params,
@@ -111,6 +113,8 @@ static bool ReindexRelationConcurrently(Oid relationOid,
 static void update_relispartition(Oid relationId, bool newval);
 static inline void set_indexsafe_procflags(void);
 
+static ReindexStmt *currentReindexStatement = NULL;
+
 /*
  * callback argument type for RangeVarCallbackForReindexIndex()
  */
@@ -2696,6 +2700,12 @@ ExecReindex(ParseState *pstate, const ReindexStmt *stmt, bool isTopLevel)
 	bool		verbose = false;
 	char	   *tablespacename = NULL;
 
+	/*
+	 * Make current stmt available for event triggers without directly passing
+	 * the context to every subsequent call.
+	 */
+	currentReindexStatement = unconstify(ReindexStmt*, stmt);
+
 	/* Parse option list */
 	foreach(lc, stmt->params)
 	{
@@ -2776,6 +2786,12 @@ ExecReindex(ParseState *pstate, const ReindexStmt *stmt, bool isTopLevel)
 				 (int) stmt->kind);
 			break;
 	}
+
+	/*
+	 * Clear the reindex stmt global reference now that triggers should have
+	 * completed
+	 */
+	currentReindexStatement = NULL;
 }
 
 /*
@@ -2827,6 +2843,8 @@ ReindexIndex(const RangeVar *indexRelation, const ReindexParams *params, bool is
 
 		newparams.options |= REINDEXOPT_REPORT_PROGRESS;
 		reindex_index(indOid, false, persistence, &newparams);
+		/* Add the index to event trigger */
+		reindex_event_trigger_collect(indOid);
 	}
 }
 
@@ -2950,6 +2968,9 @@ ReindexTable(const RangeVar *relation, const ReindexParams *params, bool isTopLe
 			ereport(NOTICE,
 					(errmsg("table \"%s\" has no indexes to reindex",
 							relation->relname)));
+
+		/* Create even for the indexes being modified */
+		reindex_event_trigger_collect_relation(heapOid);
 	}
 
 	return heapOid;
@@ -3366,6 +3387,8 @@ ReindexMultipleInternal(const List *relids, const ReindexParams *params)
 			newparams.options |=
 				REINDEXOPT_REPORT_PROGRESS | REINDEXOPT_MISSING_OK;
 			reindex_index(relid, false, relpersistence, &newparams);
+			/* Add the index to event trigger */
+			reindex_event_trigger_collect(relid);
 			PopActiveSnapshot();
 			/* reindex_index() does the verbose output */
 		}
@@ -3387,6 +3410,9 @@ ReindexMultipleInternal(const List *relids, const ReindexParams *params)
 								get_namespace_name(get_rel_namespace(relid)),
 								get_rel_name(relid))));
 
+			/* Create even for the indexes being modified */
+			reindex_event_trigger_collect_relation(relid);
+
 			PopActiveSnapshot();
 		}
 
@@ -3833,6 +3859,9 @@ ReindexRelationConcurrently(Oid relationOid, const ReindexParams *params)
 
 		newIndexIds = lappend(newIndexIds, newidx);
 
+		/* Add the index to event trigger */
+		reindex_event_trigger_collect(newIndexId);
+
 		/*
 		 * Save lockrelid to protect each relation from drop then close
 		 * relations. The lockrelid on parent relation is not taken here to
@@ -4436,3 +4465,57 @@ set_indexsafe_procflags(void)
 	ProcGlobal->statusFlags[MyProc->pgxactoff] = MyProc->statusFlags;
 	LWLockRelease(ProcArrayLock);
 }
+
+static void
+reindex_event_trigger_collect(Oid oid)
+{
+	ObjectAddress address;
+
+	if (currentReindexStatement == NULL)
+		return;
+
+	address.classId = RelationRelationId;
+	address.objectId = oid;
+	address.objectSubId = 0;
+
+	EventTriggerCollectSimpleCommand(address,
+									 InvalidObjectAddress, (Node *) currentReindexStatement);
+}
+
+static void
+reindex_event_trigger_collect_relation(Oid relid)
+{
+	Relation	rel;
+	List	   *indexIds = NULL;
+	ListCell   *indexId = NULL;
+
+	/*
+	 * Open and lock the relation.  ShareLock is sufficient since we only need
+	 * to prevent schema and data changes in it.  The lock level used here
+	 * should match catalog's reindex_relation().
+	 */
+	rel = try_table_open(relid, ShareLock);
+
+	/* if relation is gone, leave */
+	if (!rel)
+		return;
+
+	/*
+	 * Get the list of index OIDs for this relation.  (We trust to the
+	 * relcache to get this with a sequential scan if ignoring system
+	 * indexes.)
+	 */
+	indexIds = RelationGetIndexList(rel);
+
+	/*
+	 * Get the list of index OIDs for this relation.
+	 */
+	foreach(indexId, indexIds)
+	{
+		Oid			indexOid = lfirst_oid(indexId);
+
+		reindex_event_trigger_collect(indexOid);
+	}
+
+	table_close(rel, ShareLock);
+}
diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c
index e3ccf6c7..a269b5b6 100644
--- a/src/backend/tcop/utility.c
+++ b/src/backend/tcop/utility.c
@@ -961,7 +961,9 @@ standard_ProcessUtility(PlannedStmt *pstmt,
 			break;
 
 		case T_ReindexStmt:
-			ExecReindex(pstate, (ReindexStmt *) parsetree, isTopLevel);
+			ProcessUtilitySlow(pstate, pstmt, queryString,
+							   context, params, queryEnv,
+							   dest, qc);
 			break;
 
 			/*
@@ -1574,6 +1576,12 @@ ProcessUtilitySlow(ParseState *pstate,
 				}
 				break;
 
+			case T_ReindexStmt:
+				ExecReindex(pstate, (ReindexStmt *) parsetree, isTopLevel);
+				/* no commands stashed for REINDEX */
+				commandCollected = true;
+				break;
+
 			case T_CreateExtensionStmt:
 				address = CreateExtension(pstate, (CreateExtensionStmt *) parsetree);
 				break;
@@ -3620,7 +3628,7 @@ GetCommandLogLevel(Node *parsetree)
 			break;
 
 		case T_ReindexStmt:
-			lev = LOGSTMT_ALL;	/* should this be DDL? */
+			lev = LOGSTMT_DDL;
 			break;
 
 		case T_CreateConversionStmt:
diff --git a/src/include/tcop/cmdtaglist.h b/src/include/tcop/cmdtaglist.h
index e738ac1c..d0e0b186 100644
--- a/src/include/tcop/cmdtaglist.h
+++ b/src/include/tcop/cmdtaglist.h
@@ -193,7 +193,7 @@ PG_CMDTAG(CMDTAG_PREPARE, "PREPARE", false, false, false)
 PG_CMDTAG(CMDTAG_PREPARE_TRANSACTION, "PREPARE TRANSACTION", false, false, false)
 PG_CMDTAG(CMDTAG_REASSIGN_OWNED, "REASSIGN OWNED", false, false, false)
 PG_CMDTAG(CMDTAG_REFRESH_MATERIALIZED_VIEW, "REFRESH MATERIALIZED VIEW", true, false, false)
-PG_CMDTAG(CMDTAG_REINDEX, "REINDEX", false, false, false)
+PG_CMDTAG(CMDTAG_REINDEX, "REINDEX", true, false, false)
 PG_CMDTAG(CMDTAG_RELEASE, "RELEASE", false, false, false)
 PG_CMDTAG(CMDTAG_RESET, "RESET", false, false, false)
 PG_CMDTAG(CMDTAG_REVOKE, "REVOKE", true, false, false)
diff --git a/src/test/regress/expected/event_trigger.out b/src/test/regress/expected/event_trigger.out
index 2c8a6b22..874fba2a 100644
--- a/src/test/regress/expected/event_trigger.out
+++ b/src/test/regress/expected/event_trigger.out
@@ -556,6 +556,132 @@ ERROR:  cannot alter type "rewritetype" because column "rewritemetoo3.a" uses it
 drop table rewriteme;
 drop event trigger no_rewrite_allowed;
 drop function test_evtrig_no_rewrite();
+-- test Reindex Event Trigger
+CREATE OR REPLACE FUNCTION reindex_start_command()
+RETURNS event_trigger AS $$
+BEGIN
+  RAISE NOTICE 'ddl_start_command -- REINDEX: % %', tg_event, tg_tag;
+END;
+$$ LANGUAGE plpgsql;
+CREATE EVENT TRIGGER start_reindex_command ON ddl_command_start
+    WHEN TAG IN ('REINDEX') EXECUTE PROCEDURE reindex_start_command();
+CREATE OR REPLACE FUNCTION reindex_end_command()
+RETURNS event_trigger AS $$
+DECLARE
+  obj record;
+BEGIN
+  FOR obj IN SELECT * FROM pg_event_trigger_ddl_commands()
+    LOOP
+      RAISE NOTICE 'ddl_end_command -- REINDEX: %', pg_get_indexdef(obj.objid);
+    END LOOP;
+END;
+$$ LANGUAGE plpgsql;
+CREATE EVENT TRIGGER end_reindex_command ON ddl_command_end
+    WHEN TAG IN ('REINDEX') EXECUTE PROCEDURE reindex_end_command();
+-- test REINDEX event for a standard table
+DROP SCHEMA IF EXISTS reindex_test CASCADE;
+NOTICE:  schema "reindex_test" does not exist, skipping
+CREATE SCHEMA reindex_test;
+CREATE TABLE reindex_test.reindex_tester1 (a int);
+CREATE INDEX reindex_test1_idx1 ON reindex_test.reindex_tester1 (a);
+CREATE INDEX reindex_test1_idx2 ON reindex_test.reindex_tester1 (a);
+CREATE TABLE reindex_test.reindex_tester2 (a int);
+CREATE INDEX reindex_test2_idx1 ON reindex_test.reindex_tester2 (a);
+REINDEX INDEX reindex_test.reindex_test1_idx1;
+NOTICE:  ddl_start_command -- REINDEX: ddl_command_start REINDEX
+NOTICE:  ddl_end_command -- REINDEX: CREATE INDEX reindex_test1_idx1 ON reindex_test.reindex_tester1 USING btree (a)
+REINDEX TABLE reindex_test.reindex_tester1;
+NOTICE:  ddl_start_command -- REINDEX: ddl_command_start REINDEX
+NOTICE:  ddl_end_command -- REINDEX: CREATE INDEX reindex_test1_idx1 ON reindex_test.reindex_tester1 USING btree (a)
+NOTICE:  ddl_end_command -- REINDEX: CREATE INDEX reindex_test1_idx2 ON reindex_test.reindex_tester1 USING btree (a)
+REINDEX INDEX CONCURRENTLY reindex_test.reindex_test2_idx1;
+NOTICE:  ddl_start_command -- REINDEX: ddl_command_start REINDEX
+NOTICE:  ddl_end_command -- REINDEX: CREATE INDEX reindex_test2_idx1 ON reindex_test.reindex_tester2 USING btree (a)
+REINDEX TABLE CONCURRENTLY reindex_test.reindex_tester2;
+NOTICE:  ddl_start_command -- REINDEX: ddl_command_start REINDEX
+NOTICE:  ddl_end_command -- REINDEX: CREATE INDEX reindex_test2_idx1 ON reindex_test.reindex_tester2 USING btree (a)
+-- test REINDEX event for a partitioned table
+CREATE TABLE reindex_test.ptif_test (a int, b int) PARTITION BY range (a);
+CREATE TABLE reindex_test.ptif_test0 PARTITION OF reindex_test.ptif_test
+  FOR VALUES FROM (minvalue) TO (0) PARTITION BY list (b);
+CREATE TABLE reindex_test.ptif_test01 PARTITION OF reindex_test.ptif_test0 FOR VALUES IN (1);
+CREATE TABLE reindex_test.ptif_test1 PARTITION OF reindex_test.ptif_test
+  FOR VALUES FROM (0) TO (100) PARTITION BY list (b);
+CREATE TABLE reindex_test.ptif_test11 PARTITION OF reindex_test.ptif_test1 FOR VALUES IN (1);
+CREATE TABLE reindex_test.ptif_test2 PARTITION OF reindex_test.ptif_test
+  FOR VALUES FROM (100) TO (200);
+-- This partitioned table should remain with no partitions.
+CREATE TABLE reindex_test.ptif_test3 PARTITION OF reindex_test.ptif_test
+  FOR VALUES FROM (200) TO (maxvalue) PARTITION BY list (b);
+CREATE INDEX ptif_test_index ON ONLY reindex_test.ptif_test (a);
+CREATE INDEX ptif_test0_index ON ONLY reindex_test.ptif_test0 (a);
+ALTER INDEX reindex_test.ptif_test_index ATTACH PARTITION reindex_test.ptif_test0_index;
+CREATE INDEX ptif_test01_index ON reindex_test.ptif_test01 (a);
+ALTER INDEX reindex_test.ptif_test0_index ATTACH PARTITION reindex_test.ptif_test01_index;
+CREATE INDEX ptif_test1_index ON ONLY reindex_test.ptif_test1 (a);
+ALTER INDEX reindex_test.ptif_test_index ATTACH PARTITION reindex_test.ptif_test1_index;
+CREATE INDEX ptif_test11_index ON reindex_test.ptif_test11 (a);
+ALTER INDEX reindex_test.ptif_test1_index ATTACH PARTITION reindex_test.ptif_test11_index;
+CREATE INDEX ptif_test2_index ON reindex_test.ptif_test2 (a);
+ALTER INDEX reindex_test.ptif_test_index ATTACH PARTITION reindex_test.ptif_test2_index;
+CREATE INDEX ptif_test3_index ON reindex_test.ptif_test3 (a);
+ALTER INDEX reindex_test.ptif_test_index ATTACH PARTITION reindex_test.ptif_test3_index;
+-- reindex at the top level table to recursively reindex each partition
+REINDEX INDEX CONCURRENTLY reindex_test.ptif_test_index;
+NOTICE:  ddl_start_command -- REINDEX: ddl_command_start REINDEX
+NOTICE:  ddl_end_command -- REINDEX: CREATE INDEX ptif_test2_index ON reindex_test.ptif_test2 USING btree (a)
+NOTICE:  ddl_end_command -- REINDEX: CREATE INDEX ptif_test01_index ON reindex_test.ptif_test01 USING btree (a)
+NOTICE:  ddl_end_command -- REINDEX: CREATE INDEX ptif_test11_index ON reindex_test.ptif_test11 USING btree (a)
+-- ptif_test0 is partitioned table so it will index partition: ptif_test01_index
+-- event trigger will log ptif_test01_index
+REINDEX INDEX CONCURRENTLY reindex_test.ptif_test0_index;
+NOTICE:  ddl_start_command -- REINDEX: ddl_command_start REINDEX
+NOTICE:  ddl_end_command -- REINDEX: CREATE INDEX ptif_test01_index ON reindex_test.ptif_test01 USING btree (a)
+-- ptif_test1_index is partitioned index so it will index partition: ptif_test11_index
+-- event trigger will effect on partion index:ptif_test11_index
+REINDEX INDEX CONCURRENTLY reindex_test.ptif_test1_index;
+NOTICE:  ddl_start_command -- REINDEX: ddl_command_start REINDEX
+NOTICE:  ddl_end_command -- REINDEX: CREATE INDEX ptif_test11_index ON reindex_test.ptif_test11 USING btree (a)
+-- ptif_test2 is a partition so event trigger will log ptif_test2_index
+REINDEX INDEX CONCURRENTLY reindex_test.ptif_test2_index;
+NOTICE:  ddl_start_command -- REINDEX: ddl_command_start REINDEX
+NOTICE:  ddl_end_command -- REINDEX: CREATE INDEX ptif_test2_index ON reindex_test.ptif_test2 USING btree (a)
+-- no partitions: event trigger won't do anything
+REINDEX INDEX CONCURRENTLY reindex_test.ptif_test3_index;
+NOTICE:  ddl_start_command -- REINDEX: ddl_command_start REINDEX
+-- reindex at the top level table to recursively reindex each partition
+REINDEX TABLE reindex_test.ptif_test;
+NOTICE:  ddl_start_command -- REINDEX: ddl_command_start REINDEX
+NOTICE:  ddl_end_command -- REINDEX: CREATE INDEX ptif_test2_index ON reindex_test.ptif_test2 USING btree (a)
+NOTICE:  ddl_end_command -- REINDEX: CREATE INDEX ptif_test01_index ON reindex_test.ptif_test01 USING btree (a)
+NOTICE:  ddl_end_command -- REINDEX: CREATE INDEX ptif_test11_index ON reindex_test.ptif_test11 USING btree (a)
+-- will direct to ptif_test01
+REINDEX TABLE reindex_test.ptif_test0;
+NOTICE:  ddl_start_command -- REINDEX: ddl_command_start REINDEX
+NOTICE:  ddl_end_command -- REINDEX: CREATE INDEX ptif_test01_index ON reindex_test.ptif_test01 USING btree (a)
+-- will index its associtaed index
+REINDEX TABLE reindex_test.ptif_test01;
+NOTICE:  ddl_start_command -- REINDEX: ddl_command_start REINDEX
+NOTICE:  ddl_end_command -- REINDEX: CREATE INDEX ptif_test01_index ON reindex_test.ptif_test01 USING btree (a)
+-- will index its associtaed index
+REINDEX TABLE reindex_test.ptif_test11;
+NOTICE:  ddl_start_command -- REINDEX: ddl_command_start REINDEX
+NOTICE:  ddl_end_command -- REINDEX: CREATE INDEX ptif_test11_index ON reindex_test.ptif_test11 USING btree (a)
+-- will index its associtaed index
+REINDEX TABLE reindex_test.ptif_test2;
+NOTICE:  ddl_start_command -- REINDEX: ddl_command_start REINDEX
+NOTICE:  ddl_end_command -- REINDEX: CREATE INDEX ptif_test2_index ON reindex_test.ptif_test2 USING btree (a)
+-- no partion, index won't do anything
+REINDEX TABLE reindex_test.ptif_test3;
+NOTICE:  ddl_start_command -- REINDEX: ddl_command_start REINDEX
+DROP EVENT TRIGGER start_reindex_command;
+DROP EVENT TRIGGER end_reindex_command;
+DROP SCHEMA reindex_test CASCADE;
+NOTICE:  drop cascades to 3 other objects
+DETAIL:  drop cascades to table reindex_test.reindex_tester1
+drop cascades to table reindex_test.reindex_tester2
+drop cascades to table reindex_test.ptif_test
+DROP FUNCTION reindex_end_command;
 -- test Row Security Event Trigger
 RESET SESSION AUTHORIZATION;
 CREATE TABLE event_trigger_test (a integer, b text);
diff --git a/src/test/regress/sql/event_trigger.sql b/src/test/regress/sql/event_trigger.sql
index 1aeaddbe..d5d51bd9 100644
--- a/src/test/regress/sql/event_trigger.sql
+++ b/src/test/regress/sql/event_trigger.sql
@@ -418,6 +418,105 @@ drop table rewriteme;
 drop event trigger no_rewrite_allowed;
 drop function test_evtrig_no_rewrite();
 
+-- test Reindex Event Trigger
+CREATE OR REPLACE FUNCTION reindex_start_command()
+RETURNS event_trigger AS $$
+BEGIN
+  RAISE NOTICE 'ddl_start_command -- REINDEX: % %', tg_event, tg_tag;
+END;
+$$ LANGUAGE plpgsql;
+
+CREATE EVENT TRIGGER start_reindex_command ON ddl_command_start
+    WHEN TAG IN ('REINDEX') EXECUTE PROCEDURE reindex_start_command();
+
+CREATE OR REPLACE FUNCTION reindex_end_command()
+RETURNS event_trigger AS $$
+DECLARE
+  obj record;
+BEGIN
+  FOR obj IN SELECT * FROM pg_event_trigger_ddl_commands()
+    LOOP
+      RAISE NOTICE 'ddl_end_command -- REINDEX: %', pg_get_indexdef(obj.objid);
+    END LOOP;
+END;
+$$ LANGUAGE plpgsql;
+
+CREATE EVENT TRIGGER end_reindex_command ON ddl_command_end
+    WHEN TAG IN ('REINDEX') EXECUTE PROCEDURE reindex_end_command();
+
+-- test REINDEX event for a standard table
+DROP SCHEMA IF EXISTS reindex_test CASCADE;
+CREATE SCHEMA reindex_test;
+CREATE TABLE reindex_test.reindex_tester1 (a int);
+CREATE INDEX reindex_test1_idx1 ON reindex_test.reindex_tester1 (a);
+CREATE INDEX reindex_test1_idx2 ON reindex_test.reindex_tester1 (a);
+CREATE TABLE reindex_test.reindex_tester2 (a int);
+CREATE INDEX reindex_test2_idx1 ON reindex_test.reindex_tester2 (a);
+
+REINDEX INDEX reindex_test.reindex_test1_idx1;
+REINDEX TABLE reindex_test.reindex_tester1;
+REINDEX INDEX CONCURRENTLY reindex_test.reindex_test2_idx1;
+REINDEX TABLE CONCURRENTLY reindex_test.reindex_tester2;
+
+-- test REINDEX event for a partitioned table
+CREATE TABLE reindex_test.ptif_test (a int, b int) PARTITION BY range (a);
+CREATE TABLE reindex_test.ptif_test0 PARTITION OF reindex_test.ptif_test
+  FOR VALUES FROM (minvalue) TO (0) PARTITION BY list (b);
+CREATE TABLE reindex_test.ptif_test01 PARTITION OF reindex_test.ptif_test0 FOR VALUES IN (1);
+CREATE TABLE reindex_test.ptif_test1 PARTITION OF reindex_test.ptif_test
+  FOR VALUES FROM (0) TO (100) PARTITION BY list (b);
+CREATE TABLE reindex_test.ptif_test11 PARTITION OF reindex_test.ptif_test1 FOR VALUES IN (1);
+CREATE TABLE reindex_test.ptif_test2 PARTITION OF reindex_test.ptif_test
+  FOR VALUES FROM (100) TO (200);
+-- This partitioned table should remain with no partitions.
+CREATE TABLE reindex_test.ptif_test3 PARTITION OF reindex_test.ptif_test
+  FOR VALUES FROM (200) TO (maxvalue) PARTITION BY list (b);
+
+CREATE INDEX ptif_test_index ON ONLY reindex_test.ptif_test (a);
+CREATE INDEX ptif_test0_index ON ONLY reindex_test.ptif_test0 (a);
+ALTER INDEX reindex_test.ptif_test_index ATTACH PARTITION reindex_test.ptif_test0_index;
+CREATE INDEX ptif_test01_index ON reindex_test.ptif_test01 (a);
+ALTER INDEX reindex_test.ptif_test0_index ATTACH PARTITION reindex_test.ptif_test01_index;
+CREATE INDEX ptif_test1_index ON ONLY reindex_test.ptif_test1 (a);
+ALTER INDEX reindex_test.ptif_test_index ATTACH PARTITION reindex_test.ptif_test1_index;
+CREATE INDEX ptif_test11_index ON reindex_test.ptif_test11 (a);
+ALTER INDEX reindex_test.ptif_test1_index ATTACH PARTITION reindex_test.ptif_test11_index;
+CREATE INDEX ptif_test2_index ON reindex_test.ptif_test2 (a);
+ALTER INDEX reindex_test.ptif_test_index ATTACH PARTITION reindex_test.ptif_test2_index;
+CREATE INDEX ptif_test3_index ON reindex_test.ptif_test3 (a);
+ALTER INDEX reindex_test.ptif_test_index ATTACH PARTITION reindex_test.ptif_test3_index;
+
+-- reindex at the top level table to recursively reindex each partition
+REINDEX INDEX CONCURRENTLY reindex_test.ptif_test_index;
+-- ptif_test0 is partitioned table so it will index partition: ptif_test01_index
+-- event trigger will log ptif_test01_index
+REINDEX INDEX CONCURRENTLY reindex_test.ptif_test0_index;
+-- ptif_test1_index is partitioned index so it will index partition: ptif_test11_index
+-- event trigger will effect on partion index:ptif_test11_index
+REINDEX INDEX CONCURRENTLY reindex_test.ptif_test1_index;
+-- ptif_test2 is a partition so event trigger will log ptif_test2_index
+REINDEX INDEX CONCURRENTLY reindex_test.ptif_test2_index;
+-- no partitions: event trigger won't do anything
+REINDEX INDEX CONCURRENTLY reindex_test.ptif_test3_index;
+
+-- reindex at the top level table to recursively reindex each partition
+REINDEX TABLE reindex_test.ptif_test;
+-- will direct to ptif_test01
+REINDEX TABLE reindex_test.ptif_test0;
+-- will index its associtaed index
+REINDEX TABLE reindex_test.ptif_test01;
+-- will index its associtaed index
+REINDEX TABLE reindex_test.ptif_test11;
+-- will index its associtaed index
+REINDEX TABLE reindex_test.ptif_test2;
+-- no partion, index won't do anything
+REINDEX TABLE reindex_test.ptif_test3;
+
+DROP EVENT TRIGGER start_reindex_command;
+DROP EVENT TRIGGER end_reindex_command;
+DROP SCHEMA reindex_test CASCADE;
+DROP FUNCTION reindex_end_command;
+
 -- test Row Security Event Trigger
 RESET SESSION AUTHORIZATION;
 CREATE TABLE event_trigger_test (a integer, b text);
-- 
2.34.1

#13Jim Jones
jim.jones@uni-muenster.de
In reply to: jian he (#12)
Re: PATCH: Add REINDEX tag to event triggers

On 01.09.23 18:10, jian he wrote:

Thanks for pointing this out!

Thanks for the fix!

also due to commit
https://git.postgresql.org/cgit/postgresql.git/commit/?id=11af63fb48d278b86aa948a5b57f136ef03c2bb7
ExecReindex function input arguments also changed. so we need to
rebase this patch.

change to
currentReindexStatement = unconstify(ReindexStmt*, stmt);
seems to work for me. I tested it, no warning. But I am not 100% sure.

anyway I refactored the patch, making it git applyable
also change from "currentReindexStatement = stmt;" to
"currentReindexStatement = unconstify(ReindexStmt*, stmt);" due to
ExecReindex function changes.

LGTM. It applies and builds cleanly, all tests pass and documentation
also builds ok. The CFbot seems also much happier now :)

I tried this "small stress test" to see if the code was leaking .. but
it all looks ok to me:

DO $$
BEGIN
  FOR i IN 1..10000 LOOP
    REINDEX INDEX reindex_test.reindex_test1_idx1;
    REINDEX TABLE reindex_test.reindex_tester1;
  END LOOP;
END;
$$;

Jim

#14Michael Paquier
michael@paquier.xyz
In reply to: Jim Jones (#13)
Re: PATCH: Add REINDEX tag to event triggers

On Mon, Sep 04, 2023 at 08:00:52PM +0200, Jim Jones wrote:

LGTM. It applies and builds cleanly, all tests pass and documentation also
builds ok. The CFbot seems also much happier now :)

+	/*
+	 * Open and lock the relation.  ShareLock is sufficient since we only need
+	 * to prevent schema and data changes in it.  The lock level used here
+	 * should match catalog's reindex_relation().
+	 */
+	rel = try_table_open(relid, ShareLock);

I was eyeing at 0003, and this strikes me as incorrect. Sure, this
matches what reindex_relation() does, but you've missed that
CONCURRENTLY takes a lighter ShareUpdateExclusiveLock, and ShareLock
conflicts with it. See:
https://www.postgresql.org/docs/devel/explicit-locking.html

So, doesn't this disrupt a concurrent REINDEX?
--
Michael

#15jian he
jian.universality@gmail.com
In reply to: Michael Paquier (#14)
Re: PATCH: Add REINDEX tag to event triggers

On Fri, Oct 27, 2023 at 3:15 PM Michael Paquier <michael@paquier.xyz> wrote:

On Mon, Sep 04, 2023 at 08:00:52PM +0200, Jim Jones wrote:

LGTM. It applies and builds cleanly, all tests pass and documentation also
builds ok. The CFbot seems also much happier now :)

+       /*
+        * Open and lock the relation.  ShareLock is sufficient since we only need
+        * to prevent schema and data changes in it.  The lock level used here
+        * should match catalog's reindex_relation().
+        */
+       rel = try_table_open(relid, ShareLock);

I was eyeing at 0003, and this strikes me as incorrect. Sure, this
matches what reindex_relation() does, but you've missed that
CONCURRENTLY takes a lighter ShareUpdateExclusiveLock, and ShareLock
conflicts with it. See:
https://www.postgresql.org/docs/devel/explicit-locking.html

So, doesn't this disrupt a concurrent REINDEX?
--
Michael

ReindexPartitions called ReindexMultipleInternal
ReindexRelationConcurrently add reindex_event_trigger_collect to cover
it. (line 3869)
ReindexIndex has the function reindex_event_trigger_collect. (line 2853)

reindex_event_trigger_collect_relation called in
ReindexMultipleInternal, ReindexTable (line 2979).
Both are "under concurrent is false" branches.

So it should be fine.

#16Michael Paquier
michael@paquier.xyz
In reply to: jian he (#15)
Re: PATCH: Add REINDEX tag to event triggers

On Sat, Oct 28, 2023 at 12:15:22PM +0800, jian he wrote:

reindex_event_trigger_collect_relation called in
ReindexMultipleInternal, ReindexTable (line 2979).
Both are "under concurrent is false" branches.

So it should be fine.

Sorry for the delay in replying.

Indeed, you're right. reindex_event_trigger_collect_relation() gets
only called in two paths for the non-concurrent cases just after
reindex_relation(), which is not a safe location to call it because
this may be used in CLUSTER or TRUNCATE, and the intention of the
patch is only to count for the objects in REINDEX.

Anyway, I think that the current implementation is incorrect. The
object is collected after the reindex is done, which is right.
However, an object may be reindexed but not be reported if it was
dropped between the reindex and its endwhen collecting all the objects
of a single relation. Perhaps it does not matter because the object
is gone, but that still looks incorrect to me because we finish by not
reporting everything we should. I think that we should put the
collection deeper into the stack, where we know that we are holding
the locks on the objects we are collecting. Another side effect is
that the patch seems to be missing toast table indexes, which are also
reindexed for a REINDEX TABLE, for instance. That's not right.

Actually, could there be an argument for pushing the collection down
to reindex_relation() instead to count for all the commands that?
Even better, reindex_index() would be a better candidate because it is
itself called within reindex_relation() for each individual indexes?
If a code path should not be covered for event triggers, we could use
a new REINDEXOPT_ to control that, optionally. In short, it looks
like we should try to reduce the number of paths calling
reindex_event_trigger_collect(), while collect_relation() ought to be
removed.

Should REINDEX TABLE CONCURRENTLY's coverage be extended so as two new
indexes are reported instead of just one?

REINDEX SCHEMA is a command that perhaps should be tested to collect
all the indexes rebuilt through it?

A side-effect of this patch is that it impacts ddl_command_start and
ddl_command_end with the addition of REINDEX. Mixing that with the
addition of a new event is strange. I think that the addition of
REINDEX to the existing events should be done first, as a separate
patch. Then a second patch should deal with the collection and the
support of the new event.

I have also dug into the archives to note that control commands have
been proposed in the past to be added as part of event triggers, and
it happens that I've mentioned in a comment that that this was a
concept perhaps contrary to what event triggers should do, as these
are intended for DDLs:
/messages/by-id/CAB7nPqTqZ2-YcNzOQ5KVBUJYHQ4kDSd4Q55Mc-fBzM8GH0bV2Q@mail.gmail.com

Philosophically, I'm open on all that and having more commands
depending on the cases that are satisfied, FWIW, but let's organize
the changes.
--
Michael

#17jian he
jian.universality@gmail.com
In reply to: Michael Paquier (#16)
1 attachment(s)
Re: PATCH: Add REINDEX tag to event triggers

On Mon, Nov 20, 2023 at 3:34 PM Michael Paquier <michael@paquier.xyz> wrote:

On Sat, Oct 28, 2023 at 12:15:22PM +0800, jian he wrote:

reindex_event_trigger_collect_relation called in
ReindexMultipleInternal, ReindexTable (line 2979).
Both are "under concurrent is false" branches.

So it should be fine.

Sorry for the delay in replying.

Indeed, you're right. reindex_event_trigger_collect_relation() gets
only called in two paths for the non-concurrent cases just after
reindex_relation(), which is not a safe location to call it because
this may be used in CLUSTER or TRUNCATE, and the intention of the
patch is only to count for the objects in REINDEX.

Anyway, I think that the current implementation is incorrect. The
object is collected after the reindex is done, which is right.
However, an object may be reindexed but not be reported if it was
dropped between the reindex and its endwhen collecting all the objects
of a single relation. Perhaps it does not matter because the object
is gone, but that still looks incorrect to me because we finish by not
reporting everything we should. I think that we should put the
collection deeper into the stack, where we know that we are holding
the locks on the objects we are collecting. Another side effect is
that the patch seems to be missing toast table indexes, which are also
reindexed for a REINDEX TABLE, for instance. That's not right.

Actually, could there be an argument for pushing the collection down
to reindex_relation() instead to count for all the commands that?
Even better, reindex_index() would be a better candidate because it is
itself called within reindex_relation() for each individual indexes?
If a code path should not be covered for event triggers, we could use
a new REINDEXOPT_ to control that, optionally. In short, it looks
like we should try to reduce the number of paths calling
reindex_event_trigger_collect(), while collect_relation() ought to be
removed.

Should REINDEX TABLE CONCURRENTLY's coverage be extended so as two new
indexes are reported instead of just one?

REINDEX SCHEMA is a command that perhaps should be tested to collect
all the indexes rebuilt through it?

A side-effect of this patch is that it impacts ddl_command_start and
ddl_command_end with the addition of REINDEX. Mixing that with the
addition of a new event is strange. I think that the addition of
REINDEX to the existing events should be done first, as a separate
patch. Then a second patch should deal with the collection and the
support of the new event.

I have also dug into the archives to note that control commands have
been proposed in the past to be added as part of event triggers, and
it happens that I've mentioned in a comment that that this was a
concept perhaps contrary to what event triggers should do, as these
are intended for DDLs:
/messages/by-id/CAB7nPqTqZ2-YcNzOQ5KVBUJYHQ4kDSd4Q55Mc-fBzM8GH0bV2Q@mail.gmail.com

Philosophically, I'm open on all that and having more commands
depending on the cases that are satisfied, FWIW, but let's organize
the changes.
--
Michael

Hi.
Top level func is ExecReIndex. it will call {ReindexIndex,
ReindexTable, ReindexMultipleTables}
then trace deep down, it will call {ReindexRelationConcurrently, reindex_index}.

Imitate the CREATE INDEX logic, we need pass `const ReindexStmt *stmt`
to all the following functions:
ReindexIndex
ReindexTable
ReindexMultipleTables
ReindexPartitions
ReindexMultipleInternal
ReindexRelationConcurrently
reindex_relation
reindex_index

because the EventTriggerCollectSimpleCommand needs the `(ReindexStmt
*) parsetree`.
and we call EventTriggerCollectSimpleCommand at reindex_index or
ReindexRelationConcurrently.

The new test will cover the following case.
reindex (concurrently) schema;
reindex (concurrently) partitioned index;
reindex (concurrently) partitioned table;

not sure this is the right approach. But now the reindex event
collection is after we call index_open.
same static function (reindex_event_trigger_collect) in
src/backend/commands/indexcmds.c and src/backend/catalog/index.c for
now.
given that ProcessUtilitySlow supports the event triggers facility.
so probably no need for another REINDEXOPT_ option?

I also added a test for disabled event triggers.

Attachments:

v4-0001-Add-REINDEX-tag-to-event-triggers.patchtext/x-patch; charset=US-ASCII; name=v4-0001-Add-REINDEX-tag-to-event-triggers.patchDownload
From 149b4e9860f92daa810dfed4704f4c796a585b1a Mon Sep 17 00:00:00 2001
From: pgaddict <jian.universality@gmail.com>
Date: Thu, 23 Nov 2023 21:56:09 +0800
Subject: [PATCH v4 1/1] Add REINDEX tag to event triggers

This turns on the event trigger on command tag REINDEX.
This will now return a record for each index that was modified as
part of start/end ddl commands.

For concurrent reindex, this will return the OID for the new index. This
includes concurrently reindexing tables and databases. It will only
report the new index records and not the ones destroyed by the
concurrent reindex process.

We put the new index OID collection deeper into the stack, where we hold the locks on the new index we are collecting.
So event trigger REINDEX command new index oid collection will be on the reindex_index and ReindexRelationConcurrently.
specifically It will be after `index_open(newIndexId, ShareUpdateExclusiveLock);`
Author: Garrett Thornburg
---
 doc/src/sgml/event-trigger.sgml             |  8 ++
 src/backend/catalog/index.c                 | 26 +++++--
 src/backend/commands/cluster.c              |  2 +-
 src/backend/commands/indexcmds.c            | 72 +++++++++++-------
 src/backend/commands/tablecmds.c            |  2 +-
 src/backend/tcop/utility.c                  | 12 ++-
 src/include/catalog/index.h                 |  4 +-
 src/include/tcop/cmdtaglist.h               |  2 +-
 src/test/regress/expected/event_trigger.out | 81 +++++++++++++++++++++
 src/test/regress/sql/event_trigger.sql      | 71 ++++++++++++++++++
 10 files changed, 242 insertions(+), 38 deletions(-)

diff --git a/doc/src/sgml/event-trigger.sgml b/doc/src/sgml/event-trigger.sgml
index 10b20f03..234b4ffd 100644
--- a/doc/src/sgml/event-trigger.sgml
+++ b/doc/src/sgml/event-trigger.sgml
@@ -1032,6 +1032,14 @@
         <entry align="center"><literal>-</literal></entry>
         <entry align="left"></entry>
        </row>
+       <row>
+        <entry align="left"><literal>REINDEX</literal></entry>
+        <entry align="center"><literal>X</literal></entry>
+        <entry align="center"><literal>X</literal></entry>
+        <entry align="center"><literal>-</literal></entry>
+        <entry align="center"><literal>-</literal></entry>
+        <entry align="left"></entry>
+       </row>
        <row>
         <entry align="left"><literal>REVOKE</literal></entry>
         <entry align="center"><literal>X</literal></entry>
diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c
index 143fae01..2afabdeb 100644
--- a/src/backend/catalog/index.c
+++ b/src/backend/catalog/index.c
@@ -136,7 +136,7 @@ static void SetReindexProcessing(Oid heapOid, Oid indexOid);
 static void ResetReindexProcessing(void);
 static void SetReindexPending(List *indexes);
 static void RemoveReindexPending(Oid indexOid);
-
+static void reindex_event_trigger_collect(Oid oid, const ReindexStmt *stmt);
 
 /*
  * relationHasPrimaryKey
@@ -3554,11 +3554,24 @@ IndexGetRelation(Oid indexId, bool missing_ok)
 	return result;
 }
 
+static void
+reindex_event_trigger_collect(Oid oid, const ReindexStmt *stmt)
+{
+	ObjectAddress address;
+
+	address.classId = RelationRelationId;
+	address.objectId = oid;
+	address.objectSubId = 0;
+
+	EventTriggerCollectSimpleCommand(address,
+									 InvalidObjectAddress, (Node *) stmt);
+}
+
 /*
  * reindex_index - This routine is used to recreate a single index
  */
 void
-reindex_index(Oid indexId, bool skip_constraint_checks, char persistence,
+reindex_index(const ReindexStmt *stmt, Oid indexId, bool skip_constraint_checks, char persistence,
 			  const ReindexParams *params)
 {
 	Relation	iRel,
@@ -3629,6 +3642,9 @@ reindex_index(Oid indexId, bool skip_constraint_checks, char persistence,
 	if (progress)
 		pgstat_progress_update_param(PROGRESS_CREATEIDX_ACCESS_METHOD_OID,
 									 iRel->rd_rel->relam);
+	/* event trigger tracking REINDEX primary pointer */
+	if (stmt)
+		reindex_event_trigger_collect(indexId, stmt);
 
 	/*
 	 * Partitioned indexes should never get processed here, as they have no
@@ -3865,7 +3881,7 @@ reindex_index(Oid indexId, bool skip_constraint_checks, char persistence,
  * index rebuild.
  */
 bool
-reindex_relation(Oid relid, int flags, const ReindexParams *params)
+reindex_relation(const ReindexStmt *stmt, Oid relid, int flags, const ReindexParams *params)
 {
 	Relation	rel;
 	Oid			toast_relid;
@@ -3953,7 +3969,7 @@ reindex_relation(Oid relid, int flags, const ReindexParams *params)
 			continue;
 		}
 
-		reindex_index(indexOid, !(flags & REINDEX_REL_CHECK_CONSTRAINTS),
+		reindex_index(stmt, indexOid, !(flags & REINDEX_REL_CHECK_CONSTRAINTS),
 					  persistence, params);
 
 		CommandCounterIncrement();
@@ -3990,7 +4006,7 @@ reindex_relation(Oid relid, int flags, const ReindexParams *params)
 
 		newparams.options &= ~(REINDEXOPT_MISSING_OK);
 		newparams.tablespaceOid = InvalidOid;
-		result |= reindex_relation(toast_relid, flags, &newparams);
+		result |= reindex_relation(stmt, toast_relid, flags, &newparams);
 	}
 
 	return result;
diff --git a/src/backend/commands/cluster.c b/src/backend/commands/cluster.c
index a3bef6ac..1f52d391 100644
--- a/src/backend/commands/cluster.c
+++ b/src/backend/commands/cluster.c
@@ -1518,7 +1518,7 @@ finish_heap_swap(Oid OIDOldHeap, Oid OIDNewHeap,
 	pgstat_progress_update_param(PROGRESS_CLUSTER_PHASE,
 								 PROGRESS_CLUSTER_PHASE_REBUILD_INDEX);
 
-	reindex_relation(OIDOldHeap, reindex_flags, &reindex_params);
+	reindex_relation(NULL, OIDOldHeap, reindex_flags, &reindex_params);
 
 	/* Report that we are now doing clean up */
 	pgstat_progress_update_param(PROGRESS_CLUSTER_PHASE,
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index 0b3b8e98..a596c64b 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -94,20 +94,21 @@ static char *ChooseIndexName(const char *tabname, Oid namespaceId,
 							 bool primary, bool isconstraint);
 static char *ChooseIndexNameAddition(const List *colnames);
 static List *ChooseIndexColumnNames(const List *indexElems);
-static void ReindexIndex(const RangeVar *indexRelation, const ReindexParams *params,
+static void ReindexIndex(const ReindexStmt *stmt, const ReindexParams *params,
 						 bool isTopLevel);
+static void reindex_event_trigger_collect(Oid oid, const ReindexStmt *stmt);
 static void RangeVarCallbackForReindexIndex(const RangeVar *relation,
 											Oid relId, Oid oldRelId, void *arg);
-static Oid	ReindexTable(const RangeVar *relation, const ReindexParams *params,
+static Oid	ReindexTable(const ReindexStmt *stmt, const RangeVar *relation, const ReindexParams *params,
 						 bool isTopLevel);
-static void ReindexMultipleTables(const char *objectName,
+static void ReindexMultipleTables(const ReindexStmt *stmt, const char *objectName,
 								  ReindexObjectType objectKind, const ReindexParams *params);
 static void reindex_error_callback(void *arg);
-static void ReindexPartitions(Oid relid, const ReindexParams *params,
+static void ReindexPartitions(const ReindexStmt *stmt, Oid relid, const ReindexParams *params,
 							  bool isTopLevel);
-static void ReindexMultipleInternal(const List *relids,
+static void ReindexMultipleInternal(const ReindexStmt *stmt, const List *relids,
 									const ReindexParams *params);
-static bool ReindexRelationConcurrently(Oid relationOid,
+static bool ReindexRelationConcurrently(const ReindexStmt *stmt, Oid relationOid,
 										const ReindexParams *params);
 static void update_relispartition(Oid relationId, bool newval);
 static inline void set_indexsafe_procflags(void);
@@ -2729,10 +2730,10 @@ ExecReindex(ParseState *pstate, const ReindexStmt *stmt, bool isTopLevel)
 	switch (stmt->kind)
 	{
 		case REINDEX_OBJECT_INDEX:
-			ReindexIndex(stmt->relation, &params, isTopLevel);
+			ReindexIndex(stmt, &params, isTopLevel);
 			break;
 		case REINDEX_OBJECT_TABLE:
-			ReindexTable(stmt->relation, &params, isTopLevel);
+			ReindexTable(stmt, stmt->relation, &params, isTopLevel);
 			break;
 		case REINDEX_OBJECT_SCHEMA:
 		case REINDEX_OBJECT_SYSTEM:
@@ -2748,7 +2749,7 @@ ExecReindex(ParseState *pstate, const ReindexStmt *stmt, bool isTopLevel)
 									  (stmt->kind == REINDEX_OBJECT_SCHEMA) ? "REINDEX SCHEMA" :
 									  (stmt->kind == REINDEX_OBJECT_SYSTEM) ? "REINDEX SYSTEM" :
 									  "REINDEX DATABASE");
-			ReindexMultipleTables(stmt->name, stmt->kind, &params);
+			ReindexMultipleTables(stmt, stmt->name, stmt->kind, &params);
 			break;
 		default:
 			elog(ERROR, "unrecognized object type: %d",
@@ -2762,13 +2763,15 @@ ExecReindex(ParseState *pstate, const ReindexStmt *stmt, bool isTopLevel)
  *		Recreate a specific index.
  */
 static void
-ReindexIndex(const RangeVar *indexRelation, const ReindexParams *params, bool isTopLevel)
+ReindexIndex(const ReindexStmt *stmt, const ReindexParams *params, bool isTopLevel)
 {
+	const RangeVar *indexRelation;
 	struct ReindexIndexCallbackState state;
 	Oid			indOid;
 	char		persistence;
 	char		relkind;
 
+	indexRelation = stmt->relation;
 	/*
 	 * Find and lock index, and check permissions on table; use callback to
 	 * obtain lock on table first, to avoid deadlock hazard.  The lock level
@@ -2796,16 +2799,16 @@ ReindexIndex(const RangeVar *indexRelation, const ReindexParams *params, bool is
 	relkind = get_rel_relkind(indOid);
 
 	if (relkind == RELKIND_PARTITIONED_INDEX)
-		ReindexPartitions(indOid, params, isTopLevel);
+		ReindexPartitions(stmt, indOid, params, isTopLevel);
 	else if ((params->options & REINDEXOPT_CONCURRENTLY) != 0 &&
 			 persistence != RELPERSISTENCE_TEMP)
-		ReindexRelationConcurrently(indOid, params);
+		ReindexRelationConcurrently(stmt, indOid, params);
 	else
 	{
 		ReindexParams newparams = *params;
 
 		newparams.options |= REINDEXOPT_REPORT_PROGRESS;
-		reindex_index(indOid, false, persistence, &newparams);
+		reindex_index(stmt, indOid, false, persistence, &newparams);
 	}
 }
 
@@ -2885,7 +2888,7 @@ RangeVarCallbackForReindexIndex(const RangeVar *relation,
  *		Recreate all indexes of a table (and of its toast table, if any)
  */
 static Oid
-ReindexTable(const RangeVar *relation, const ReindexParams *params, bool isTopLevel)
+ReindexTable(const ReindexStmt *stmt, const RangeVar *relation, const ReindexParams *params, bool isTopLevel)
 {
 	Oid			heapOid;
 	bool		result;
@@ -2905,11 +2908,11 @@ ReindexTable(const RangeVar *relation, const ReindexParams *params, bool isTopLe
 									   RangeVarCallbackOwnsTable, NULL);
 
 	if (get_rel_relkind(heapOid) == RELKIND_PARTITIONED_TABLE)
-		ReindexPartitions(heapOid, params, isTopLevel);
+		ReindexPartitions(stmt, heapOid, params, isTopLevel);
 	else if ((params->options & REINDEXOPT_CONCURRENTLY) != 0 &&
 			 get_rel_persistence(heapOid) != RELPERSISTENCE_TEMP)
 	{
-		result = ReindexRelationConcurrently(heapOid, params);
+		result = ReindexRelationConcurrently(stmt, heapOid, params);
 
 		if (!result)
 			ereport(NOTICE,
@@ -2921,7 +2924,7 @@ ReindexTable(const RangeVar *relation, const ReindexParams *params, bool isTopLe
 		ReindexParams newparams = *params;
 
 		newparams.options |= REINDEXOPT_REPORT_PROGRESS;
-		result = reindex_relation(heapOid,
+		result = reindex_relation(stmt, heapOid,
 								  REINDEX_REL_PROCESS_TOAST |
 								  REINDEX_REL_CHECK_CONSTRAINTS,
 								  &newparams);
@@ -2943,7 +2946,7 @@ ReindexTable(const RangeVar *relation, const ReindexParams *params, bool isTopLe
  * That means this must not be called within a user transaction block!
  */
 static void
-ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind,
+ReindexMultipleTables(const ReindexStmt *stmt, const char *objectName, ReindexObjectType objectKind,
 					  const ReindexParams *params)
 {
 	Oid			objectOid;
@@ -3152,7 +3155,7 @@ ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind,
 	 * Process each relation listed in a separate transaction.  Note that this
 	 * commits and then starts a new transaction immediately.
 	 */
-	ReindexMultipleInternal(relids, params);
+	ReindexMultipleInternal(stmt, relids, params);
 
 	MemoryContextDelete(private_context);
 }
@@ -3182,7 +3185,7 @@ reindex_error_callback(void *arg)
  * by the caller.
  */
 static void
-ReindexPartitions(Oid relid, const ReindexParams *params, bool isTopLevel)
+ReindexPartitions(const ReindexStmt *stmt, Oid relid, const ReindexParams *params, bool isTopLevel)
 {
 	List	   *partitions = NIL;
 	char		relkind = get_rel_relkind(relid);
@@ -3258,7 +3261,7 @@ ReindexPartitions(Oid relid, const ReindexParams *params, bool isTopLevel)
 	 * Process each partition listed in a separate transaction.  Note that
 	 * this commits and then starts a new transaction immediately.
 	 */
-	ReindexMultipleInternal(partitions, params);
+	ReindexMultipleInternal(stmt, partitions, params);
 
 	/*
 	 * Clean up working storage --- note we must do this after
@@ -3276,7 +3279,7 @@ ReindexPartitions(Oid relid, const ReindexParams *params, bool isTopLevel)
  * and starts a new transaction when finished.
  */
 static void
-ReindexMultipleInternal(const List *relids, const ReindexParams *params)
+ReindexMultipleInternal(const ReindexStmt *stmt, const List *relids, const ReindexParams *params)
 {
 	ListCell   *l;
 
@@ -3335,7 +3338,7 @@ ReindexMultipleInternal(const List *relids, const ReindexParams *params)
 			ReindexParams newparams = *params;
 
 			newparams.options |= REINDEXOPT_MISSING_OK;
-			(void) ReindexRelationConcurrently(relid, &newparams);
+			(void) ReindexRelationConcurrently(stmt, relid, &newparams);
 			/* ReindexRelationConcurrently() does the verbose output */
 		}
 		else if (relkind == RELKIND_INDEX)
@@ -3344,7 +3347,7 @@ ReindexMultipleInternal(const List *relids, const ReindexParams *params)
 
 			newparams.options |=
 				REINDEXOPT_REPORT_PROGRESS | REINDEXOPT_MISSING_OK;
-			reindex_index(relid, false, relpersistence, &newparams);
+			reindex_index(stmt, relid, false, relpersistence, &newparams);
 			PopActiveSnapshot();
 			/* reindex_index() does the verbose output */
 		}
@@ -3355,7 +3358,7 @@ ReindexMultipleInternal(const List *relids, const ReindexParams *params)
 
 			newparams.options |=
 				REINDEXOPT_REPORT_PROGRESS | REINDEXOPT_MISSING_OK;
-			result = reindex_relation(relid,
+			result = reindex_relation(stmt, relid,
 									  REINDEX_REL_PROCESS_TOAST |
 									  REINDEX_REL_CHECK_CONSTRAINTS,
 									  &newparams);
@@ -3400,7 +3403,7 @@ ReindexMultipleInternal(const List *relids, const ReindexParams *params)
  * anyway, and a non-concurrent reindex is more efficient.
  */
 static bool
-ReindexRelationConcurrently(Oid relationOid, const ReindexParams *params)
+ReindexRelationConcurrently(const ReindexStmt *stmt, Oid relationOid, const ReindexParams *params)
 {
 	typedef struct ReindexIndexInfo
 	{
@@ -3812,6 +3815,10 @@ ReindexRelationConcurrently(Oid relationOid, const ReindexParams *params)
 
 		newIndexIds = lappend(newIndexIds, newidx);
 
+		/* Add the index to event trigger */
+		if (stmt)
+			reindex_event_trigger_collect(newIndexId, stmt);
+
 		/*
 		 * Save lockrelid to protect each relation from drop then close
 		 * relations. The lockrelid on parent relation is not taken here to
@@ -4415,3 +4422,16 @@ set_indexsafe_procflags(void)
 	ProcGlobal->statusFlags[MyProc->pgxactoff] = MyProc->statusFlags;
 	LWLockRelease(ProcArrayLock);
 }
+
+static void
+reindex_event_trigger_collect(Oid oid, const ReindexStmt *stmt)
+{
+	ObjectAddress address;
+
+	address.classId = RelationRelationId;
+	address.objectId = oid;
+	address.objectSubId = 0;
+
+	EventTriggerCollectSimpleCommand(address,
+									 InvalidObjectAddress, (Node *) stmt);
+}
\ No newline at end of file
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 323d9bf8..85ee7c63 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -2172,7 +2172,7 @@ ExecuteTruncateGuts(List *explicit_rels,
 			/*
 			 * Reconstruct the indexes to match, and we're done.
 			 */
-			reindex_relation(heap_relid, REINDEX_REL_PROCESS_TOAST,
+			reindex_relation(NULL, heap_relid, REINDEX_REL_PROCESS_TOAST,
 							 &reindex_params);
 		}
 
diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c
index e3ccf6c7..a269b5b6 100644
--- a/src/backend/tcop/utility.c
+++ b/src/backend/tcop/utility.c
@@ -961,7 +961,9 @@ standard_ProcessUtility(PlannedStmt *pstmt,
 			break;
 
 		case T_ReindexStmt:
-			ExecReindex(pstate, (ReindexStmt *) parsetree, isTopLevel);
+			ProcessUtilitySlow(pstate, pstmt, queryString,
+							   context, params, queryEnv,
+							   dest, qc);
 			break;
 
 			/*
@@ -1574,6 +1576,12 @@ ProcessUtilitySlow(ParseState *pstate,
 				}
 				break;
 
+			case T_ReindexStmt:
+				ExecReindex(pstate, (ReindexStmt *) parsetree, isTopLevel);
+				/* no commands stashed for REINDEX */
+				commandCollected = true;
+				break;
+
 			case T_CreateExtensionStmt:
 				address = CreateExtension(pstate, (CreateExtensionStmt *) parsetree);
 				break;
@@ -3620,7 +3628,7 @@ GetCommandLogLevel(Node *parsetree)
 			break;
 
 		case T_ReindexStmt:
-			lev = LOGSTMT_ALL;	/* should this be DDL? */
+			lev = LOGSTMT_DDL;
 			break;
 
 		case T_CreateConversionStmt:
diff --git a/src/include/catalog/index.h b/src/include/catalog/index.h
index a4770eaf..e1272662 100644
--- a/src/include/catalog/index.h
+++ b/src/include/catalog/index.h
@@ -149,7 +149,7 @@ extern void index_set_state_flags(Oid indexId, IndexStateFlagsAction action);
 
 extern Oid	IndexGetRelation(Oid indexId, bool missing_ok);
 
-extern void reindex_index(Oid indexId, bool skip_constraint_checks,
+extern void reindex_index(const ReindexStmt *stmt, Oid indexId, bool skip_constraint_checks,
 						  char persistence, const ReindexParams *params);
 
 /* Flag bits for reindex_relation(): */
@@ -159,7 +159,7 @@ extern void reindex_index(Oid indexId, bool skip_constraint_checks,
 #define REINDEX_REL_FORCE_INDEXES_UNLOGGED	0x08
 #define REINDEX_REL_FORCE_INDEXES_PERMANENT 0x10
 
-extern bool reindex_relation(Oid relid, int flags, const ReindexParams *params);
+extern bool reindex_relation(const ReindexStmt *stmt, Oid relid, int flags, const ReindexParams *params);
 
 extern bool ReindexIsProcessingHeap(Oid heapOid);
 extern bool ReindexIsProcessingIndex(Oid indexOid);
diff --git a/src/include/tcop/cmdtaglist.h b/src/include/tcop/cmdtaglist.h
index 553a3187..320ee915 100644
--- a/src/include/tcop/cmdtaglist.h
+++ b/src/include/tcop/cmdtaglist.h
@@ -194,7 +194,7 @@ PG_CMDTAG(CMDTAG_PREPARE, "PREPARE", false, false, false)
 PG_CMDTAG(CMDTAG_PREPARE_TRANSACTION, "PREPARE TRANSACTION", false, false, false)
 PG_CMDTAG(CMDTAG_REASSIGN_OWNED, "REASSIGN OWNED", false, false, false)
 PG_CMDTAG(CMDTAG_REFRESH_MATERIALIZED_VIEW, "REFRESH MATERIALIZED VIEW", true, false, false)
-PG_CMDTAG(CMDTAG_REINDEX, "REINDEX", false, false, false)
+PG_CMDTAG(CMDTAG_REINDEX, "REINDEX", true, false, false)
 PG_CMDTAG(CMDTAG_RELEASE, "RELEASE", false, false, false)
 PG_CMDTAG(CMDTAG_RESET, "RESET", false, false, false)
 PG_CMDTAG(CMDTAG_REVOKE, "REVOKE", true, false, false)
diff --git a/src/test/regress/expected/event_trigger.out b/src/test/regress/expected/event_trigger.out
index 0b87a42d..1f0f4aad 100644
--- a/src/test/regress/expected/event_trigger.out
+++ b/src/test/regress/expected/event_trigger.out
@@ -556,6 +556,87 @@ ERROR:  cannot alter type "rewritetype" because column "rewritemetoo3.a" uses it
 drop table rewriteme;
 drop event trigger no_rewrite_allowed;
 drop function test_evtrig_no_rewrite();
+--reindex command, event trigger test setup.
+DROP SCHEMA IF EXISTS schema_to_reindex CASCADE;
+NOTICE:  schema "schema_to_reindex" does not exist, skipping
+CREATE SCHEMA schema_to_reindex;
+SET search_path TO schema_to_reindex;
+CREATE TABLE concur_reindex_tab (c1 int,c2 text);
+CREATE UNIQUE INDEX concur_reindex_ind1 ON concur_reindex_tab (c1);
+CREATE INDEX concur_reindex_ind2 ON concur_reindex_tab (c2);
+CREATE INDEX concur_reindex_ind4 ON concur_reindex_tab (c1, c1, c2);
+ALTER TABLE concur_reindex_tab ADD PRIMARY KEY USING INDEX concur_reindex_ind1;
+INSERT INTO concur_reindex_tab VALUES (1, 'a'),(2, 'a');
+CREATE TABLE parted_irreg_ancestor (b text,a int) PARTITION BY RANGE (b);
+CREATE TABLE parted_irreg (a int,b text) PARTITION BY RANGE (b);
+ALTER TABLE parted_irreg_ancestor ATTACH PARTITION parted_irreg
+FOR VALUES FROM ('aaaa') TO ('zzzz');
+CREATE TABLE parted1_irreg1 (b text NOT NULL,a int);
+ALTER TABLE parted_irreg ATTACH PARTITION parted1_irreg1
+FOR VALUES FROM ('aaaa') TO ('jjjj');
+CREATE TABLE parted1_irreg2 (b text NOT NULL, a int);
+ALTER TABLE parted_irreg ATTACH PARTITION parted1_irreg2
+FOR VALUES FROM ('jjjj') TO ('zzzz');
+INSERT INTO parted_irreg_ancestor (b)
+VALUES ('daasvog'),('asdhjksd'),('sssdjk'),('jssdjk');
+ALTER TABLE parted_irreg_ancestor ADD PRIMARY KEY (b);
+CREATE OR REPLACE FUNCTION public.reindex_start_command()
+RETURNS event_trigger AS $$
+BEGIN
+    RAISE NOTICE 'ddl_start_command -- REINDEX: % %', tg_event, tg_tag;
+END;
+$$ LANGUAGE plpgsql;
+CREATE EVENT TRIGGER start_reindex_command ON ddl_command_start
+    WHEN TAG IN ('REINDEX') EXECUTE PROCEDURE public.reindex_start_command();
+CREATE OR REPLACE FUNCTION public.reindex_end_command()
+RETURNS event_trigger AS $$
+DECLARE
+    obj record;
+BEGIN
+FOR obj IN SELECT * FROM pg_event_trigger_ddl_commands()
+    LOOP
+        if  obj.schema_name = 'pg_toast' then
+            RAISE NOTICE 'reindexing toast related index! object_type: %, schema_name: %'
+                        ,obj.object_type, obj.schema_name;
+        else
+            RAISE NOTICE 'ddl_end_command -- REINDEX: %', pg_get_indexdef(obj.objid);
+        end if;
+    END LOOP;
+END;
+$$ LANGUAGE plpgsql;
+CREATE EVENT TRIGGER end_reindex_command ON ddl_command_end
+    WHEN TAG IN ('REINDEX') EXECUTE PROCEDURE public.reindex_end_command();
+REINDEX (CONCURRENTLY) INDEX  parted_irreg_ancestor_pkey;
+NOTICE:  ddl_start_command -- REINDEX: ddl_command_start REINDEX
+NOTICE:  ddl_end_command -- REINDEX: CREATE UNIQUE INDEX parted1_irreg1_pkey ON schema_to_reindex.parted1_irreg1 USING btree (b)
+NOTICE:  ddl_end_command -- REINDEX: CREATE UNIQUE INDEX parted1_irreg2_pkey ON schema_to_reindex.parted1_irreg2 USING btree (b)
+ALTER EVENT TRIGGER end_reindex_command DISABLE;
+REINDEX (CONCURRENTLY) TABLE  parted_irreg_ancestor; --reindex_start_command part will invoke, reindex_end_command won't.
+NOTICE:  ddl_start_command -- REINDEX: ddl_command_start REINDEX
+ALTER EVENT TRIGGER end_reindex_command ENABLE;
+REINDEX (CONCURRENTLY) TABLE  parted_irreg_ancestor;  --reindex partitioned table
+NOTICE:  ddl_start_command -- REINDEX: ddl_command_start REINDEX
+NOTICE:  ddl_end_command -- REINDEX: CREATE UNIQUE INDEX parted1_irreg1_pkey ON schema_to_reindex.parted1_irreg1 USING btree (b)
+NOTICE:  reindexing toast related index! object_type: index, schema_name: pg_toast
+NOTICE:  ddl_end_command -- REINDEX: CREATE UNIQUE INDEX parted1_irreg2_pkey ON schema_to_reindex.parted1_irreg2 USING btree (b)
+NOTICE:  reindexing toast related index! object_type: index, schema_name: pg_toast
+REINDEX (CONCURRENTLY) SCHEMA  schema_to_reindex;  --should cover reindex most cases.
+NOTICE:  ddl_start_command -- REINDEX: ddl_command_start REINDEX
+NOTICE:  ddl_end_command -- REINDEX: CREATE UNIQUE INDEX concur_reindex_ind1 ON schema_to_reindex.concur_reindex_tab USING btree (c1)
+NOTICE:  ddl_end_command -- REINDEX: CREATE INDEX concur_reindex_ind2 ON schema_to_reindex.concur_reindex_tab USING btree (c2)
+NOTICE:  ddl_end_command -- REINDEX: CREATE INDEX concur_reindex_ind4 ON schema_to_reindex.concur_reindex_tab USING btree (c1, c1, c2)
+NOTICE:  reindexing toast related index! object_type: index, schema_name: pg_toast
+NOTICE:  ddl_end_command -- REINDEX: CREATE UNIQUE INDEX parted1_irreg1_pkey ON schema_to_reindex.parted1_irreg1 USING btree (b)
+NOTICE:  reindexing toast related index! object_type: index, schema_name: pg_toast
+NOTICE:  ddl_end_command -- REINDEX: CREATE UNIQUE INDEX parted1_irreg2_pkey ON schema_to_reindex.parted1_irreg2 USING btree (b)
+NOTICE:  reindexing toast related index! object_type: index, schema_name: pg_toast
+drop event trigger if exists  start_reindex_command,end_reindex_command;
+drop function if exists  public.reindex_end_command, public.reindex_start_command cascade;
+drop schema if exists schema_to_reindex cascade;
+NOTICE:  drop cascades to 2 other objects
+DETAIL:  drop cascades to table concur_reindex_tab
+drop cascades to table parted_irreg_ancestor
+RESET search_path;
 -- test Row Security Event Trigger
 RESET SESSION AUTHORIZATION;
 CREATE TABLE event_trigger_test (a integer, b text);
diff --git a/src/test/regress/sql/event_trigger.sql b/src/test/regress/sql/event_trigger.sql
index 6f0933b9..fbe1a858 100644
--- a/src/test/regress/sql/event_trigger.sql
+++ b/src/test/regress/sql/event_trigger.sql
@@ -418,6 +418,77 @@ drop table rewriteme;
 drop event trigger no_rewrite_allowed;
 drop function test_evtrig_no_rewrite();
 
+--reindex command, event trigger test setup.
+DROP SCHEMA IF EXISTS schema_to_reindex CASCADE;
+CREATE SCHEMA schema_to_reindex;
+SET search_path TO schema_to_reindex;
+
+CREATE TABLE concur_reindex_tab (c1 int,c2 text);
+CREATE UNIQUE INDEX concur_reindex_ind1 ON concur_reindex_tab (c1);
+CREATE INDEX concur_reindex_ind2 ON concur_reindex_tab (c2);
+CREATE INDEX concur_reindex_ind4 ON concur_reindex_tab (c1, c1, c2);
+ALTER TABLE concur_reindex_tab ADD PRIMARY KEY USING INDEX concur_reindex_ind1;
+INSERT INTO concur_reindex_tab VALUES (1, 'a'),(2, 'a');
+
+CREATE TABLE parted_irreg_ancestor (b text,a int) PARTITION BY RANGE (b);
+CREATE TABLE parted_irreg (a int,b text) PARTITION BY RANGE (b);
+
+ALTER TABLE parted_irreg_ancestor ATTACH PARTITION parted_irreg
+FOR VALUES FROM ('aaaa') TO ('zzzz');
+
+CREATE TABLE parted1_irreg1 (b text NOT NULL,a int);
+ALTER TABLE parted_irreg ATTACH PARTITION parted1_irreg1
+FOR VALUES FROM ('aaaa') TO ('jjjj');
+
+CREATE TABLE parted1_irreg2 (b text NOT NULL, a int);
+ALTER TABLE parted_irreg ATTACH PARTITION parted1_irreg2
+FOR VALUES FROM ('jjjj') TO ('zzzz');
+
+INSERT INTO parted_irreg_ancestor (b)
+VALUES ('daasvog'),('asdhjksd'),('sssdjk'),('jssdjk');
+
+ALTER TABLE parted_irreg_ancestor ADD PRIMARY KEY (b);
+
+CREATE OR REPLACE FUNCTION public.reindex_start_command()
+RETURNS event_trigger AS $$
+BEGIN
+    RAISE NOTICE 'ddl_start_command -- REINDEX: % %', tg_event, tg_tag;
+END;
+$$ LANGUAGE plpgsql;
+CREATE EVENT TRIGGER start_reindex_command ON ddl_command_start
+    WHEN TAG IN ('REINDEX') EXECUTE PROCEDURE public.reindex_start_command();
+
+CREATE OR REPLACE FUNCTION public.reindex_end_command()
+RETURNS event_trigger AS $$
+DECLARE
+    obj record;
+BEGIN
+FOR obj IN SELECT * FROM pg_event_trigger_ddl_commands()
+    LOOP
+        if  obj.schema_name = 'pg_toast' then
+            RAISE NOTICE 'reindexing toast related index! object_type: %, schema_name: %'
+                        ,obj.object_type, obj.schema_name;
+        else
+            RAISE NOTICE 'ddl_end_command -- REINDEX: %', pg_get_indexdef(obj.objid);
+        end if;
+    END LOOP;
+END;
+$$ LANGUAGE plpgsql;
+CREATE EVENT TRIGGER end_reindex_command ON ddl_command_end
+    WHEN TAG IN ('REINDEX') EXECUTE PROCEDURE public.reindex_end_command();
+
+REINDEX (CONCURRENTLY) INDEX  parted_irreg_ancestor_pkey;
+ALTER EVENT TRIGGER end_reindex_command DISABLE;
+REINDEX (CONCURRENTLY) TABLE  parted_irreg_ancestor; --reindex_start_command part will invoke, reindex_end_command won't.
+ALTER EVENT TRIGGER end_reindex_command ENABLE;
+REINDEX (CONCURRENTLY) TABLE  parted_irreg_ancestor;  --reindex partitioned table
+REINDEX (CONCURRENTLY) SCHEMA  schema_to_reindex;  --should cover reindex most cases.
+
+drop event trigger if exists  start_reindex_command,end_reindex_command;
+drop function if exists  public.reindex_end_command, public.reindex_start_command cascade;
+drop schema if exists schema_to_reindex cascade;
+RESET search_path;
+
 -- test Row Security Event Trigger
 RESET SESSION AUTHORIZATION;
 CREATE TABLE event_trigger_test (a integer, b text);
-- 
2.34.1

#18Michael Paquier
michael@paquier.xyz
In reply to: jian he (#17)
Re: PATCH: Add REINDEX tag to event triggers

On Fri, Nov 24, 2023 at 08:00:00AM +0800, jian he wrote:

not sure this is the right approach. But now the reindex event
collection is after we call index_open.
same static function (reindex_event_trigger_collect) in
src/backend/commands/indexcmds.c and src/backend/catalog/index.c for
now.

Knowing that there are two calls of reindex_relation() and four
callers of reindex_index(), passing the stmt looks acceptable to me to
be able to get more context from the reindex statement where a given
index has been rebuilt.

Anyway, as a whole, I am still confused by the shape of the patch..
This relies on pg_event_trigger_ddl_commands(), that reports a list of
DDL commands, but the use case you are proposing is to report a list
of the *indexes* rebuilt. So, in its current shape, we'd report the
equivalent of N DDL commands matching the N indexes rebuilt in a
single command. Shouldn't we use a new event type and a dedicated SQL
function to report the list of indexes newly rebuilt to get a full
list of the work that has happened?

given that ProcessUtilitySlow supports the event triggers facility.
so probably no need for another REINDEXOPT_ option?

Right, perhaps we don't need that if we just supply the stmt. If
there is no requirement for it, let's avoid that, then.

-     ReindexMultipleTables(stmt->name, stmt->kind, &params);
+     ReindexMultipleTables(stmt, stmt->name, stmt->kind, &params);

If we begin including the stmt, there is no need for these extra
arguments in the extra routines of indexcmds.c.

As far as I can see, this patch is doing too much as presented. Could
you split the patch into more pieces, please? Based on v4 you have
sent, there are refactoring and basic piece parts like:
- Patch to make event triggers ddl_command_start and ddl_command_stop
react on ReindexStmt, so as a command is reported in
pg_event_trigger_ddl_commands().
- Patch for GetCommandLogLevel() to switch ReindexStmt from
LOGSTMT_ALL to LOGSTMT_DDL. True that this could be switched.
- Patch to refactor the routines of indexcmds.c and index.c to use the
ReindexStmt as argument, as a preparation of the next patch...
- Patch to add a new event type with a new SQL function to return a
list of the indexes rebuilt, with the ReindexStmt involved when the
index OID was trapped by the collection.

1) and 2) are a minimal feature in themselves, which may be enough to
satisfy your original case, and where you'd know when a REINDEX has
been run in event triggers. 3) and 4) are what you are trying to
achieve, to get the list of the indexes rebuilt knowing the context of
a given command.
--
Michael

#19jian he
jian.universality@gmail.com
In reply to: Michael Paquier (#18)
4 attachment(s)
Re: PATCH: Add REINDEX tag to event triggers

On Fri, Nov 24, 2023 at 10:44 AM Michael Paquier <michael@paquier.xyz> wrote:

As far as I can see, this patch is doing too much as presented. Could
you split the patch into more pieces, please? Based on v4 you have
sent, there are refactoring and basic piece parts like:
- Patch to make event triggers ddl_command_start and ddl_command_stop
react on ReindexStmt, so as a command is reported in
pg_event_trigger_ddl_commands().
- Patch for GetCommandLogLevel() to switch ReindexStmt from
LOGSTMT_ALL to LOGSTMT_DDL. True that this could be switched.
- Patch to refactor the routines of indexcmds.c and index.c to use the
ReindexStmt as argument, as a preparation of the next patch...
- Patch to add a new event type with a new SQL function to return a
list of the indexes rebuilt, with the ReindexStmt involved when the
index OID was trapped by the collection.

1) and 2) are a minimal feature in themselves, which may be enough to
satisfy your original case, and where you'd know when a REINDEX has
been run in event triggers. 3) and 4) are what you are trying to
achieve, to get the list of the indexes rebuilt knowing the context of
a given command.
--
Michael

hi.
v5-0001. changed the REINDEX command tag from event_trigger_ok: false
to event_trigger_ok: true.
Move ReindexStmt moves from standard_ProcessUtility to ProcessUtilitySlow.
By default ProcessUtilitySlow will call trigger related functions.
So the event trigger facility can support reindex statements.

v5-0002. In GetCommandLogLevel, change T_ReindexStmt from lev =
LOGSTMT_ALL to lev = LOGSTMT_DDL. so log_statement (enum) >= 'ddl'
will make the reindex statement be logged.

v5-0003. Refactor the following functions: {ReindexIndex,
ReindexTable, ReindexMultipleTables,
ReindexPartitions,ReindexMultipleInternal
,ReindexRelationConcurrently, reindex_relation,reindex_index} by
adding `const ReindexStmt *stmt` as their first argument.
This is for event trigger support reindex. We need to pass both the
newly generated indexId and the ReindexStmt to
EventTriggerCollectSimpleCommand right after the newly index gets
their lock. To do that, we have to refactor related functions.

v5-0004. Event trigger support reindex command implementation,
documentation, regress test, helper function pass reindex info to
function EventTriggerCollectSimpleCommand.

Attachments:

v5-0002-tag-ReindexStmt-as-ddl-in-GetCommandLogLevel.patchtext/x-patch; charset=US-ASCII; name=v5-0002-tag-ReindexStmt-as-ddl-in-GetCommandLogLevel.patchDownload
From a72aaf4dcc2ae70766f5380855e7013035c63d9c Mon Sep 17 00:00:00 2001
From: pgaddict <jian.universality@gmail.com>
Date: Fri, 24 Nov 2023 20:44:44 +0800
Subject: [PATCH v5 2/4] tag ReindexStmt as ddl in GetCommandLogLevel.

In GetCommandLogLevel, change T_ReindexStmt from lev = LOGSTMT_ALL to lev = LOGSTMT_DDL.
so log_statement (enum) >= 'ddl' will make the reindex statement be logged.
---
 src/backend/tcop/utility.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c
index 5a7f63cb..a269b5b6 100644
--- a/src/backend/tcop/utility.c
+++ b/src/backend/tcop/utility.c
@@ -3628,7 +3628,7 @@ GetCommandLogLevel(Node *parsetree)
 			break;
 
 		case T_ReindexStmt:
-			lev = LOGSTMT_ALL;	/* should this be DDL? */
+			lev = LOGSTMT_DDL;
 			break;
 
 		case T_CreateConversionStmt:
-- 
2.34.1

v5-0004-Event-trigger-support-reindex-command.patchtext/x-patch; charset=US-ASCII; name=v5-0004-Event-trigger-support-reindex-command.patchDownload
From 864f53c932cd2b692b5042bbfddc021d5e49ef0c Mon Sep 17 00:00:00 2001
From: pgaddict <jian.universality@gmail.com>
Date: Sat, 25 Nov 2023 09:28:07 +0800
Subject: [PATCH v5 4/4] Event trigger support reindex command. this add
 documentation, regress test, static function pass reindex info to function
 EventTriggerCollectSimpleCommand.

---
 doc/src/sgml/event-trigger.sgml             |   8 ++
 src/backend/catalog/index.c                 |   5 +-
 src/backend/commands/indexcmds.c            |  17 ++++
 src/test/regress/expected/event_trigger.out | 103 ++++++++++++++++++++
 src/test/regress/sql/event_trigger.sql      |  93 ++++++++++++++++++
 5 files changed, 225 insertions(+), 1 deletion(-)

diff --git a/doc/src/sgml/event-trigger.sgml b/doc/src/sgml/event-trigger.sgml
index 10b20f03..234b4ffd 100644
--- a/doc/src/sgml/event-trigger.sgml
+++ b/doc/src/sgml/event-trigger.sgml
@@ -1032,6 +1032,14 @@
         <entry align="center"><literal>-</literal></entry>
         <entry align="left"></entry>
        </row>
+       <row>
+        <entry align="left"><literal>REINDEX</literal></entry>
+        <entry align="center"><literal>X</literal></entry>
+        <entry align="center"><literal>X</literal></entry>
+        <entry align="center"><literal>-</literal></entry>
+        <entry align="center"><literal>-</literal></entry>
+        <entry align="left"></entry>
+       </row>
        <row>
         <entry align="left"><literal>REVOKE</literal></entry>
         <entry align="center"><literal>X</literal></entry>
diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c
index fc09191c..2afabdeb 100644
--- a/src/backend/catalog/index.c
+++ b/src/backend/catalog/index.c
@@ -136,7 +136,7 @@ static void SetReindexProcessing(Oid heapOid, Oid indexOid);
 static void ResetReindexProcessing(void);
 static void SetReindexPending(List *indexes);
 static void RemoveReindexPending(Oid indexOid);
-
+static void reindex_event_trigger_collect(Oid oid, const ReindexStmt *stmt);
 
 /*
  * relationHasPrimaryKey
@@ -3642,6 +3642,9 @@ reindex_index(const ReindexStmt *stmt, Oid indexId, bool skip_constraint_checks,
 	if (progress)
 		pgstat_progress_update_param(PROGRESS_CREATEIDX_ACCESS_METHOD_OID,
 									 iRel->rd_rel->relam);
+	/* event trigger tracking REINDEX primary pointer */
+	if (stmt)
+		reindex_event_trigger_collect(indexId, stmt);
 
 	/*
 	 * Partitioned indexes should never get processed here, as they have no
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index f63647e3..cb31767d 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -3817,6 +3817,10 @@ ReindexRelationConcurrently(const ReindexStmt *stmt, Oid relationOid, const Rein
 
 		newIndexIds = lappend(newIndexIds, newidx);
 
+		/* Add the index to event trigger */
+		if (stmt)
+			reindex_event_trigger_collect(newIndexId, stmt);
+
 		/*
 		 * Save lockrelid to protect each relation from drop then close
 		 * relations. The lockrelid on parent relation is not taken here to
@@ -4420,3 +4424,16 @@ set_indexsafe_procflags(void)
 	ProcGlobal->statusFlags[MyProc->pgxactoff] = MyProc->statusFlags;
 	LWLockRelease(ProcArrayLock);
 }
+
+static void
+reindex_event_trigger_collect(Oid oid, const ReindexStmt *stmt)
+{
+	ObjectAddress address;
+
+	address.classId = RelationRelationId;
+	address.objectId = oid;
+	address.objectSubId = 0;
+
+	EventTriggerCollectSimpleCommand(address,
+									 InvalidObjectAddress, (Node *) stmt);
+}
\ No newline at end of file
diff --git a/src/test/regress/expected/event_trigger.out b/src/test/regress/expected/event_trigger.out
index 0b87a42d..5dfb2ee1 100644
--- a/src/test/regress/expected/event_trigger.out
+++ b/src/test/regress/expected/event_trigger.out
@@ -556,6 +556,109 @@ ERROR:  cannot alter type "rewritetype" because column "rewritemetoo3.a" uses it
 drop table rewriteme;
 drop event trigger no_rewrite_allowed;
 drop function test_evtrig_no_rewrite();
+--reindex command, event trigger test setup.
+DROP SCHEMA IF EXISTS schema_to_reindex CASCADE;
+NOTICE:  schema "schema_to_reindex" does not exist, skipping
+CREATE SCHEMA schema_to_reindex;
+SET search_path TO schema_to_reindex;
+CREATE TABLE concur_reindex_tab (c1 int,c2 text);
+CREATE UNIQUE INDEX concur_reindex_ind1 ON concur_reindex_tab (c1);
+CREATE INDEX concur_reindex_ind2 ON concur_reindex_tab (c2);
+CREATE INDEX concur_reindex_ind4 ON concur_reindex_tab (c1, c1, c2);
+ALTER TABLE concur_reindex_tab ADD PRIMARY KEY USING INDEX concur_reindex_ind1;
+INSERT INTO concur_reindex_tab VALUES (1, 'a'),(2, 'a');
+CREATE TABLE parted_irreg_ancestor (b text,a int) PARTITION BY RANGE (b);
+CREATE TABLE parted_irreg (a int,b text) PARTITION BY RANGE (b);
+ALTER TABLE parted_irreg_ancestor ATTACH PARTITION parted_irreg
+FOR VALUES FROM ('aaaa') TO ('zzzz');
+CREATE TABLE parted1_irreg1 (b text NOT NULL,a int);
+ALTER TABLE parted_irreg ATTACH PARTITION parted1_irreg1
+FOR VALUES FROM ('aaaa') TO ('jjjj');
+CREATE TABLE parted1_irreg2 (b text NOT NULL, a int);
+ALTER TABLE parted_irreg ATTACH PARTITION parted1_irreg2
+FOR VALUES FROM ('jjjj') TO ('zzzz');
+INSERT INTO parted_irreg_ancestor (b)
+VALUES ('daasvog'),('asdhjksd'),('sssdjk'),('jssdjk');
+ALTER TABLE parted_irreg_ancestor ADD PRIMARY KEY (b);
+CREATE OR REPLACE FUNCTION public.reindex_start_command()
+RETURNS event_trigger AS $$
+BEGIN
+    RAISE NOTICE 'ddl_start_command -- REINDEX: % %', tg_event, tg_tag;
+END;
+$$ LANGUAGE plpgsql;
+CREATE EVENT TRIGGER start_reindex_command ON ddl_command_start
+    WHEN TAG IN ('REINDEX') EXECUTE PROCEDURE public.reindex_start_command();
+CREATE OR REPLACE FUNCTION public.reindex_end_command()
+RETURNS event_trigger AS $$
+DECLARE
+	obj record;
+	toast_main_table text;
+BEGIN
+FOR obj IN SELECT * FROM pg_event_trigger_ddl_commands()
+    LOOP
+        IF  obj.schema_name = 'pg_toast' THEN
+            RAISE NOTICE 'reindexing toast related index! object_type: %, schema_name: %'
+                          ,obj.object_type, obj.schema_name;
+            /* get the toast table will be reindexed main table.
+             * toast table name auto generated, cannot use to test.
+            */
+            SELECT t1.relname into toast_main_table
+            FROM (
+                SELECT t1.oid
+                FROM pg_class t1 INNER JOIN pg_index t2 ON t1.oid = t2.indrelid
+                WHERE t2.indexrelid = obj.objid AND relkind = 't') sub
+                INNER JOIN pg_class t1 ON t1.reltoastrelid = sub.oid;
+
+            RAISE NOTICE 'toast table related main relation: %', toast_main_table;
+        ELSE
+            RAISE NOTICE 'ddl_end_command -- REINDEX: %', pg_get_indexdef(obj.objid);
+        END IF;
+    END LOOP;
+END;
+$$ LANGUAGE plpgsql;
+CREATE EVENT TRIGGER end_reindex_command ON ddl_command_end
+    WHEN TAG IN ('REINDEX') EXECUTE PROCEDURE public.reindex_end_command();
+REINDEX (CONCURRENTLY) INDEX  parted_irreg_ancestor_pkey;
+NOTICE:  ddl_start_command -- REINDEX: ddl_command_start REINDEX
+NOTICE:  ddl_end_command -- REINDEX: CREATE UNIQUE INDEX parted1_irreg1_pkey ON schema_to_reindex.parted1_irreg1 USING btree (b)
+NOTICE:  ddl_end_command -- REINDEX: CREATE UNIQUE INDEX parted1_irreg2_pkey ON schema_to_reindex.parted1_irreg2 USING btree (b)
+ALTER EVENT TRIGGER end_reindex_command DISABLE;
+-- event trigger disabled.
+--so reindex_start_command part will invoke, reindex_end_command won't.
+REINDEX (CONCURRENTLY) TABLE  parted_irreg_ancestor;
+NOTICE:  ddl_start_command -- REINDEX: ddl_command_start REINDEX
+ALTER EVENT TRIGGER end_reindex_command ENABLE;
+REINDEX (CONCURRENTLY) TABLE  parted_irreg_ancestor;
+NOTICE:  ddl_start_command -- REINDEX: ddl_command_start REINDEX
+NOTICE:  ddl_end_command -- REINDEX: CREATE UNIQUE INDEX parted1_irreg1_pkey ON schema_to_reindex.parted1_irreg1 USING btree (b)
+NOTICE:  reindexing toast related index! object_type: index, schema_name: pg_toast
+NOTICE:  toast table related main relation: parted1_irreg1
+NOTICE:  ddl_end_command -- REINDEX: CREATE UNIQUE INDEX parted1_irreg2_pkey ON schema_to_reindex.parted1_irreg2 USING btree (b)
+NOTICE:  reindexing toast related index! object_type: index, schema_name: pg_toast
+NOTICE:  toast table related main relation: parted1_irreg2
+/* REINDEX SCHEMA (CONCURRENTLY) generated test result order unstable,
+ * So just do normal reindex schema. should cover reindex statement most cases.
+*/
+REINDEX SCHEMA  schema_to_reindex;
+NOTICE:  ddl_start_command -- REINDEX: ddl_command_start REINDEX
+NOTICE:  ddl_end_command -- REINDEX: CREATE UNIQUE INDEX concur_reindex_ind1 ON schema_to_reindex.concur_reindex_tab USING btree (c1)
+NOTICE:  ddl_end_command -- REINDEX: CREATE INDEX concur_reindex_ind2 ON schema_to_reindex.concur_reindex_tab USING btree (c2)
+NOTICE:  ddl_end_command -- REINDEX: CREATE INDEX concur_reindex_ind4 ON schema_to_reindex.concur_reindex_tab USING btree (c1, c1, c2)
+NOTICE:  reindexing toast related index! object_type: index, schema_name: pg_toast
+NOTICE:  toast table related main relation: concur_reindex_tab
+NOTICE:  ddl_end_command -- REINDEX: CREATE UNIQUE INDEX parted1_irreg1_pkey ON schema_to_reindex.parted1_irreg1 USING btree (b)
+NOTICE:  reindexing toast related index! object_type: index, schema_name: pg_toast
+NOTICE:  toast table related main relation: parted1_irreg1
+NOTICE:  ddl_end_command -- REINDEX: CREATE UNIQUE INDEX parted1_irreg2_pkey ON schema_to_reindex.parted1_irreg2 USING btree (b)
+NOTICE:  reindexing toast related index! object_type: index, schema_name: pg_toast
+NOTICE:  toast table related main relation: parted1_irreg2
+drop event trigger if exists  start_reindex_command,end_reindex_command;
+drop function if exists  public.reindex_end_command, public.reindex_start_command cascade;
+drop schema if exists schema_to_reindex cascade;
+NOTICE:  drop cascades to 2 other objects
+DETAIL:  drop cascades to table concur_reindex_tab
+drop cascades to table parted_irreg_ancestor
+RESET search_path;
 -- test Row Security Event Trigger
 RESET SESSION AUTHORIZATION;
 CREATE TABLE event_trigger_test (a integer, b text);
diff --git a/src/test/regress/sql/event_trigger.sql b/src/test/regress/sql/event_trigger.sql
index 6f0933b9..ace58308 100644
--- a/src/test/regress/sql/event_trigger.sql
+++ b/src/test/regress/sql/event_trigger.sql
@@ -418,6 +418,99 @@ drop table rewriteme;
 drop event trigger no_rewrite_allowed;
 drop function test_evtrig_no_rewrite();
 
+--reindex command, event trigger test setup.
+DROP SCHEMA IF EXISTS schema_to_reindex CASCADE;
+CREATE SCHEMA schema_to_reindex;
+SET search_path TO schema_to_reindex;
+
+CREATE TABLE concur_reindex_tab (c1 int,c2 text);
+CREATE UNIQUE INDEX concur_reindex_ind1 ON concur_reindex_tab (c1);
+CREATE INDEX concur_reindex_ind2 ON concur_reindex_tab (c2);
+CREATE INDEX concur_reindex_ind4 ON concur_reindex_tab (c1, c1, c2);
+ALTER TABLE concur_reindex_tab ADD PRIMARY KEY USING INDEX concur_reindex_ind1;
+INSERT INTO concur_reindex_tab VALUES (1, 'a'),(2, 'a');
+
+CREATE TABLE parted_irreg_ancestor (b text,a int) PARTITION BY RANGE (b);
+CREATE TABLE parted_irreg (a int,b text) PARTITION BY RANGE (b);
+
+ALTER TABLE parted_irreg_ancestor ATTACH PARTITION parted_irreg
+FOR VALUES FROM ('aaaa') TO ('zzzz');
+
+CREATE TABLE parted1_irreg1 (b text NOT NULL,a int);
+ALTER TABLE parted_irreg ATTACH PARTITION parted1_irreg1
+FOR VALUES FROM ('aaaa') TO ('jjjj');
+
+CREATE TABLE parted1_irreg2 (b text NOT NULL, a int);
+ALTER TABLE parted_irreg ATTACH PARTITION parted1_irreg2
+FOR VALUES FROM ('jjjj') TO ('zzzz');
+
+INSERT INTO parted_irreg_ancestor (b)
+VALUES ('daasvog'),('asdhjksd'),('sssdjk'),('jssdjk');
+
+ALTER TABLE parted_irreg_ancestor ADD PRIMARY KEY (b);
+
+CREATE OR REPLACE FUNCTION public.reindex_start_command()
+RETURNS event_trigger AS $$
+BEGIN
+    RAISE NOTICE 'ddl_start_command -- REINDEX: % %', tg_event, tg_tag;
+END;
+$$ LANGUAGE plpgsql;
+
+CREATE EVENT TRIGGER start_reindex_command ON ddl_command_start
+    WHEN TAG IN ('REINDEX') EXECUTE PROCEDURE public.reindex_start_command();
+
+CREATE OR REPLACE FUNCTION public.reindex_end_command()
+RETURNS event_trigger AS $$
+DECLARE
+	obj record;
+	toast_main_table text;
+BEGIN
+FOR obj IN SELECT * FROM pg_event_trigger_ddl_commands()
+    LOOP
+        IF  obj.schema_name = 'pg_toast' THEN
+            RAISE NOTICE 'reindexing toast related index! object_type: %, schema_name: %'
+                          ,obj.object_type, obj.schema_name;
+            /* get the toast table will be reindexed main table.
+             * toast table name auto generated, cannot use to test.
+            */
+            SELECT t1.relname into toast_main_table
+            FROM (
+                SELECT t1.oid
+                FROM pg_class t1 INNER JOIN pg_index t2 ON t1.oid = t2.indrelid
+                WHERE t2.indexrelid = obj.objid AND relkind = 't') sub
+                INNER JOIN pg_class t1 ON t1.reltoastrelid = sub.oid;
+
+            RAISE NOTICE 'toast table related main relation: %', toast_main_table;
+        ELSE
+            RAISE NOTICE 'ddl_end_command -- REINDEX: %', pg_get_indexdef(obj.objid);
+        END IF;
+    END LOOP;
+END;
+$$ LANGUAGE plpgsql;
+
+CREATE EVENT TRIGGER end_reindex_command ON ddl_command_end
+    WHEN TAG IN ('REINDEX') EXECUTE PROCEDURE public.reindex_end_command();
+
+REINDEX (CONCURRENTLY) INDEX  parted_irreg_ancestor_pkey;
+ALTER EVENT TRIGGER end_reindex_command DISABLE;
+
+-- event trigger disabled.
+--so reindex_start_command part will invoke, reindex_end_command won't.
+REINDEX (CONCURRENTLY) TABLE  parted_irreg_ancestor;
+
+ALTER EVENT TRIGGER end_reindex_command ENABLE;
+REINDEX (CONCURRENTLY) TABLE  parted_irreg_ancestor;
+
+/* REINDEX SCHEMA (CONCURRENTLY) generated test result order unstable,
+ * So just do normal reindex schema. should cover reindex statement most cases.
+*/
+REINDEX SCHEMA  schema_to_reindex;
+
+drop event trigger if exists  start_reindex_command,end_reindex_command;
+drop function if exists  public.reindex_end_command, public.reindex_start_command cascade;
+drop schema if exists schema_to_reindex cascade;
+RESET search_path;
+
 -- test Row Security Event Trigger
 RESET SESSION AUTHORIZATION;
 CREATE TABLE event_trigger_test (a integer, b text);
-- 
2.34.1

v5-0003-refactor-multiple-functions-to-support-tracking-r.patchtext/x-patch; charset=US-ASCII; name=v5-0003-refactor-multiple-functions-to-support-tracking-r.patchDownload
From 463d0ce9e8c5d37f989aaf47cc28194b3375bea2 Mon Sep 17 00:00:00 2001
From: pgaddict <jian.universality@gmail.com>
Date: Fri, 24 Nov 2023 20:52:08 +0800
Subject: [PATCH v5 3/4] refactor multiple functions to support tracking
 reindex using event trigger.

Refactor the following functions:
ReindexIndex,
ReindexTable,
ReindexMultipleTables,
ReindexPartitions,ReindexMultipleInternal,
ReindexRelationConcurrently,
reindex_relation,
reindex_index
by adding `const ReindexStmt *stmt` as their first argument.
---
 src/backend/catalog/index.c      | 21 ++++++++---
 src/backend/commands/cluster.c   |  2 +-
 src/backend/commands/indexcmds.c | 61 +++++++++++++++++---------------
 src/backend/commands/tablecmds.c |  2 +-
 src/include/catalog/index.h      |  4 +--
 5 files changed, 54 insertions(+), 36 deletions(-)

diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c
index 143fae01..fc09191c 100644
--- a/src/backend/catalog/index.c
+++ b/src/backend/catalog/index.c
@@ -3554,11 +3554,24 @@ IndexGetRelation(Oid indexId, bool missing_ok)
 	return result;
 }
 
+static void
+reindex_event_trigger_collect(Oid oid, const ReindexStmt *stmt)
+{
+	ObjectAddress address;
+
+	address.classId = RelationRelationId;
+	address.objectId = oid;
+	address.objectSubId = 0;
+
+	EventTriggerCollectSimpleCommand(address,
+									 InvalidObjectAddress, (Node *) stmt);
+}
+
 /*
  * reindex_index - This routine is used to recreate a single index
  */
 void
-reindex_index(Oid indexId, bool skip_constraint_checks, char persistence,
+reindex_index(const ReindexStmt *stmt, Oid indexId, bool skip_constraint_checks, char persistence,
 			  const ReindexParams *params)
 {
 	Relation	iRel,
@@ -3865,7 +3878,7 @@ reindex_index(Oid indexId, bool skip_constraint_checks, char persistence,
  * index rebuild.
  */
 bool
-reindex_relation(Oid relid, int flags, const ReindexParams *params)
+reindex_relation(const ReindexStmt *stmt, Oid relid, int flags, const ReindexParams *params)
 {
 	Relation	rel;
 	Oid			toast_relid;
@@ -3953,7 +3966,7 @@ reindex_relation(Oid relid, int flags, const ReindexParams *params)
 			continue;
 		}
 
-		reindex_index(indexOid, !(flags & REINDEX_REL_CHECK_CONSTRAINTS),
+		reindex_index(stmt, indexOid, !(flags & REINDEX_REL_CHECK_CONSTRAINTS),
 					  persistence, params);
 
 		CommandCounterIncrement();
@@ -3990,7 +4003,7 @@ reindex_relation(Oid relid, int flags, const ReindexParams *params)
 
 		newparams.options &= ~(REINDEXOPT_MISSING_OK);
 		newparams.tablespaceOid = InvalidOid;
-		result |= reindex_relation(toast_relid, flags, &newparams);
+		result |= reindex_relation(stmt, toast_relid, flags, &newparams);
 	}
 
 	return result;
diff --git a/src/backend/commands/cluster.c b/src/backend/commands/cluster.c
index a3bef6ac..1f52d391 100644
--- a/src/backend/commands/cluster.c
+++ b/src/backend/commands/cluster.c
@@ -1518,7 +1518,7 @@ finish_heap_swap(Oid OIDOldHeap, Oid OIDNewHeap,
 	pgstat_progress_update_param(PROGRESS_CLUSTER_PHASE,
 								 PROGRESS_CLUSTER_PHASE_REBUILD_INDEX);
 
-	reindex_relation(OIDOldHeap, reindex_flags, &reindex_params);
+	reindex_relation(NULL, OIDOldHeap, reindex_flags, &reindex_params);
 
 	/* Report that we are now doing clean up */
 	pgstat_progress_update_param(PROGRESS_CLUSTER_PHASE,
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index 0b3b8e98..f63647e3 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -94,20 +94,20 @@ static char *ChooseIndexName(const char *tabname, Oid namespaceId,
 							 bool primary, bool isconstraint);
 static char *ChooseIndexNameAddition(const List *colnames);
 static List *ChooseIndexColumnNames(const List *indexElems);
-static void ReindexIndex(const RangeVar *indexRelation, const ReindexParams *params,
+static void ReindexIndex(const ReindexStmt *stmt, const ReindexParams *params,
 						 bool isTopLevel);
+static void reindex_event_trigger_collect(Oid oid, const ReindexStmt *stmt);
 static void RangeVarCallbackForReindexIndex(const RangeVar *relation,
 											Oid relId, Oid oldRelId, void *arg);
-static Oid	ReindexTable(const RangeVar *relation, const ReindexParams *params,
+static Oid	ReindexTable(const ReindexStmt *stmt, const ReindexParams *params,
 						 bool isTopLevel);
-static void ReindexMultipleTables(const char *objectName,
-								  ReindexObjectType objectKind, const ReindexParams *params);
+static void ReindexMultipleTables(const ReindexStmt *stmt, const ReindexParams *params);
 static void reindex_error_callback(void *arg);
-static void ReindexPartitions(Oid relid, const ReindexParams *params,
+static void ReindexPartitions(const ReindexStmt *stmt, Oid relid, const ReindexParams *params,
 							  bool isTopLevel);
-static void ReindexMultipleInternal(const List *relids,
+static void ReindexMultipleInternal(const ReindexStmt *stmt, const List *relids,
 									const ReindexParams *params);
-static bool ReindexRelationConcurrently(Oid relationOid,
+static bool ReindexRelationConcurrently(const ReindexStmt *stmt, Oid relationOid,
 										const ReindexParams *params);
 static void update_relispartition(Oid relationId, bool newval);
 static inline void set_indexsafe_procflags(void);
@@ -2729,10 +2729,10 @@ ExecReindex(ParseState *pstate, const ReindexStmt *stmt, bool isTopLevel)
 	switch (stmt->kind)
 	{
 		case REINDEX_OBJECT_INDEX:
-			ReindexIndex(stmt->relation, &params, isTopLevel);
+			ReindexIndex(stmt, &params, isTopLevel);
 			break;
 		case REINDEX_OBJECT_TABLE:
-			ReindexTable(stmt->relation, &params, isTopLevel);
+			ReindexTable(stmt, &params, isTopLevel);
 			break;
 		case REINDEX_OBJECT_SCHEMA:
 		case REINDEX_OBJECT_SYSTEM:
@@ -2748,7 +2748,7 @@ ExecReindex(ParseState *pstate, const ReindexStmt *stmt, bool isTopLevel)
 									  (stmt->kind == REINDEX_OBJECT_SCHEMA) ? "REINDEX SCHEMA" :
 									  (stmt->kind == REINDEX_OBJECT_SYSTEM) ? "REINDEX SYSTEM" :
 									  "REINDEX DATABASE");
-			ReindexMultipleTables(stmt->name, stmt->kind, &params);
+			ReindexMultipleTables(stmt, &params);
 			break;
 		default:
 			elog(ERROR, "unrecognized object type: %d",
@@ -2762,13 +2762,15 @@ ExecReindex(ParseState *pstate, const ReindexStmt *stmt, bool isTopLevel)
  *		Recreate a specific index.
  */
 static void
-ReindexIndex(const RangeVar *indexRelation, const ReindexParams *params, bool isTopLevel)
+ReindexIndex(const ReindexStmt *stmt, const ReindexParams *params, bool isTopLevel)
 {
+	const RangeVar *indexRelation;
 	struct ReindexIndexCallbackState state;
 	Oid			indOid;
 	char		persistence;
 	char		relkind;
 
+	indexRelation = stmt->relation;
 	/*
 	 * Find and lock index, and check permissions on table; use callback to
 	 * obtain lock on table first, to avoid deadlock hazard.  The lock level
@@ -2796,16 +2798,16 @@ ReindexIndex(const RangeVar *indexRelation, const ReindexParams *params, bool is
 	relkind = get_rel_relkind(indOid);
 
 	if (relkind == RELKIND_PARTITIONED_INDEX)
-		ReindexPartitions(indOid, params, isTopLevel);
+		ReindexPartitions(stmt, indOid, params, isTopLevel);
 	else if ((params->options & REINDEXOPT_CONCURRENTLY) != 0 &&
 			 persistence != RELPERSISTENCE_TEMP)
-		ReindexRelationConcurrently(indOid, params);
+		ReindexRelationConcurrently(stmt, indOid, params);
 	else
 	{
 		ReindexParams newparams = *params;
 
 		newparams.options |= REINDEXOPT_REPORT_PROGRESS;
-		reindex_index(indOid, false, persistence, &newparams);
+		reindex_index(stmt, indOid, false, persistence, &newparams);
 	}
 }
 
@@ -2885,11 +2887,12 @@ RangeVarCallbackForReindexIndex(const RangeVar *relation,
  *		Recreate all indexes of a table (and of its toast table, if any)
  */
 static Oid
-ReindexTable(const RangeVar *relation, const ReindexParams *params, bool isTopLevel)
+ReindexTable(const ReindexStmt *stmt, const ReindexParams *params, bool isTopLevel)
 {
 	Oid			heapOid;
 	bool		result;
 
+	const RangeVar *relation = stmt->relation;
 	/*
 	 * The lock level used here should match reindex_relation().
 	 *
@@ -2905,11 +2908,11 @@ ReindexTable(const RangeVar *relation, const ReindexParams *params, bool isTopLe
 									   RangeVarCallbackOwnsTable, NULL);
 
 	if (get_rel_relkind(heapOid) == RELKIND_PARTITIONED_TABLE)
-		ReindexPartitions(heapOid, params, isTopLevel);
+		ReindexPartitions(stmt, heapOid, params, isTopLevel);
 	else if ((params->options & REINDEXOPT_CONCURRENTLY) != 0 &&
 			 get_rel_persistence(heapOid) != RELPERSISTENCE_TEMP)
 	{
-		result = ReindexRelationConcurrently(heapOid, params);
+		result = ReindexRelationConcurrently(stmt, heapOid, params);
 
 		if (!result)
 			ereport(NOTICE,
@@ -2921,7 +2924,7 @@ ReindexTable(const RangeVar *relation, const ReindexParams *params, bool isTopLe
 		ReindexParams newparams = *params;
 
 		newparams.options |= REINDEXOPT_REPORT_PROGRESS;
-		result = reindex_relation(heapOid,
+		result = reindex_relation(stmt, heapOid,
 								  REINDEX_REL_PROCESS_TOAST |
 								  REINDEX_REL_CHECK_CONSTRAINTS,
 								  &newparams);
@@ -2943,9 +2946,9 @@ ReindexTable(const RangeVar *relation, const ReindexParams *params, bool isTopLe
  * That means this must not be called within a user transaction block!
  */
 static void
-ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind,
-					  const ReindexParams *params)
+ReindexMultipleTables(const ReindexStmt *stmt, const ReindexParams *params)
 {
+
 	Oid			objectOid;
 	Relation	relationRelation;
 	TableScanDesc scan;
@@ -2957,6 +2960,8 @@ ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind,
 	int			num_keys;
 	bool		concurrent_warning = false;
 	bool		tablespace_warning = false;
+	const char *objectName = stmt->name;
+	const ReindexObjectType objectKind = stmt->kind;
 
 	Assert(objectKind == REINDEX_OBJECT_SCHEMA ||
 		   objectKind == REINDEX_OBJECT_SYSTEM ||
@@ -3152,7 +3157,7 @@ ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind,
 	 * Process each relation listed in a separate transaction.  Note that this
 	 * commits and then starts a new transaction immediately.
 	 */
-	ReindexMultipleInternal(relids, params);
+	ReindexMultipleInternal(stmt, relids, params);
 
 	MemoryContextDelete(private_context);
 }
@@ -3182,7 +3187,7 @@ reindex_error_callback(void *arg)
  * by the caller.
  */
 static void
-ReindexPartitions(Oid relid, const ReindexParams *params, bool isTopLevel)
+ReindexPartitions(const ReindexStmt *stmt, Oid relid, const ReindexParams *params, bool isTopLevel)
 {
 	List	   *partitions = NIL;
 	char		relkind = get_rel_relkind(relid);
@@ -3258,7 +3263,7 @@ ReindexPartitions(Oid relid, const ReindexParams *params, bool isTopLevel)
 	 * Process each partition listed in a separate transaction.  Note that
 	 * this commits and then starts a new transaction immediately.
 	 */
-	ReindexMultipleInternal(partitions, params);
+	ReindexMultipleInternal(stmt, partitions, params);
 
 	/*
 	 * Clean up working storage --- note we must do this after
@@ -3276,7 +3281,7 @@ ReindexPartitions(Oid relid, const ReindexParams *params, bool isTopLevel)
  * and starts a new transaction when finished.
  */
 static void
-ReindexMultipleInternal(const List *relids, const ReindexParams *params)
+ReindexMultipleInternal(const ReindexStmt *stmt, const List *relids, const ReindexParams *params)
 {
 	ListCell   *l;
 
@@ -3335,7 +3340,7 @@ ReindexMultipleInternal(const List *relids, const ReindexParams *params)
 			ReindexParams newparams = *params;
 
 			newparams.options |= REINDEXOPT_MISSING_OK;
-			(void) ReindexRelationConcurrently(relid, &newparams);
+			(void) ReindexRelationConcurrently(stmt, relid, &newparams);
 			/* ReindexRelationConcurrently() does the verbose output */
 		}
 		else if (relkind == RELKIND_INDEX)
@@ -3344,7 +3349,7 @@ ReindexMultipleInternal(const List *relids, const ReindexParams *params)
 
 			newparams.options |=
 				REINDEXOPT_REPORT_PROGRESS | REINDEXOPT_MISSING_OK;
-			reindex_index(relid, false, relpersistence, &newparams);
+			reindex_index(stmt, relid, false, relpersistence, &newparams);
 			PopActiveSnapshot();
 			/* reindex_index() does the verbose output */
 		}
@@ -3355,7 +3360,7 @@ ReindexMultipleInternal(const List *relids, const ReindexParams *params)
 
 			newparams.options |=
 				REINDEXOPT_REPORT_PROGRESS | REINDEXOPT_MISSING_OK;
-			result = reindex_relation(relid,
+			result = reindex_relation(stmt, relid,
 									  REINDEX_REL_PROCESS_TOAST |
 									  REINDEX_REL_CHECK_CONSTRAINTS,
 									  &newparams);
@@ -3400,7 +3405,7 @@ ReindexMultipleInternal(const List *relids, const ReindexParams *params)
  * anyway, and a non-concurrent reindex is more efficient.
  */
 static bool
-ReindexRelationConcurrently(Oid relationOid, const ReindexParams *params)
+ReindexRelationConcurrently(const ReindexStmt *stmt, Oid relationOid, const ReindexParams *params)
 {
 	typedef struct ReindexIndexInfo
 	{
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 323d9bf8..85ee7c63 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -2172,7 +2172,7 @@ ExecuteTruncateGuts(List *explicit_rels,
 			/*
 			 * Reconstruct the indexes to match, and we're done.
 			 */
-			reindex_relation(heap_relid, REINDEX_REL_PROCESS_TOAST,
+			reindex_relation(NULL, heap_relid, REINDEX_REL_PROCESS_TOAST,
 							 &reindex_params);
 		}
 
diff --git a/src/include/catalog/index.h b/src/include/catalog/index.h
index a4770eaf..e1272662 100644
--- a/src/include/catalog/index.h
+++ b/src/include/catalog/index.h
@@ -149,7 +149,7 @@ extern void index_set_state_flags(Oid indexId, IndexStateFlagsAction action);
 
 extern Oid	IndexGetRelation(Oid indexId, bool missing_ok);
 
-extern void reindex_index(Oid indexId, bool skip_constraint_checks,
+extern void reindex_index(const ReindexStmt *stmt, Oid indexId, bool skip_constraint_checks,
 						  char persistence, const ReindexParams *params);
 
 /* Flag bits for reindex_relation(): */
@@ -159,7 +159,7 @@ extern void reindex_index(Oid indexId, bool skip_constraint_checks,
 #define REINDEX_REL_FORCE_INDEXES_UNLOGGED	0x08
 #define REINDEX_REL_FORCE_INDEXES_PERMANENT 0x10
 
-extern bool reindex_relation(Oid relid, int flags, const ReindexParams *params);
+extern bool reindex_relation(const ReindexStmt *stmt, Oid relid, int flags, const ReindexParams *params);
 
 extern bool ReindexIsProcessingHeap(Oid heapOid);
 extern bool ReindexIsProcessingIndex(Oid indexOid);
-- 
2.34.1

v5-0001-make-event-triggers-facility-react-to-ReindexStmt.patchtext/x-patch; charset=US-ASCII; name=v5-0001-make-event-triggers-facility-react-to-ReindexStmt.patchDownload
From c6d19985d559db51a9b588d2e5b5adeb202a2e5a Mon Sep 17 00:00:00 2001
From: pgaddict <jian.universality@gmail.com>
Date: Fri, 24 Nov 2023 21:22:12 +0800
Subject: [PATCH v5 1/4] make event triggers facility react to ReindexStmt

Move ReindexStmt moves from standard_ProcessUtility to ProcessUtilitySlow.
By default ProcessUtilitySlow will call trigger related functions.
So the event trigger facility can support reindex statements.
---
 src/backend/tcop/utility.c    | 10 +++++++++-
 src/include/tcop/cmdtaglist.h |  2 +-
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c
index e3ccf6c7..5a7f63cb 100644
--- a/src/backend/tcop/utility.c
+++ b/src/backend/tcop/utility.c
@@ -961,7 +961,9 @@ standard_ProcessUtility(PlannedStmt *pstmt,
 			break;
 
 		case T_ReindexStmt:
-			ExecReindex(pstate, (ReindexStmt *) parsetree, isTopLevel);
+			ProcessUtilitySlow(pstate, pstmt, queryString,
+							   context, params, queryEnv,
+							   dest, qc);
 			break;
 
 			/*
@@ -1574,6 +1576,12 @@ ProcessUtilitySlow(ParseState *pstate,
 				}
 				break;
 
+			case T_ReindexStmt:
+				ExecReindex(pstate, (ReindexStmt *) parsetree, isTopLevel);
+				/* no commands stashed for REINDEX */
+				commandCollected = true;
+				break;
+
 			case T_CreateExtensionStmt:
 				address = CreateExtension(pstate, (CreateExtensionStmt *) parsetree);
 				break;
diff --git a/src/include/tcop/cmdtaglist.h b/src/include/tcop/cmdtaglist.h
index 553a3187..320ee915 100644
--- a/src/include/tcop/cmdtaglist.h
+++ b/src/include/tcop/cmdtaglist.h
@@ -194,7 +194,7 @@ PG_CMDTAG(CMDTAG_PREPARE, "PREPARE", false, false, false)
 PG_CMDTAG(CMDTAG_PREPARE_TRANSACTION, "PREPARE TRANSACTION", false, false, false)
 PG_CMDTAG(CMDTAG_REASSIGN_OWNED, "REASSIGN OWNED", false, false, false)
 PG_CMDTAG(CMDTAG_REFRESH_MATERIALIZED_VIEW, "REFRESH MATERIALIZED VIEW", true, false, false)
-PG_CMDTAG(CMDTAG_REINDEX, "REINDEX", false, false, false)
+PG_CMDTAG(CMDTAG_REINDEX, "REINDEX", true, false, false)
 PG_CMDTAG(CMDTAG_RELEASE, "RELEASE", false, false, false)
 PG_CMDTAG(CMDTAG_RESET, "RESET", false, false, false)
 PG_CMDTAG(CMDTAG_REVOKE, "REVOKE", true, false, false)
-- 
2.34.1

#20Ajin Cherian
itsajin@gmail.com
In reply to: jian he (#19)
Re: PATCH: Add REINDEX tag to event triggers

On Mon, Nov 27, 2023 at 11:00 AM jian he <jian.universality@gmail.com> wrote:

On Fri, Nov 24, 2023 at 10:44 AM Michael Paquier <michael@paquier.xyz> wrote:
hi.
v5-0001. changed the REINDEX command tag from event_trigger_ok: false
to event_trigger_ok: true.
Move ReindexStmt moves from standard_ProcessUtility to ProcessUtilitySlow.
By default ProcessUtilitySlow will call trigger related functions.
So the event trigger facility can support reindex statements.

v5-0002. In GetCommandLogLevel, change T_ReindexStmt from lev =
LOGSTMT_ALL to lev = LOGSTMT_DDL. so log_statement (enum) >= 'ddl'
will make the reindex statement be logged.

v5-0003. Refactor the following functions: {ReindexIndex,
ReindexTable, ReindexMultipleTables,
ReindexPartitions,ReindexMultipleInternal
,ReindexRelationConcurrently, reindex_relation,reindex_index} by
adding `const ReindexStmt *stmt` as their first argument.
This is for event trigger support reindex. We need to pass both the
newly generated indexId and the ReindexStmt to
EventTriggerCollectSimpleCommand right after the newly index gets
their lock. To do that, we have to refactor related functions.

v5-0004. Event trigger support reindex command implementation,
documentation, regress test, helper function pass reindex info to
function EventTriggerCollectSimpleCommand.

I just started reviewing the patch. Some minor comments:
In patch 0001:
In standard_ProcessUtility(), since you are unconditionally calling
ProcessUtilitySlow() in case of T_ReindexStmt, you really don't need
the case statement for T_ReindexStmt just like all the other commands
which have event trigger support. It will call ProcessUtilitySlow() as
default.

In patch 0004:
No need to duplicate reindex_event_trigger_collect in indexcmds.c
since it is already present in index.c. Add it to index.h and make the
function extern so that it can be accessed in both index.c and
indexcmds.c

regards,
Ajin Cherian
Fujitsu Australia

#21jian he
jian.universality@gmail.com
In reply to: Ajin Cherian (#20)
4 attachment(s)
Re: PATCH: Add REINDEX tag to event triggers

On Mon, Nov 27, 2023 at 6:58 PM Ajin Cherian <itsajin@gmail.com> wrote:

I just started reviewing the patch. Some minor comments:
In patch 0001:
In standard_ProcessUtility(), since you are unconditionally calling
ProcessUtilitySlow() in case of T_ReindexStmt, you really don't need
the case statement for T_ReindexStmt just like all the other commands
which have event trigger support. It will call ProcessUtilitySlow() as
default.

In patch 0004:
No need to duplicate reindex_event_trigger_collect in indexcmds.c
since it is already present in index.c. Add it to index.h and make the
function extern so that it can be accessed in both index.c and
indexcmds.c

regards,
Ajin Cherian
Fujitsu Australia

Thanks for reviewing it!
The above 2 suggestions are good, now the code is more neat.

Attachments:

v6-0001-make-event-triggers-facility-react-to-ReindexStmt.patchtext/x-patch; charset=US-ASCII; name=v6-0001-make-event-triggers-facility-react-to-ReindexStmt.patchDownload
From 6a4f1bee35867565b5f65902db642a0d111be6e6 Mon Sep 17 00:00:00 2001
From: pgaddict <jian.universality@gmail.com>
Date: Mon, 27 Nov 2023 23:43:01 +0800
Subject: [PATCH v6 1/4] make event triggers facility react to ReindexStmt

Move ReindexStmt moves from standard_ProcessUtility to ProcessUtilitySlow.
By default ProcessUtilitySlow will call trigger related functions.
So the event trigger facility able to support reindex statements.
---
 src/backend/tcop/utility.c    | 10 ++++++----
 src/include/tcop/cmdtaglist.h |  2 +-
 2 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c
index e3ccf6c7..37161eda 100644
--- a/src/backend/tcop/utility.c
+++ b/src/backend/tcop/utility.c
@@ -960,10 +960,6 @@ standard_ProcessUtility(PlannedStmt *pstmt,
 							  (RecoveryInProgress() ? 0 : CHECKPOINT_FORCE));
 			break;
 
-		case T_ReindexStmt:
-			ExecReindex(pstate, (ReindexStmt *) parsetree, isTopLevel);
-			break;
-
 			/*
 			 * The following statements are supported by Event Triggers only
 			 * in some cases, so we "fast path" them in the other cases.
@@ -1574,6 +1570,12 @@ ProcessUtilitySlow(ParseState *pstate,
 				}
 				break;
 
+			case T_ReindexStmt:
+				ExecReindex(pstate, (ReindexStmt *) parsetree, isTopLevel);
+				/* commandCollected done in the deep inside of ExecReindex */
+				commandCollected = true;
+				break;
+
 			case T_CreateExtensionStmt:
 				address = CreateExtension(pstate, (CreateExtensionStmt *) parsetree);
 				break;
diff --git a/src/include/tcop/cmdtaglist.h b/src/include/tcop/cmdtaglist.h
index 553a3187..320ee915 100644
--- a/src/include/tcop/cmdtaglist.h
+++ b/src/include/tcop/cmdtaglist.h
@@ -194,7 +194,7 @@ PG_CMDTAG(CMDTAG_PREPARE, "PREPARE", false, false, false)
 PG_CMDTAG(CMDTAG_PREPARE_TRANSACTION, "PREPARE TRANSACTION", false, false, false)
 PG_CMDTAG(CMDTAG_REASSIGN_OWNED, "REASSIGN OWNED", false, false, false)
 PG_CMDTAG(CMDTAG_REFRESH_MATERIALIZED_VIEW, "REFRESH MATERIALIZED VIEW", true, false, false)
-PG_CMDTAG(CMDTAG_REINDEX, "REINDEX", false, false, false)
+PG_CMDTAG(CMDTAG_REINDEX, "REINDEX", true, false, false)
 PG_CMDTAG(CMDTAG_RELEASE, "RELEASE", false, false, false)
 PG_CMDTAG(CMDTAG_RESET, "RESET", false, false, false)
 PG_CMDTAG(CMDTAG_REVOKE, "REVOKE", true, false, false)
-- 
2.34.1

v6-0002-tag-ReindexStmt-as-ddl-in-GetCommandLogLevel.patchtext/x-patch; charset=US-ASCII; name=v6-0002-tag-ReindexStmt-as-ddl-in-GetCommandLogLevel.patchDownload
From 141b176ba6226e61698a08e0c429d7ba61939e7b Mon Sep 17 00:00:00 2001
From: pgaddict <jian.universality@gmail.com>
Date: Fri, 24 Nov 2023 20:44:44 +0800
Subject: [PATCH v6 2/4] tag ReindexStmt as ddl in GetCommandLogLevel.

In GetCommandLogLevel, change T_ReindexStmt from lev = LOGSTMT_ALL to lev = LOGSTMT_DDL.
so log_statement (enum) >= 'ddl' will make the reindex statement be logged.
---
 src/backend/tcop/utility.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c
index 37161eda..e115b0ca 100644
--- a/src/backend/tcop/utility.c
+++ b/src/backend/tcop/utility.c
@@ -3622,7 +3622,7 @@ GetCommandLogLevel(Node *parsetree)
 			break;
 
 		case T_ReindexStmt:
-			lev = LOGSTMT_ALL;	/* should this be DDL? */
+			lev = LOGSTMT_DDL;
 			break;
 
 		case T_CreateConversionStmt:
-- 
2.34.1

v6-0003-refactor-multiple-functions-to-support-tracking-r.patchtext/x-patch; charset=US-ASCII; name=v6-0003-refactor-multiple-functions-to-support-tracking-r.patchDownload
From f6a660d14b3649f046c2d54c810ccf28f51f65db Mon Sep 17 00:00:00 2001
From: pgaddict <jian.universality@gmail.com>
Date: Tue, 28 Nov 2023 00:05:50 +0800
Subject: [PATCH v6 3/4] refactor multiple functions to support tracking
 reindex using event trigger.

---
 src/backend/catalog/index.c      |  8 ++---
 src/backend/commands/cluster.c   |  2 +-
 src/backend/commands/indexcmds.c | 60 +++++++++++++++++---------------
 src/backend/commands/tablecmds.c |  2 +-
 src/include/catalog/index.h      |  4 +--
 5 files changed, 40 insertions(+), 36 deletions(-)

diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c
index 143fae01..88ff8c5a 100644
--- a/src/backend/catalog/index.c
+++ b/src/backend/catalog/index.c
@@ -3558,7 +3558,7 @@ IndexGetRelation(Oid indexId, bool missing_ok)
  * reindex_index - This routine is used to recreate a single index
  */
 void
-reindex_index(Oid indexId, bool skip_constraint_checks, char persistence,
+reindex_index(const ReindexStmt *stmt, Oid indexId, bool skip_constraint_checks, char persistence,
 			  const ReindexParams *params)
 {
 	Relation	iRel,
@@ -3865,7 +3865,7 @@ reindex_index(Oid indexId, bool skip_constraint_checks, char persistence,
  * index rebuild.
  */
 bool
-reindex_relation(Oid relid, int flags, const ReindexParams *params)
+reindex_relation(const ReindexStmt *stmt, Oid relid, int flags, const ReindexParams *params)
 {
 	Relation	rel;
 	Oid			toast_relid;
@@ -3953,7 +3953,7 @@ reindex_relation(Oid relid, int flags, const ReindexParams *params)
 			continue;
 		}
 
-		reindex_index(indexOid, !(flags & REINDEX_REL_CHECK_CONSTRAINTS),
+		reindex_index(stmt, indexOid, !(flags & REINDEX_REL_CHECK_CONSTRAINTS),
 					  persistence, params);
 
 		CommandCounterIncrement();
@@ -3990,7 +3990,7 @@ reindex_relation(Oid relid, int flags, const ReindexParams *params)
 
 		newparams.options &= ~(REINDEXOPT_MISSING_OK);
 		newparams.tablespaceOid = InvalidOid;
-		result |= reindex_relation(toast_relid, flags, &newparams);
+		result |= reindex_relation(stmt, toast_relid, flags, &newparams);
 	}
 
 	return result;
diff --git a/src/backend/commands/cluster.c b/src/backend/commands/cluster.c
index a3bef6ac..1f52d391 100644
--- a/src/backend/commands/cluster.c
+++ b/src/backend/commands/cluster.c
@@ -1518,7 +1518,7 @@ finish_heap_swap(Oid OIDOldHeap, Oid OIDNewHeap,
 	pgstat_progress_update_param(PROGRESS_CLUSTER_PHASE,
 								 PROGRESS_CLUSTER_PHASE_REBUILD_INDEX);
 
-	reindex_relation(OIDOldHeap, reindex_flags, &reindex_params);
+	reindex_relation(NULL, OIDOldHeap, reindex_flags, &reindex_params);
 
 	/* Report that we are now doing clean up */
 	pgstat_progress_update_param(PROGRESS_CLUSTER_PHASE,
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index 0b3b8e98..f47a4aa7 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -94,20 +94,19 @@ static char *ChooseIndexName(const char *tabname, Oid namespaceId,
 							 bool primary, bool isconstraint);
 static char *ChooseIndexNameAddition(const List *colnames);
 static List *ChooseIndexColumnNames(const List *indexElems);
-static void ReindexIndex(const RangeVar *indexRelation, const ReindexParams *params,
+static void ReindexIndex(const ReindexStmt *stmt, const ReindexParams *params,
 						 bool isTopLevel);
 static void RangeVarCallbackForReindexIndex(const RangeVar *relation,
 											Oid relId, Oid oldRelId, void *arg);
-static Oid	ReindexTable(const RangeVar *relation, const ReindexParams *params,
+static Oid	ReindexTable(const ReindexStmt *stmt, const ReindexParams *params,
 						 bool isTopLevel);
-static void ReindexMultipleTables(const char *objectName,
-								  ReindexObjectType objectKind, const ReindexParams *params);
+static void ReindexMultipleTables(const ReindexStmt *stmt, const ReindexParams *params);
 static void reindex_error_callback(void *arg);
-static void ReindexPartitions(Oid relid, const ReindexParams *params,
+static void ReindexPartitions(const ReindexStmt *stmt, Oid relid, const ReindexParams *params,
 							  bool isTopLevel);
-static void ReindexMultipleInternal(const List *relids,
+static void ReindexMultipleInternal(const ReindexStmt *stmt, const List *relids,
 									const ReindexParams *params);
-static bool ReindexRelationConcurrently(Oid relationOid,
+static bool ReindexRelationConcurrently(const ReindexStmt *stmt, Oid relationOid,
 										const ReindexParams *params);
 static void update_relispartition(Oid relationId, bool newval);
 static inline void set_indexsafe_procflags(void);
@@ -2729,10 +2728,10 @@ ExecReindex(ParseState *pstate, const ReindexStmt *stmt, bool isTopLevel)
 	switch (stmt->kind)
 	{
 		case REINDEX_OBJECT_INDEX:
-			ReindexIndex(stmt->relation, &params, isTopLevel);
+			ReindexIndex(stmt, &params, isTopLevel);
 			break;
 		case REINDEX_OBJECT_TABLE:
-			ReindexTable(stmt->relation, &params, isTopLevel);
+			ReindexTable(stmt, &params, isTopLevel);
 			break;
 		case REINDEX_OBJECT_SCHEMA:
 		case REINDEX_OBJECT_SYSTEM:
@@ -2748,7 +2747,7 @@ ExecReindex(ParseState *pstate, const ReindexStmt *stmt, bool isTopLevel)
 									  (stmt->kind == REINDEX_OBJECT_SCHEMA) ? "REINDEX SCHEMA" :
 									  (stmt->kind == REINDEX_OBJECT_SYSTEM) ? "REINDEX SYSTEM" :
 									  "REINDEX DATABASE");
-			ReindexMultipleTables(stmt->name, stmt->kind, &params);
+			ReindexMultipleTables(stmt, &params);
 			break;
 		default:
 			elog(ERROR, "unrecognized object type: %d",
@@ -2762,13 +2761,15 @@ ExecReindex(ParseState *pstate, const ReindexStmt *stmt, bool isTopLevel)
  *		Recreate a specific index.
  */
 static void
-ReindexIndex(const RangeVar *indexRelation, const ReindexParams *params, bool isTopLevel)
+ReindexIndex(const ReindexStmt *stmt, const ReindexParams *params, bool isTopLevel)
 {
+	const RangeVar *indexRelation;
 	struct ReindexIndexCallbackState state;
 	Oid			indOid;
 	char		persistence;
 	char		relkind;
 
+	indexRelation = stmt->relation;
 	/*
 	 * Find and lock index, and check permissions on table; use callback to
 	 * obtain lock on table first, to avoid deadlock hazard.  The lock level
@@ -2796,16 +2797,16 @@ ReindexIndex(const RangeVar *indexRelation, const ReindexParams *params, bool is
 	relkind = get_rel_relkind(indOid);
 
 	if (relkind == RELKIND_PARTITIONED_INDEX)
-		ReindexPartitions(indOid, params, isTopLevel);
+		ReindexPartitions(stmt, indOid, params, isTopLevel);
 	else if ((params->options & REINDEXOPT_CONCURRENTLY) != 0 &&
 			 persistence != RELPERSISTENCE_TEMP)
-		ReindexRelationConcurrently(indOid, params);
+		ReindexRelationConcurrently(stmt, indOid, params);
 	else
 	{
 		ReindexParams newparams = *params;
 
 		newparams.options |= REINDEXOPT_REPORT_PROGRESS;
-		reindex_index(indOid, false, persistence, &newparams);
+		reindex_index(stmt, indOid, false, persistence, &newparams);
 	}
 }
 
@@ -2885,11 +2886,12 @@ RangeVarCallbackForReindexIndex(const RangeVar *relation,
  *		Recreate all indexes of a table (and of its toast table, if any)
  */
 static Oid
-ReindexTable(const RangeVar *relation, const ReindexParams *params, bool isTopLevel)
+ReindexTable(const ReindexStmt *stmt, const ReindexParams *params, bool isTopLevel)
 {
 	Oid			heapOid;
 	bool		result;
 
+	const RangeVar *relation = stmt->relation;
 	/*
 	 * The lock level used here should match reindex_relation().
 	 *
@@ -2905,11 +2907,11 @@ ReindexTable(const RangeVar *relation, const ReindexParams *params, bool isTopLe
 									   RangeVarCallbackOwnsTable, NULL);
 
 	if (get_rel_relkind(heapOid) == RELKIND_PARTITIONED_TABLE)
-		ReindexPartitions(heapOid, params, isTopLevel);
+		ReindexPartitions(stmt, heapOid, params, isTopLevel);
 	else if ((params->options & REINDEXOPT_CONCURRENTLY) != 0 &&
 			 get_rel_persistence(heapOid) != RELPERSISTENCE_TEMP)
 	{
-		result = ReindexRelationConcurrently(heapOid, params);
+		result = ReindexRelationConcurrently(stmt, heapOid, params);
 
 		if (!result)
 			ereport(NOTICE,
@@ -2921,7 +2923,7 @@ ReindexTable(const RangeVar *relation, const ReindexParams *params, bool isTopLe
 		ReindexParams newparams = *params;
 
 		newparams.options |= REINDEXOPT_REPORT_PROGRESS;
-		result = reindex_relation(heapOid,
+		result = reindex_relation(stmt, heapOid,
 								  REINDEX_REL_PROCESS_TOAST |
 								  REINDEX_REL_CHECK_CONSTRAINTS,
 								  &newparams);
@@ -2943,9 +2945,9 @@ ReindexTable(const RangeVar *relation, const ReindexParams *params, bool isTopLe
  * That means this must not be called within a user transaction block!
  */
 static void
-ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind,
-					  const ReindexParams *params)
+ReindexMultipleTables(const ReindexStmt *stmt, const ReindexParams *params)
 {
+
 	Oid			objectOid;
 	Relation	relationRelation;
 	TableScanDesc scan;
@@ -2957,6 +2959,8 @@ ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind,
 	int			num_keys;
 	bool		concurrent_warning = false;
 	bool		tablespace_warning = false;
+	const char *objectName = stmt->name;
+	const ReindexObjectType objectKind = stmt->kind;
 
 	Assert(objectKind == REINDEX_OBJECT_SCHEMA ||
 		   objectKind == REINDEX_OBJECT_SYSTEM ||
@@ -3152,7 +3156,7 @@ ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind,
 	 * Process each relation listed in a separate transaction.  Note that this
 	 * commits and then starts a new transaction immediately.
 	 */
-	ReindexMultipleInternal(relids, params);
+	ReindexMultipleInternal(stmt, relids, params);
 
 	MemoryContextDelete(private_context);
 }
@@ -3182,7 +3186,7 @@ reindex_error_callback(void *arg)
  * by the caller.
  */
 static void
-ReindexPartitions(Oid relid, const ReindexParams *params, bool isTopLevel)
+ReindexPartitions(const ReindexStmt *stmt, Oid relid, const ReindexParams *params, bool isTopLevel)
 {
 	List	   *partitions = NIL;
 	char		relkind = get_rel_relkind(relid);
@@ -3258,7 +3262,7 @@ ReindexPartitions(Oid relid, const ReindexParams *params, bool isTopLevel)
 	 * Process each partition listed in a separate transaction.  Note that
 	 * this commits and then starts a new transaction immediately.
 	 */
-	ReindexMultipleInternal(partitions, params);
+	ReindexMultipleInternal(stmt, partitions, params);
 
 	/*
 	 * Clean up working storage --- note we must do this after
@@ -3276,7 +3280,7 @@ ReindexPartitions(Oid relid, const ReindexParams *params, bool isTopLevel)
  * and starts a new transaction when finished.
  */
 static void
-ReindexMultipleInternal(const List *relids, const ReindexParams *params)
+ReindexMultipleInternal(const ReindexStmt *stmt, const List *relids, const ReindexParams *params)
 {
 	ListCell   *l;
 
@@ -3335,7 +3339,7 @@ ReindexMultipleInternal(const List *relids, const ReindexParams *params)
 			ReindexParams newparams = *params;
 
 			newparams.options |= REINDEXOPT_MISSING_OK;
-			(void) ReindexRelationConcurrently(relid, &newparams);
+			(void) ReindexRelationConcurrently(stmt, relid, &newparams);
 			/* ReindexRelationConcurrently() does the verbose output */
 		}
 		else if (relkind == RELKIND_INDEX)
@@ -3344,7 +3348,7 @@ ReindexMultipleInternal(const List *relids, const ReindexParams *params)
 
 			newparams.options |=
 				REINDEXOPT_REPORT_PROGRESS | REINDEXOPT_MISSING_OK;
-			reindex_index(relid, false, relpersistence, &newparams);
+			reindex_index(stmt, relid, false, relpersistence, &newparams);
 			PopActiveSnapshot();
 			/* reindex_index() does the verbose output */
 		}
@@ -3355,7 +3359,7 @@ ReindexMultipleInternal(const List *relids, const ReindexParams *params)
 
 			newparams.options |=
 				REINDEXOPT_REPORT_PROGRESS | REINDEXOPT_MISSING_OK;
-			result = reindex_relation(relid,
+			result = reindex_relation(stmt, relid,
 									  REINDEX_REL_PROCESS_TOAST |
 									  REINDEX_REL_CHECK_CONSTRAINTS,
 									  &newparams);
@@ -3400,7 +3404,7 @@ ReindexMultipleInternal(const List *relids, const ReindexParams *params)
  * anyway, and a non-concurrent reindex is more efficient.
  */
 static bool
-ReindexRelationConcurrently(Oid relationOid, const ReindexParams *params)
+ReindexRelationConcurrently(const ReindexStmt *stmt, Oid relationOid, const ReindexParams *params)
 {
 	typedef struct ReindexIndexInfo
 	{
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 323d9bf8..85ee7c63 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -2172,7 +2172,7 @@ ExecuteTruncateGuts(List *explicit_rels,
 			/*
 			 * Reconstruct the indexes to match, and we're done.
 			 */
-			reindex_relation(heap_relid, REINDEX_REL_PROCESS_TOAST,
+			reindex_relation(NULL, heap_relid, REINDEX_REL_PROCESS_TOAST,
 							 &reindex_params);
 		}
 
diff --git a/src/include/catalog/index.h b/src/include/catalog/index.h
index a4770eaf..e1272662 100644
--- a/src/include/catalog/index.h
+++ b/src/include/catalog/index.h
@@ -149,7 +149,7 @@ extern void index_set_state_flags(Oid indexId, IndexStateFlagsAction action);
 
 extern Oid	IndexGetRelation(Oid indexId, bool missing_ok);
 
-extern void reindex_index(Oid indexId, bool skip_constraint_checks,
+extern void reindex_index(const ReindexStmt *stmt, Oid indexId, bool skip_constraint_checks,
 						  char persistence, const ReindexParams *params);
 
 /* Flag bits for reindex_relation(): */
@@ -159,7 +159,7 @@ extern void reindex_index(Oid indexId, bool skip_constraint_checks,
 #define REINDEX_REL_FORCE_INDEXES_UNLOGGED	0x08
 #define REINDEX_REL_FORCE_INDEXES_PERMANENT 0x10
 
-extern bool reindex_relation(Oid relid, int flags, const ReindexParams *params);
+extern bool reindex_relation(const ReindexStmt *stmt, Oid relid, int flags, const ReindexParams *params);
 
 extern bool ReindexIsProcessingHeap(Oid heapOid);
 extern bool ReindexIsProcessingIndex(Oid indexOid);
-- 
2.34.1

v6-0004-make-Event-trigger-support-reindex-command.patchtext/x-patch; charset=US-ASCII; name=v6-0004-make-Event-trigger-support-reindex-command.patchDownload
From cdf69292a19c76a1e15d331a64e732eabf384dfd Mon Sep 17 00:00:00 2001
From: pgaddict <jian.universality@gmail.com>
Date: Tue, 28 Nov 2023 00:12:29 +0800
Subject: [PATCH v6 4/4] make Event trigger support reindex command.

documentation event trigger suppport index, regress test.
also mark reindex_event_trigger_collect as extern,
since we use reindex_event_trigger_collect both in indexcmds.c and index.c.
---
 doc/src/sgml/event-trigger.sgml             |   8 ++
 src/backend/catalog/index.c                 |  15 +++
 src/backend/commands/indexcmds.c            |   4 +
 src/include/catalog/index.h                 |   2 +-
 src/test/regress/expected/event_trigger.out | 103 ++++++++++++++++++++
 src/test/regress/sql/event_trigger.sql      |  93 ++++++++++++++++++
 6 files changed, 224 insertions(+), 1 deletion(-)

diff --git a/doc/src/sgml/event-trigger.sgml b/doc/src/sgml/event-trigger.sgml
index 10b20f03..234b4ffd 100644
--- a/doc/src/sgml/event-trigger.sgml
+++ b/doc/src/sgml/event-trigger.sgml
@@ -1032,6 +1032,14 @@
         <entry align="center"><literal>-</literal></entry>
         <entry align="left"></entry>
        </row>
+       <row>
+        <entry align="left"><literal>REINDEX</literal></entry>
+        <entry align="center"><literal>X</literal></entry>
+        <entry align="center"><literal>X</literal></entry>
+        <entry align="center"><literal>-</literal></entry>
+        <entry align="center"><literal>-</literal></entry>
+        <entry align="left"></entry>
+       </row>
        <row>
         <entry align="left"><literal>REVOKE</literal></entry>
         <entry align="center"><literal>X</literal></entry>
diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c
index 88ff8c5a..2af9efb8 100644
--- a/src/backend/catalog/index.c
+++ b/src/backend/catalog/index.c
@@ -3554,6 +3554,18 @@ IndexGetRelation(Oid indexId, bool missing_ok)
 	return result;
 }
 
+void
+reindex_event_trigger_collect(Oid oid, const ReindexStmt *stmt)
+{
+	ObjectAddress address;
+
+	address.classId = RelationRelationId;
+	address.objectId = oid;
+	address.objectSubId = 0;
+
+	EventTriggerCollectSimpleCommand(address,
+									 InvalidObjectAddress, (Node *) stmt);
+}
 /*
  * reindex_index - This routine is used to recreate a single index
  */
@@ -3629,6 +3641,9 @@ reindex_index(const ReindexStmt *stmt, Oid indexId, bool skip_constraint_checks,
 	if (progress)
 		pgstat_progress_update_param(PROGRESS_CREATEIDX_ACCESS_METHOD_OID,
 									 iRel->rd_rel->relam);
+	/* event trigger tracking REINDEX primary pointer */
+	if (stmt)
+		reindex_event_trigger_collect(indexId, stmt);
 
 	/*
 	 * Partitioned indexes should never get processed here, as they have no
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index f47a4aa7..fa874307 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -3816,6 +3816,10 @@ ReindexRelationConcurrently(const ReindexStmt *stmt, Oid relationOid, const Rein
 
 		newIndexIds = lappend(newIndexIds, newidx);
 
+		/* Add the index to event trigger */
+		if (stmt)
+			reindex_event_trigger_collect(newIndexId, stmt);
+
 		/*
 		 * Save lockrelid to protect each relation from drop then close
 		 * relations. The lockrelid on parent relation is not taken here to
diff --git a/src/include/catalog/index.h b/src/include/catalog/index.h
index e1272662..6f757f77 100644
--- a/src/include/catalog/index.h
+++ b/src/include/catalog/index.h
@@ -170,7 +170,7 @@ extern void SerializeReindexState(Size maxsize, char *start_address);
 extern void RestoreReindexState(const void *reindexstate);
 
 extern void IndexSetParentIndex(Relation partitionIdx, Oid parentOid);
-
+extern void reindex_event_trigger_collect(Oid oid, const ReindexStmt *stmt);
 
 /*
  * itemptr_encode - Encode ItemPointer as int64/int8
diff --git a/src/test/regress/expected/event_trigger.out b/src/test/regress/expected/event_trigger.out
index 0b87a42d..266cccec 100644
--- a/src/test/regress/expected/event_trigger.out
+++ b/src/test/regress/expected/event_trigger.out
@@ -556,6 +556,109 @@ ERROR:  cannot alter type "rewritetype" because column "rewritemetoo3.a" uses it
 drop table rewriteme;
 drop event trigger no_rewrite_allowed;
 drop function test_evtrig_no_rewrite();
+--reindex command, event trigger test setup.
+DROP SCHEMA IF EXISTS schema_to_reindex CASCADE;
+NOTICE:  schema "schema_to_reindex" does not exist, skipping
+CREATE SCHEMA schema_to_reindex;
+SET search_path TO schema_to_reindex;
+CREATE TABLE concur_reindex_tab (c1 int,c2 text);
+CREATE UNIQUE INDEX concur_reindex_ind1 ON concur_reindex_tab (c1);
+CREATE INDEX concur_reindex_ind2 ON concur_reindex_tab (c2);
+CREATE INDEX concur_reindex_ind4 ON concur_reindex_tab (c1, c1, c2);
+ALTER TABLE concur_reindex_tab ADD PRIMARY KEY USING INDEX concur_reindex_ind1;
+INSERT INTO concur_reindex_tab VALUES (1, 'a'),(2, 'a');
+CREATE TABLE parted_irreg_ancestor (b text,a int) PARTITION BY RANGE (b);
+CREATE TABLE parted_irreg (a int,b text) PARTITION BY RANGE (b);
+ALTER TABLE parted_irreg_ancestor ATTACH PARTITION parted_irreg
+FOR VALUES FROM ('aaaa') TO ('zzzz');
+CREATE TABLE parted1_irreg1 (b text NOT NULL,a int);
+ALTER TABLE parted_irreg ATTACH PARTITION parted1_irreg1
+FOR VALUES FROM ('aaaa') TO ('jjjj');
+CREATE TABLE parted1_irreg2 (b text NOT NULL, a int);
+ALTER TABLE parted_irreg ATTACH PARTITION parted1_irreg2
+FOR VALUES FROM ('jjjj') TO ('zzzz');
+INSERT INTO parted_irreg_ancestor (b)
+VALUES ('daasvog'),('asdhjksd'),('sssdjk'),('jssdjk');
+ALTER TABLE parted_irreg_ancestor ADD PRIMARY KEY (b);
+CREATE OR REPLACE FUNCTION public.reindex_start_command()
+RETURNS event_trigger AS $$
+BEGIN
+    RAISE NOTICE 'ddl_start_command -- REINDEX: % %', tg_event, tg_tag;
+END;
+$$ LANGUAGE plpgsql;
+CREATE EVENT TRIGGER start_reindex_command ON ddl_command_start
+    WHEN TAG IN ('REINDEX') EXECUTE PROCEDURE public.reindex_start_command();
+CREATE OR REPLACE FUNCTION public.reindex_end_command()
+RETURNS event_trigger AS $$
+DECLARE
+	obj record;
+	toast_main_table text;
+BEGIN
+FOR obj IN SELECT * FROM pg_event_trigger_ddl_commands()
+    LOOP
+        IF  obj.schema_name = 'pg_toast' THEN
+            RAISE NOTICE 'reindexing toast related index! object_type: %, schema_name: %'
+                          ,obj.object_type, obj.schema_name;
+            /* get the toast table will be reindexed main table.
+             * toast table name auto generated, cannot use to test.
+            */
+            SELECT t1.relname into toast_main_table
+            FROM (
+                SELECT t1.oid
+                FROM pg_class t1 INNER JOIN pg_index t2 ON t1.oid = t2.indrelid
+                WHERE t2.indexrelid = obj.objid AND relkind = 't') sub
+                INNER JOIN pg_class t1 ON t1.reltoastrelid = sub.oid;
+
+            RAISE NOTICE 'toast table related main relation: %', toast_main_table;
+        ELSE
+            RAISE NOTICE 'ddl_end_command -- REINDEX: %', pg_get_indexdef(obj.objid);
+        END IF;
+    END LOOP;
+END;
+$$ LANGUAGE plpgsql;
+CREATE EVENT TRIGGER end_reindex_command ON ddl_command_end
+    WHEN TAG IN ('REINDEX') EXECUTE PROCEDURE public.reindex_end_command();
+REINDEX (CONCURRENTLY) INDEX  parted_irreg_ancestor_pkey;
+NOTICE:  ddl_start_command -- REINDEX: ddl_command_start REINDEX
+NOTICE:  ddl_end_command -- REINDEX: CREATE UNIQUE INDEX parted1_irreg1_pkey ON schema_to_reindex.parted1_irreg1 USING btree (b)
+NOTICE:  ddl_end_command -- REINDEX: CREATE UNIQUE INDEX parted1_irreg2_pkey ON schema_to_reindex.parted1_irreg2 USING btree (b)
+ALTER EVENT TRIGGER end_reindex_command DISABLE;
+-- event trigger disabled.
+--so reindex_start_command part will invoke, reindex_end_command won't.
+REINDEX (CONCURRENTLY) TABLE  parted_irreg_ancestor;
+NOTICE:  ddl_start_command -- REINDEX: ddl_command_start REINDEX
+ALTER EVENT TRIGGER end_reindex_command ENABLE;
+REINDEX (CONCURRENTLY) TABLE  parted_irreg_ancestor;
+NOTICE:  ddl_start_command -- REINDEX: ddl_command_start REINDEX
+NOTICE:  ddl_end_command -- REINDEX: CREATE UNIQUE INDEX parted1_irreg1_pkey ON schema_to_reindex.parted1_irreg1 USING btree (b)
+NOTICE:  reindexing toast related index! object_type: index, schema_name: pg_toast
+NOTICE:  toast table related main relation: parted1_irreg1
+NOTICE:  ddl_end_command -- REINDEX: CREATE UNIQUE INDEX parted1_irreg2_pkey ON schema_to_reindex.parted1_irreg2 USING btree (b)
+NOTICE:  reindexing toast related index! object_type: index, schema_name: pg_toast
+NOTICE:  toast table related main relation: parted1_irreg2
+/* REINDEX SCHEMA (CONCURRENTLY) generated test result order unstable,
+ * So just do normal reindex schema. should cover reindex statement most cases.
+*/
+REINDEX SCHEMA  schema_to_reindex;
+NOTICE:  ddl_start_command -- REINDEX: ddl_command_start REINDEX
+NOTICE:  ddl_end_command -- REINDEX: CREATE UNIQUE INDEX concur_reindex_ind1 ON schema_to_reindex.concur_reindex_tab USING btree (c1)
+NOTICE:  ddl_end_command -- REINDEX: CREATE INDEX concur_reindex_ind2 ON schema_to_reindex.concur_reindex_tab USING btree (c2)
+NOTICE:  ddl_end_command -- REINDEX: CREATE INDEX concur_reindex_ind4 ON schema_to_reindex.concur_reindex_tab USING btree (c1, c1, c2)
+NOTICE:  reindexing toast related index! object_type: index, schema_name: pg_toast
+NOTICE:  toast table related main relation: concur_reindex_tab
+NOTICE:  ddl_end_command -- REINDEX: CREATE UNIQUE INDEX parted1_irreg1_pkey ON schema_to_reindex.parted1_irreg1 USING btree (b)
+NOTICE:  reindexing toast related index! object_type: index, schema_name: pg_toast
+NOTICE:  toast table related main relation: parted1_irreg1
+NOTICE:  ddl_end_command -- REINDEX: CREATE UNIQUE INDEX parted1_irreg2_pkey ON schema_to_reindex.parted1_irreg2 USING btree (b)
+NOTICE:  reindexing toast related index! object_type: index, schema_name: pg_toast
+NOTICE:  toast table related main relation: parted1_irreg2
+drop event trigger if exists  start_reindex_command,end_reindex_command;
+drop function if exists  public.reindex_end_command, public.reindex_start_command cascade;
+drop schema if exists schema_to_reindex cascade;
+NOTICE:  drop cascades to 2 other objects
+DETAIL:  drop cascades to table concur_reindex_tab
+drop cascades to table parted_irreg_ancestor
+RESET search_path;
 -- test Row Security Event Trigger
 RESET SESSION AUTHORIZATION;
 CREATE TABLE event_trigger_test (a integer, b text);
diff --git a/src/test/regress/sql/event_trigger.sql b/src/test/regress/sql/event_trigger.sql
index 6f0933b9..ace58308 100644
--- a/src/test/regress/sql/event_trigger.sql
+++ b/src/test/regress/sql/event_trigger.sql
@@ -418,6 +418,99 @@ drop table rewriteme;
 drop event trigger no_rewrite_allowed;
 drop function test_evtrig_no_rewrite();
 
+--reindex command, event trigger test setup.
+DROP SCHEMA IF EXISTS schema_to_reindex CASCADE;
+CREATE SCHEMA schema_to_reindex;
+SET search_path TO schema_to_reindex;
+
+CREATE TABLE concur_reindex_tab (c1 int,c2 text);
+CREATE UNIQUE INDEX concur_reindex_ind1 ON concur_reindex_tab (c1);
+CREATE INDEX concur_reindex_ind2 ON concur_reindex_tab (c2);
+CREATE INDEX concur_reindex_ind4 ON concur_reindex_tab (c1, c1, c2);
+ALTER TABLE concur_reindex_tab ADD PRIMARY KEY USING INDEX concur_reindex_ind1;
+INSERT INTO concur_reindex_tab VALUES (1, 'a'),(2, 'a');
+
+CREATE TABLE parted_irreg_ancestor (b text,a int) PARTITION BY RANGE (b);
+CREATE TABLE parted_irreg (a int,b text) PARTITION BY RANGE (b);
+
+ALTER TABLE parted_irreg_ancestor ATTACH PARTITION parted_irreg
+FOR VALUES FROM ('aaaa') TO ('zzzz');
+
+CREATE TABLE parted1_irreg1 (b text NOT NULL,a int);
+ALTER TABLE parted_irreg ATTACH PARTITION parted1_irreg1
+FOR VALUES FROM ('aaaa') TO ('jjjj');
+
+CREATE TABLE parted1_irreg2 (b text NOT NULL, a int);
+ALTER TABLE parted_irreg ATTACH PARTITION parted1_irreg2
+FOR VALUES FROM ('jjjj') TO ('zzzz');
+
+INSERT INTO parted_irreg_ancestor (b)
+VALUES ('daasvog'),('asdhjksd'),('sssdjk'),('jssdjk');
+
+ALTER TABLE parted_irreg_ancestor ADD PRIMARY KEY (b);
+
+CREATE OR REPLACE FUNCTION public.reindex_start_command()
+RETURNS event_trigger AS $$
+BEGIN
+    RAISE NOTICE 'ddl_start_command -- REINDEX: % %', tg_event, tg_tag;
+END;
+$$ LANGUAGE plpgsql;
+
+CREATE EVENT TRIGGER start_reindex_command ON ddl_command_start
+    WHEN TAG IN ('REINDEX') EXECUTE PROCEDURE public.reindex_start_command();
+
+CREATE OR REPLACE FUNCTION public.reindex_end_command()
+RETURNS event_trigger AS $$
+DECLARE
+	obj record;
+	toast_main_table text;
+BEGIN
+FOR obj IN SELECT * FROM pg_event_trigger_ddl_commands()
+    LOOP
+        IF  obj.schema_name = 'pg_toast' THEN
+            RAISE NOTICE 'reindexing toast related index! object_type: %, schema_name: %'
+                          ,obj.object_type, obj.schema_name;
+            /* get the toast table will be reindexed main table.
+             * toast table name auto generated, cannot use to test.
+            */
+            SELECT t1.relname into toast_main_table
+            FROM (
+                SELECT t1.oid
+                FROM pg_class t1 INNER JOIN pg_index t2 ON t1.oid = t2.indrelid
+                WHERE t2.indexrelid = obj.objid AND relkind = 't') sub
+                INNER JOIN pg_class t1 ON t1.reltoastrelid = sub.oid;
+
+            RAISE NOTICE 'toast table related main relation: %', toast_main_table;
+        ELSE
+            RAISE NOTICE 'ddl_end_command -- REINDEX: %', pg_get_indexdef(obj.objid);
+        END IF;
+    END LOOP;
+END;
+$$ LANGUAGE plpgsql;
+
+CREATE EVENT TRIGGER end_reindex_command ON ddl_command_end
+    WHEN TAG IN ('REINDEX') EXECUTE PROCEDURE public.reindex_end_command();
+
+REINDEX (CONCURRENTLY) INDEX  parted_irreg_ancestor_pkey;
+ALTER EVENT TRIGGER end_reindex_command DISABLE;
+
+-- event trigger disabled.
+--so reindex_start_command part will invoke, reindex_end_command won't.
+REINDEX (CONCURRENTLY) TABLE  parted_irreg_ancestor;
+
+ALTER EVENT TRIGGER end_reindex_command ENABLE;
+REINDEX (CONCURRENTLY) TABLE  parted_irreg_ancestor;
+
+/* REINDEX SCHEMA (CONCURRENTLY) generated test result order unstable,
+ * So just do normal reindex schema. should cover reindex statement most cases.
+*/
+REINDEX SCHEMA  schema_to_reindex;
+
+drop event trigger if exists  start_reindex_command,end_reindex_command;
+drop function if exists  public.reindex_end_command, public.reindex_start_command cascade;
+drop schema if exists schema_to_reindex cascade;
+RESET search_path;
+
 -- test Row Security Event Trigger
 RESET SESSION AUTHORIZATION;
 CREATE TABLE event_trigger_test (a integer, b text);
-- 
2.34.1

#22Michael Paquier
michael@paquier.xyz
In reply to: jian he (#21)
Re: PATCH: Add REINDEX tag to event triggers

On Tue, Nov 28, 2023 at 12:28:50AM +0800, jian he wrote:

On Mon, Nov 27, 2023 at 6:58 PM Ajin Cherian <itsajin@gmail.com> wrote:

Ajin Cherian

Dammit. I've just noticed that I missed mentioning you in the commit
log as a reviewer. Sorry about that.

The above 2 suggestions are good, now the code is more neat.

Right, the extra bits for the default case of
standard_ProcessUtility() was not necessary, because we don't need to
apply any object-level checks to see as we just collect a list of
indexes.

Anyway, I've been working on the patch for the last few days, and
applied it after tweaking a bit its style, code and comments.

Mainly, I did not see a need for reindex_event_trigger_collect() as
wrapper for EventTriggerCollectSimpleCommand() as it is only used in
two code paths across two files. I have come back to the regression
tests, and trimmed them down to a minimum as event_trigger.sql cannot
be run in parallel so this eats in run time, concluding that knowing
the list of indexes rebuilt is also something that we track with
relfilenode change tracking in the other test suites (including the
SCHEMA, toast and partition cases), so doing that again had limited
impact.

One thing that I have been unable to conclude is if we should change
GetCommandLogLevel() for ReindexStmt. I agree that there's a good
argument for it, but I am going to raise a different thread about that
to raise awareness, and this does not prevent the feature to work
(even if it feels inconsistent with pg_event_trigger_ddl_commands()).
--
Michael

#23Alexander Lakhin
exclusion@gmail.com
In reply to: Michael Paquier (#22)
Re: PATCH: Add REINDEX tag to event triggers

Hi Michael,

04.12.2023 04:04, Michael Paquier wrote:

Anyway, I've been working on the patch for the last few days, and
applied it after tweaking a bit its style, code and comments.

Please look at the assertion failure triggered when REINDEX processed by an event trigger:
CREATE FUNCTION etf() RETURNS EVENT_TRIGGER AS $$ BEGIN PERFORM 1; END $$ LANGUAGE plpgsql;
CREATE EVENT TRIGGER et ON ddl_command_end EXECUTE FUNCTION etf();
CREATE TABLE t (i int);
REINDEX (CONCURRENTLY) TABLE t;

Core was generated by `postgres: law regression [local] REINDEX                                      '.
Program terminated with signal SIGABRT, Aborted.

warning: Section `.reg-xstate/338911' in core file too small.
#0  __pthread_kill_implementation (no_tid=0, signo=6, threadid=140462399108928) at ./nptl/pthread_kill.c:44
44      ./nptl/pthread_kill.c: No such file or directory.
(gdb) bt
#0  __pthread_kill_implementation (no_tid=0, signo=6, threadid=140462399108928) at ./nptl/pthread_kill.c:44
#1  __pthread_kill_internal (signo=6, threadid=140462399108928) at ./nptl/pthread_kill.c:78
#2  __GI___pthread_kill (threadid=140462399108928, signo=signo@entry=6) at ./nptl/pthread_kill.c:89
#3  0x00007fbff2c09476 in __GI_raise (sig=sig@entry=6) at ../sysdeps/posix/raise.c:26
#4  0x00007fbff2bef7f3 in __GI_abort () at ./stdlib/abort.c:79
#5  0x000055d88b8bb8de in ExceptionalCondition (conditionName=0x55d88baac6c0 "portal->portalSnapshot == NULL",
fileName=0x55d88baac2c4 "pquery.c", lineNumber=1796) at assert.c:66
#6  0x000055d88b6d6186 in EnsurePortalSnapshotExists () at pquery.c:1796
#7  0x000055d88b47d21a in _SPI_execute_plan (plan=0x55d88bfd2a80, options=0x7ffdc53b94c0, snapshot=0x0,
crosscheck_snapshot=0x0, fire_triggers=true) at spi.c:2579
#8  0x000055d88b479f37 in SPI_execute_plan_with_paramlist (plan=0x55d88bfd2a80, params=0x0, read_only=false, tcount=0)
at spi.c:749
#9  0x00007fbfe6de57ed in exec_run_select (estate=0x7ffdc53b97c0, expr=0x55d88bfba7f8, maxtuples=0, portalP=0x0) at
pl_exec.c:5815
#10 0x00007fbfe6dddce9 in exec_stmt_perform (estate=0x7ffdc53b97c0, stmt=0x55d88bfba950) at pl_exec.c:2171
#11 0x00007fbfe6ddd87c in exec_stmts (estate=0x7ffdc53b97c0, stmts=0x55d88bfbad90) at pl_exec.c:2023
#12 0x00007fbfe6ddd5f8 in exec_stmt_block (estate=0x7ffdc53b97c0, block=0x55d88bfbade0) at pl_exec.c:1942
#13 0x00007fbfe6ddccff in exec_toplevel_block (estate=0x7ffdc53b97c0, block=0x55d88bfbade0) at pl_exec.c:1633
#14 0x00007fbfe6ddbb6a in plpgsql_exec_event_trigger (func=0x55d88bf66840, trigdata=0x7ffdc53b9b00) at pl_exec.c:1198
#15 0x00007fbfe6df74c7 in plpgsql_call_handler (fcinfo=0x7ffdc53b9aa0) at pl_handler.c:272
#16 0x000055d88b34f592 in EventTriggerInvoke (fn_oid_list=0x55d88beffa40, trigdata=0x7ffdc53b9b00) at event_trigger.c:1087
#17 0x000055d88b34edea in EventTriggerDDLCommandEnd (parsetree=0x55d88bed6000) at event_trigger.c:803
#18 0x000055d88b6d9a41 in ProcessUtilitySlow (pstate=0x55d88beff930, pstmt=0x55d88bed60b0, queryString=0x55d88bed54f0
"REINDEX (CONCURRENTLY) TABLE t;", context=PROCESS_UTILITY_TOPLEVEL, params=0x0, queryEnv=0x0, dest=0x55d88bed6370,
    qc=0x7ffdc53ba310) at utility.c:1937
#19 0x000055d88b6d7ae9 in standard_ProcessUtility (pstmt=0x55d88bed60b0, queryString=0x55d88bed54f0 "REINDEX
(CONCURRENTLY) TABLE t;", readOnlyTree=false, context=PROCESS_UTILITY_TOPLEVEL, params=0x0, queryEnv=0x0,
    dest=0x55d88bed6370, qc=0x7ffdc53ba310) at utility.c:1074
#20 0x000055d88b6d69ea in ProcessUtility (pstmt=0x55d88bed60b0, queryString=0x55d88bed54f0 "REINDEX (CONCURRENTLY) TABLE
t;", readOnlyTree=false, context=PROCESS_UTILITY_TOPLEVEL, params=0x0, queryEnv=0x0, dest=0x55d88bed6370,
    qc=0x7ffdc53ba310) at utility.c:530
#21 0x000055d88b6d52b8 in PortalRunUtility (portal=0x55d88bf509f0, pstmt=0x55d88bed60b0, isTopLevel=true,
setHoldSnapshot=false, dest=0x55d88bed6370, qc=0x7ffdc53ba310) at pquery.c:1158
#22 0x000055d88b6d552f in PortalRunMulti (portal=0x55d88bf509f0, isTopLevel=true, setHoldSnapshot=false,
dest=0x55d88bed6370, altdest=0x55d88bed6370, qc=0x7ffdc53ba310) at pquery.c:1315
#23 0x000055d88b6d4979 in PortalRun (portal=0x55d88bf509f0, count=9223372036854775807, isTopLevel=true, run_once=true,
dest=0x55d88bed6370, altdest=0x55d88bed6370, qc=0x7ffdc53ba310) at pquery.c:791
#24 0x000055d88b6cd74a in exec_simple_query (query_string=0x55d88bed54f0 "REINDEX (CONCURRENTLY) TABLE t;") at
postgres.c:1273
#25 0x000055d88b6d26f6 in PostgresMain (dbname=0x55d88bf0c860 "regression", username=0x55d88bf0c848 "law") at
postgres.c:4653
#26 0x000055d88b5f2014 in BackendRun (port=0x55d88bf00f80) at postmaster.c:4425
#27 0x000055d88b5f1636 in BackendStartup (port=0x55d88bf00f80) at postmaster.c:4104
#28 0x000055d88b5edcf6 in ServerLoop () at postmaster.c:1772
#29 0x000055d88b5ed5f3 in PostmasterMain (argc=3, argv=0x55d88becf700) at postmaster.c:1471
#30 0x000055d88b49d2ca in main (argc=3, argv=0x55d88becf700) at main.c:198

Best regards,
Alexander

#24jian he
jian.universality@gmail.com
In reply to: Alexander Lakhin (#23)
Re: PATCH: Add REINDEX tag to event triggers

On Mon, Dec 4, 2023 at 11:00 PM Alexander Lakhin <exclusion@gmail.com> wrote:

Hi Michael,

04.12.2023 04:04, Michael Paquier wrote:

Anyway, I've been working on the patch for the last few days, and
applied it after tweaking a bit its style, code and comments.

Please look at the assertion failure triggered when REINDEX processed by an event trigger:
CREATE FUNCTION etf() RETURNS EVENT_TRIGGER AS $$ BEGIN PERFORM 1; END $$ LANGUAGE plpgsql;
CREATE EVENT TRIGGER et ON ddl_command_end EXECUTE FUNCTION etf();
CREATE TABLE t (i int);
REINDEX (CONCURRENTLY) TABLE t;

Core was generated by `postgres: law regression [local] REINDEX '.
Program terminated with signal SIGABRT, Aborted.

warning: Section `.reg-xstate/338911' in core file too small.
#0 __pthread_kill_implementation (no_tid=0, signo=6, threadid=140462399108928) at ./nptl/pthread_kill.c:44
44 ./nptl/pthread_kill.c: No such file or directory.
(gdb) bt
#0 __pthread_kill_implementation (no_tid=0, signo=6, threadid=140462399108928) at ./nptl/pthread_kill.c:44
#1 __pthread_kill_internal (signo=6, threadid=140462399108928) at ./nptl/pthread_kill.c:78
#2 __GI___pthread_kill (threadid=140462399108928, signo=signo@entry=6) at ./nptl/pthread_kill.c:89
#3 0x00007fbff2c09476 in __GI_raise (sig=sig@entry=6) at ../sysdeps/posix/raise.c:26
#4 0x00007fbff2bef7f3 in __GI_abort () at ./stdlib/abort.c:79
#5 0x000055d88b8bb8de in ExceptionalCondition (conditionName=0x55d88baac6c0 "portal->portalSnapshot == NULL",
fileName=0x55d88baac2c4 "pquery.c", lineNumber=1796) at assert.c:66
#6 0x000055d88b6d6186 in EnsurePortalSnapshotExists () at pquery.c:1796
#7 0x000055d88b47d21a in _SPI_execute_plan (plan=0x55d88bfd2a80, options=0x7ffdc53b94c0, snapshot=0x0,
crosscheck_snapshot=0x0, fire_triggers=true) at spi.c:2579
#8 0x000055d88b479f37 in SPI_execute_plan_with_paramlist (plan=0x55d88bfd2a80, params=0x0, read_only=false, tcount=0)
at spi.c:749
#9 0x00007fbfe6de57ed in exec_run_select (estate=0x7ffdc53b97c0, expr=0x55d88bfba7f8, maxtuples=0, portalP=0x0) at
pl_exec.c:5815
#10 0x00007fbfe6dddce9 in exec_stmt_perform (estate=0x7ffdc53b97c0, stmt=0x55d88bfba950) at pl_exec.c:2171
#11 0x00007fbfe6ddd87c in exec_stmts (estate=0x7ffdc53b97c0, stmts=0x55d88bfbad90) at pl_exec.c:2023
#12 0x00007fbfe6ddd5f8 in exec_stmt_block (estate=0x7ffdc53b97c0, block=0x55d88bfbade0) at pl_exec.c:1942
#13 0x00007fbfe6ddccff in exec_toplevel_block (estate=0x7ffdc53b97c0, block=0x55d88bfbade0) at pl_exec.c:1633
#14 0x00007fbfe6ddbb6a in plpgsql_exec_event_trigger (func=0x55d88bf66840, trigdata=0x7ffdc53b9b00) at pl_exec.c:1198
#15 0x00007fbfe6df74c7 in plpgsql_call_handler (fcinfo=0x7ffdc53b9aa0) at pl_handler.c:272
#16 0x000055d88b34f592 in EventTriggerInvoke (fn_oid_list=0x55d88beffa40, trigdata=0x7ffdc53b9b00) at event_trigger.c:1087
#17 0x000055d88b34edea in EventTriggerDDLCommandEnd (parsetree=0x55d88bed6000) at event_trigger.c:803
#18 0x000055d88b6d9a41 in ProcessUtilitySlow (pstate=0x55d88beff930, pstmt=0x55d88bed60b0, queryString=0x55d88bed54f0
"REINDEX (CONCURRENTLY) TABLE t;", context=PROCESS_UTILITY_TOPLEVEL, params=0x0, queryEnv=0x0, dest=0x55d88bed6370,
qc=0x7ffdc53ba310) at utility.c:1937
#19 0x000055d88b6d7ae9 in standard_ProcessUtility (pstmt=0x55d88bed60b0, queryString=0x55d88bed54f0 "REINDEX
(CONCURRENTLY) TABLE t;", readOnlyTree=false, context=PROCESS_UTILITY_TOPLEVEL, params=0x0, queryEnv=0x0,
dest=0x55d88bed6370, qc=0x7ffdc53ba310) at utility.c:1074
#20 0x000055d88b6d69ea in ProcessUtility (pstmt=0x55d88bed60b0, queryString=0x55d88bed54f0 "REINDEX (CONCURRENTLY) TABLE
t;", readOnlyTree=false, context=PROCESS_UTILITY_TOPLEVEL, params=0x0, queryEnv=0x0, dest=0x55d88bed6370,
qc=0x7ffdc53ba310) at utility.c:530
#21 0x000055d88b6d52b8 in PortalRunUtility (portal=0x55d88bf509f0, pstmt=0x55d88bed60b0, isTopLevel=true,
setHoldSnapshot=false, dest=0x55d88bed6370, qc=0x7ffdc53ba310) at pquery.c:1158
#22 0x000055d88b6d552f in PortalRunMulti (portal=0x55d88bf509f0, isTopLevel=true, setHoldSnapshot=false,
dest=0x55d88bed6370, altdest=0x55d88bed6370, qc=0x7ffdc53ba310) at pquery.c:1315
#23 0x000055d88b6d4979 in PortalRun (portal=0x55d88bf509f0, count=9223372036854775807, isTopLevel=true, run_once=true,
dest=0x55d88bed6370, altdest=0x55d88bed6370, qc=0x7ffdc53ba310) at pquery.c:791
#24 0x000055d88b6cd74a in exec_simple_query (query_string=0x55d88bed54f0 "REINDEX (CONCURRENTLY) TABLE t;") at
postgres.c:1273
#25 0x000055d88b6d26f6 in PostgresMain (dbname=0x55d88bf0c860 "regression", username=0x55d88bf0c848 "law") at
postgres.c:4653
#26 0x000055d88b5f2014 in BackendRun (port=0x55d88bf00f80) at postmaster.c:4425
#27 0x000055d88b5f1636 in BackendStartup (port=0x55d88bf00f80) at postmaster.c:4104
#28 0x000055d88b5edcf6 in ServerLoop () at postmaster.c:1772
#29 0x000055d88b5ed5f3 in PostmasterMain (argc=3, argv=0x55d88becf700) at postmaster.c:1471
#30 0x000055d88b49d2ca in main (argc=3, argv=0x55d88becf700) at main.c:198

Best regards,
Alexander

In indexcmd.c, function, ReindexRelationConcurrently
if (indexIds == NIL)
{
PopActiveSnapshot();
return false;
}

So there is no snapshot left, then PERFORM 1; need a snapshot.

#25Michael Paquier
michael@paquier.xyz
In reply to: jian he (#24)
1 attachment(s)
Re: PATCH: Add REINDEX tag to event triggers

On Tue, Dec 05, 2023 at 12:49:12AM +0800, jian he wrote:

On Mon, Dec 4, 2023 at 11:00 PM Alexander Lakhin <exclusion@gmail.com> wrote:

Please look at the assertion failure triggered when REINDEX processed by an event trigger:
CREATE FUNCTION etf() RETURNS EVENT_TRIGGER AS $$ BEGIN PERFORM 1; END $$ LANGUAGE plpgsql;
CREATE EVENT TRIGGER et ON ddl_command_end EXECUTE FUNCTION etf();
CREATE TABLE t (i int);
REINDEX (CONCURRENTLY) TABLE t;

In indexcmd.c, function, ReindexRelationConcurrently
if (indexIds == NIL)
{
PopActiveSnapshot();
return false;
}

So there is no snapshot left, then PERFORM 1; need a snapshot.

Popping a snapshot at this stage when there are no indexes has been a
decision taken by the original commit in 5dc92b844e68 because we had
no need for it yet, but we may do now depending on the function
triggered. I have been looking at the whole stack and something like
the attached to make a pop conditional seems to be sufficient to
satisfy all the cases I've been able to come up with, including the
one reported here.

Alexander, do you see any hole in that? Perhaps the snapshot push
should be more aggressive at the end of ReindexRelationConcurrently()
as well (in the last transaction when rebuilds happen)?
--
Michael

Attachments:

reindex-event-snap.patchtext/x-diff; charset=us-asciiDownload
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index 412e1ba84f..4ee498d985 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -3347,6 +3347,8 @@ ReindexMultipleInternal(const ReindexStmt *stmt, const List *relids, const Reind
 
 			newparams.options |= REINDEXOPT_MISSING_OK;
 			(void) ReindexRelationConcurrently(stmt, relid, &newparams);
+			if (ActiveSnapshotSet())
+				PopActiveSnapshot();
 			/* ReindexRelationConcurrently() does the verbose output */
 		}
 		else if (relkind == RELKIND_INDEX)
@@ -3698,10 +3700,7 @@ ReindexRelationConcurrently(const ReindexStmt *stmt, Oid relationOid, const Rein
 	 * session until this operation completes.
 	 */
 	if (indexIds == NIL)
-	{
-		PopActiveSnapshot();
 		return false;
-	}
 
 	/* It's not a shared catalog, so refuse to move it to shared tablespace */
 	if (params->tablespaceOid == GLOBALTABLESPACE_OID)
diff --git a/src/test/regress/expected/event_trigger.out b/src/test/regress/expected/event_trigger.out
index fdcd127945..a6ce337be5 100644
--- a/src/test/regress/expected/event_trigger.out
+++ b/src/test/regress/expected/event_trigger.out
@@ -581,6 +581,36 @@ $$ LANGUAGE plpgsql;
 CREATE EVENT TRIGGER regress_reindex_end ON ddl_command_end
     WHEN TAG IN ('REINDEX')
     EXECUTE PROCEDURE reindex_end_command();
+-- Extra event to force the use of a snapshot.
+CREATE FUNCTION reindex_end_command_snap() RETURNS EVENT_TRIGGER
+  AS $$ BEGIN PERFORM 1; END $$ LANGUAGE plpgsql;
+CREATE EVENT TRIGGER regress_reindex_end_snap ON ddl_command_end
+  EXECUTE FUNCTION reindex_end_command_snap();
+-- With a Schema
+CREATE SCHEMA concur_reindex_schema;
+-- No indexes
+REINDEX SCHEMA concur_reindex_schema;
+NOTICE:  REINDEX START: ddl_command_start REINDEX
+REINDEX SCHEMA CONCURRENTLY concur_reindex_schema;
+NOTICE:  REINDEX START: ddl_command_start REINDEX
+CREATE TABLE concur_reindex_schema.tab (a int);
+CREATE INDEX ind ON concur_reindex_schema.tab (a);
+-- One index reported
+REINDEX SCHEMA concur_reindex_schema;
+NOTICE:  REINDEX START: ddl_command_start REINDEX
+NOTICE:  REINDEX END: command_tag=REINDEX type=index identity=concur_reindex_schema.ind
+REINDEX SCHEMA CONCURRENTLY concur_reindex_schema;
+NOTICE:  REINDEX START: ddl_command_start REINDEX
+NOTICE:  REINDEX END: command_tag=REINDEX type=index identity=concur_reindex_schema.ind
+-- One table on schema but no indexes
+DROP INDEX concur_reindex_schema.ind;
+REINDEX SCHEMA concur_reindex_schema;
+NOTICE:  REINDEX START: ddl_command_start REINDEX
+REINDEX SCHEMA CONCURRENTLY concur_reindex_schema;
+NOTICE:  REINDEX START: ddl_command_start REINDEX
+DROP SCHEMA concur_reindex_schema CASCADE;
+NOTICE:  drop cascades to table concur_reindex_schema.tab
+-- With relation
 CREATE TABLE concur_reindex_tab (c1 int);
 CREATE INDEX concur_reindex_ind ON concur_reindex_tab (c1);
 -- Both start and end triggers enabled.
@@ -602,10 +632,18 @@ REINDEX INDEX concur_reindex_ind;
 NOTICE:  REINDEX END: command_tag=REINDEX type=index identity=public.concur_reindex_ind
 REINDEX INDEX CONCURRENTLY concur_reindex_ind;
 NOTICE:  REINDEX END: command_tag=REINDEX type=index identity=public.concur_reindex_ind
+-- without an index
+DROP INDEX concur_reindex_ind;
+REINDEX TABLE concur_reindex_tab;
+NOTICE:  table "concur_reindex_tab" has no indexes to reindex
+REINDEX TABLE CONCURRENTLY concur_reindex_tab;
+NOTICE:  table "concur_reindex_tab" has no indexes that can be reindexed concurrently
 -- Clean up
 DROP EVENT TRIGGER regress_reindex_start;
 DROP EVENT TRIGGER regress_reindex_end;
+DROP EVENT TRIGGER regress_reindex_end_snap;
 DROP FUNCTION reindex_end_command();
+DROP FUNCTION reindex_end_command_snap();
 DROP FUNCTION reindex_start_command();
 DROP TABLE concur_reindex_tab;
 -- test Row Security Event Trigger
diff --git a/src/test/regress/sql/event_trigger.sql b/src/test/regress/sql/event_trigger.sql
index e5c8bf8412..18bac8820d 100644
--- a/src/test/regress/sql/event_trigger.sql
+++ b/src/test/regress/sql/event_trigger.sql
@@ -443,7 +443,29 @@ $$ LANGUAGE plpgsql;
 CREATE EVENT TRIGGER regress_reindex_end ON ddl_command_end
     WHEN TAG IN ('REINDEX')
     EXECUTE PROCEDURE reindex_end_command();
+-- Extra event to force the use of a snapshot.
+CREATE FUNCTION reindex_end_command_snap() RETURNS EVENT_TRIGGER
+  AS $$ BEGIN PERFORM 1; END $$ LANGUAGE plpgsql;
+CREATE EVENT TRIGGER regress_reindex_end_snap ON ddl_command_end
+  EXECUTE FUNCTION reindex_end_command_snap();
 
+-- With a Schema
+CREATE SCHEMA concur_reindex_schema;
+-- No indexes
+REINDEX SCHEMA concur_reindex_schema;
+REINDEX SCHEMA CONCURRENTLY concur_reindex_schema;
+CREATE TABLE concur_reindex_schema.tab (a int);
+CREATE INDEX ind ON concur_reindex_schema.tab (a);
+-- One index reported
+REINDEX SCHEMA concur_reindex_schema;
+REINDEX SCHEMA CONCURRENTLY concur_reindex_schema;
+-- One table on schema but no indexes
+DROP INDEX concur_reindex_schema.ind;
+REINDEX SCHEMA concur_reindex_schema;
+REINDEX SCHEMA CONCURRENTLY concur_reindex_schema;
+DROP SCHEMA concur_reindex_schema CASCADE;
+
+-- With relation
 CREATE TABLE concur_reindex_tab (c1 int);
 CREATE INDEX concur_reindex_ind ON concur_reindex_tab (c1);
 -- Both start and end triggers enabled.
@@ -455,11 +477,17 @@ REINDEX TABLE CONCURRENTLY concur_reindex_tab;
 ALTER EVENT TRIGGER regress_reindex_start DISABLE;
 REINDEX INDEX concur_reindex_ind;
 REINDEX INDEX CONCURRENTLY concur_reindex_ind;
+-- without an index
+DROP INDEX concur_reindex_ind;
+REINDEX TABLE concur_reindex_tab;
+REINDEX TABLE CONCURRENTLY concur_reindex_tab;
 
 -- Clean up
 DROP EVENT TRIGGER regress_reindex_start;
 DROP EVENT TRIGGER regress_reindex_end;
+DROP EVENT TRIGGER regress_reindex_end_snap;
 DROP FUNCTION reindex_end_command();
+DROP FUNCTION reindex_end_command_snap();
 DROP FUNCTION reindex_start_command();
 DROP TABLE concur_reindex_tab;
 
#26Alexander Lakhin
exclusion@gmail.com
In reply to: Michael Paquier (#25)
Re: PATCH: Add REINDEX tag to event triggers

Hi Michael,

05.12.2023 02:45, Michael Paquier wrote:

Popping a snapshot at this stage when there are no indexes has been a
decision taken by the original commit in 5dc92b844e68 because we had
no need for it yet, but we may do now depending on the function
triggered. I have been looking at the whole stack and something like
the attached to make a pop conditional seems to be sufficient to
satisfy all the cases I've been able to come up with, including the
one reported here.

Alexander, do you see any hole in that? Perhaps the snapshot push
should be more aggressive at the end of ReindexRelationConcurrently()
as well (in the last transaction when rebuilds happen)?

Thank you for the fix proposed!

I agree with it. I had worried a bit about ReindexRelationConcurrently()
becoming twofold for callers (it can leave the snapshot or pop it), but I
couldn't find a way to hide this twofoldness inside without adding more
complexity. On the other hand, ReindexRelationConcurrently() now satisfies
EnsurePortalSnapshotExists() in all cases.

Best regards,
Alexander

#27Michael Paquier
michael@paquier.xyz
In reply to: Alexander Lakhin (#26)
Re: PATCH: Add REINDEX tag to event triggers

On Wed, Dec 06, 2023 at 10:00:01AM +0300, Alexander Lakhin wrote:

I agree with it. I had worried a bit about ReindexRelationConcurrently()
becoming twofold for callers (it can leave the snapshot or pop it), but I
couldn't find a way to hide this twofoldness inside without adding more
complexity. On the other hand, ReindexRelationConcurrently() now satisfies
EnsurePortalSnapshotExists() in all cases.

Thanks, applied that with a few more tests, covering a bit more than
the code path you've reported with a failure.

I was wondering if this should be backpatched, actually, but could not
make a case for it as we've never needed a snapshot after a reindex
until now, AFAIK.
--
Michael