On 28.05.2024 07:05, Alexander Pyhalov wrote:
Ilya Gladyshev писал(а) 2024-05-28 02:52:
Also I'd like to note that in new patch version there's a strange
wording in documentation:
"This can be very convenient as not only will all existing
partitions be
indexed, but any future partitions will be as well.
<command>CREATE INDEX ... CONCURRENTLY</command> can incur long
lock times
on huge partitioned tables, to avoid that you can
use <command>CREATE INDEX ON ONLY</command> the partitioned table,
which
creates the new index marked as invalid, preventing automatic
application
to existing partitions."
All the point of CIC is to avoid long lock times. So it seems this
paragraph should be rewritten in the following way:
"To avoid long lock times, you can use CREATE INDEX CONCURRENTLY or
CREATE INDEX ON ONLY</command> the partitioned table..."
True, the current wording doesn't look right. Right now CREATE INDEX
ON ONLY is described as a workaround for the missing CIC. I think it
rather makes sense to say that it gives more fine-grained control of
partition locking than both CIC and ordinary CREATE INDEX. See the
updated patch.
Hi.
Not sure if it's worth removing mentioning of CIC in
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
but at least now it looks better.
The current patch version locks all the partitions in the first
transaction up until each of them is built, which makes for long lock
times for partitions that are built last. Having looked at the
implementation of REINDEX CONCURRENTLY for partitioned tables, I think
we can improve this by using the same approach of just skipping the
relations that we find out are dropped when trying to lock them.
Incidentally, this implementation in the new patch version is also simpler.
In addition, I noticed that progress tracking is once again broken for
partitioned tables, while looking at REINDEX implementation, attaching
the second patch to fix it.
From 884be03aaeabee5c6eeb5a3f639ac9afe712c24b 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 v4 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 5da2df2d3b..17b30ad6aa 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;
}
@@ -1682,7 +1683,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 */
@@ -3327,6 +3328,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));
@@ -3388,11 +3397,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
@@ -3413,6 +3439,7 @@ static void
ReindexMultipleInternal(const ReindexStmt *stmt, const List *relids, const ReindexParams *params)
{
ListCell *l;
+ bool partitions = ((params->options & REINDEXOPT_PARTITION) != 0);
PopActiveSnapshot();
CommitTransactionCommand();
@@ -3506,6 +3533,9 @@ ReindexMultipleInternal(const ReindexStmt *stmt, const List *relids, const Reind
}
CommitTransactionCommand();
+
+ if (partitions)
+ progress_index_partition_done();
}
StartTransactionCommand();
@@ -3558,6 +3588,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,
@@ -3901,7 +3932,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 */
@@ -4375,7 +4407,8 @@ ReindexRelationConcurrently(const ReindexStmt *stmt, Oid relationOid, const Rein
MemoryContextDelete(private_context);
- pgstat_progress_end_command();
+ if (!partition)
+ pgstat_progress_end_command();
return true;
}
@@ -4565,3 +4598,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 45576292012468f2d3deea0e568ee74891cb73b1 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 v4 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 | 224 +++++++++++++++++++------
src/test/regress/expected/indexing.out | 127 +++++++++++++-
src/test/regress/sql/indexing.sql | 26 ++-
5 files changed, 323 insertions(+), 78 deletions(-)
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..5da2df2d3b 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,137 @@ 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;
+
+ /*
+ * Pre-existing partitions which were ATTACHED were already
+ * counted in the progress report.
+ */
+ if (get_index_isvalid(indrelid))
+ 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);
+ continue;
+ }
+
+ /*
+ * 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;
+ }
+
+ 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 +1913,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/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
