On 15.06.2024 20:40, Justin Pryzby wrote:
On Thu, May 23, 2024 at 10:14:57PM +0100, Ilya Gladyshev wrote:
Hi,
I think it's well worth the effort to revive the patch, so I rebased it on
master, updated it and will return it back to the commitfest. Alexander,
Justin feel free to add yourselves as authors
Thanks -- I was intending to write about this.
I realized that the patch will need some isolation tests to exercise its
concurrent behavior.
Thanks for the suggestion, added an isolation test that verifies
behaviour of partitioned CIC with simultaneous partition drop/detach
going on. Also fixed some issues in the new patch that I found while
writing the test.
From 45f2ec9ee57a5337b77b66db3c8c5092f305a176 Mon Sep 17 00:00:00 2001
From: Ilya Gladyshev <ilya.v.gladys...@gmail.com>
Date: Tue, 11 Jun 2024 17:48:08 +0100
Subject: [PATCH v5 2/2] Fix progress report for partitioned REINDEX
---
src/backend/catalog/index.c | 11 ++++--
src/backend/commands/indexcmds.c | 63 +++++++++++++++++++++++++++++---
src/include/catalog/index.h | 1 +
3 files changed, 66 insertions(+), 9 deletions(-)
diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c
index 55fdde4b24..c5bc72b350 100644
--- a/src/backend/catalog/index.c
+++ b/src/backend/catalog/index.c
@@ -3559,6 +3559,7 @@ reindex_index(const ReindexStmt *stmt, Oid indexId,
volatile bool skipped_constraint = false;
PGRUsage ru0;
bool progress = ((params->options & REINDEXOPT_REPORT_PROGRESS) != 0);
+ bool partition = ((params->options & REINDEXOPT_PARTITION) != 0);
bool set_tablespace = false;
pg_rusage_init(&ru0);
@@ -3604,8 +3605,9 @@ reindex_index(const ReindexStmt *stmt, Oid indexId,
indexId
};
- pgstat_progress_start_command(PROGRESS_COMMAND_CREATE_INDEX,
- heapId);
+ if (!partition)
+ pgstat_progress_start_command(PROGRESS_COMMAND_CREATE_INDEX,
+ heapId);
pgstat_progress_update_multi_param(2, progress_cols, progress_vals);
}
@@ -3845,8 +3847,11 @@ reindex_index(const ReindexStmt *stmt, Oid indexId,
index_close(iRel, NoLock);
table_close(heapRelation, NoLock);
- if (progress)
+ if (progress && !partition)
+ {
+ /* progress for partitions is tracked in the caller */
pgstat_progress_end_command();
+ }
}
/*
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index dcb4ea89e9..6abe1f017c 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -118,6 +118,7 @@ static bool ReindexRelationConcurrently(const ReindexStmt *stmt,
const ReindexParams *params);
static void update_relispartition(Oid relationId, bool newval);
static inline void set_indexsafe_procflags(void);
+static inline void progress_index_partition_done(void);
/*
* callback argument type for RangeVarCallbackForReindexIndex()
@@ -1550,7 +1551,7 @@ DefineIndex(Oid tableId,
* Update progress for an intermediate partitioned index
* itself
*/
- pgstat_progress_incr_param(PROGRESS_CREATEIDX_PARTITIONS_DONE, 1);
+ progress_index_partition_done();
}
return address;
@@ -1577,7 +1578,7 @@ DefineIndex(Oid tableId,
if (!OidIsValid(parentIndexId))
pgstat_progress_end_command();
else if (!concurrent)
- pgstat_progress_incr_param(PROGRESS_CREATEIDX_PARTITIONS_DONE, 1);
+ progress_index_partition_done();
return address;
}
@@ -1686,7 +1687,7 @@ DefineIndex(Oid tableId,
heaplocktag, heaprelid);
PushActiveSnapshot(GetTransactionSnapshot());
- pgstat_progress_incr_param(PROGRESS_CREATEIDX_PARTITIONS_DONE, 1);
+ progress_index_partition_done();
}
/* Set as valid all partitioned indexes, including the parent */
@@ -3331,6 +3332,14 @@ ReindexPartitions(const ReindexStmt *stmt, Oid relid, const ReindexParams *param
ListCell *lc;
ErrorContextCallback errcallback;
ReindexErrorInfo errinfo;
+ ReindexParams newparams;
+ int progress_params[3] = {
+ PROGRESS_CREATEIDX_COMMAND,
+ PROGRESS_CREATEIDX_PHASE,
+ PROGRESS_CREATEIDX_PARTITIONS_TOTAL
+ };
+ int64 progress_values[3];
+ Oid heapId = relid;
Assert(RELKIND_HAS_PARTITIONS(relkind));
@@ -3392,11 +3401,28 @@ ReindexPartitions(const ReindexStmt *stmt, Oid relid, const ReindexParams *param
MemoryContextSwitchTo(old_context);
}
+ if (relkind == RELKIND_PARTITIONED_INDEX)
+ {
+ heapId = IndexGetRelation(relid, true);
+ }
+
+ progress_values[0] = (params->options & REINDEXOPT_CONCURRENTLY) != 0 ?
+ PROGRESS_CREATEIDX_COMMAND_REINDEX_CONCURRENTLY :
+ PROGRESS_CREATEIDX_COMMAND_REINDEX;
+ progress_values[1] = 0;
+ progress_values[2] = list_length(partitions);
+ pgstat_progress_start_command(PROGRESS_COMMAND_CREATE_INDEX, heapId);
+ pgstat_progress_update_multi_param(3, progress_params, progress_values);
+
/*
* Process each partition listed in a separate transaction. Note that
* this commits and then starts a new transaction immediately.
*/
- ReindexMultipleInternal(stmt, partitions, params);
+ newparams = *params;
+ newparams.options |= REINDEXOPT_PARTITION;
+ ReindexMultipleInternal(stmt, partitions, &newparams);
+
+ pgstat_progress_end_command();
/*
* Clean up working storage --- note we must do this after
@@ -3417,6 +3443,7 @@ static void
ReindexMultipleInternal(const ReindexStmt *stmt, const List *relids, const ReindexParams *params)
{
ListCell *l;
+ bool partitions = ((params->options & REINDEXOPT_PARTITION) != 0);
PopActiveSnapshot();
CommitTransactionCommand();
@@ -3510,6 +3537,9 @@ ReindexMultipleInternal(const ReindexStmt *stmt, const List *relids, const Reind
}
CommitTransactionCommand();
+
+ if (partitions)
+ progress_index_partition_done();
}
StartTransactionCommand();
@@ -3562,6 +3592,7 @@ ReindexRelationConcurrently(const ReindexStmt *stmt, Oid relationOid, const Rein
char *relationName = NULL;
char *relationNamespace = NULL;
PGRUsage ru0;
+ bool partition = ((params->options & REINDEXOPT_PARTITION) != 0);
const int progress_index[] = {
PROGRESS_CREATEIDX_COMMAND,
PROGRESS_CREATEIDX_PHASE,
@@ -3905,7 +3936,8 @@ ReindexRelationConcurrently(const ReindexStmt *stmt, Oid relationOid, const Rein
if (indexRel->rd_rel->relpersistence == RELPERSISTENCE_TEMP)
elog(ERROR, "cannot reindex a temporary table concurrently");
- pgstat_progress_start_command(PROGRESS_COMMAND_CREATE_INDEX, idx->tableId);
+ if (!partition)
+ pgstat_progress_start_command(PROGRESS_COMMAND_CREATE_INDEX, idx->tableId);
progress_vals[0] = PROGRESS_CREATEIDX_COMMAND_REINDEX_CONCURRENTLY;
progress_vals[1] = 0; /* initializing */
@@ -4379,7 +4411,8 @@ ReindexRelationConcurrently(const ReindexStmt *stmt, Oid relationOid, const Rein
MemoryContextDelete(private_context);
- pgstat_progress_end_command();
+ if (!partition)
+ pgstat_progress_end_command();
return true;
}
@@ -4569,3 +4602,21 @@ set_indexsafe_procflags(void)
ProcGlobal->statusFlags[MyProc->pgxactoff] = MyProc->statusFlags;
LWLockRelease(ProcArrayLock);
}
+
+static inline void
+progress_index_partition_done(void)
+{
+ int nparam = 6;
+ const int progress_idx[] = {
+ PROGRESS_CREATEIDX_INDEX_OID,
+ PROGRESS_CREATEIDX_ACCESS_METHOD_OID,
+ PROGRESS_CREATEIDX_PHASE,
+ PROGRESS_CREATEIDX_SUBPHASE,
+ PROGRESS_CREATEIDX_TUPLES_TOTAL,
+ PROGRESS_CREATEIDX_TUPLES_DONE
+ };
+ const int64 progress_vals[] = {0, 0, 0, 0, 0, 0};
+
+ pgstat_progress_update_multi_param(nparam, progress_idx, progress_vals);
+ pgstat_progress_incr_param(PROGRESS_CREATEIDX_PARTITIONS_DONE, 1);
+}
diff --git a/src/include/catalog/index.h b/src/include/catalog/index.h
index 7d434f8e65..ccba65fbbf 100644
--- a/src/include/catalog/index.h
+++ b/src/include/catalog/index.h
@@ -42,6 +42,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_PARTITION 0x10 /* reindexing is done as part of partitioned table/index reindex */
/* state info for validate_index bulkdelete callback */
typedef struct ValidateIndexState
--
2.43.0
From acf5cf5d4a984c0f8635a25e03c23409601c0c93 Mon Sep 17 00:00:00 2001
From: Ilya Gladyshev <ilya.v.gladys...@gmail.com>
Date: Thu, 23 May 2024 18:13:41 +0100
Subject: [PATCH v5 1/2] Allow CREATE INDEX CONCURRENTLY on partitioned table
---
doc/src/sgml/ddl.sgml | 10 +-
doc/src/sgml/ref/create_index.sgml | 14 +-
src/backend/commands/indexcmds.c | 228 +++++++++++++-----
.../isolation/expected/partitioned-cic.out | 135 +++++++++++
src/test/isolation/isolation_schedule | 1 +
src/test/isolation/specs/partitioned-cic.spec | 57 +++++
src/test/regress/expected/indexing.out | 127 +++++++++-
src/test/regress/sql/indexing.sql | 26 +-
8 files changed, 520 insertions(+), 78 deletions(-)
create mode 100644 src/test/isolation/expected/partitioned-cic.out
create mode 100644 src/test/isolation/specs/partitioned-cic.spec
diff --git a/doc/src/sgml/ddl.sgml b/doc/src/sgml/ddl.sgml
index 6aab79e901..904978c6e5 100644
--- a/doc/src/sgml/ddl.sgml
+++ b/doc/src/sgml/ddl.sgml
@@ -4314,14 +4314,12 @@ ALTER TABLE measurement ATTACH PARTITION measurement_y2008m02
As mentioned earlier, it is possible to create indexes on partitioned
tables so that they are applied automatically to the entire hierarchy.
This can be very convenient as not only will all existing partitions be
- indexed, but any future partitions will be as well. However, one
- limitation when creating new indexes on partitioned tables is that it
- is not possible to use the <literal>CONCURRENTLY</literal>
- qualifier, which could lead to long lock times. To avoid this, you can
- use <command>CREATE INDEX ON ONLY</command> the partitioned table, which
+ indexed, but any future partitions will be as well. For more control over
+ locking of the partitions you can use <command>CREATE INDEX ON ONLY</command>
+ on the partitioned table, which
creates the new index marked as invalid, preventing automatic application
to existing partitions. Instead, indexes can then be created individually
- on each partition using <literal>CONCURRENTLY</literal> and
+ on each partition and
<firstterm>attached</firstterm> to the partitioned index on the parent
using <command>ALTER INDEX ... ATTACH PARTITION</command>. Once indexes for
all the partitions are attached to the parent index, the parent index will
diff --git a/doc/src/sgml/ref/create_index.sgml b/doc/src/sgml/ref/create_index.sgml
index 621bc0e253..2366cfd9b5 100644
--- a/doc/src/sgml/ref/create_index.sgml
+++ b/doc/src/sgml/ref/create_index.sgml
@@ -645,7 +645,10 @@ CREATE [ UNIQUE ] INDEX [ CONCURRENTLY ] [ [ IF NOT EXISTS ] <replaceable class=
<para>
If a problem arises while scanning the table, such as a deadlock or a
uniqueness violation in a unique index, the <command>CREATE INDEX</command>
- command will fail but leave behind an <quote>invalid</quote> index. This index
+ command will fail but leave behind an <quote>invalid</quote> index.
+ If this happens while build an index concurrently on a partitioned
+ table, the command can also leave behind <quote>valid</quote> or
+ <quote>invalid</quote> indexes on table partitions. The invalid index
will be ignored for querying purposes because it might be incomplete;
however it will still consume update overhead. The <application>psql</application>
<command>\d</command> command will report such an index as <literal>INVALID</literal>:
@@ -692,15 +695,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 309389e20d..dcb4ea89e9 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -95,6 +95,11 @@ static char *ChooseIndexName(const char *tabname, Oid namespaceId,
bool primary, bool isconstraint);
static char *ChooseIndexNameAddition(const List *colnames);
static List *ChooseIndexColumnNames(const List *indexElems);
+static void DefineIndexConcurrentInternal(Oid relationId,
+ Oid indexRelationId,
+ IndexInfo *indexInfo,
+ LOCKTAG heaplocktag,
+ LockRelId heaprelid);
static void ReindexIndex(const ReindexStmt *stmt, const ReindexParams *params,
bool isTopLevel);
static void RangeVarCallbackForReindexIndex(const RangeVar *relation,
@@ -561,7 +566,6 @@ DefineIndex(Oid tableId,
bool amissummarizing;
amoptions_function amoptions;
bool partitioned;
- bool safe_index;
Datum reloptions;
int16 *coloptions;
IndexInfo *indexInfo;
@@ -569,12 +573,10 @@ DefineIndex(Oid tableId,
bits16 constr_flags;
int numberOfAttributes;
int numberOfKeyAttributes;
- TransactionId limitXmin;
ObjectAddress address;
LockRelId heaprelid;
LOCKTAG heaplocktag;
LOCKMODE lockmode;
- Snapshot snapshot;
Oid root_save_userid;
int root_save_sec_context;
int root_save_nestlevel;
@@ -706,20 +708,6 @@ DefineIndex(Oid tableId,
* partition.
*/
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))));
- }
/*
* Don't try to CREATE INDEX on temp tables of other backends.
@@ -1116,10 +1104,6 @@ DefineIndex(Oid tableId,
}
}
- /* Is index safe for others to ignore? See set_indexsafe_procflags() */
- safe_index = indexInfo->ii_Expressions == NIL &&
- indexInfo->ii_Predicate == NIL;
-
/*
* Report index creation if appropriate (delay this till after most of the
* error checks)
@@ -1184,6 +1168,11 @@ DefineIndex(Oid tableId,
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;
@@ -1533,21 +1522,7 @@ DefineIndex(Oid tableId,
*/
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);
/*
* CCI here to make this update visible, in case this recurses
@@ -1559,37 +1534,49 @@ DefineIndex(Oid tableId,
/*
* Indexes on partitioned tables are not themselves built, so we're
- * done here.
+ * done here in the non-concurrent case.
*/
- AtEOXact_GUC(false, root_save_nestlevel);
- SetUserIdAndSecContext(root_save_userid, root_save_sec_context);
- table_close(rel, NoLock);
- if (!OidIsValid(parentIndexId))
- pgstat_progress_end_command();
- else
+ if (!concurrent)
{
- /* Update progress for an intermediate partitioned index itself */
- pgstat_progress_incr_param(PROGRESS_CREATEIDX_PARTITIONS_DONE, 1);
- }
+ AtEOXact_GUC(false, root_save_nestlevel);
+ SetUserIdAndSecContext(root_save_userid, root_save_sec_context);
+ table_close(rel, NoLock);
- return address;
+ if (!OidIsValid(parentIndexId))
+ pgstat_progress_end_command();
+ else
+ {
+ /*
+ * Update progress for an intermediate partitioned index
+ * itself
+ */
+ pgstat_progress_incr_param(PROGRESS_CREATEIDX_PARTITIONS_DONE, 1);
+ }
+
+ return address;
+ }
}
AtEOXact_GUC(false, root_save_nestlevel);
SetUserIdAndSecContext(root_save_userid, root_save_sec_context);
- if (!concurrent)
+ /*
+ * All done in the non-concurrent case, and when building catalog entries
+ * of partitions for CIC.
+ */
+ if (!concurrent || OidIsValid(parentIndexId))
{
- /* Close the heap and we're done, in the non-concurrent case */
table_close(rel, NoLock);
/*
* If this is the top-level index, the command is done overall;
- * otherwise, increment progress to report one child index is done.
+ * otherwise (when being called recursively), increment progress to
+ * report that one child index is done. Except in the concurrent
+ * (catalog-only) case, which is handled later.
*/
if (!OidIsValid(parentIndexId))
pgstat_progress_end_command();
- else
+ else if (!concurrent)
pgstat_progress_incr_param(PROGRESS_CREATEIDX_PARTITIONS_DONE, 1);
return address;
@@ -1600,6 +1587,141 @@ DefineIndex(Oid tableId,
SET_LOCKTAG_RELATION(heaplocktag, heaprelid.dbId, heaprelid.relId);
table_close(rel, NoLock);
+ if (!partitioned)
+ {
+ /* CREATE INDEX CONCURRENTLY on a nonpartitioned table */
+ DefineIndexConcurrentInternal(tableId, indexRelationId,
+ indexInfo, heaplocktag, heaprelid);
+ pgstat_progress_end_command();
+ return address;
+ }
+ else
+ {
+ /*
+ * For CIC on a partitioned table, finish by building indexes on
+ * partitions
+ */
+
+ ListCell *lc;
+ List *childs;
+ List *tosetvalid = NIL;
+ MemoryContext cic_context,
+ old_context;
+
+ /* Create special memory context for cross-transaction storage */
+ cic_context = AllocSetContextCreate(PortalContext,
+ "Create index concurrently",
+ ALLOCSET_DEFAULT_SIZES);
+
+ old_context = MemoryContextSwitchTo(cic_context);
+ childs = find_all_inheritors(indexRelationId, ShareUpdateExclusiveLock, NULL);
+ MemoryContextSwitchTo(old_context);
+
+ foreach(lc, childs)
+ {
+ Oid indrelid = lfirst_oid(lc);
+ Oid tabrelid;
+ char relkind;
+
+ /*
+ * Partition could have been dropped, since we looked it up. In
+ * this case consider it done and go to the next one.
+ */
+ tabrelid = IndexGetRelation(indrelid, true);
+ if (!tabrelid)
+ {
+ pgstat_progress_incr_param(PROGRESS_CREATEIDX_PARTITIONS_DONE, 1);
+ continue;
+ }
+ rel = try_table_open(tabrelid, ShareUpdateExclusiveLock);
+ if (!rel)
+ {
+ pgstat_progress_incr_param(PROGRESS_CREATEIDX_PARTITIONS_DONE, 1);
+ continue;
+ }
+
+ /*
+ * Pre-existing partitions which were ATTACHED were already
+ * counted in the progress report.
+ */
+ if (get_index_isvalid(indrelid))
+ {
+ table_close(rel, ShareUpdateExclusiveLock);
+ continue;
+ }
+
+ /*
+ * Partitioned indexes are counted in the progress report, but
+ * don't need to be further processed.
+ */
+ relkind = get_rel_relkind(indrelid);
+ if (!RELKIND_HAS_STORAGE(relkind))
+ {
+ /* The toplevel index doesn't count towards "partitions done" */
+ if (indrelid != indexRelationId)
+ pgstat_progress_incr_param(PROGRESS_CREATEIDX_PARTITIONS_DONE, 1);
+
+ /*
+ * Build up a list of all the intermediate partitioned tables
+ * which will later need to be set valid.
+ */
+ old_context = MemoryContextSwitchTo(cic_context);
+ tosetvalid = lappend_oid(tosetvalid, indrelid);
+ MemoryContextSwitchTo(old_context);
+ table_close(rel, ShareUpdateExclusiveLock);
+ continue;
+ }
+
+ heaprelid = rel->rd_lockInfo.lockRelId;
+
+ /*
+ * Close the table but retain the lock, that should be extended to
+ * session level in DefineIndexConcurrentInternal.
+ */
+ table_close(rel, NoLock);
+ SET_LOCKTAG_RELATION(heaplocktag, heaprelid.dbId, heaprelid.relId);
+
+ /* Process each partition in a separate transaction */
+ DefineIndexConcurrentInternal(tabrelid, indrelid, indexInfo,
+ heaplocktag, heaprelid);
+
+ PushActiveSnapshot(GetTransactionSnapshot());
+ pgstat_progress_incr_param(PROGRESS_CREATEIDX_PARTITIONS_DONE, 1);
+ }
+
+ /* Set as valid all partitioned indexes, including the parent */
+ foreach(lc, tosetvalid)
+ {
+ Oid indrelid = lfirst_oid(lc);
+ Relation indrel = try_index_open(indrelid, ShareUpdateExclusiveLock);
+
+ if (!indrel)
+ continue;
+ index_set_state_flags(indrelid, INDEX_CREATE_SET_READY);
+ CommandCounterIncrement();
+ index_set_state_flags(indrelid, INDEX_CREATE_SET_VALID);
+ index_close(indrel, ShareUpdateExclusiveLock);
+ }
+
+ MemoryContextDelete(cic_context);
+ pgstat_progress_end_command();
+ PopActiveSnapshot();
+ return address;
+ }
+}
+
+
+static void
+DefineIndexConcurrentInternal(Oid tableId, Oid indexRelationId, IndexInfo *indexInfo,
+ LOCKTAG heaplocktag, LockRelId heaprelid)
+{
+ TransactionId limitXmin;
+ Snapshot snapshot;
+
+ /* Is index safe for others to ignore? See set_indexsafe_procflags() */
+ bool safe_index = indexInfo->ii_Expressions == NIL &&
+ indexInfo->ii_Predicate == NIL;
+
/*
* For a concurrent build, it's important to make the catalog entries
* visible to other transactions before we start to build the index. That
@@ -1795,10 +1917,6 @@ DefineIndex(Oid tableId,
* Last thing to do is release the session-level lock on the parent table.
*/
UnlockRelationIdForSession(&heaprelid, ShareUpdateExclusiveLock);
-
- pgstat_progress_end_command();
-
- return address;
}
diff --git a/src/test/isolation/expected/partitioned-cic.out b/src/test/isolation/expected/partitioned-cic.out
new file mode 100644
index 0000000000..b66acc6f6a
--- /dev/null
+++ b/src/test/isolation/expected/partitioned-cic.out
@@ -0,0 +1,135 @@
+Parsed test spec with 3 sessions
+
+starting permutation: lock_p1 cic insert drop2 commit chk_content
+step lock_p1: lock cictab_part_1 in row exclusive mode;
+step cic: CREATE INDEX CONCURRENTLY ON cictab(i); <waiting ...>
+step insert: insert into cictab values (1, 1), (11, 1);
+step drop2: DROP TABLE cictab_part_2;
+step commit: COMMIT;
+step cic: <... completed>
+step chk_content:
+ set enable_seqscan to off;
+ explain (costs off) select * from cictab where i > 0;
+ select * from cictab where i > 0;
+
+QUERY PLAN
+------------------------------------------------------------
+Index Scan using cictab_part_1_i_idx on cictab_part_1 cictab
+ Index Cond: (i > 0)
+(2 rows)
+
+i|j
+-+-
+1|0
+1|1
+(2 rows)
+
+
+starting permutation: lock_p2 cic insert drop1 commit chk_content
+step lock_p2: lock cictab_part_2 in row exclusive mode;
+step cic: CREATE INDEX CONCURRENTLY ON cictab(i); <waiting ...>
+step insert: insert into cictab values (1, 1), (11, 1);
+step drop1: DROP TABLE cictab_part_1;
+step commit: COMMIT;
+step cic: <... completed>
+step chk_content:
+ set enable_seqscan to off;
+ explain (costs off) select * from cictab where i > 0;
+ select * from cictab where i > 0;
+
+QUERY PLAN
+------------------------------------------------------------
+Index Scan using cictab_part_2_i_idx on cictab_part_2 cictab
+ Index Cond: (i > 0)
+(2 rows)
+
+ i|j
+--+-
+11|0
+11|1
+(2 rows)
+
+
+starting permutation: lock_p1 cic insert detach2 commit chk_content chk_content_part2
+step lock_p1: lock cictab_part_1 in row exclusive mode;
+step cic: CREATE INDEX CONCURRENTLY ON cictab(i); <waiting ...>
+step insert: insert into cictab values (1, 1), (11, 1);
+step detach2: ALTER TABLE cictab DETACH PARTITION cictab_part_2;
+step commit: COMMIT;
+step cic: <... completed>
+step chk_content:
+ set enable_seqscan to off;
+ explain (costs off) select * from cictab where i > 0;
+ select * from cictab where i > 0;
+
+QUERY PLAN
+------------------------------------------------------------
+Index Scan using cictab_part_1_i_idx on cictab_part_1 cictab
+ Index Cond: (i > 0)
+(2 rows)
+
+i|j
+-+-
+1|0
+1|1
+(2 rows)
+
+step chk_content_part2:
+ set enable_seqscan to off;
+ explain (costs off) select * from cictab_part_2 where i > 0;
+ select * from cictab_part_2 where i > 0;
+
+QUERY PLAN
+-----------------------------------------------------
+Index Scan using cictab_part_2_i_idx on cictab_part_2
+ Index Cond: (i > 0)
+(2 rows)
+
+ i|j
+--+-
+11|0
+11|1
+(2 rows)
+
+
+starting permutation: lock_p2 cic insert detach1 commit chk_content chk_content_part1
+step lock_p2: lock cictab_part_2 in row exclusive mode;
+step cic: CREATE INDEX CONCURRENTLY ON cictab(i); <waiting ...>
+step insert: insert into cictab values (1, 1), (11, 1);
+step detach1: ALTER TABLE cictab DETACH PARTITION cictab_part_1;
+step commit: COMMIT;
+step cic: <... completed>
+step chk_content:
+ set enable_seqscan to off;
+ explain (costs off) select * from cictab where i > 0;
+ select * from cictab where i > 0;
+
+QUERY PLAN
+------------------------------------------------------------
+Index Scan using cictab_part_2_i_idx on cictab_part_2 cictab
+ Index Cond: (i > 0)
+(2 rows)
+
+ i|j
+--+-
+11|0
+11|1
+(2 rows)
+
+step chk_content_part1:
+ set enable_seqscan to off;
+ explain (costs off) select * from cictab_part_1 where i > 0;
+ select * from cictab_part_1 where i > 0;
+
+QUERY PLAN
+-----------------------------------------------------
+Index Scan using cictab_part_1_i_idx on cictab_part_1
+ Index Cond: (i > 0)
+(2 rows)
+
+i|j
+-+-
+1|0
+1|1
+(2 rows)
+
diff --git a/src/test/isolation/isolation_schedule b/src/test/isolation/isolation_schedule
index 0342eb39e4..57b7948687 100644
--- a/src/test/isolation/isolation_schedule
+++ b/src/test/isolation/isolation_schedule
@@ -114,3 +114,4 @@ test: serializable-parallel-2
test: serializable-parallel-3
test: matview-write-skew
test: lock-nowait
+test: partitioned-cic
diff --git a/src/test/isolation/specs/partitioned-cic.spec b/src/test/isolation/specs/partitioned-cic.spec
new file mode 100644
index 0000000000..95f0bb2b47
--- /dev/null
+++ b/src/test/isolation/specs/partitioned-cic.spec
@@ -0,0 +1,57 @@
+# Test the ability to drop/detach partitions while CREATE INDEX CONCURRENTLY is running.
+# To achieve this, start a transaction that will pause CIC in progress by
+# locking a partition in row exclusive mode, giving us a change to drop/detach another partition.
+# Dropping/detaching is tested for each partition to test two scenarios:
+# when the partition has already been indexed and when it's yet to be indexed.
+
+setup {
+ create table cictab(i int, j int) partition by range(i);
+ create table cictab_part_1 partition of cictab for values from (0) to (10);
+ create table cictab_part_2 partition of cictab for values from (10) to (20);
+
+ insert into cictab values (1, 0), (11, 0);
+}
+
+teardown {
+ drop table if exists cictab_part_1;
+ drop table if exists cictab_part_2;
+ drop table cictab;
+}
+
+session s1
+setup {BEGIN;}
+step lock_p1 { lock cictab_part_1 in row exclusive mode; }
+step lock_p2 { lock cictab_part_2 in row exclusive mode; }
+step commit { COMMIT; }
+
+session s2
+step cic { CREATE INDEX CONCURRENTLY ON cictab(i); }
+step chk_content {
+ set enable_seqscan to off;
+ explain (costs off) select * from cictab where i > 0;
+ select * from cictab where i > 0;
+}
+
+step chk_content_part1 {
+ set enable_seqscan to off;
+ explain (costs off) select * from cictab_part_1 where i > 0;
+ select * from cictab_part_1 where i > 0;
+}
+
+step chk_content_part2 {
+ set enable_seqscan to off;
+ explain (costs off) select * from cictab_part_2 where i > 0;
+ select * from cictab_part_2 where i > 0;
+}
+
+session s3
+step detach1 { ALTER TABLE cictab DETACH PARTITION cictab_part_1; }
+step detach2 { ALTER TABLE cictab DETACH PARTITION cictab_part_2; }
+step drop1 { DROP TABLE cictab_part_1; }
+step drop2 { DROP TABLE cictab_part_2; }
+step insert { insert into cictab values (1, 1), (11, 1); }
+
+permutation lock_p1 cic insert drop2 commit chk_content
+permutation lock_p2 cic insert drop1 commit chk_content
+permutation lock_p1 cic insert detach2 commit chk_content chk_content_part2
+permutation lock_p2 cic insert detach1 commit chk_content chk_content_part1
diff --git a/src/test/regress/expected/indexing.out b/src/test/regress/expected/indexing.out
index f25723da92..44a6cf39df 100644
--- a/src/test/regress/expected/indexing.out
+++ b/src/test/regress/expected/indexing.out
@@ -50,11 +50,130 @@ 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);
+create table idxpart3 partition of idxpart for values from (30) to (40) partition by range(a);
+create table idxpart31 partition of idxpart3 default;
+insert into idxpart2 values(10),(10); -- not unique
+create index concurrently on idxpart11 (a); -- partitioned and partition, with no leaves
+create index concurrently on idxpart1 (a); -- partitioned and partition
+create index concurrently on idxpart2 (a); -- leaf
+create index concurrently on idxpart (a); -- partitioned
+create unique index concurrently on idxpart (a); -- partitioned, unique failure
+ERROR: could not create unique index "idxpart2_a_idx1"
+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: 3 (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)
+ "idxpart1_a_idx1" UNIQUE, btree (a) INVALID
+Number of partitions: 1 (Use \d+ to list them.)
+
+\d idxpart11
+ Partitioned table "public.idxpart11"
+ Column | Type | Collation | Nullable | Default
+--------+---------+-----------+----------+---------
+ a | integer | | |
+ b | integer | | |
+ c | text | | |
+Partition of: idxpart1 FOR VALUES FROM (0) TO (10)
+Partition key: RANGE (a)
+Indexes:
+ "idxpart11_a_idx" btree (a)
+ "idxpart11_a_idx1" UNIQUE, btree (a) INVALID
+Number of partitions: 1 (Use \d+ to list them.)
+
+\d idxpart111
+ Partitioned table "public.idxpart111"
+ Column | Type | Collation | Nullable | Default
+--------+---------+-----------+----------+---------
+ a | integer | | |
+ b | integer | | |
+ c | text | | |
+Partition of: idxpart11 DEFAULT
+Partition key: RANGE (a)
+Indexes:
+ "idxpart111_a_idx" btree (a)
+ "idxpart111_a_idx1" UNIQUE, btree (a) INVALID
+Number of partitions: 1 (Use \d+ to list them.)
+
+\d idxpart1111
+ Partitioned table "public.idxpart1111"
+ Column | Type | Collation | Nullable | Default
+--------+---------+-----------+----------+---------
+ a | integer | | |
+ b | integer | | |
+ c | text | | |
+Partition of: idxpart111 DEFAULT
+Partition key: RANGE (a)
+Indexes:
+ "idxpart1111_a_idx" btree (a)
+ "idxpart1111_a_idx1" UNIQUE, btree (a) INVALID
+Number of partitions: 0
+
+\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" UNIQUE, btree (a) INVALID
+
+\d idxpart3
+ Partitioned table "public.idxpart3"
+ Column | Type | Collation | Nullable | Default
+--------+---------+-----------+----------+---------
+ a | integer | | |
+ b | integer | | |
+ c | text | | |
+Partition of: idxpart FOR VALUES FROM (30) TO (40)
+Partition key: RANGE (a)
+Indexes:
+ "idxpart3_a_idx" btree (a)
+ "idxpart3_a_idx1" UNIQUE, btree (a) INVALID
+Number of partitions: 1 (Use \d+ to list them.)
+
+\d idxpart31
+ Table "public.idxpart31"
+ Column | Type | Collation | Nullable | Default
+--------+---------+-----------+----------+---------
+ a | integer | | |
+ b | integer | | |
+ c | text | | |
+Partition of: idxpart3 DEFAULT
+Indexes:
+ "idxpart31_a_idx" btree (a)
+ "idxpart31_a_idx1" 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 5f1f4b80c9..38a730d877 100644
--- a/src/test/regress/sql/indexing.sql
+++ b/src/test/regress/sql/indexing.sql
@@ -29,10 +29,30 @@ 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);
+create table idxpart3 partition of idxpart for values from (30) to (40) partition by range(a);
+create table idxpart31 partition of idxpart3 default;
+
+insert into idxpart2 values(10),(10); -- not unique
+create index concurrently on idxpart11 (a); -- partitioned and partition, with no leaves
+create index concurrently on idxpart1 (a); -- partitioned and partition
+create index concurrently on idxpart2 (a); -- leaf
+create index concurrently on idxpart (a); -- partitioned
+create unique index concurrently on idxpart (a); -- partitioned, unique failure
+\d idxpart
+\d idxpart1
+\d idxpart11
+\d idxpart111
+\d idxpart1111
+\d idxpart2
+\d idxpart3
+\d idxpart31
drop table idxpart;
-- Verify bugfix with query on indexed partitioned table with no partitions
--
2.43.0
