On Wed, Aug 12, 2020 at 12:28:20AM -0500, Justin Pryzby wrote: > On Wed, Aug 12, 2020 at 01:54:38PM +0900, Michael Paquier wrote: >> +++ b/src/backend/catalog/index.c >> @@ -3661,20 +3662,12 @@ reindex_relation(Oid relid, int flags, int options) >> + elog(ERROR, "unsupported relation kind for relation \"%s\"", >> + RelationGetRelationName(rel)); > > I guess it should show the relkind(%c) in the message, like these:
Sure, but I don't see much the point in adding the relkind here knowing that we know which one it is. > ISTM reindex_index is missing that, too: > > 8b08f7d4820fd7a8ef6152a9dd8c6e3cb01e5f99 > + if (iRel->rd_rel->relkind == RELKIND_PARTITIONED_INDEX) > + elog(ERROR, "unsupported relation kind for index \"%s\"", > + RelationGetRelationName(iRel)); The error string does not follow the usual project style either, so I have updated both. >> <para> >> - Reindexing partitioned tables or partitioned indexes is not supported. >> - Each individual partition can be reindexed separately instead. >> + Reindexing partitioned indexes or partitioned tables is supported >> + with respectively <command>REINDEX INDEX</command> or >> + <command>REINDEX TABLE</command>. > > Should say "..with REINDEX INDEX or REINDEX TABLE, respectively". > >> + Each partition of the partitioned >> + relation defined is rebuilt in its own transaction. > > => Each partition of the specified partitioned relation is reindexed in a > separate transaction. Thanks, good idea. I have been able to work more on this patch today, and finally added an error context for the transaction block check, as that's cleaner. In my manual tests, I have also bumped into a case that failed with the original patch (where there were no session locks), and created an isolation test based on it: drop of a partition leaf concurrently to REINDEX done on the parent. One last thing I have spotted is that we failed to discard properly foreign tables defined as leaves of a partition tree, causing a reindex to fail, so reindex_partitions() ought to just use RELKIND_HAS_STORAGE() to do its filtering work. I am leaving this patch alone for a couple of days now, and I'll try to come back to it after and potentially commit it. The attached has been indented by the way. -- Michael
diff --git a/src/include/commands/defrem.h b/src/include/commands/defrem.h
index c26a102b17..22db3f660d 100644
--- a/src/include/commands/defrem.h
+++ b/src/include/commands/defrem.h
@@ -34,8 +34,10 @@ extern ObjectAddress DefineIndex(Oid relationId,
bool check_not_in_use,
bool skip_build,
bool quiet);
-extern void ReindexIndex(RangeVar *indexRelation, int options, bool concurrent);
-extern Oid ReindexTable(RangeVar *relation, int options, bool concurrent);
+extern void ReindexIndex(RangeVar *indexRelation, int options, bool concurrent,
+ bool isTopLevel);
+extern Oid ReindexTable(RangeVar *relation, int options, bool concurrent,
+ bool isTopLevel);
extern void ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind,
int options, bool concurrent);
extern char *makeObjectName(const char *name1, const char *name2,
diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c
index 1be27eec52..14e3464d0e 100644
--- a/src/backend/catalog/index.c
+++ b/src/backend/catalog/index.c
@@ -77,6 +77,7 @@
#include "utils/lsyscache.h"
#include "utils/memutils.h"
#include "utils/pg_rusage.h"
+#include "utils/rel.h"
#include "utils/snapmgr.h"
#include "utils/syscache.h"
#include "utils/tuplesort.h"
@@ -3447,11 +3448,12 @@ reindex_index(Oid indexId, bool skip_constraint_checks, char persistence,
iRel->rd_rel->relam);
/*
- * The case of reindexing partitioned tables and indexes is handled
- * differently by upper layers, so this case shouldn't arise.
+ * Partitioned indexes should never get processed here, as they have no
+ * physical storage.
*/
if (iRel->rd_rel->relkind == RELKIND_PARTITIONED_INDEX)
- elog(ERROR, "unsupported relation kind for index \"%s\"",
+ elog(ERROR, "cannot reindex partitioned index \"%s.%s\"",
+ get_namespace_name(RelationGetNamespace(iRel)),
RelationGetRelationName(iRel));
/*
@@ -3661,20 +3663,13 @@ reindex_relation(Oid relid, int flags, int options)
rel = table_open(relid, ShareLock);
/*
- * This may be useful when implemented someday; but that day is not today.
- * For now, avoid erroring out when called in a multi-table context
- * (REINDEX SCHEMA) and happen to come across a partitioned table. The
- * partitions may be reindexed on their own anyway.
+ * Partitioned tables should never get processed here, as they have no
+ * physical storage.
*/
if (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)
- {
- ereport(WARNING,
- (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
- errmsg("REINDEX of partitioned tables is not yet implemented, skipping \"%s\"",
- RelationGetRelationName(rel))));
- table_close(rel, ShareLock);
- return false;
- }
+ elog(ERROR, "cannot reindex partitioned table \"%s.%s\"",
+ get_namespace_name(RelationGetNamespace(rel)),
+ RelationGetRelationName(rel));
toast_relid = rel->rd_rel->reltoastrelid;
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index 7819266a63..0f787661de 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -88,7 +88,12 @@ static List *ChooseIndexColumnNames(List *indexElems);
static void RangeVarCallbackForReindexIndex(const RangeVar *relation,
Oid relId, Oid oldRelId, void *arg);
static bool ReindexRelationConcurrently(Oid relationOid, int options);
-static void ReindexPartitionedIndex(Relation parentIdx);
+
+static void reindex_partitions(Oid relid, int options, bool concurrent,
+ bool isTopLevel);
+static void reindex_multiple_internal(List *relids, int options,
+ bool concurrent);
+static void reindex_error_callback(void *args);
static void update_relispartition(Oid relationId, bool newval);
static bool CompareOpclassOptions(Datum *opts1, Datum *opts2, int natts);
@@ -101,6 +106,16 @@ struct ReindexIndexCallbackState
Oid locked_table_oid; /* tracks previously locked table */
};
+/*
+ * callback arguments for reindex_error_callback()
+ */
+typedef struct ReindexErrorInfo
+{
+ char *relname;
+ char *relnamespace;
+ char relkind;
+} ReindexErrorInfo;
+
/*
* CheckIndexCompatible
* Determine whether an existing index definition is compatible with a
@@ -2420,11 +2435,10 @@ ChooseIndexColumnNames(List *indexElems)
* Recreate a specific index.
*/
void
-ReindexIndex(RangeVar *indexRelation, int options, bool concurrent)
+ReindexIndex(RangeVar *indexRelation, int options, bool concurrent, bool isTopLevel)
{
struct ReindexIndexCallbackState state;
Oid indOid;
- Relation irel;
char persistence;
/*
@@ -2445,22 +2459,10 @@ ReindexIndex(RangeVar *indexRelation, int options, bool concurrent)
RangeVarCallbackForReindexIndex,
&state);
- /*
- * Obtain the current persistence of the existing index. We already hold
- * lock on the index.
- */
- irel = index_open(indOid, NoLock);
-
- if (irel->rd_rel->relkind == RELKIND_PARTITIONED_INDEX)
- {
- ReindexPartitionedIndex(irel);
- return;
- }
-
- persistence = irel->rd_rel->relpersistence;
- index_close(irel, NoLock);
-
- if (concurrent && persistence != RELPERSISTENCE_TEMP)
+ persistence = get_rel_persistence(indOid);
+ if (get_rel_relkind(indOid) == RELKIND_PARTITIONED_INDEX)
+ reindex_partitions(indOid, options, concurrent, isTopLevel);
+ else if (concurrent && persistence != RELPERSISTENCE_TEMP)
ReindexRelationConcurrently(indOid, options);
else
reindex_index(indOid, false, persistence,
@@ -2542,7 +2544,7 @@ RangeVarCallbackForReindexIndex(const RangeVar *relation,
* Recreate all indexes of a table (and of its toast table, if any)
*/
Oid
-ReindexTable(RangeVar *relation, int options, bool concurrent)
+ReindexTable(RangeVar *relation, int options, bool concurrent, bool isTopLevel)
{
Oid heapOid;
bool result;
@@ -2560,7 +2562,9 @@ ReindexTable(RangeVar *relation, int options, bool concurrent)
0,
RangeVarCallbackOwnsTable, NULL);
- if (concurrent && get_rel_persistence(heapOid) != RELPERSISTENCE_TEMP)
+ if (get_rel_relkind(heapOid) == RELKIND_PARTITIONED_TABLE)
+ reindex_partitions(heapOid, options, concurrent, isTopLevel);
+ else if (concurrent && get_rel_persistence(heapOid) != RELPERSISTENCE_TEMP)
{
result = ReindexRelationConcurrently(heapOid, options);
@@ -2604,7 +2608,6 @@ ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind,
MemoryContext private_context;
MemoryContext old;
List *relids = NIL;
- ListCell *l;
int num_keys;
bool concurrent_warning = false;
@@ -2688,11 +2691,8 @@ ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind,
* Only regular tables and matviews can have indexes, so ignore any
* other kind of relation.
*
- * It is tempting to also consider partitioned tables here, but that
- * has the problem that if the children are in the same schema, they
- * would be processed twice. Maybe we could have a separate list of
- * partitioned tables, and expand that afterwards into relids,
- * ignoring any duplicates.
+ * Partitioned tables/indexes are skipped but matching leaf partitions
+ * are processed.
*/
if (classtuple->relkind != RELKIND_RELATION &&
classtuple->relkind != RELKIND_MATVIEW)
@@ -2755,22 +2755,202 @@ ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind,
table_endscan(scan);
table_close(relationRelation, AccessShareLock);
- /* Now reindex each rel in a separate transaction */
+ /*
+ * Process each relation listed in a separate transaction. Note that this
+ * commits and then starts a new transaction immediately.
+ */
+ reindex_multiple_internal(relids, options, concurrent);
+
+ MemoryContextDelete(private_context);
+}
+
+/*
+ * Error callback specific to reindex_partitions().
+ */
+static void
+reindex_error_callback(void *arg)
+{
+ ReindexErrorInfo *errinfo = (ReindexErrorInfo *) arg;
+
+ Assert(errinfo->relkind == RELKIND_PARTITIONED_INDEX ||
+ errinfo->relkind == RELKIND_PARTITIONED_TABLE);
+
+ if (errinfo->relkind == RELKIND_PARTITIONED_TABLE)
+ errcontext("while reindexing partitioned table \"%s.%s\"",
+ errinfo->relnamespace, errinfo->relname);
+ else if (errinfo->relkind == RELKIND_PARTITIONED_INDEX)
+ errcontext("while reindexing partitioned index \"%s.%s\"",
+ errinfo->relnamespace, errinfo->relname);
+}
+
+/*
+ * reindex_partitions
+ *
+ * Reindex a set of partitions, per the partitioned index or table given
+ * by the caller.
+ */
+static void
+reindex_partitions(Oid relid, int options, bool concurrent,
+ bool isTopLevel)
+{
+ List *partitions = NIL;
+ char relkind = get_rel_relkind(relid);
+ char *relname = get_rel_name(relid);
+ char *relnamespace = get_namespace_name(get_rel_namespace(relid));
+ MemoryContext reindex_context;
+ List *inhoids;
+ ListCell *lc;
+ LOCKMODE lockmode = concurrent ?
+ ShareUpdateExclusiveLock : AccessExclusiveLock;
+ List *partLocks = NIL;
+ ErrorContextCallback errcallback;
+ ReindexErrorInfo errinfo;
+
+ Assert(relkind == RELKIND_PARTITIONED_INDEX ||
+ relkind == RELKIND_PARTITIONED_TABLE);
+
+ /*
+ * Check if this runs in a transaction block, with an error callback to
+ * provide the context under which a problem happens.
+ */
+ errinfo.relname = pstrdup(relname);
+ errinfo.relnamespace = pstrdup(relnamespace);
+ errinfo.relkind = relkind;
+ errcallback.callback = reindex_error_callback;
+ errcallback.arg = (void *) &errinfo;
+ errcallback.previous = error_context_stack;
+ error_context_stack = &errcallback;
+
+ PreventInTransactionBlock(isTopLevel,
+ relkind == RELKIND_PARTITIONED_TABLE ?
+ "REINDEX TABLE" : "REINDEX INDEX");
+
+ /* Pop the error context stack */
+ error_context_stack = errcallback.previous;
+
+ /*
+ * Create special memory context for cross-transaction storage.
+ *
+ * Since it is a child of PortalContext, it will go away eventually even
+ * if we suffer an error so there is no need for special abort cleanup
+ * logic.
+ */
+ reindex_context = AllocSetContextCreate(PortalContext, "Reindex",
+ ALLOCSET_DEFAULT_SIZES);
+
+ /* ShareLock is enough to prevent schema modifications */
+ inhoids = find_all_inheritors(relid, ShareLock, NULL);
+
+ /*
+ * The list of relations to reindex are the physical partitions of the
+ * tree so discard any partitioned table or index, and take some session
+ * locks. For partition indexes, the parent table needs to be locked.
+ */
+ foreach(lc, inhoids)
+ {
+ Oid partoid = lfirst_oid(lc);
+ Oid parentoid;
+ char partkind = get_rel_relkind(partoid);
+ Relation rel;
+ MemoryContext old_context;
+ LockRelId *lockrelid;
+
+ /*
+ * This discards partitioned tables, partitioned indexes and foreign
+ * tables.
+ */
+ if (!RELKIND_HAS_STORAGE(partkind))
+ continue;
+
+ Assert(partkind == RELKIND_INDEX ||
+ partkind == RELKIND_RELATION);
+
+ parentoid = (partkind == RELKIND_INDEX) ?
+ IndexGetRelation(partoid, false) : partoid;
+
+ rel = relation_open(parentoid, lockmode);
+
+ /* Save partition OID and the session lock of parent table */
+ old_context = MemoryContextSwitchTo(reindex_context);
+ partitions = lappend_oid(partitions, partoid);
+ lockrelid = palloc(sizeof(LockRelId));
+ *lockrelid = rel->rd_lockInfo.lockRelId;
+ partLocks = lappend(partLocks, lockrelid);
+ MemoryContextSwitchTo(old_context);
+
+ relation_close(rel, NoLock);
+ LockRelationIdForSession(lockrelid, lockmode);
+ }
+
+ /*
+ * Process each partition listed in a separate transaction. Note that
+ * this commits and then starts a new transaction immediately.
+ */
+ reindex_multiple_internal(partitions, options, concurrent);
+
+ /* Finally, release the session-level locks */
+ foreach(lc, partLocks)
+ {
+ LockRelId *lockrelid = (LockRelId *) lfirst(lc);
+
+ UnlockRelationIdForSession(lockrelid, lockmode);
+ }
+
+ /*
+ * Clean up working storage --- note we must do this after
+ * StartTransactionCommand, else we might be trying to delete the active
+ * context!
+ */
+ MemoryContextDelete(reindex_context);
+}
+
+/*
+ * reindex_multiple_internal
+ *
+ * Reindex a list of relations, each one being processed in its own
+ * transaction. This commits the existing transaction immediately,
+ * and starts a new transaction when done.
+ */
+static void
+reindex_multiple_internal(List *relids, int options, bool concurrent)
+{
+ ListCell *l;
+
PopActiveSnapshot();
CommitTransactionCommand();
+
foreach(l, relids)
{
Oid relid = lfirst_oid(l);
+ char relkind;
+ char relpersistence;
StartTransactionCommand();
+
/* functions in indexes may want a snapshot set */
PushActiveSnapshot(GetTransactionSnapshot());
+ relkind = get_rel_relkind(relid);
+ relpersistence = get_rel_persistence(relid);
+
+ /*
+ * Partitioned tables and indexes can never be processed directly, and
+ * a list of their leaves should be built first.
+ */
+ Assert(relkind != RELKIND_PARTITIONED_INDEX &&
+ relkind != RELKIND_PARTITIONED_TABLE);
+
if (concurrent && get_rel_persistence(relid) != RELPERSISTENCE_TEMP)
{
(void) ReindexRelationConcurrently(relid, options);
/* ReindexRelationConcurrently() does the verbose output */
}
+ else if (relkind == RELKIND_INDEX)
+ {
+ reindex_index(relid, false, relpersistence,
+ options | REINDEXOPT_REPORT_PROGRESS);
+ PopActiveSnapshot();
+ }
else
{
bool result;
@@ -2791,9 +2971,8 @@ ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind,
CommitTransactionCommand();
}
- StartTransactionCommand();
- MemoryContextDelete(private_context);
+ StartTransactionCommand();
}
@@ -2805,8 +2984,7 @@ ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind,
* view. For tables and materialized views, all its indexes will be rebuilt,
* excluding invalid indexes and any indexes used in exclusion constraints,
* but including its associated toast table indexes. For indexes, the index
- * itself will be rebuilt. If 'relationOid' belongs to a partitioned table
- * then we issue a warning to mention these are not yet supported.
+ * itself will be rebuilt.
*
* The locks taken on parent tables and involved indexes are kept until the
* transaction is committed, at which point a session lock is taken on each
@@ -3010,13 +3188,9 @@ ReindexRelationConcurrently(Oid relationOid, int options)
MemoryContextSwitchTo(oldcontext);
break;
}
+
case RELKIND_PARTITIONED_TABLE:
- /* see reindex_relation() */
- ereport(WARNING,
- (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
- errmsg("REINDEX of partitioned tables is not yet implemented, skipping \"%s\"",
- get_rel_name(relationOid))));
- return false;
+ case RELKIND_PARTITIONED_INDEX:
default:
/* Return error if type of relation is not supported */
ereport(ERROR,
@@ -3477,20 +3651,6 @@ ReindexRelationConcurrently(Oid relationOid, int options)
return true;
}
-/*
- * ReindexPartitionedIndex
- * Reindex each child of the given partitioned index.
- *
- * Not yet implemented.
- */
-static void
-ReindexPartitionedIndex(Relation parentIdx)
-{
- ereport(ERROR,
- (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
- errmsg("REINDEX is not yet implemented for partitioned indexes")));
-}
-
/*
* Insert or delete an appropriate pg_inherits tuple to make the given index
* be a partition of the indicated parent index.
diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c
index 9b0c376c8c..fef4064d1c 100644
--- a/src/backend/tcop/utility.c
+++ b/src/backend/tcop/utility.c
@@ -926,10 +926,12 @@ standard_ProcessUtility(PlannedStmt *pstmt,
switch (stmt->kind)
{
case REINDEX_OBJECT_INDEX:
- ReindexIndex(stmt->relation, stmt->options, stmt->concurrent);
+ ReindexIndex(stmt->relation, stmt->options,
+ stmt->concurrent, isTopLevel);
break;
case REINDEX_OBJECT_TABLE:
- ReindexTable(stmt->relation, stmt->options, stmt->concurrent);
+ ReindexTable(stmt->relation, stmt->options,
+ stmt->concurrent, isTopLevel);
break;
case REINDEX_OBJECT_SCHEMA:
case REINDEX_OBJECT_SYSTEM:
diff --git a/src/test/isolation/expected/reindex-partitions.out b/src/test/isolation/expected/reindex-partitions.out
new file mode 100644
index 0000000000..ab7e6bbaa0
--- /dev/null
+++ b/src/test/isolation/expected/reindex-partitions.out
@@ -0,0 +1,19 @@
+Parsed test spec with 3 sessions
+
+starting permutation: begin1 lock1 reindex2 drop3 end1
+step begin1: BEGIN;
+step lock1: LOCK reind_conc_parent IN ACCESS EXCLUSIVE MODE;
+step reindex2: REINDEX TABLE reind_conc_parent; <waiting ...>
+step drop3: DROP TABLE reind_conc_10_20; <waiting ...>
+step end1: COMMIT;
+step reindex2: <... completed>
+step drop3: <... completed>
+
+starting permutation: begin1 lock1 reindex_conc2 drop3 end1
+step begin1: BEGIN;
+step lock1: LOCK reind_conc_parent IN ACCESS EXCLUSIVE MODE;
+step reindex_conc2: REINDEX TABLE CONCURRENTLY reind_conc_parent; <waiting ...>
+step drop3: DROP TABLE reind_conc_10_20; <waiting ...>
+step end1: COMMIT;
+step reindex_conc2: <... completed>
+step drop3: <... completed>
diff --git a/src/test/isolation/isolation_schedule b/src/test/isolation/isolation_schedule
index 218c87b24b..2edb8f2662 100644
--- a/src/test/isolation/isolation_schedule
+++ b/src/test/isolation/isolation_schedule
@@ -49,6 +49,7 @@ test: lock-committed-update
test: lock-committed-keyupdate
test: update-locked-tuple
test: reindex-concurrently
+test: reindex-partitions
test: propagate-lock-delete
test: tuplelock-conflict
test: tuplelock-update
diff --git a/src/test/isolation/specs/reindex-partitions.spec b/src/test/isolation/specs/reindex-partitions.spec
new file mode 100644
index 0000000000..b50ab376ab
--- /dev/null
+++ b/src/test/isolation/specs/reindex-partitions.spec
@@ -0,0 +1,35 @@
+# REINDEX with partitioned tables
+#
+# Ensure that concurrent and non-concurrent operations work correctly when
+# a REINDEX is performed on a partitioned table or index.
+
+setup
+{
+ CREATE TABLE reind_conc_parent (id int) PARTITION BY RANGE (id);
+ CREATE TABLE reind_conc_0_10 PARTITION OF reind_conc_parent
+ FOR VALUES FROM (0) TO (10);
+ CREATE TABLE reind_conc_10_20 PARTITION OF reind_conc_parent
+ FOR VALUES FROM (10) TO (20);
+ INSERT INTO reind_conc_parent VALUES (generate_series(0, 19));
+}
+
+teardown
+{
+ DROP TABLE reind_conc_parent;
+}
+
+session "s1"
+step "begin1" { BEGIN; }
+step "lock1" { LOCK reind_conc_parent IN ACCESS EXCLUSIVE MODE; }
+step "end1" { COMMIT; }
+
+session "s2"
+step "reindex2" { REINDEX TABLE reind_conc_parent; }
+step "reindex_conc2" { REINDEX TABLE CONCURRENTLY reind_conc_parent; }
+
+session "s3"
+step "drop3" { DROP TABLE reind_conc_10_20; }
+
+# The partition leaf can be dropped, after a successful reindex.
+permutation "begin1" "lock1" "reindex2" "drop3" "end1"
+permutation "begin1" "lock1" "reindex_conc2" "drop3" "end1"
diff --git a/src/test/regress/expected/create_index.out b/src/test/regress/expected/create_index.out
index e3e6634d7e..3ccf3719d4 100644
--- a/src/test/regress/expected/create_index.out
+++ b/src/test/regress/expected/create_index.out
@@ -2196,18 +2196,6 @@ SELECT relid, parentrelid, level FROM pg_partition_tree('concur_reindex_part_ind
concur_reindex_part_index_0_2 | concur_reindex_part_index_0 | 2
(5 rows)
--- REINDEX fails for partitioned indexes
-REINDEX INDEX concur_reindex_part_index_10;
-ERROR: REINDEX is not yet implemented for partitioned indexes
-REINDEX INDEX CONCURRENTLY concur_reindex_part_index_10;
-ERROR: REINDEX is not yet implemented for partitioned indexes
--- REINDEX is a no-op for partitioned tables
-REINDEX TABLE concur_reindex_part_10;
-WARNING: REINDEX of partitioned tables is not yet implemented, skipping "concur_reindex_part_10"
-NOTICE: table "concur_reindex_part_10" has no indexes to reindex
-REINDEX TABLE CONCURRENTLY concur_reindex_part_10;
-WARNING: REINDEX of partitioned tables is not yet implemented, skipping "concur_reindex_part_10"
-NOTICE: table "concur_reindex_part_10" has no indexes that can be reindexed concurrently
SELECT relid, parentrelid, level FROM pg_partition_tree('concur_reindex_part_index')
ORDER BY relid, level;
relid | parentrelid | level
@@ -2320,6 +2308,152 @@ SELECT relid, parentrelid, level FROM pg_partition_tree('concur_reindex_part_ind
concur_reindex_part_index_0_2 | concur_reindex_part_index_0 | 2
(5 rows)
+-- REINDEX for partitioned indexes
+-- REINDEX TABLE fails for partitioned indexes
+-- Top-most parent index
+REINDEX TABLE concur_reindex_part_index;
+ERROR: "concur_reindex_part_index" is not a table or materialized view
+REINDEX TABLE CONCURRENTLY concur_reindex_part_index;
+ERROR: "concur_reindex_part_index" is not a table or materialized view
+-- Partitioned index with no leaves
+REINDEX TABLE concur_reindex_part_index_10;
+ERROR: "concur_reindex_part_index_10" is not a table or materialized view
+REINDEX TABLE CONCURRENTLY concur_reindex_part_index_10;
+ERROR: "concur_reindex_part_index_10" is not a table or materialized view
+-- Cannot run in a transaction block
+BEGIN;
+REINDEX INDEX concur_reindex_part_index;
+ERROR: REINDEX INDEX cannot run inside a transaction block
+CONTEXT: while reindexing partitioned index "public.concur_reindex_part_index"
+ROLLBACK;
+-- Helper functions to track changes of relfilenodes in a partition tree.
+-- Create a table tracking the relfilenode state.
+CREATE OR REPLACE FUNCTION create_relfilenode_part(relname text, indname text)
+ RETURNS VOID AS
+ $func$
+ BEGIN
+ EXECUTE format('
+ CREATE TABLE %I AS
+ SELECT oid, relname, relfilenode, relkind, reltoastrelid
+ FROM pg_class
+ WHERE oid IN
+ (SELECT relid FROM pg_partition_tree(''%I''));',
+ relname, indname);
+ END
+ $func$ LANGUAGE plpgsql;
+CREATE OR REPLACE FUNCTION compare_relfilenode_part(tabname text)
+ RETURNS TABLE (relname name, relkind "char", state text) AS
+ $func$
+ BEGIN
+ RETURN QUERY EXECUTE
+ format(
+ 'SELECT b.relname,
+ b.relkind,
+ CASE WHEN a.relfilenode = b.relfilenode THEN ''relfilenode is unchanged''
+ ELSE ''relfilenode has changed'' END
+ -- Do not join with OID here as CONCURRENTLY changes it.
+ FROM %I b JOIN pg_class a ON b.relname = a.relname
+ ORDER BY 1;', tabname);
+ END
+ $func$ LANGUAGE plpgsql;
+-- Check that expected relfilenodes are changed, non-concurrent case.
+SELECT create_relfilenode_part('reindex_index_status', 'concur_reindex_part_index');
+ create_relfilenode_part
+-------------------------
+
+(1 row)
+
+REINDEX INDEX concur_reindex_part_index;
+SELECT * FROM compare_relfilenode_part('reindex_index_status');
+ relname | relkind | state
+-------------------------------+---------+--------------------------
+ concur_reindex_part_index | I | relfilenode is unchanged
+ concur_reindex_part_index_0 | I | relfilenode is unchanged
+ concur_reindex_part_index_0_1 | i | relfilenode has changed
+ concur_reindex_part_index_0_2 | i | relfilenode has changed
+ concur_reindex_part_index_10 | I | relfilenode is unchanged
+(5 rows)
+
+DROP TABLE reindex_index_status;
+-- concurrent case.
+SELECT create_relfilenode_part('reindex_index_status', 'concur_reindex_part_index');
+ create_relfilenode_part
+-------------------------
+
+(1 row)
+
+REINDEX INDEX CONCURRENTLY concur_reindex_part_index;
+SELECT * FROM compare_relfilenode_part('reindex_index_status');
+ relname | relkind | state
+-------------------------------+---------+--------------------------
+ concur_reindex_part_index | I | relfilenode is unchanged
+ concur_reindex_part_index_0 | I | relfilenode is unchanged
+ concur_reindex_part_index_0_1 | i | relfilenode has changed
+ concur_reindex_part_index_0_2 | i | relfilenode has changed
+ concur_reindex_part_index_10 | I | relfilenode is unchanged
+(5 rows)
+
+DROP TABLE reindex_index_status;
+-- REINDEX for partitioned tables
+-- REINDEX INDEX fails for partitioned tables
+-- Top-most parent
+REINDEX INDEX concur_reindex_part;
+ERROR: "concur_reindex_part" is not an index
+REINDEX INDEX CONCURRENTLY concur_reindex_part;
+ERROR: "concur_reindex_part" is not an index
+-- Partitioned with no leaves
+REINDEX INDEX concur_reindex_part_10;
+ERROR: "concur_reindex_part_10" is not an index
+REINDEX INDEX CONCURRENTLY concur_reindex_part_10;
+ERROR: "concur_reindex_part_10" is not an index
+-- Cannot run in a transaction block
+BEGIN;
+REINDEX TABLE concur_reindex_part;
+ERROR: REINDEX TABLE cannot run inside a transaction block
+CONTEXT: while reindexing partitioned table "public.concur_reindex_part"
+ROLLBACK;
+-- Check that expected relfilenodes are changed, non-concurrent case.
+-- Note that the partition tree changes of the *indexes* need to be checked.
+SELECT create_relfilenode_part('reindex_index_status', 'concur_reindex_part_index');
+ create_relfilenode_part
+-------------------------
+
+(1 row)
+
+REINDEX TABLE concur_reindex_part;
+SELECT * FROM compare_relfilenode_part('reindex_index_status');
+ relname | relkind | state
+-------------------------------+---------+--------------------------
+ concur_reindex_part_index | I | relfilenode is unchanged
+ concur_reindex_part_index_0 | I | relfilenode is unchanged
+ concur_reindex_part_index_0_1 | i | relfilenode has changed
+ concur_reindex_part_index_0_2 | i | relfilenode has changed
+ concur_reindex_part_index_10 | I | relfilenode is unchanged
+(5 rows)
+
+DROP TABLE reindex_index_status;
+-- concurrent case.
+SELECT create_relfilenode_part('reindex_index_status', 'concur_reindex_part_index');
+ create_relfilenode_part
+-------------------------
+
+(1 row)
+
+REINDEX TABLE CONCURRENTLY concur_reindex_part;
+SELECT * FROM compare_relfilenode_part('reindex_index_status');
+ relname | relkind | state
+-------------------------------+---------+--------------------------
+ concur_reindex_part_index | I | relfilenode is unchanged
+ concur_reindex_part_index_0 | I | relfilenode is unchanged
+ concur_reindex_part_index_0_1 | i | relfilenode has changed
+ concur_reindex_part_index_0_2 | i | relfilenode has changed
+ concur_reindex_part_index_10 | I | relfilenode is unchanged
+(5 rows)
+
+DROP TABLE reindex_index_status;
+DROP FUNCTION create_relfilenode_part;
+DROP FUNCTION compare_relfilenode_part;
+-- Cleanup of partition tree used for REINDEX test.
DROP TABLE concur_reindex_part;
-- Check errors
-- Cannot run inside a transaction block
diff --git a/src/test/regress/sql/create_index.sql b/src/test/regress/sql/create_index.sql
index f3667bacdc..6d98b73365 100644
--- a/src/test/regress/sql/create_index.sql
+++ b/src/test/regress/sql/create_index.sql
@@ -903,12 +903,6 @@ CREATE INDEX concur_reindex_part_index_0_2 ON ONLY concur_reindex_part_0_2 (c1);
ALTER INDEX concur_reindex_part_index_0 ATTACH PARTITION concur_reindex_part_index_0_2;
SELECT relid, parentrelid, level FROM pg_partition_tree('concur_reindex_part_index')
ORDER BY relid, level;
--- REINDEX fails for partitioned indexes
-REINDEX INDEX concur_reindex_part_index_10;
-REINDEX INDEX CONCURRENTLY concur_reindex_part_index_10;
--- REINDEX is a no-op for partitioned tables
-REINDEX TABLE concur_reindex_part_10;
-REINDEX TABLE CONCURRENTLY concur_reindex_part_10;
SELECT relid, parentrelid, level FROM pg_partition_tree('concur_reindex_part_index')
ORDER BY relid, level;
-- REINDEX should preserve dependencies of partition tree.
@@ -948,6 +942,88 @@ WHERE classid = 'pg_class'::regclass AND
ORDER BY 1, 2;
SELECT relid, parentrelid, level FROM pg_partition_tree('concur_reindex_part_index')
ORDER BY relid, level;
+
+-- REINDEX for partitioned indexes
+-- REINDEX TABLE fails for partitioned indexes
+-- Top-most parent index
+REINDEX TABLE concur_reindex_part_index;
+REINDEX TABLE CONCURRENTLY concur_reindex_part_index;
+-- Partitioned index with no leaves
+REINDEX TABLE concur_reindex_part_index_10;
+REINDEX TABLE CONCURRENTLY concur_reindex_part_index_10;
+-- Cannot run in a transaction block
+BEGIN;
+REINDEX INDEX concur_reindex_part_index;
+ROLLBACK;
+-- Helper functions to track changes of relfilenodes in a partition tree.
+-- Create a table tracking the relfilenode state.
+CREATE OR REPLACE FUNCTION create_relfilenode_part(relname text, indname text)
+ RETURNS VOID AS
+ $func$
+ BEGIN
+ EXECUTE format('
+ CREATE TABLE %I AS
+ SELECT oid, relname, relfilenode, relkind, reltoastrelid
+ FROM pg_class
+ WHERE oid IN
+ (SELECT relid FROM pg_partition_tree(''%I''));',
+ relname, indname);
+ END
+ $func$ LANGUAGE plpgsql;
+CREATE OR REPLACE FUNCTION compare_relfilenode_part(tabname text)
+ RETURNS TABLE (relname name, relkind "char", state text) AS
+ $func$
+ BEGIN
+ RETURN QUERY EXECUTE
+ format(
+ 'SELECT b.relname,
+ b.relkind,
+ CASE WHEN a.relfilenode = b.relfilenode THEN ''relfilenode is unchanged''
+ ELSE ''relfilenode has changed'' END
+ -- Do not join with OID here as CONCURRENTLY changes it.
+ FROM %I b JOIN pg_class a ON b.relname = a.relname
+ ORDER BY 1;', tabname);
+ END
+ $func$ LANGUAGE plpgsql;
+-- Check that expected relfilenodes are changed, non-concurrent case.
+SELECT create_relfilenode_part('reindex_index_status', 'concur_reindex_part_index');
+REINDEX INDEX concur_reindex_part_index;
+SELECT * FROM compare_relfilenode_part('reindex_index_status');
+DROP TABLE reindex_index_status;
+-- concurrent case.
+SELECT create_relfilenode_part('reindex_index_status', 'concur_reindex_part_index');
+REINDEX INDEX CONCURRENTLY concur_reindex_part_index;
+SELECT * FROM compare_relfilenode_part('reindex_index_status');
+DROP TABLE reindex_index_status;
+
+-- REINDEX for partitioned tables
+-- REINDEX INDEX fails for partitioned tables
+-- Top-most parent
+REINDEX INDEX concur_reindex_part;
+REINDEX INDEX CONCURRENTLY concur_reindex_part;
+-- Partitioned with no leaves
+REINDEX INDEX concur_reindex_part_10;
+REINDEX INDEX CONCURRENTLY concur_reindex_part_10;
+-- Cannot run in a transaction block
+BEGIN;
+REINDEX TABLE concur_reindex_part;
+ROLLBACK;
+-- Check that expected relfilenodes are changed, non-concurrent case.
+-- Note that the partition tree changes of the *indexes* need to be checked.
+SELECT create_relfilenode_part('reindex_index_status', 'concur_reindex_part_index');
+REINDEX TABLE concur_reindex_part;
+SELECT * FROM compare_relfilenode_part('reindex_index_status');
+DROP TABLE reindex_index_status;
+-- concurrent case.
+SELECT create_relfilenode_part('reindex_index_status', 'concur_reindex_part_index');
+REINDEX TABLE CONCURRENTLY concur_reindex_part;
+SELECT * FROM compare_relfilenode_part('reindex_index_status');
+DROP TABLE reindex_index_status;
+
+DROP FUNCTION create_relfilenode_part;
+DROP FUNCTION compare_relfilenode_part;
+
+-- Cleanup of partition tree used for REINDEX test.
DROP TABLE concur_reindex_part;
-- Check errors
diff --git a/doc/src/sgml/ref/reindex.sgml b/doc/src/sgml/ref/reindex.sgml
index aac5d5be23..665d6d33f7 100644
--- a/doc/src/sgml/ref/reindex.sgml
+++ b/doc/src/sgml/ref/reindex.sgml
@@ -88,7 +88,9 @@ REINDEX [ ( <replaceable class="parameter">option</replaceable> [, ...] ) ] { IN
<term><literal>INDEX</literal></term>
<listitem>
<para>
- Recreate the specified index.
+ Recreate the specified index. This form of <command>REINDEX</command>
+ cannot be executed inside a transaction block when used with a
+ partitioned index.
</para>
</listitem>
</varlistentry>
@@ -99,6 +101,8 @@ REINDEX [ ( <replaceable class="parameter">option</replaceable> [, ...] ) ] { IN
<para>
Recreate all indexes of the specified table. If the table has a
secondary <quote>TOAST</quote> table, that is reindexed as well.
+ This form of <command>REINDEX</command> cannot be executed inside a
+ transaction block when used with a partitioned table.
</para>
</listitem>
</varlistentry>
@@ -259,8 +263,11 @@ REINDEX [ ( <replaceable class="parameter">option</replaceable> [, ...] ) ] { IN
</para>
<para>
- Reindexing partitioned tables or partitioned indexes is not supported.
- Each individual partition can be reindexed separately instead.
+ Reindexing partitioned indexes or partitioned tables is supported
+ with <command>REINDEX INDEX</command> or <command>REINDEX TABLE</command>,
+ respectively. Each partition of the specified partitioned relation is
+ reindexed in a separate transaction. Those commands cannot be used inside
+ a transaction block when working on a partitioned table or index.
</para>
<refsect2 id="sql-reindex-concurrently" xreflabel="Rebuilding Indexes Concurrently">
diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list
index 7eaaad1e14..26eb2400d2 100644
--- a/src/tools/pgindent/typedefs.list
+++ b/src/tools/pgindent/typedefs.list
@@ -2035,6 +2035,7 @@ RegProcedure
Regis
RegisNode
RegisteredBgWorker
+ReindexErrorInfo
ReindexObjectType
ReindexStmt
ReindexType
signature.asc
Description: PGP signature
