diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out index 90db550b92..84bc0ee381 100644 --- a/contrib/postgres_fdw/expected/postgres_fdw.out +++ b/contrib/postgres_fdw/expected/postgres_fdw.out @@ -4044,6 +4044,27 @@ SELECT f_test(100); DROP FUNCTION f_test(int); -- =================================================================== +-- REINDEX +-- =================================================================== +-- remote table is not created here +CREATE FOREIGN TABLE reindex_foreign (c1 int, c2 int) + SERVER loopback2 OPTIONS (table_name 'reindex_local'); +REINDEX TABLE reindex_foreign; -- error +ERROR: "reindex_foreign" is not a table or materialized view +REINDEX TABLE CONCURRENTLY reindex_foreign; -- error +ERROR: "reindex_foreign" is not a table or materialized view +DROP FOREIGN TABLE reindex_foreign; +-- partitions and foreign tables +CREATE TABLE reind_fdw_parent (c1 int) PARTITION BY RANGE (c1); +CREATE TABLE reind_fdw_0_10 PARTITION OF reind_fdw_parent + FOR VALUES FROM (0) TO (10); +CREATE FOREIGN TABLE reind_fdw_10_20 PARTITION OF reind_fdw_parent + FOR VALUES FROM (10) TO (20) + SERVER loopback OPTIONS (table_name 'reind_local_10_20'); +REINDEX TABLE reind_fdw_parent; -- ok +REINDEX TABLE CONCURRENTLY reind_fdw_parent; -- ok +DROP TABLE reind_fdw_parent; +-- =================================================================== -- conversion error -- =================================================================== ALTER FOREIGN TABLE ft1 ALTER COLUMN c8 TYPE int; diff --git a/contrib/postgres_fdw/sql/postgres_fdw.sql b/contrib/postgres_fdw/sql/postgres_fdw.sql index 83971665e3..d452d06343 100644 --- a/contrib/postgres_fdw/sql/postgres_fdw.sql +++ b/contrib/postgres_fdw/sql/postgres_fdw.sql @@ -1081,6 +1081,26 @@ $$ LANGUAGE plpgsql; SELECT f_test(100); DROP FUNCTION f_test(int); +-- =================================================================== +-- REINDEX +-- =================================================================== +-- remote table is not created here +CREATE FOREIGN TABLE reindex_foreign (c1 int, c2 int) + SERVER loopback2 OPTIONS (table_name 'reindex_local'); +REINDEX TABLE reindex_foreign; -- error +REINDEX TABLE CONCURRENTLY reindex_foreign; -- error +DROP FOREIGN TABLE reindex_foreign; +-- partitions and foreign tables +CREATE TABLE reind_fdw_parent (c1 int) PARTITION BY RANGE (c1); +CREATE TABLE reind_fdw_0_10 PARTITION OF reind_fdw_parent + FOR VALUES FROM (0) TO (10); +CREATE FOREIGN TABLE reind_fdw_10_20 PARTITION OF reind_fdw_parent + FOR VALUES FROM (10) TO (20) + SERVER loopback OPTIONS (table_name 'reind_local_10_20'); +REINDEX TABLE reind_fdw_parent; -- ok +REINDEX TABLE CONCURRENTLY reind_fdw_parent; -- ok +DROP TABLE reind_fdw_parent; + -- =================================================================== -- conversion error -- =================================================================== diff --git a/doc/src/sgml/ref/reindex.sgml b/doc/src/sgml/ref/reindex.sgml index c16f223e4e..33af4ae02a 100644 --- a/doc/src/sgml/ref/reindex.sgml +++ b/doc/src/sgml/ref/reindex.sgml @@ -88,7 +88,9 @@ REINDEX [ ( option [, ...] ) ] { IN INDEX - Recreate the specified index. + Recreate the specified index. This form of REINDEX + cannot be executed inside a transaction block when used with a + partitioned index. @@ -99,6 +101,8 @@ REINDEX [ ( option [, ...] ) ] { IN Recreate all indexes of the specified table. If the table has a secondary TOAST table, that is reindexed as well. + This form of REINDEX cannot be executed inside a + transaction block when used with a partitioned table. @@ -259,8 +263,11 @@ REINDEX [ ( option [, ...] ) ] { IN - 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 REINDEX INDEX or REINDEX TABLE, + 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. diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c index b2c8cb320e..117e3fdef7 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" @@ -3486,11 +3487,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)); /* @@ -3707,20 +3709,13 @@ reindex_relation(Oid relid, int flags, int options) return false; /* - * 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 430e88b4c9..f1b5f87e6a 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 void ReindexPartitions(Oid relid, int options, bool isTopLevel); +static void ReindexMultipleInternal(List *relids, int options); +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 +104,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,12 +2433,12 @@ ChooseIndexColumnNames(List *indexElems) * Recreate a specific index. */ void -ReindexIndex(RangeVar *indexRelation, int options) +ReindexIndex(RangeVar *indexRelation, int options, bool isTopLevel) { struct ReindexIndexCallbackState state; Oid indOid; - Relation irel; char persistence; + char relkind; /* * Find and lock index, and check permissions on table; use callback to @@ -2447,22 +2460,16 @@ ReindexIndex(RangeVar *indexRelation, int options) &state); /* - * Obtain the current persistence of the existing index. We already hold - * lock on the index. + * Obtain the current persistence and kind of the existing index. We + * already hold a lock on the index. */ - irel = index_open(indOid, NoLock); + persistence = get_rel_persistence(indOid); + relkind = get_rel_relkind(indOid); - if (irel->rd_rel->relkind == RELKIND_PARTITIONED_INDEX) - { - ReindexPartitionedIndex(irel); - return; - } - - persistence = irel->rd_rel->relpersistence; - index_close(irel, NoLock); - - if ((options & REINDEXOPT_CONCURRENTLY) != 0 && - persistence != RELPERSISTENCE_TEMP) + if (relkind == RELKIND_PARTITIONED_INDEX) + ReindexPartitions(indOid, options, isTopLevel); + else if ((options & REINDEXOPT_CONCURRENTLY) != 0 && + persistence != RELPERSISTENCE_TEMP) ReindexRelationConcurrently(indOid, options); else reindex_index(indOid, false, persistence, @@ -2545,7 +2552,7 @@ RangeVarCallbackForReindexIndex(const RangeVar *relation, * Recreate all indexes of a table (and of its toast table, if any) */ Oid -ReindexTable(RangeVar *relation, int options) +ReindexTable(RangeVar *relation, int options, bool isTopLevel) { Oid heapOid; bool result; @@ -2564,8 +2571,10 @@ ReindexTable(RangeVar *relation, int options) 0, RangeVarCallbackOwnsTable, NULL); - if ((options & REINDEXOPT_CONCURRENTLY) != 0 && - get_rel_persistence(heapOid) != RELPERSISTENCE_TEMP) + if (get_rel_relkind(heapOid) == RELKIND_PARTITIONED_TABLE) + ReindexPartitions(heapOid, options, isTopLevel); + else if ((options & REINDEXOPT_CONCURRENTLY) != 0 && + get_rel_persistence(heapOid) != RELPERSISTENCE_TEMP) { result = ReindexRelationConcurrently(heapOid, options); @@ -2609,7 +2618,6 @@ ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind, MemoryContext private_context; MemoryContext old; List *relids = NIL; - ListCell *l; int num_keys; bool concurrent_warning = false; @@ -2694,11 +2702,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) @@ -2761,14 +2766,151 @@ 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. + */ + ReindexMultipleInternal(relids, options); + + MemoryContextDelete(private_context); +} + +/* + * Error callback specific to ReindexPartitions(). + */ +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); +} + +/* + * ReindexPartitions + * + * Reindex a set of partitions, per the partitioned index or table given + * by the caller. + */ +static void +ReindexPartitions(Oid relid, int options, 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; + 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 more 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. + */ + foreach(lc, inhoids) + { + Oid partoid = lfirst_oid(lc); + char partkind = get_rel_relkind(partoid); + MemoryContext old_context; + + /* + * This discards partitioned tables, partitioned indexes and foreign + * tables. + */ + if (!RELKIND_HAS_STORAGE(partkind)) + continue; + + Assert(partkind == RELKIND_INDEX || + partkind == RELKIND_RELATION); + + /* Save partition OID */ + old_context = MemoryContextSwitchTo(reindex_context); + partitions = lappend_oid(partitions, partoid); + MemoryContextSwitchTo(old_context); + } + + /* + * Process each partition listed in a separate transaction. Note that + * this commits and then starts a new transaction immediately. + */ + ReindexMultipleInternal(partitions, options); + + /* + * Clean up working storage --- note we must do this after + * StartTransactionCommand, else we might be trying to delete the active + * context! + */ + MemoryContextDelete(reindex_context); +} + +/* + * ReindexMultipleInternal + * + * 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 finished. + */ +static void +ReindexMultipleInternal(List *relids, int options) +{ + 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()); @@ -2780,14 +2922,33 @@ ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind, continue; } + 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 ((options & REINDEXOPT_CONCURRENTLY) != 0 && - get_rel_persistence(relid) != RELPERSISTENCE_TEMP) + relpersistence != RELPERSISTENCE_TEMP) { (void) ReindexRelationConcurrently(relid, options | REINDEXOPT_MISSING_OK); /* ReindexRelationConcurrently() does the verbose output */ } + else if (relkind == RELKIND_INDEX) + { + reindex_index(relid, false, relpersistence, + options | + REINDEXOPT_REPORT_PROGRESS | + REINDEXOPT_MISSING_OK); + PopActiveSnapshot(); + /* reindex_index() does the verbose output */ + } else { bool result; @@ -2810,9 +2971,8 @@ ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind, CommitTransactionCommand(); } - StartTransactionCommand(); - MemoryContextDelete(private_context); + StartTransactionCommand(); } @@ -2824,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 @@ -3064,13 +3223,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, @@ -3537,20 +3692,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 b4cde5565e..9713a7ac41 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); + ReindexIndex(stmt->relation, stmt->options, + isTopLevel); break; case REINDEX_OBJECT_TABLE: - ReindexTable(stmt->relation, stmt->options); + ReindexTable(stmt->relation, stmt->options, + 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 3129b684f6..7a079ef07f 100644 --- a/src/include/commands/defrem.h +++ b/src/include/commands/defrem.h @@ -34,8 +34,8 @@ extern ObjectAddress DefineIndex(Oid relationId, bool check_not_in_use, bool skip_build, bool quiet); -extern void ReindexIndex(RangeVar *indexRelation, int options); -extern Oid ReindexTable(RangeVar *relation, int options); +extern void ReindexIndex(RangeVar *indexRelation, int options, bool isTopLevel); +extern Oid ReindexTable(RangeVar *relation, int options, bool isTopLevel); extern void ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind, int options); extern char *makeObjectName(const char *name1, const char *name2, diff --git a/src/test/isolation/expected/reindex-partitions.out b/src/test/isolation/expected/reindex-partitions.out new file mode 100644 index 0000000000..5373abde25 --- /dev/null +++ b/src/test/isolation/expected/reindex-partitions.out @@ -0,0 +1,107 @@ +Parsed test spec with 3 sessions + +starting permutation: begin1 lockexcl1 reindex2 drop3 end1 +step begin1: BEGIN; +step lockexcl1: LOCK reind_conc_parent IN ACCESS EXCLUSIVE MODE; +step reindex2: REINDEX TABLE reind_conc_parent; +step drop3: DROP TABLE reind_conc_10_20; +step end1: COMMIT; +step reindex2: <... completed> +step drop3: <... completed> + +starting permutation: begin1 lockexcl1 reindex_conc2 drop3 end1 +step begin1: BEGIN; +step lockexcl1: LOCK reind_conc_parent IN ACCESS EXCLUSIVE MODE; +step reindex_conc2: REINDEX TABLE CONCURRENTLY reind_conc_parent; +step drop3: DROP TABLE reind_conc_10_20; +step end1: COMMIT; +step reindex_conc2: <... completed> +step drop3: <... completed> + +starting permutation: begin1 lockshare1 reindex2 drop3 end1 +step begin1: BEGIN; +step lockshare1: LOCK reind_conc_parent IN SHARE MODE; +step reindex2: REINDEX TABLE reind_conc_parent; +step drop3: DROP TABLE reind_conc_10_20; +step end1: COMMIT; +step drop3: <... completed> + +starting permutation: begin1 lockshare1 reindex_conc2 drop3 end1 +step begin1: BEGIN; +step lockshare1: LOCK reind_conc_parent IN SHARE MODE; +step reindex_conc2: REINDEX TABLE CONCURRENTLY reind_conc_parent; +step drop3: DROP TABLE reind_conc_10_20; +step end1: COMMIT; +step reindex_conc2: <... completed> +step drop3: <... completed> + +starting permutation: begin1 lockupdate1 reindex2 drop3 end1 +step begin1: BEGIN; +step lockupdate1: LOCK reind_conc_parent IN SHARE UPDATE EXCLUSIVE MODE; +step reindex2: REINDEX TABLE reind_conc_parent; +step drop3: DROP TABLE reind_conc_10_20; +step end1: COMMIT; +step reindex2: <... completed> +step drop3: <... completed> + +starting permutation: begin1 lockupdate1 reindex_conc2 drop3 end1 +step begin1: BEGIN; +step lockupdate1: LOCK reind_conc_parent IN SHARE UPDATE EXCLUSIVE MODE; +step reindex_conc2: REINDEX TABLE CONCURRENTLY reind_conc_parent; +step drop3: DROP TABLE reind_conc_10_20; +step end1: COMMIT; +step reindex_conc2: <... completed> +step drop3: <... completed> + +starting permutation: begin1 lockpartexcl1 reindex2 drop3 end1 +step begin1: BEGIN; +step lockpartexcl1: LOCK reind_conc_10_20 IN ACCESS EXCLUSIVE MODE; +step reindex2: REINDEX TABLE reind_conc_parent; +step drop3: DROP TABLE reind_conc_10_20; +step end1: COMMIT; +step reindex2: <... completed> +step drop3: <... completed> + +starting permutation: begin1 lockpartexcl1 reindex_conc2 drop3 end1 +step begin1: BEGIN; +step lockpartexcl1: LOCK reind_conc_10_20 IN ACCESS EXCLUSIVE MODE; +step reindex_conc2: REINDEX TABLE CONCURRENTLY reind_conc_parent; +step drop3: DROP TABLE reind_conc_10_20; +step end1: COMMIT; +step reindex_conc2: <... completed> +step drop3: <... completed> + +starting permutation: begin1 lockpartshare1 reindex2 drop3 end1 +step begin1: BEGIN; +step lockpartshare1: LOCK reind_conc_10_20 IN SHARE MODE; +step reindex2: REINDEX TABLE reind_conc_parent; +step drop3: DROP TABLE reind_conc_10_20; +step end1: COMMIT; +step drop3: <... completed> + +starting permutation: begin1 lockpartshare1 reindex_conc2 drop3 end1 +step begin1: BEGIN; +step lockpartshare1: LOCK reind_conc_10_20 IN SHARE MODE; +step reindex_conc2: REINDEX TABLE CONCURRENTLY reind_conc_parent; +step drop3: DROP TABLE reind_conc_10_20; +step end1: COMMIT; +step reindex_conc2: <... completed> +step drop3: <... completed> + +starting permutation: begin1 lockpartupdate1 reindex2 drop3 end1 +step begin1: BEGIN; +step lockpartupdate1: LOCK reind_conc_10_20 IN SHARE UPDATE EXCLUSIVE MODE; +step reindex2: REINDEX TABLE reind_conc_parent; +step drop3: DROP TABLE reind_conc_10_20; +step end1: COMMIT; +step reindex2: <... completed> +step drop3: <... completed> + +starting permutation: begin1 lockpartupdate1 reindex_conc2 drop3 end1 +step begin1: BEGIN; +step lockpartupdate1: LOCK reind_conc_10_20 IN SHARE UPDATE EXCLUSIVE MODE; +step reindex_conc2: REINDEX TABLE CONCURRENTLY reind_conc_parent; +step drop3: DROP TABLE reind_conc_10_20; +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 6acbb695ec..65d1443ac6 100644 --- a/src/test/isolation/isolation_schedule +++ b/src/test/isolation/isolation_schedule @@ -50,6 +50,7 @@ test: lock-committed-keyupdate test: update-locked-tuple test: reindex-concurrently test: reindex-schema +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..1a4dd1bf28 --- /dev/null +++ b/src/test/isolation/specs/reindex-partitions.spec @@ -0,0 +1,59 @@ +# REINDEX with partitioned tables +# +# Ensure that concurrent and non-concurrent operations work correctly when +# a REINDEX is performed on a partitioned table or index. + +# In the cases dealt with here, partition leaves are dropped in parallel of +# a REINDEX. DROP TABLE gets blocked by the first transaction of REINDEX +# building the list of partitions, so it will finish executing once REINDEX +# is done. + +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 "lockexcl1" { LOCK reind_conc_parent IN ACCESS EXCLUSIVE MODE; } +step "lockshare1" { LOCK reind_conc_parent IN SHARE MODE; } +step "lockupdate1" { LOCK reind_conc_parent IN SHARE UPDATE EXCLUSIVE MODE; } +step "lockpartexcl1" { LOCK reind_conc_10_20 IN ACCESS EXCLUSIVE MODE; } +step "lockpartshare1" { LOCK reind_conc_10_20 IN SHARE MODE; } +step "lockpartupdate1" { LOCK reind_conc_10_20 IN SHARE UPDATE 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; } + +# An existing partition leaf is dropped after reindex is done when the +# parent is locked. +permutation "begin1" "lockexcl1" "reindex2" "drop3" "end1" +permutation "begin1" "lockexcl1" "reindex_conc2" "drop3" "end1" +permutation "begin1" "lockshare1" "reindex2" "drop3" "end1" +permutation "begin1" "lockshare1" "reindex_conc2" "drop3" "end1" +permutation "begin1" "lockupdate1" "reindex2" "drop3" "end1" +permutation "begin1" "lockupdate1" "reindex_conc2" "drop3" "end1" + +# An existing partition leaf is dropped after reindex is done when this +# leaf is locked. +permutation "begin1" "lockpartexcl1" "reindex2" "drop3" "end1" +permutation "begin1" "lockpartexcl1" "reindex_conc2" "drop3" "end1" +permutation "begin1" "lockpartshare1" "reindex2" "drop3" "end1" +permutation "begin1" "lockpartshare1" "reindex_conc2" "drop3" "end1" +permutation "begin1" "lockpartupdate1" "reindex2" "drop3" "end1" +permutation "begin1" "lockpartupdate1" "reindex_conc2" "drop3" "end1" diff --git a/src/test/regress/expected/create_index.out b/src/test/regress/expected/create_index.out index 814416d936..64c0c66859 100644 --- a/src/test/regress/expected/create_index.out +++ b/src/test/regress/expected/create_index.out @@ -2194,18 +2194,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 @@ -2318,6 +2306,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 +ERROR: "concur_reindex_part_index" is not a table or materialized view +REINDEX TABLE CONCURRENTLY concur_reindex_part_index; -- error +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 +ERROR: "concur_reindex_part_index_10" is not a table or materialized view +REINDEX TABLE CONCURRENTLY concur_reindex_part_index_10; -- error +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 +ERROR: "concur_reindex_part" is not an index +REINDEX INDEX CONCURRENTLY concur_reindex_part; -- error +ERROR: "concur_reindex_part" is not an index +-- Partitioned with no leaves +REINDEX INDEX concur_reindex_part_10; -- error +ERROR: "concur_reindex_part_10" is not an index +REINDEX INDEX CONCURRENTLY concur_reindex_part_10; -- error +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..37f7259da9 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; -- error +REINDEX TABLE CONCURRENTLY concur_reindex_part_index; -- error +-- Partitioned index with no leaves +REINDEX TABLE concur_reindex_part_index_10; -- error +REINDEX TABLE CONCURRENTLY concur_reindex_part_index_10; -- error +-- 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; -- error +REINDEX INDEX CONCURRENTLY concur_reindex_part; -- error +-- Partitioned with no leaves +REINDEX INDEX concur_reindex_part_10; -- error +REINDEX INDEX CONCURRENTLY concur_reindex_part_10; -- error +-- 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/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list index 391c204f72..f151e13d7f 100644 --- a/src/tools/pgindent/typedefs.list +++ b/src/tools/pgindent/typedefs.list @@ -2038,6 +2038,7 @@ RegProcedure Regis RegisNode RegisteredBgWorker +ReindexErrorInfo ReindexObjectType ReindexStmt ReindexType