On Mon, Nov 20, 2023 at 3:34 PM Michael Paquier <mich...@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:
> https://www.postgresql.org/message-id/cab7npqtqz2-ycnzoq5kvbujyhq4kdsd4q55mc-fbzm8gh0b...@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.
From 149b4e9860f92daa810dfed4704f4c796a585b1a Mon Sep 17 00:00:00 2001
From: pgaddict <jian.universal...@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

Reply via email to