From 3c465187a190ecfe21dc24dd36e64cbf304cbd17 Mon Sep 17 00:00:00 2001 From: Justin Pryzby Date: Sun, 7 Jun 2020 15:59:54 -0500 Subject: [PATCH v4 1/3] Implement REINDEX of partitioned tables/indexes --- doc/src/sgml/ref/reindex.sgml | 5 - src/backend/catalog/index.c | 7 +- src/backend/commands/indexcmds.c | 119 +++++++++++++-------- src/backend/tcop/utility.c | 6 +- src/include/commands/defrem.h | 6 +- src/test/regress/expected/create_index.out | 18 ++-- src/test/regress/sql/create_index.sql | 13 ++- 7 files changed, 106 insertions(+), 68 deletions(-) diff --git a/doc/src/sgml/ref/reindex.sgml b/doc/src/sgml/ref/reindex.sgml index aac5d5be23..e2e32b3ba0 100644 --- a/doc/src/sgml/ref/reindex.sgml +++ b/doc/src/sgml/ref/reindex.sgml @@ -258,11 +258,6 @@ REINDEX [ ( option [, ...] ) ] { IN can always reindex anything. - - Reindexing partitioned tables or partitioned indexes is not supported. - Each individual partition can be reindexed separately instead. - - Rebuilding Indexes Concurrently diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c index 1be27eec52..7abd7f5cfc 100644 --- a/src/backend/catalog/index.c +++ b/src/backend/catalog/index.c @@ -3660,12 +3660,7 @@ 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. - */ + /* Avoid erroring out */ if (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE) { ereport(WARNING, diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c index 2baca12c5f..df3e567acb 100644 --- a/src/backend/commands/indexcmds.c +++ b/src/backend/commands/indexcmds.c @@ -88,7 +88,8 @@ 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 ReindexPartitionedRel(Oid reloid, int options, bool concurrent, + bool isTopLevel); static void update_relispartition(Oid relationId, bool newval); static bool CompareOpclassOptions(Datum *opts1, Datum *opts2, int natts); @@ -2420,11 +2421,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 +2445,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) + ReindexPartitionedRel(indOid, options, concurrent, isTopLevel); + else if (concurrent && persistence != RELPERSISTENCE_TEMP) ReindexRelationConcurrently(indOid, options); else reindex_index(indOid, false, persistence, @@ -2542,7 +2530,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 +2548,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) + ReindexPartitionedRel(heapOid, options, concurrent, isTopLevel); + else if (concurrent && get_rel_persistence(heapOid) != RELPERSISTENCE_TEMP) { result = ReindexRelationConcurrently(heapOid, options); @@ -2688,11 +2678,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) @@ -2805,8 +2792,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 +2996,6 @@ 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; default: /* Return error if type of relation is not supported */ ereport(ERROR, @@ -3478,17 +3457,71 @@ ReindexRelationConcurrently(Oid relationOid, int options) } /* - * ReindexPartitionedIndex - * Reindex each child of the given partitioned index. - * - * Not yet implemented. + * ReindexPartitionedRel + * Reindex each child of the given partitioned relation. */ static void -ReindexPartitionedIndex(Relation parentIdx) +ReindexPartitionedRel(Oid reloid, int options, bool concurrent, bool isTopLevel) { - ereport(ERROR, - (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("REINDEX is not yet implemented for partitioned indexes"))); + MemoryContext oldcontext, reindex_context; + List *inhoids; + ListCell *lc; + + /* + * This cannot run inside a user transaction block; if + * we were inside a transaction, then its commit- and + * start-transaction-command calls would not have the + * intended effect! + */ + PreventInTransactionBlock(isTopLevel, + "REINDEX of partitioned relations"); // XXX + + /* + * Create list of children in longlived context, since we process each + * child in a separate transaction + */ + reindex_context = AllocSetContextCreate(PortalContext, "Reindex", + ALLOCSET_DEFAULT_SIZES); + oldcontext = MemoryContextSwitchTo(reindex_context); + inhoids = find_all_inheritors(reloid, NoLock, NULL); + MemoryContextSwitchTo(oldcontext); + + foreach (lc, inhoids) + { + Oid inhrelid = lfirst_oid(lc); + + PopActiveSnapshot(); + CommitTransactionCommand(); + + StartTransactionCommand(); + /* functions in indexes may want a snapshot set */ + PushActiveSnapshot(GetTransactionSnapshot()); + + switch (get_rel_relkind(inhrelid)) + { + case RELKIND_PARTITIONED_INDEX: + case RELKIND_PARTITIONED_TABLE: + /* + * We have a full list of direct and indirect children, so skip + * partitioned relations and just handle their children. + */ + continue; + case RELKIND_INDEX: + reindex_index(inhrelid, false, get_rel_persistence(inhrelid), + options | REINDEXOPT_REPORT_PROGRESS); + break; + case RELKIND_RELATION: + (void) reindex_relation(inhrelid, + REINDEX_REL_PROCESS_TOAST | + REINDEX_REL_CHECK_CONSTRAINTS, + options | REINDEXOPT_REPORT_PROGRESS); + break; + } + } + + PopActiveSnapshot(); + CommitTransactionCommand(); + StartTransactionCommand(); } /* diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c index 9b0c376c8c..fd6bc65c18 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/include/commands/defrem.h b/src/include/commands/defrem.h index c26a102b17..df32f5b201 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/test/regress/expected/create_index.out b/src/test/regress/expected/create_index.out index e3e6634d7e..61b4a30db4 100644 --- a/src/test/regress/expected/create_index.out +++ b/src/test/regress/expected/create_index.out @@ -2196,18 +2196,20 @@ 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 for partitioned indexes +REINDEX INDEX concur_reindex_part_index; +REINDEX INDEX CONCURRENTLY concur_reindex_part_index; +REINDEX INDEX concur_reindex_part_index_0; +REINDEX INDEX CONCURRENTLY concur_reindex_part_index_0; 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 for partitioned tables +REINDEX TABLE concur_reindex_part_10; +REINDEX TABLE CONCURRENTLY concur_reindex_part_10; +REINDEX TABLE concur_reindex_part_0; +REINDEX TABLE CONCURRENTLY concur_reindex_part_0; 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 diff --git a/src/test/regress/sql/create_index.sql b/src/test/regress/sql/create_index.sql index f3667bacdc..a751dd5caa 100644 --- a/src/test/regress/sql/create_index.sql +++ b/src/test/regress/sql/create_index.sql @@ -903,12 +903,21 @@ 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 for partitioned indexes +REINDEX INDEX concur_reindex_part_index; +REINDEX INDEX CONCURRENTLY concur_reindex_part_index; +REINDEX INDEX concur_reindex_part_index_0; +REINDEX INDEX CONCURRENTLY concur_reindex_part_index_0; REINDEX INDEX concur_reindex_part_index_10; REINDEX INDEX CONCURRENTLY concur_reindex_part_index_10; --- REINDEX is a no-op for partitioned tables +-- REINDEX for partitioned tables REINDEX TABLE concur_reindex_part_10; REINDEX TABLE CONCURRENTLY concur_reindex_part_10; +REINDEX TABLE concur_reindex_part_0; +REINDEX TABLE CONCURRENTLY concur_reindex_part_0; +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. -- 2.17.0