From 25486d56303de512368f322c87528c2be29af0de Mon Sep 17 00:00:00 2001 From: Justin Pryzby Date: Sun, 7 Jun 2020 15:59:54 -0500 Subject: [PATCH v5 1/3] Implement REINDEX of partitioned tables/indexes --- doc/src/sgml/ref/reindex.sgml | 5 - src/backend/catalog/index.c | 8 +- src/backend/commands/indexcmds.c | 137 ++++++++++++++------- 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, 122 insertions(+), 71 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..f69f027890 100644 --- a/src/backend/catalog/index.c +++ b/src/backend/catalog/index.c @@ -3660,12 +3660,8 @@ 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. - */ + /* Skip partitioned tables if called in a multi-table context. The + * partitions are reindexed on their own. */ 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..68d75916ae 100644 --- a/src/backend/commands/indexcmds.c +++ b/src/backend/commands/indexcmds.c @@ -88,7 +88,10 @@ 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 List *PartitionedLeaves(Oid reloid); +static void ReindexMultipleRelsWorker(List *relids, int options, + bool concurrent); static void update_relispartition(Oid relationId, bool newval); static bool CompareOpclassOptions(Datum *opts1, Datum *opts2, int natts); @@ -2420,11 +2423,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 +2447,13 @@ 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) + persistence = get_rel_persistence(indOid); + if (get_rel_relkind(indOid) == RELKIND_PARTITIONED_INDEX) { - ReindexPartitionedIndex(irel); - return; + List *relids = PartitionedLeaves(indOid); + ReindexMultipleRelsWorker(relids, options, concurrent); } - - persistence = irel->rd_rel->relpersistence; - index_close(irel, NoLock); - - if (concurrent && persistence != RELPERSISTENCE_TEMP) + else if (concurrent && persistence != RELPERSISTENCE_TEMP) ReindexRelationConcurrently(indOid, options); else reindex_index(indOid, false, persistence, @@ -2542,7 +2535,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 +2553,12 @@ 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) + { + List *relids = PartitionedLeaves(heapOid); + ReindexMultipleRelsWorker(relids, options, concurrent); + } + else if (concurrent && get_rel_persistence(heapOid) != RELPERSISTENCE_TEMP) { result = ReindexRelationConcurrently(heapOid, options); @@ -2604,7 +2602,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 +2685,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 +2749,59 @@ ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind, table_endscan(scan); table_close(relationRelation, AccessShareLock); - /* Now reindex each rel in a separate transaction */ + ReindexMultipleRelsWorker(relids, options, concurrent); + MemoryContextDelete(private_context); +} + +/* Reindex each rel in a separate transaction */ +static void +ReindexMultipleRelsWorker(List *relids, int options, bool concurrent) +{ + ListCell *l; + + /* + * 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 + PopActiveSnapshot(); CommitTransactionCommand(); + foreach(l, relids) { Oid relid = lfirst_oid(l); + char relkind; StartTransactionCommand(); + /* functions in indexes may want a snapshot set */ PushActiveSnapshot(GetTransactionSnapshot()); + relkind = get_rel_relkind(relid); + + if (relkind == RELKIND_PARTITIONED_INDEX || + relkind == RELKIND_PARTITIONED_TABLE) + { + abort(); + // ReindexPartitionedRel(relid, options, concurrent, true); + } + if (concurrent && get_rel_persistence(relid) != RELPERSISTENCE_TEMP) { + /* Handles concurrent indexing of both indexes and tables */ (void) ReindexRelationConcurrently(relid, options); /* ReindexRelationConcurrently() does the verbose output */ } + else if (relkind == RELKIND_INDEX) + { + reindex_index(relid, false, get_rel_persistence(relid), + options | REINDEXOPT_REPORT_PROGRESS); + PopActiveSnapshot(); + } else { bool result; @@ -2792,8 +2823,6 @@ ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind, CommitTransactionCommand(); } StartTransactionCommand(); - - MemoryContextDelete(private_context); } @@ -2805,8 +2834,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 +3038,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 +3499,41 @@ ReindexRelationConcurrently(Oid relationOid, int options) } /* - * ReindexPartitionedIndex - * Reindex each child of the given partitioned index. - * - * Not yet implemented. + * PartitionedLeaves + * Return a list of leaf partitions to be reindexed. */ -static void -ReindexPartitionedIndex(Relation parentIdx) +static List * +PartitionedLeaves(Oid reloid) { - ereport(ERROR, - (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("REINDEX is not yet implemented for partitioned indexes"))); + MemoryContext oldcontext, reindex_context; + List *inhoids; + ListCell *lc; + + /* + * Create list of children in longlived context, since we will 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); + + /* + * We have a full list of direct and indirect children, so remove from + * the list any partitioned relations (including the rel itself) and + * handle the leaves directly. + */ + foreach (lc, inhoids) + { + Oid relid = lfirst_oid(lc); + char relkind = get_rel_relkind(relid); + if (relkind == RELKIND_PARTITIONED_INDEX || + relkind == RELKIND_PARTITIONED_TABLE) + inhoids = foreach_delete_current(inhoids, lc); + } + + MemoryContextSwitchTo(oldcontext); + return inhoids; } /* 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