Reindex toast before its main relation in reindex_relation()

michaelpq · michaelpq · commit f2bf8fb04886 · 2024-01-26T17:39:58.000+09:00
This commit changes the order of reindex on a relation so as a toast relation is processed before its main relation. The original order, where a rebuild was first done for the indexes on the main table, could be a problem in the event of a corruption of a toast index, because, as scans of a toast index may be required to rebuild the indexes on the main relation, this could lead to failures with REINDEX TABLE without being able to fix anything. Rebuilding corrupted toast indexes before this change was possible but troublesome, as it was necessary to issue a REINDEX on the toast relation first, followed by a REINDEX on the main relation. Changing the order of these operations should make things easier when rebuilding corrupted indexes, as toast indexes would be rebuilt before they are used for the indexes on the main relation. Per request from Richard Vesely. Author: Gurjeet Singh Reviewed-by: Nathan Bossart, Michael Paquier Discussion: https://postgr.es/m/18016-2bd9b549b1fe49b3@postgresql.org
diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c
@@ -3917,7 +3917,7 @@ reindex_relation(const ReindexStmt *stmt, Oid relid, int flags,
 	Oid			toast_relid;
 	List	   *indexIds;
 	char		persistence;
-	bool		result;
+	bool		result = false;
 	ListCell   *indexId;
 	int			i;
 
@@ -3964,6 +3964,35 @@ reindex_relation(const ReindexStmt *stmt, Oid relid, int flags,
 		CommandCounterIncrement();
 	}
 
+	/*
+	 * Reindex the toast table, if any, before the main table.
+	 *
+	 * This helps in cases where a corruption in the toast table's index would
+	 * otherwise error and stop REINDEX TABLE command when it tries to fetch a
+	 * toasted datum.  This way. the toast table's index is rebuilt and fixed
+	 * before it is used for reindexing the main table.
+	 *
+	 * It is critical to call reindex_relation() *after* the call to
+	 * RelationGetIndexList() returning the list of indexes on the relation,
+	 * because reindex_relation() will call CommandCounterIncrement() after
+	 * every reindex_index().  See REINDEX_REL_SUPPRESS_INDEX_USE for more
+	 * details.
+	 */
+	if ((flags & REINDEX_REL_PROCESS_TOAST) && OidIsValid(toast_relid))
+	{
+		/*
+		 * Note that this should fail if the toast relation is missing, so
+		 * reset REINDEXOPT_MISSING_OK.  Even if a new tablespace is set for
+		 * the parent relation, the indexes on its toast table are not moved.
+		 * This rule is enforced by setting tablespaceOid to InvalidOid.
+		 */
+		ReindexParams newparams = *params;
+
+		newparams.options &= ~(REINDEXOPT_MISSING_OK);
+		newparams.tablespaceOid = InvalidOid;
+		result |= reindex_relation(stmt, toast_relid, flags, &newparams);
+	}
+
 	/*
 	 * Compute persistence of indexes: same as that of owning rel, unless
 	 * caller specified otherwise.
@@ -4017,26 +4046,7 @@ reindex_relation(const ReindexStmt *stmt, Oid relid, int flags,
 	 */
 	table_close(rel, NoLock);
 
-	result = (indexIds != NIL);
-
-	/*
-	 * If the relation has a secondary toast rel, reindex that too while we
-	 * still hold the lock on the main table.
-	 */
-	if ((flags & REINDEX_REL_PROCESS_TOAST) && OidIsValid(toast_relid))
-	{
-		/*
-		 * Note that this should fail if the toast relation is missing, so
-		 * reset REINDEXOPT_MISSING_OK.  Even if a new tablespace is set for
-		 * the parent relation, the indexes on its toast table are not moved.
-		 * This rule is enforced by setting tablespaceOid to InvalidOid.
-		 */
-		ReindexParams newparams = *params;
-
-		newparams.options &= ~(REINDEXOPT_MISSING_OK);
-		newparams.tablespaceOid = InvalidOid;
-		result |= reindex_relation(stmt, toast_relid, flags, &newparams);
-	}
+	result |= (indexIds != NIL);
 
 	return result;
 }
