On Thu, Jan 28, 2021 at 09:51:51PM +0900, Masahiko Sawada wrote:
> On Mon, Nov 30, 2020 at 5:22 AM Justin Pryzby <pry...@telsasoft.com> wrote:
> > On Sat, Oct 31, 2020 at 01:31:17AM -0500, Justin Pryzby wrote:
> > > Forking this thread, since the existing CFs have been closed.
> > > https://www.postgresql.org/message-id/flat/20200914143102.GX18552%40telsasoft.com#58b1056488451f8594b0f0ba40996afd
> > >
> > > The strategy is to create catalog entries for all tables with
> > > indisvalid=false,
> > > and then process them like REINDEX CONCURRENTLY. If it's interrupted, it
> > > leaves INVALID indexes, which can be cleaned up with DROP or REINDEX,
> > > same as
> > > CIC on a plain table.
> > >
> > > On Sat, Aug 08, 2020 at 01:37:44AM -0500, Justin Pryzby wrote:
> > > > On Mon, Jun 15, 2020 at 09:37:42PM +0900, Michael Paquier wrote:
> > > > Note that the mentioned problem wasn't serious: there was missing index
> > > > on
> > > > child table, therefor the parent index was invalid, as intended.
> > > > However I
> > > > agree that it's not nice that the command can fail so easily and leave
> > > > behind
> > > > some indexes created successfully and some failed some not created at
> > > > all.
> > > >
> > > > But I took your advice initially creating invalid inds.
> > > ...
> > > > That gave me the idea to layer CIC on top of Reindex, since I think it
> > > > does
> > > > exactly what's needed.
> > >
> > > On Sat, Sep 26, 2020 at 02:56:55PM -0500, Justin Pryzby wrote:
> > > > On Thu, Sep 24, 2020 at 05:11:03PM +0900, Michael Paquier wrote:
> > > > > It would be good also to check if
> > > > > we have a partition index tree that maps partially with a partition
> > > > > table tree (aka no all table partitions have a partition index), where
> > > > > these don't get clustered because there is no index to work on.
> > > >
> > > > This should not happen, since a incomplete partitioned index is
> > > > "invalid".
> >
> > @cfbot: rebased over recent changes to indexcmds.c
>
> Status update for a commitfest entry.
>
> This patch has not been updated and "Waiting on Author" status since
> Nov 30. Are you still planning to work on this, Justin? If no, I'm
> going to set this entry to "Returned with Feedback" barring
> objections.
I had been waiting to rebase since there hasn't been any review comments and I
expected additional, future conflicts.
--
Justin
>From 2840a6d355961ea6fdd29c2851b9c333c17c849f Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryz...@telsasoft.com>
Date: Sat, 6 Jun 2020 17:42:23 -0500
Subject: [PATCH v12 1/5] Allow CREATE INDEX CONCURRENTLY on partitioned table
Note, this effectively reverts 050098b14, so take care to not reintroduce the
bug it fixed.
XXX: does pgstat_progress_update_param() break other commands progress ?
---
doc/src/sgml/ref/create_index.sgml | 9 --
src/backend/commands/indexcmds.c | 142 ++++++++++++++++++-------
src/test/regress/expected/indexing.out | 60 ++++++++++-
src/test/regress/sql/indexing.sql | 18 +++-
4 files changed, 173 insertions(+), 56 deletions(-)
diff --git a/doc/src/sgml/ref/create_index.sgml b/doc/src/sgml/ref/create_index.sgml
index a5271a9f8f..6869a18968 100644
--- a/doc/src/sgml/ref/create_index.sgml
+++ b/doc/src/sgml/ref/create_index.sgml
@@ -686,15 +686,6 @@ Indexes:
cannot.
</para>
- <para>
- Concurrent builds for indexes on partitioned tables are currently not
- supported. However, you may concurrently build the index on each
- partition individually and then finally create the partitioned index
- non-concurrently in order to reduce the time where writes to the
- partitioned table will be locked out. In this case, building the
- partitioned index is a metadata only operation.
- </para>
-
</refsect2>
</refsect1>
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index f9f3ff3b62..c513e8a6bd 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -68,6 +68,7 @@
/* non-export function prototypes */
+static void reindex_invalid_child_indexes(Oid indexRelationId);
static bool CompareOpclassOptions(Datum *opts1, Datum *opts2, int natts);
static void CheckPredicate(Expr *predicate);
static void ComputeIndexAttrs(IndexInfo *indexInfo,
@@ -680,17 +681,6 @@ DefineIndex(Oid relationId,
partitioned = rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE;
if (partitioned)
{
- /*
- * Note: we check 'stmt->concurrent' rather than 'concurrent', so that
- * the error is thrown also for temporary tables. Seems better to be
- * consistent, even though we could do it on temporary table because
- * we're not actually doing it concurrently.
- */
- if (stmt->concurrent)
- ereport(ERROR,
- (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
- errmsg("cannot create index on partitioned table \"%s\" concurrently",
- RelationGetRelationName(rel))));
if (stmt->excludeOpNames)
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
@@ -1128,6 +1118,11 @@ DefineIndex(Oid relationId,
if (pd->nparts != 0)
flags |= INDEX_CREATE_INVALID;
}
+ else if (concurrent && OidIsValid(parentIndexId))
+ {
+ /* If concurrent, initially build index partitions as "invalid" */
+ flags |= INDEX_CREATE_INVALID;
+ }
if (stmt->deferrable)
constr_flags |= INDEX_CONSTR_CREATE_DEFERRABLE;
@@ -1183,18 +1178,29 @@ DefineIndex(Oid relationId,
partdesc = RelationGetPartitionDesc(rel);
if ((!stmt->relation || stmt->relation->inh) && partdesc->nparts > 0)
{
+ /*
+ * Need to close the relation before recursing into children, so
+ * copy needed data into a longlived context.
+ */
+
+ MemoryContext ind_context = AllocSetContextCreate(PortalContext, "CREATE INDEX",
+ ALLOCSET_DEFAULT_SIZES);
+ MemoryContext oldcontext = MemoryContextSwitchTo(ind_context);
int nparts = partdesc->nparts;
Oid *part_oids = palloc(sizeof(Oid) * nparts);
bool invalidate_parent = false;
TupleDesc parentDesc;
Oid *opfamOids;
+ // If concurrent, maybe this should be done after excluding indexes which already exist ?
pgstat_progress_update_param(PROGRESS_CREATEIDX_PARTITIONS_TOTAL,
nparts);
memcpy(part_oids, partdesc->oids, sizeof(Oid) * nparts);
+ parentDesc = CreateTupleDescCopy(RelationGetDescr(rel));
+ table_close(rel, NoLock);
+ MemoryContextSwitchTo(oldcontext);
- parentDesc = RelationGetDescr(rel);
opfamOids = palloc(sizeof(Oid) * numberOfKeyAttributes);
for (i = 0; i < numberOfKeyAttributes; i++)
opfamOids[i] = get_opclass_family(classObjectId[i]);
@@ -1237,10 +1243,12 @@ DefineIndex(Oid relationId,
continue;
}
+ oldcontext = MemoryContextSwitchTo(ind_context);
childidxs = RelationGetIndexList(childrel);
attmap =
build_attrmap_by_name(RelationGetDescr(childrel),
parentDesc);
+ MemoryContextSwitchTo(oldcontext);
foreach(cell, childidxs)
{
@@ -1311,10 +1319,14 @@ DefineIndex(Oid relationId,
*/
if (!found)
{
- IndexStmt *childStmt = copyObject(stmt);
+ IndexStmt *childStmt;
bool found_whole_row;
ListCell *lc;
+ oldcontext = MemoryContextSwitchTo(ind_context);
+ childStmt = copyObject(stmt);
+ MemoryContextSwitchTo(oldcontext);
+
/*
* We can't use the same index name for the child index,
* so clear idxname to let the recursive invocation choose
@@ -1366,10 +1378,18 @@ DefineIndex(Oid relationId,
createdConstraintId,
is_alter_table, check_rights, check_not_in_use,
skip_build, quiet);
+ if (concurrent)
+ {
+ PopActiveSnapshot();
+ PushActiveSnapshot(GetTransactionSnapshot());
+ invalidate_parent = true;
+ }
}
- pgstat_progress_update_param(PROGRESS_CREATEIDX_PARTITIONS_DONE,
- i + 1);
+ /* For concurrent build, this is a catalog-only stage */
+ if (!concurrent)
+ pgstat_progress_update_param(PROGRESS_CREATEIDX_PARTITIONS_DONE,
+ i + 1);
free_attrmap(attmap);
}
@@ -1379,51 +1399,42 @@ DefineIndex(Oid relationId,
* invalid, this is incorrect, so update our row to invalid too.
*/
if (invalidate_parent)
- {
- Relation pg_index = table_open(IndexRelationId, RowExclusiveLock);
- HeapTuple tup,
- newtup;
-
- tup = SearchSysCache1(INDEXRELID,
- ObjectIdGetDatum(indexRelationId));
- if (!HeapTupleIsValid(tup))
- elog(ERROR, "cache lookup failed for index %u",
- indexRelationId);
- newtup = heap_copytuple(tup);
- ((Form_pg_index) GETSTRUCT(newtup))->indisvalid = false;
- CatalogTupleUpdate(pg_index, &tup->t_self, newtup);
- ReleaseSysCache(tup);
- table_close(pg_index, RowExclusiveLock);
- heap_freetuple(newtup);
- }
- }
+ index_set_state_flags(indexRelationId, INDEX_DROP_CLEAR_VALID);
+ } else
+ table_close(rel, NoLock);
/*
* Indexes on partitioned tables are not themselves built, so we're
* done here.
*/
- table_close(rel, NoLock);
if (!OidIsValid(parentIndexId))
+ {
+ if (concurrent)
+ reindex_invalid_child_indexes(indexRelationId);
+
pgstat_progress_end_command();
+ }
+
return address;
}
- if (!concurrent)
+ table_close(rel, NoLock);
+ if (!concurrent || OidIsValid(parentIndexId))
{
- /* Close the heap and we're done, in the non-concurrent case */
- table_close(rel, NoLock);
+ /*
+ * We're done if this is the top-level index,
+ * or the catalog-only phase of a partition built concurrently
+ */
- /* If this is the top-level index, we're done. */
if (!OidIsValid(parentIndexId))
pgstat_progress_end_command();
return address;
}
- /* save lockrelid and locktag for below, then close rel */
+ /* save lockrelid and locktag for below */
heaprelid = rel->rd_lockInfo.lockRelId;
SET_LOCKTAG_RELATION(heaplocktag, heaprelid.dbId, heaprelid.relId);
- table_close(rel, NoLock);
/*
* For a concurrent build, it's important to make the catalog entries
@@ -1617,6 +1628,57 @@ DefineIndex(Oid relationId,
return address;
}
+/* Reindex invalid child indexes created earlier */
+static void
+reindex_invalid_child_indexes(Oid indexRelationId)
+{
+ ListCell *lc;
+ int npart = 0;
+ ReindexParams params = { .options = REINDEXOPT_CONCURRENTLY };
+
+ MemoryContext ind_context = AllocSetContextCreate(PortalContext, "CREATE INDEX",
+ ALLOCSET_DEFAULT_SIZES);
+ MemoryContext oldcontext;
+ List *childs = find_inheritance_children(indexRelationId, ShareLock);
+ List *partitions = NIL;
+
+ PreventInTransactionBlock(true, "REINDEX INDEX");
+
+ foreach (lc, childs)
+ {
+ Oid partoid = lfirst_oid(lc);
+
+ pgstat_progress_update_param(PROGRESS_CREATEIDX_PARTITIONS_DONE,
+ npart++);
+
+ if (get_index_isvalid(partoid) ||
+ !RELKIND_HAS_STORAGE(get_rel_relkind(partoid)))
+ continue;
+
+ /* Save partition OID */
+ oldcontext = MemoryContextSwitchTo(ind_context);
+ partitions = lappend_oid(partitions, partoid);
+ MemoryContextSwitchTo(oldcontext);
+ }
+
+ /*
+ * Process each partition listed in a separate transaction. Note that
+ * this commits and then starts a new transaction immediately.
+ */
+ ReindexMultipleInternal(partitions, ¶ms);
+
+ /*
+ * CIC needs to mark a partitioned index as VALID, which itself
+ * requires setting READY, which is unset for CIC (even though
+ * it's meaningless for an index without storage).
+ * This must be done only while holding a lock which precludes adding
+ * partitions.
+ * See also: validatePartitionedIndex().
+ */
+ index_set_state_flags(indexRelationId, INDEX_CREATE_SET_READY);
+ CommandCounterIncrement();
+ index_set_state_flags(indexRelationId, INDEX_CREATE_SET_VALID);
+}
/*
* CheckMutability
diff --git a/src/test/regress/expected/indexing.out b/src/test/regress/expected/indexing.out
index c93f4470c9..f04abc6897 100644
--- a/src/test/regress/expected/indexing.out
+++ b/src/test/regress/expected/indexing.out
@@ -50,11 +50,63 @@ select relname, relkind, relhassubclass, inhparent::regclass
(8 rows)
drop table idxpart;
--- Some unsupported features
+-- CIC on partitioned table
create table idxpart (a int, b int, c text) partition by range (a);
-create table idxpart1 partition of idxpart for values from (0) to (10);
-create index concurrently on idxpart (a);
-ERROR: cannot create index on partitioned table "idxpart" concurrently
+create table idxpart1 partition of idxpart for values from (0) to (10) partition by range(a);
+create table idxpart11 partition of idxpart1 for values from (0) to (10) partition by range(a);
+create table idxpart111 partition of idxpart11 default partition by range(a);
+create table idxpart1111 partition of idxpart111 default partition by range(a);
+create table idxpart2 partition of idxpart for values from (10) to (20);
+insert into idxpart2 values(10),(10); -- not unique
+create index concurrently on idxpart (a); -- partitioned
+create index concurrently on idxpart1 (a); -- partitioned and partition
+create index concurrently on idxpart11 (a); -- partitioned and partition, with no leaves
+create index concurrently on idxpart2 (a); -- leaf
+create unique index concurrently on idxpart (a); -- partitioned, unique failure
+ERROR: could not create unique index "idxpart2_a_idx2_ccnew"
+DETAIL: Key (a)=(10) is duplicated.
+\d idxpart
+ Partitioned table "public.idxpart"
+ Column | Type | Collation | Nullable | Default
+--------+---------+-----------+----------+---------
+ a | integer | | |
+ b | integer | | |
+ c | text | | |
+Partition key: RANGE (a)
+Indexes:
+ "idxpart_a_idx" btree (a)
+ "idxpart_a_idx1" UNIQUE, btree (a) INVALID
+Number of partitions: 2 (Use \d+ to list them.)
+
+\d idxpart1
+ Partitioned table "public.idxpart1"
+ Column | Type | Collation | Nullable | Default
+--------+---------+-----------+----------+---------
+ a | integer | | |
+ b | integer | | |
+ c | text | | |
+Partition of: idxpart FOR VALUES FROM (0) TO (10)
+Partition key: RANGE (a)
+Indexes:
+ "idxpart1_a_idx" btree (a) INVALID
+ "idxpart1_a_idx1" btree (a)
+ "idxpart1_a_idx2" UNIQUE, btree (a) INVALID
+Number of partitions: 1 (Use \d+ to list them.)
+
+\d idxpart2
+ Table "public.idxpart2"
+ Column | Type | Collation | Nullable | Default
+--------+---------+-----------+----------+---------
+ a | integer | | |
+ b | integer | | |
+ c | text | | |
+Partition of: idxpart FOR VALUES FROM (10) TO (20)
+Indexes:
+ "idxpart2_a_idx" btree (a)
+ "idxpart2_a_idx1" btree (a)
+ "idxpart2_a_idx2" UNIQUE, btree (a) INVALID
+ "idxpart2_a_idx2_ccnew" UNIQUE, btree (a) INVALID
+
drop table idxpart;
-- Verify bugfix with query on indexed partitioned table with no partitions
-- https://postgr.es/m/20180124162006.pmapfiznhgngwtjf@alvherre.pgsql
diff --git a/src/test/regress/sql/indexing.sql b/src/test/regress/sql/indexing.sql
index 42f398b67c..3d4b6e9bc9 100644
--- a/src/test/regress/sql/indexing.sql
+++ b/src/test/regress/sql/indexing.sql
@@ -29,10 +29,22 @@ select relname, relkind, relhassubclass, inhparent::regclass
where relname like 'idxpart%' order by relname;
drop table idxpart;
--- Some unsupported features
+-- CIC on partitioned table
create table idxpart (a int, b int, c text) partition by range (a);
-create table idxpart1 partition of idxpart for values from (0) to (10);
-create index concurrently on idxpart (a);
+create table idxpart1 partition of idxpart for values from (0) to (10) partition by range(a);
+create table idxpart11 partition of idxpart1 for values from (0) to (10) partition by range(a);
+create table idxpart111 partition of idxpart11 default partition by range(a);
+create table idxpart1111 partition of idxpart111 default partition by range(a);
+create table idxpart2 partition of idxpart for values from (10) to (20);
+insert into idxpart2 values(10),(10); -- not unique
+create index concurrently on idxpart (a); -- partitioned
+create index concurrently on idxpart1 (a); -- partitioned and partition
+create index concurrently on idxpart11 (a); -- partitioned and partition, with no leaves
+create index concurrently on idxpart2 (a); -- leaf
+create unique index concurrently on idxpart (a); -- partitioned, unique failure
+\d idxpart
+\d idxpart1
+\d idxpart2
drop table idxpart;
-- Verify bugfix with query on indexed partitioned table with no partitions
--
2.17.0
>From 2ce39870407fa8bb486df75f51bc9f20dd626045 Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryz...@telsasoft.com>
Date: Fri, 30 Oct 2020 16:23:02 -0500
Subject: [PATCH v12 2/5] Add SKIPVALID flag for more integration
---
src/backend/commands/indexcmds.c | 52 +++++++++++---------------------
src/include/catalog/index.h | 1 +
2 files changed, 18 insertions(+), 35 deletions(-)
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index c513e8a6bd..89c2f62eda 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -1628,54 +1628,31 @@ DefineIndex(Oid relationId,
return address;
}
-/* Reindex invalid child indexes created earlier */
+/*
+ * Reindex invalid child indexes created earlier thereby validating
+ * the parent index.
+ */
static void
reindex_invalid_child_indexes(Oid indexRelationId)
{
- ListCell *lc;
- int npart = 0;
ReindexParams params = { .options = REINDEXOPT_CONCURRENTLY };
- MemoryContext ind_context = AllocSetContextCreate(PortalContext, "CREATE INDEX",
- ALLOCSET_DEFAULT_SIZES);
- MemoryContext oldcontext;
- List *childs = find_inheritance_children(indexRelationId, ShareLock);
- List *partitions = NIL;
-
- PreventInTransactionBlock(true, "REINDEX INDEX");
-
- foreach (lc, childs)
- {
- Oid partoid = lfirst_oid(lc);
-
- pgstat_progress_update_param(PROGRESS_CREATEIDX_PARTITIONS_DONE,
- npart++);
-
- if (get_index_isvalid(partoid) ||
- !RELKIND_HAS_STORAGE(get_rel_relkind(partoid)))
- continue;
-
- /* Save partition OID */
- oldcontext = MemoryContextSwitchTo(ind_context);
- partitions = lappend_oid(partitions, partoid);
- MemoryContextSwitchTo(oldcontext);
- }
-
- /*
- * Process each partition listed in a separate transaction. Note that
- * this commits and then starts a new transaction immediately.
- */
- ReindexMultipleInternal(partitions, ¶ms);
-
/*
* CIC needs to mark a partitioned index as VALID, which itself
* requires setting READY, which is unset for CIC (even though
* it's meaningless for an index without storage).
* This must be done only while holding a lock which precludes adding
* partitions.
- * See also: validatePartitionedIndex().
*/
+ CommandCounterIncrement();
index_set_state_flags(indexRelationId, INDEX_CREATE_SET_READY);
+
+ /*
+ * Process each partition listed in a separate transaction. Note that
+ * this commits and then starts a new transaction immediately.
+ */
+ ReindexPartitions(indexRelationId, ¶ms, true);
+
CommandCounterIncrement();
index_set_state_flags(indexRelationId, INDEX_CREATE_SET_VALID);
}
@@ -3037,6 +3014,11 @@ ReindexPartitions(Oid relid, ReindexParams *params, bool isTopLevel)
if (!RELKIND_HAS_STORAGE(partkind))
continue;
+ /* Skip invalid indexes, if requested */
+ if ((params->options & REINDEXOPT_SKIPVALID) != 0 &&
+ get_index_isvalid(partoid))
+ continue;
+
Assert(partkind == RELKIND_INDEX ||
partkind == RELKIND_RELATION);
diff --git a/src/include/catalog/index.h b/src/include/catalog/index.h
index 266f8950dc..6673122ec2 100644
--- a/src/include/catalog/index.h
+++ b/src/include/catalog/index.h
@@ -40,6 +40,7 @@ typedef struct ReindexParams
#define REINDEXOPT_REPORT_PROGRESS 0x02 /* report pgstat progress */
#define REINDEXOPT_MISSING_OK 0x04 /* skip missing relations */
#define REINDEXOPT_CONCURRENTLY 0x08 /* concurrent mode */
+#define REINDEXOPT_SKIPVALID 0x10 /* skip valid indexes */
/* state info for validate_index bulkdelete callback */
typedef struct ValidateIndexState
--
2.17.0
>From 15643b52a721f4c43d00296cd87f152543b018f7 Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryz...@telsasoft.com>
Date: Fri, 30 Oct 2020 23:52:31 -0500
Subject: [PATCH v12 3/5] ReindexPartitions() to set indisvalid..
Something like this should probably have been included in
a6642b3ae060976b42830b7dc8f29ec190ab05e4
See also 71a05b223, which mentioned the absence of any way to validate an
index.
---
src/backend/commands/indexcmds.c | 23 ++++++++++++++++++-----
1 file changed, 18 insertions(+), 5 deletions(-)
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index 89c2f62eda..739bd14001 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -1641,8 +1641,6 @@ reindex_invalid_child_indexes(Oid indexRelationId)
* CIC needs to mark a partitioned index as VALID, which itself
* requires setting READY, which is unset for CIC (even though
* it's meaningless for an index without storage).
- * This must be done only while holding a lock which precludes adding
- * partitions.
*/
CommandCounterIncrement();
index_set_state_flags(indexRelationId, INDEX_CREATE_SET_READY);
@@ -1652,9 +1650,6 @@ reindex_invalid_child_indexes(Oid indexRelationId)
* this commits and then starts a new transaction immediately.
*/
ReindexPartitions(indexRelationId, ¶ms, true);
-
- CommandCounterIncrement();
- index_set_state_flags(indexRelationId, INDEX_CREATE_SET_VALID);
}
/*
@@ -3034,6 +3029,24 @@ ReindexPartitions(Oid relid, ReindexParams *params, bool isTopLevel)
*/
ReindexMultipleInternal(partitions, params);
+ /*
+ * If indexes exist on all of the partitioned table's children, and we
+ * just reindexed them, then we know they're valid, and so can mark the
+ * parent index as valid.
+ * This handles the case of CREATE INDEX CONCURRENTLY.
+ * See also: validatePartitionedIndex().
+ */
+ if (get_rel_relkind(relid) == RELKIND_PARTITIONED_INDEX
+ && !get_index_isvalid(relid))
+ {
+ Oid tableoid = IndexGetRelation(relid, false);
+ List *child_tables = find_all_inheritors(tableoid, ShareLock, NULL);
+
+ /* Both lists include their parent relation as well as any intermediate partitioned rels */
+ if (list_length(inhoids) == list_length(child_tables))
+ index_set_state_flags(relid, INDEX_CREATE_SET_VALID);
+ }
+
/*
* Clean up working storage --- note we must do this after
* StartTransactionCommand, else we might be trying to delete the active
--
2.17.0
>From 66c0421963c7f98b004c21ab1b29dc2cd2713d2e Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryz...@telsasoft.com>
Date: Sun, 1 Nov 2020 12:25:15 -0600
Subject: [PATCH v12 4/5] Refactor to allow reindexing all index partitions at
once
---
src/backend/commands/indexcmds.c | 262 ++++++++++++++-------
src/test/regress/expected/create_index.out | 4 +-
2 files changed, 185 insertions(+), 81 deletions(-)
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index 739bd14001..08d44a1999 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -102,6 +102,8 @@ static void ReindexMultipleInternal(List *relids,
ReindexParams *params);
static bool ReindexRelationConcurrently(Oid relationOid,
ReindexParams *params);
+static List *ReindexIndexesConcurrently(List *indexIds, List *heapRelationIds,
+ int options, MemoryContext private_context);
static void update_relispartition(Oid relationId, bool newval);
static inline void set_indexsafe_procflags(void);
@@ -124,6 +126,15 @@ typedef struct ReindexErrorInfo
char relkind;
} ReindexErrorInfo;
+/* Argument to ReindexIndexConcurrently takes a List* of these */
+typedef struct ReindexIndexInfo
+{
+ Oid indexId;
+ Oid tableId;
+ Oid amId;
+ bool safe; /* for set_indexsafe_procflags */
+} ReindexIndexInfo;
+
/*
* CheckIndexCompatible
* Determine whether an existing index definition is compatible with a
@@ -1635,7 +1646,9 @@ DefineIndex(Oid relationId,
static void
reindex_invalid_child_indexes(Oid indexRelationId)
{
- ReindexParams params = { .options = REINDEXOPT_CONCURRENTLY };
+ ReindexParams params = {
+ .options = REINDEXOPT_CONCURRENTLY | REINDEXOPT_SKIPVALID,
+ };
/*
* CIC needs to mark a partitioned index as VALID, which itself
@@ -2607,7 +2620,15 @@ ReindexIndex(RangeVar *indexRelation, ReindexParams *params, bool isTopLevel)
ReindexPartitions(indOid, params, isTopLevel);
else if ((params->options & REINDEXOPT_CONCURRENTLY) != 0 &&
persistence != RELPERSISTENCE_TEMP)
- ReindexRelationConcurrently(indOid, params);
+ {
+ ReindexIndexInfo idxinfo = {
+ .indexId = indOid,
+ /* other fields set later */
+ };
+ ReindexIndexesConcurrently(list_make1(&idxinfo),
+ list_make1_oid(IndexGetRelation(indOid, false)),
+ params->options, CurrentMemoryContext);
+ }
else
{
ReindexParams newparams = *params;
@@ -2938,20 +2959,69 @@ reindex_error_callback(void *arg)
errinfo->relnamespace, errinfo->relname);
}
+
+/*
+ * Given a list of index oids, return a list of leaf partitions by removing
+ * any intermediate parents. heaprels is populated with the corresponding
+ * tables.
+ */
+static List *
+leaf_partitions(List *inhoids, int options, List **heaprels)
+{
+ List *partitions = NIL;
+ ListCell *lc;
+
+ foreach(lc, inhoids)
+ {
+ Oid partoid = lfirst_oid(lc);
+ Oid tableoid;
+ Relation table;
+ char partkind = get_rel_relkind(partoid);
+
+ /*
+ * This discards partitioned indexes and foreign tables.
+ */
+ if (!RELKIND_HAS_STORAGE(partkind))
+ continue;
+
+ Assert(partkind == RELKIND_INDEX);
+
+ /* Skip invalid indexes, if requested */
+ if ((options & REINDEXOPT_SKIPVALID) != 0 &&
+ get_index_isvalid(partoid))
+ continue;
+
+ /* (try to) Open the table, with lock */
+ tableoid = IndexGetRelation(partoid, false);
+ table = table_open(tableoid, ShareLock);
+ table_close(table, NoLock);
+
+ /* Save partition OID in current MemoryContext */
+ partitions = lappend_oid(partitions, partoid);
+ *heaprels = lappend_oid(*heaprels, tableoid);
+ }
+
+ return partitions;
+}
+
+
/*
* ReindexPartitions
*
* Reindex a set of partitions, per the partitioned index or table given
* by the caller.
+ * XXX: should be further refactored with logic from ReindexRelationConcurrently
*/
static void
ReindexPartitions(Oid relid, ReindexParams *params, bool isTopLevel)
{
- List *partitions = NIL;
+ List *partitions = NIL,
+ *heaprels = 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;
+ MemoryContext old_context;
List *inhoids;
ListCell *lc;
ErrorContextCallback errcallback;
@@ -2996,38 +3066,58 @@ ReindexPartitions(Oid relid, ReindexParams *params, bool isTopLevel)
* 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;
-
- /* Skip invalid indexes, if requested */
- if ((params->options & REINDEXOPT_SKIPVALID) != 0 &&
- get_index_isvalid(partoid))
- continue;
+ if (relkind == RELKIND_PARTITIONED_INDEX)
+ {
+ old_context = MemoryContextSwitchTo(reindex_context);
+ partitions = leaf_partitions(inhoids, params->options, &heaprels);
+ MemoryContextSwitchTo(old_context);
+ } else {
+ /* Loop over parent tables */
+ foreach(lc, inhoids)
+ {
+ Oid partoid = lfirst_oid(lc);
+ Relation parttable;
+ List *partindexes;
+
+ parttable = table_open(partoid, ShareLock);
+ old_context = MemoryContextSwitchTo(reindex_context);
+ partindexes = RelationGetIndexList(parttable);
+ partindexes = leaf_partitions(partindexes, params->options, &heaprels);
+ partitions = list_concat(partitions, partindexes);
+
+ MemoryContextSwitchTo(old_context);
+ table_close(parttable, ShareLock);
+ }
+ }
- Assert(partkind == RELKIND_INDEX ||
- partkind == RELKIND_RELATION);
+ if ((params->options & REINDEXOPT_CONCURRENTLY) != 0 &&
+ relkind == RELKIND_PARTITIONED_INDEX &&
+ get_rel_persistence(relid) != RELPERSISTENCE_TEMP)
+ {
+ List *idxinfos = NIL;
+ ReindexIndexInfo *idxinfo;
- /* Save partition OID */
old_context = MemoryContextSwitchTo(reindex_context);
- partitions = lappend_oid(partitions, partoid);
+ foreach (lc, partitions)
+ {
+ Oid partoid = lfirst_oid(lc);
+ idxinfo = palloc(sizeof(ReindexIndexInfo));
+ idxinfo->indexId = partoid;
+ /* other fields set later */
+ idxinfos = lappend(idxinfos, idxinfo);
+ }
MemoryContextSwitchTo(old_context);
- }
- /*
- * Process each partition listed in a separate transaction. Note that
- * this commits and then starts a new transaction immediately.
- */
- ReindexMultipleInternal(partitions, params);
+ /* Process all indexes in a single loop */
+ ReindexIndexesConcurrently(idxinfos, heaprels, params->options, reindex_context);
+ } else {
+ /*
+ * Process each partition listed in a separate transaction. Note that
+ * this commits and then starts a new transaction immediately.
+ */
+ ReindexMultipleInternal(partitions, params);
+ }
/*
* If indexes exist on all of the partitioned table's children, and we
@@ -3172,18 +3262,9 @@ ReindexMultipleInternal(List *relids, ReindexParams *params)
static bool
ReindexRelationConcurrently(Oid relationOid, ReindexParams *params)
{
- typedef struct ReindexIndexInfo
- {
- Oid indexId;
- Oid tableId;
- Oid amId;
- bool safe; /* for set_indexsafe_procflags */
- } ReindexIndexInfo;
List *heapRelationIds = NIL;
List *indexIds = NIL;
List *newIndexIds = NIL;
- List *relationLocks = NIL;
- List *lockTags = NIL;
ListCell *lc,
*lc2;
MemoryContext private_context;
@@ -3192,13 +3273,6 @@ ReindexRelationConcurrently(Oid relationOid, ReindexParams *params)
char *relationName = NULL;
char *relationNamespace = NULL;
PGRUsage ru0;
- const int progress_index[] = {
- PROGRESS_CREATEIDX_COMMAND,
- PROGRESS_CREATEIDX_PHASE,
- PROGRESS_CREATEIDX_INDEX_OID,
- PROGRESS_CREATEIDX_ACCESS_METHOD_OID
- };
- int64 progress_vals[4];
/*
* Create a memory context that will survive forced transaction commits we
@@ -3449,6 +3523,69 @@ ReindexRelationConcurrently(Oid relationOid, ReindexParams *params)
Assert(heapRelationIds != NIL);
+ /* Do the work */
+ newIndexIds = ReindexIndexesConcurrently(indexIds, heapRelationIds, params->options, private_context);
+
+ /* Log what we did */
+ if ((params->options & REINDEXOPT_VERBOSE) != 0)
+ {
+ if (relkind == RELKIND_INDEX)
+ ereport(INFO,
+ (errmsg("index \"%s.%s\" was reindexed",
+ relationNamespace, relationName),
+ errdetail("%s.",
+ pg_rusage_show(&ru0))));
+ else
+ {
+ foreach(lc, newIndexIds)
+ {
+ Oid indOid = lfirst_oid(lc);
+ ereport(INFO,
+ (errmsg("index \"%s.%s\" was reindexed",
+ get_namespace_name(get_rel_namespace(indOid)),
+ get_rel_name(indOid))));
+ /* Don't show rusage here, since it's not per index. */
+ }
+
+ ereport(INFO,
+ (errmsg("table \"%s.%s\" was reindexed",
+ relationNamespace, relationName),
+ errdetail("%s.",
+ pg_rusage_show(&ru0))));
+ }
+ }
+
+
+ MemoryContextDelete(private_context);
+
+ return true;
+}
+
+/*
+ * Reindex concurrently for an arbitrary list of index relations
+ * This is called by ReindexRelationConcurrently and
+ */
+static List *
+ReindexIndexesConcurrently(List *indexIds, List *heapRelationIds, int options,
+ MemoryContext private_context)
+{
+ List *newIndexIds = NIL;
+ List *relationLocks = NIL;
+ List *lockTags = NIL;
+
+ ListCell *lc,
+ *lc2;
+
+ MemoryContext oldcontext;
+
+ const int progress_index[] = {
+ PROGRESS_CREATEIDX_COMMAND,
+ PROGRESS_CREATEIDX_PHASE,
+ PROGRESS_CREATEIDX_INDEX_OID,
+ PROGRESS_CREATEIDX_ACCESS_METHOD_OID
+ };
+ int64 progress_vals[4];
+
/*-----
* Now we have all the indexes we want to process in indexIds.
*
@@ -3913,42 +4050,9 @@ ReindexRelationConcurrently(Oid relationOid, ReindexParams *params)
/* Start a new transaction to finish process properly */
StartTransactionCommand();
- /* Log what we did */
- if ((params->options & REINDEXOPT_VERBOSE) != 0)
- {
- if (relkind == RELKIND_INDEX)
- ereport(INFO,
- (errmsg("index \"%s.%s\" was reindexed",
- relationNamespace, relationName),
- errdetail("%s.",
- pg_rusage_show(&ru0))));
- else
- {
- foreach(lc, newIndexIds)
- {
- ReindexIndexInfo *idx = lfirst(lc);
- Oid indOid = idx->indexId;
-
- ereport(INFO,
- (errmsg("index \"%s.%s\" was reindexed",
- get_namespace_name(get_rel_namespace(indOid)),
- get_rel_name(indOid))));
- /* Don't show rusage here, since it's not per index. */
- }
-
- ereport(INFO,
- (errmsg("table \"%s.%s\" was reindexed",
- relationNamespace, relationName),
- errdetail("%s.",
- pg_rusage_show(&ru0))));
- }
- }
-
- MemoryContextDelete(private_context);
-
pgstat_progress_end_command();
- return true;
+ return newIndexIds;
}
/*
diff --git a/src/test/regress/expected/create_index.out b/src/test/regress/expected/create_index.out
index fc6afab58a..4a03ab2abb 100644
--- a/src/test/regress/expected/create_index.out
+++ b/src/test/regress/expected/create_index.out
@@ -2470,12 +2470,12 @@ COMMIT;
REINDEX TABLE CONCURRENTLY pg_class; -- no catalog relation
ERROR: cannot reindex system catalogs concurrently
REINDEX INDEX CONCURRENTLY pg_class_oid_index; -- no catalog index
-ERROR: cannot reindex system catalogs concurrently
+ERROR: concurrent index creation on system catalog tables is not supported
-- These are the toast table and index of pg_authid.
REINDEX TABLE CONCURRENTLY pg_toast.pg_toast_1260; -- no catalog toast table
ERROR: cannot reindex system catalogs concurrently
REINDEX INDEX CONCURRENTLY pg_toast.pg_toast_1260_index; -- no catalog toast index
-ERROR: cannot reindex system catalogs concurrently
+ERROR: concurrent index creation on system catalog tables is not supported
REINDEX SYSTEM CONCURRENTLY postgres; -- not allowed for SYSTEM
ERROR: cannot reindex system catalogs concurrently
-- Warns about catalog relations
--
2.17.0
>From fa128228d21fce54babf6f736e833eea3d2f82a3 Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryz...@telsasoft.com>
Date: Sun, 1 Nov 2020 13:46:18 -0600
Subject: [PATCH v12 5/5] More refactoring
---
src/backend/commands/indexcmds.c | 183 +++++++++------------
src/test/regress/expected/create_index.out | 4 +-
2 files changed, 83 insertions(+), 104 deletions(-)
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index 08d44a1999..4586942960 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -102,8 +102,8 @@ static void ReindexMultipleInternal(List *relids,
ReindexParams *params);
static bool ReindexRelationConcurrently(Oid relationOid,
ReindexParams *params);
-static List *ReindexIndexesConcurrently(List *indexIds, List *heapRelationIds,
- int options, MemoryContext private_context);
+static List *ReindexIndexesConcurrently(List *indexIds, int options,
+ MemoryContext private_context);
static void update_relispartition(Oid relationId, bool newval);
static inline void set_indexsafe_procflags(void);
@@ -2625,8 +2625,8 @@ ReindexIndex(RangeVar *indexRelation, ReindexParams *params, bool isTopLevel)
.indexId = indOid,
/* other fields set later */
};
+
ReindexIndexesConcurrently(list_make1(&idxinfo),
- list_make1_oid(IndexGetRelation(indOid, false)),
params->options, CurrentMemoryContext);
}
else
@@ -2961,12 +2961,11 @@ reindex_error_callback(void *arg)
/*
- * Given a list of index oids, return a list of leaf partitions by removing
- * any intermediate parents. heaprels is populated with the corresponding
- * tables.
+ * Given a list of index oids, return a new list of leaf partitions by
+ * excluding any intermediate parents.
*/
static List *
-leaf_partitions(List *inhoids, int options, List **heaprels)
+leaf_partitions(List *inhoids, int options)
{
List *partitions = NIL;
ListCell *lc;
@@ -2998,7 +2997,6 @@ leaf_partitions(List *inhoids, int options, List **heaprels)
/* Save partition OID in current MemoryContext */
partitions = lappend_oid(partitions, partoid);
- *heaprels = lappend_oid(*heaprels, tableoid);
}
return partitions;
@@ -3010,13 +3008,11 @@ leaf_partitions(List *inhoids, int options, List **heaprels)
*
* Reindex a set of partitions, per the partitioned index or table given
* by the caller.
- * XXX: should be further refactored with logic from ReindexRelationConcurrently
*/
static void
ReindexPartitions(Oid relid, ReindexParams *params, bool isTopLevel)
{
- List *partitions = NIL,
- *heaprels = NIL;
+ List *partitions = NIL;
char relkind = get_rel_relkind(relid);
char *relname = get_rel_name(relid);
char *relnamespace = get_namespace_name(get_rel_namespace(relid));
@@ -3070,7 +3066,7 @@ ReindexPartitions(Oid relid, ReindexParams *params, bool isTopLevel)
if (relkind == RELKIND_PARTITIONED_INDEX)
{
old_context = MemoryContextSwitchTo(reindex_context);
- partitions = leaf_partitions(inhoids, params->options, &heaprels);
+ partitions = leaf_partitions(inhoids, params->options);
MemoryContextSwitchTo(old_context);
} else {
/* Loop over parent tables */
@@ -3083,7 +3079,7 @@ ReindexPartitions(Oid relid, ReindexParams *params, bool isTopLevel)
parttable = table_open(partoid, ShareLock);
old_context = MemoryContextSwitchTo(reindex_context);
partindexes = RelationGetIndexList(parttable);
- partindexes = leaf_partitions(partindexes, params->options, &heaprels);
+ partindexes = leaf_partitions(partindexes, params->options);
partitions = list_concat(partitions, partindexes);
MemoryContextSwitchTo(old_context);
@@ -3092,10 +3088,9 @@ ReindexPartitions(Oid relid, ReindexParams *params, bool isTopLevel)
}
if ((params->options & REINDEXOPT_CONCURRENTLY) != 0 &&
- relkind == RELKIND_PARTITIONED_INDEX &&
get_rel_persistence(relid) != RELPERSISTENCE_TEMP)
{
- List *idxinfos = NIL;
+ List *idxinfos = NIL;
ReindexIndexInfo *idxinfo;
old_context = MemoryContextSwitchTo(reindex_context);
@@ -3110,7 +3105,7 @@ ReindexPartitions(Oid relid, ReindexParams *params, bool isTopLevel)
MemoryContextSwitchTo(old_context);
/* Process all indexes in a single loop */
- ReindexIndexesConcurrently(idxinfos, heaprels, params->options, reindex_context);
+ ReindexIndexesConcurrently(idxinfos, params->options, reindex_context);
} else {
/*
* Process each partition listed in a separate transaction. Note that
@@ -3262,7 +3257,6 @@ ReindexMultipleInternal(List *relids, ReindexParams *params)
static bool
ReindexRelationConcurrently(Oid relationOid, ReindexParams *params)
{
- List *heapRelationIds = NIL;
List *indexIds = NIL;
List *newIndexIds = NIL;
ListCell *lc,
@@ -3315,14 +3309,6 @@ ReindexRelationConcurrently(Oid relationOid, ReindexParams *params)
*/
Relation heapRelation;
- /* Save the list of relation OIDs in private context */
- oldcontext = MemoryContextSwitchTo(private_context);
-
- /* Track this relation for session locks */
- heapRelationIds = lappend_oid(heapRelationIds, relationOid);
-
- MemoryContextSwitchTo(oldcontext);
-
if (IsCatalogRelationOid(relationOid))
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
@@ -3335,7 +3321,7 @@ ReindexRelationConcurrently(Oid relationOid, ReindexParams *params)
ShareUpdateExclusiveLock);
/* leave if relation does not exist */
if (!heapRelation)
- break;
+ break; // XXX: lremove
}
else
heapRelation = table_open(relationOid,
@@ -3386,14 +3372,6 @@ ReindexRelationConcurrently(Oid relationOid, ReindexParams *params)
Relation toastRelation = table_open(toastOid,
ShareUpdateExclusiveLock);
- /* Save the list of relation OIDs in private context */
- oldcontext = MemoryContextSwitchTo(private_context);
-
- /* Track this relation for session locks */
- heapRelationIds = lappend_oid(heapRelationIds, toastOid);
-
- MemoryContextSwitchTo(oldcontext);
-
foreach(lc2, RelationGetIndexList(toastRelation))
{
Oid cellOid = lfirst_oid(lc2);
@@ -3434,70 +3412,6 @@ ReindexRelationConcurrently(Oid relationOid, ReindexParams *params)
break;
}
case RELKIND_INDEX:
- {
- Oid heapId = IndexGetRelation(relationOid,
- (params->options & REINDEXOPT_MISSING_OK) != 0);
- Relation heapRelation;
- ReindexIndexInfo *idx;
-
- /* if relation is missing, leave */
- if (!OidIsValid(heapId))
- break;
-
- if (IsCatalogRelationOid(heapId))
- ereport(ERROR,
- (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
- errmsg("cannot reindex system catalogs concurrently")));
-
- /*
- * Don't allow reindex for an invalid index on TOAST table, as
- * if rebuilt it would not be possible to drop it. Match
- * error message in reindex_index().
- */
- if (IsToastNamespace(get_rel_namespace(relationOid)) &&
- !get_index_isvalid(relationOid))
- ereport(ERROR,
- (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
- errmsg("cannot reindex invalid index on TOAST table")));
-
- /*
- * Check if parent relation can be locked and if it exists,
- * this needs to be done at this stage as the list of indexes
- * to rebuild is not complete yet, and REINDEXOPT_MISSING_OK
- * should not be used once all the session locks are taken.
- */
- if ((params->options & REINDEXOPT_MISSING_OK) != 0)
- {
- heapRelation = try_table_open(heapId,
- ShareUpdateExclusiveLock);
- /* leave if relation does not exist */
- if (!heapRelation)
- break;
- }
- else
- heapRelation = table_open(heapId,
- ShareUpdateExclusiveLock);
- table_close(heapRelation, NoLock);
-
- /* Save the list of relation OIDs in private context */
- oldcontext = MemoryContextSwitchTo(private_context);
-
- /* Track the heap relation of this index for session locks */
- heapRelationIds = list_make1_oid(heapId);
-
- /*
- * Save the list of relation OIDs in private context. Note
- * that invalid indexes are allowed here.
- */
- idx = palloc(sizeof(ReindexIndexInfo));
- idx->indexId = relationOid;
- indexIds = lappend(indexIds, idx);
- /* other fields set later */
-
- MemoryContextSwitchTo(oldcontext);
- break;
- }
-
case RELKIND_PARTITIONED_TABLE:
case RELKIND_PARTITIONED_INDEX:
default:
@@ -3521,10 +3435,8 @@ ReindexRelationConcurrently(Oid relationOid, ReindexParams *params)
return false;
}
- Assert(heapRelationIds != NIL);
-
/* Do the work */
- newIndexIds = ReindexIndexesConcurrently(indexIds, heapRelationIds, params->options, private_context);
+ newIndexIds = ReindexIndexesConcurrently(indexIds, params->options, private_context);
/* Log what we did */
if ((params->options & REINDEXOPT_VERBOSE) != 0)
@@ -3566,9 +3478,10 @@ ReindexRelationConcurrently(Oid relationOid, ReindexParams *params)
* This is called by ReindexRelationConcurrently and
*/
static List *
-ReindexIndexesConcurrently(List *indexIds, List *heapRelationIds, int options,
+ReindexIndexesConcurrently(List *indexIds, int options,
MemoryContext private_context)
{
+ List *heapRelationIds = NIL;
List *newIndexIds = NIL;
List *relationLocks = NIL;
List *lockTags = NIL;
@@ -3586,6 +3499,72 @@ ReindexIndexesConcurrently(List *indexIds, List *heapRelationIds, int options,
};
int64 progress_vals[4];
+ foreach(lc, indexIds)
+ {
+ ReindexIndexInfo *idx = lfirst(lc);
+ Oid indexrelid = idx->indexId;
+ Oid heapId = IndexGetRelation(indexrelid,
+ (options & REINDEXOPT_MISSING_OK) != 0);
+ Relation heapRelation;
+
+ /* if relation is missing, leave */
+ if (!OidIsValid(heapId))
+ break; // XXX: ldelete?
+
+ if (IsCatalogRelationOid(heapId))
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("cannot reindex system catalogs concurrently")));
+
+ /*
+ * Don't allow reindex for an invalid index on TOAST table, as
+ * if rebuilt it would not be possible to drop it. Match
+ * error message in reindex_index().
+ */
+ if (IsToastNamespace(get_rel_namespace(indexrelid)) &&
+ !get_index_isvalid(indexrelid))
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("cannot reindex invalid index on TOAST table")));
+
+ if (IsToastNamespace(get_rel_namespace(indexrelid)) &&
+ !get_index_isvalid(indexrelid))
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("cannot reindex invalid index on TOAST table")));
+
+ /*
+ * Check if parent relation can be locked and if it exists,
+ * this needs to be done at this stage as the list of indexes
+ * to rebuild is not complete yet, and REINDEXOPT_MISSING_OK
+ * should not be used once all the session locks are taken.
+ */
+ if ((options & REINDEXOPT_MISSING_OK) != 0)
+ {
+ heapRelation = try_table_open(heapId,
+ ShareUpdateExclusiveLock);
+ /* leave if relation does not exist */
+ if (!heapRelation)
+ break; // ldelete
+ }
+ else
+ heapRelation = table_open(heapId,
+ ShareUpdateExclusiveLock);
+ table_close(heapRelation, NoLock);
+
+ /* Save the list of relation OIDs in private context */
+ oldcontext = MemoryContextSwitchTo(private_context);
+
+ /* Track the heap relation of this index for session locks */
+ heapRelationIds = lappend_oid(heapRelationIds, heapId);
+ // heapRelationIds = list_make1_oid(heapId);
+
+ /* Note that invalid indexes are allowed here. */
+
+ MemoryContextSwitchTo(oldcontext);
+ // break;
+ }
+
/*-----
* Now we have all the indexes we want to process in indexIds.
*
diff --git a/src/test/regress/expected/create_index.out b/src/test/regress/expected/create_index.out
index 4a03ab2abb..fc6afab58a 100644
--- a/src/test/regress/expected/create_index.out
+++ b/src/test/regress/expected/create_index.out
@@ -2470,12 +2470,12 @@ COMMIT;
REINDEX TABLE CONCURRENTLY pg_class; -- no catalog relation
ERROR: cannot reindex system catalogs concurrently
REINDEX INDEX CONCURRENTLY pg_class_oid_index; -- no catalog index
-ERROR: concurrent index creation on system catalog tables is not supported
+ERROR: cannot reindex system catalogs concurrently
-- These are the toast table and index of pg_authid.
REINDEX TABLE CONCURRENTLY pg_toast.pg_toast_1260; -- no catalog toast table
ERROR: cannot reindex system catalogs concurrently
REINDEX INDEX CONCURRENTLY pg_toast.pg_toast_1260_index; -- no catalog toast index
-ERROR: concurrent index creation on system catalog tables is not supported
+ERROR: cannot reindex system catalogs concurrently
REINDEX SYSTEM CONCURRENTLY postgres; -- not allowed for SYSTEM
ERROR: cannot reindex system catalogs concurrently
-- Warns about catalog relations
--
2.17.0
