From: Justin Pryzby <pryzby@telsasoft.com>
To: 李杰(慎追) <adger.lj@alibaba-inc.com>
Cc: pgsql-hackers <pgsql-hackers@lists.postgresql.org>
Cc: 曾文旌(义从) <wenjing.zwj@alibaba-inc.com>
Cc: Alvaro Herrera <alvherre@2ndquadrant.com>
Subject: Re: how to create index concurrently on partitioned table
Date: Thu, 11 Jun 2020 10:35:02 -0500
Message-ID: <20200611153502.GT14879@telsasoft.com> (raw)
In-Reply-To: <20200607180448.GL22473@telsasoft.com>
References: <db12e897-73ff-467e-94cb-4af03705435f.adger.lj@alibaba-inc.com>
<20200606142332.GH22473@telsasoft.com>
<20200607180448.GL22473@telsasoft.com>
On Sun, Jun 07, 2020 at 01:04:48PM -0500, Justin Pryzby wrote:
> On Sat, Jun 06, 2020 at 09:23:32AM -0500, Justin Pryzby wrote:
> > On Wed, Jun 03, 2020 at 08:22:29PM +0800, 李杰(慎追) wrote:
> > > Partitioning is necessary for very large tables. However, I found that
> > > postgresql does not support create index concurrently on partitioned
> > > tables. The document show that we need to create an index on each
> > > partition individually and then finally create the partitioned index
> > > non-concurrently. This is undoubtedly a complex operation for DBA,
> > > especially when there are many partitions.
I added functionality for C-I-C, REINDEX-CONCURRENTLY, and CLUSTER of
partitioned tables. We already recursively handle VACUUM and ANALYZE since
v10.
And added here:
https://commitfest.postgresql.org/28/2584/
Adger, if you're familiar with compilation and patching, do you want to try the
patch ?
Note, you could do this now using psql like:
SELECT format('CREATE INDEX CONCURRENTLY ... ON %s(col)', a::regclass) FROM pg_partition_ancestors() AS a;
\gexec
--
Justin
Attachments:download all (3 files, .tar.gz)v2-0001-Allow-CREATE-INDEX-CONCURRENTLY-on-partitioned-ta.patch (text/x-diff, 7.6K) download
From b889658992c20011c10ecb05fd43e59ac5475d9e Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryzbyj@telsasoft.com>
Date: Sat, 6 Jun 2020 17:42:23 -0500
Subject: [PATCH v2 1/3] Allow CREATE INDEX CONCURRENTLY on partitioned table
---
doc/src/sgml/ref/create_index.sgml | 9 ----
src/backend/commands/indexcmds.c | 68 ++++++++++++++------------
src/test/regress/expected/indexing.out | 14 +++++-
src/test/regress/sql/indexing.sql | 3 +-
4 files changed, 52 insertions(+), 42 deletions(-)
diff --git a/doc/src/sgml/ref/create_index.sgml b/doc/src/sgml/ref/create_index.sgmlindex ff87b2d28f..e1298b8523 100644--- a/doc/src/sgml/ref/create_index.sgml+++ b/doc/src/sgml/ref/create_index.sgml@@ -657,15 +657,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.cindex 2baca12c5f..344cabaf52 100644--- a/src/backend/commands/indexcmds.c+++ b/src/backend/commands/indexcmds.c@@ -652,17 +652,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),
@@ -1139,8 +1128,26 @@ DefineIndex(Oid relationId,
CreateComments(indexRelationId, RelationRelationId, 0,
stmt->idxcomment);
+ /* save lockrelid and locktag for below */+ heaprelid = rel->rd_lockInfo.lockRelId;+ SET_LOCKTAG_RELATION(heaplocktag, heaprelid.dbId, heaprelid.relId);+
if (partitioned)
{
+ /*+ * Need to close the relation before recursing into children, so copy+ * needed data.+ */+ PartitionDesc partdesc = RelationGetPartitionDesc(rel);+ int nparts = partdesc->nparts;+ TupleDesc parentDesc = CreateTupleDescCopy(RelationGetDescr(rel));+ char *relname = pstrdup(RelationGetRelationName(rel));+ Oid *part_oids;++ part_oids = palloc(sizeof(Oid) * nparts);+ memcpy(part_oids, partdesc->oids, sizeof(Oid) * nparts);+ table_close(rel, NoLock);+
/*
* Unless caller specified to skip this step (via ONLY), process each
* partition to make sure they all contain a corresponding index.
@@ -1149,19 +1156,11 @@ DefineIndex(Oid relationId,
*/
if (!stmt->relation || stmt->relation->inh)
{
- PartitionDesc partdesc = RelationGetPartitionDesc(rel);- int nparts = partdesc->nparts;- Oid *part_oids = palloc(sizeof(Oid) * nparts);
bool invalidate_parent = false;
- TupleDesc parentDesc;
Oid *opfamOids;
pgstat_progress_update_param(PROGRESS_CREATEIDX_PARTITIONS_TOTAL,
nparts);
-- memcpy(part_oids, partdesc->oids, sizeof(Oid) * nparts);-- parentDesc = RelationGetDescr(rel);
opfamOids = palloc(sizeof(Oid) * numberOfKeyAttributes);
for (i = 0; i < numberOfKeyAttributes; i++)
opfamOids[i] = get_opclass_family(classObjectId[i]);
@@ -1196,9 +1195,9 @@ DefineIndex(Oid relationId,
ereport(ERROR,
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
errmsg("cannot create unique index on partitioned table \"%s\"",
- RelationGetRelationName(rel)),+ relname),
errdetail("Table \"%s\" contains partitions that are foreign tables.",
- RelationGetRelationName(rel))));+ relname)));
table_close(childrel, lockmode);
continue;
@@ -1365,21 +1364,35 @@ DefineIndex(Oid relationId,
}
}
+ /*+ * CIC needs to mark a partitioned table as VALID, which itself+ * requires setting READY, which is unset for CIC (even though+ * it's meaningless for an index without storage).+ */+ if (concurrent)+ {+ if (ActiveSnapshotSet())+ PopActiveSnapshot();+ CommitTransactionCommand();+ StartTransactionCommand();+ index_set_state_flags(indexRelationId, INDEX_CREATE_SET_READY);++ CommandCounterIncrement();+ index_set_state_flags(indexRelationId, INDEX_CREATE_SET_VALID);+ }+
/*
* Indexes on partitioned tables are not themselves built, so we're
* done here.
*/
- table_close(rel, NoLock);
if (!OidIsValid(parentIndexId))
pgstat_progress_end_command();
return address;
}
+ table_close(rel, NoLock);
if (!concurrent)
{
- /* Close the heap and we're done, in the non-concurrent case */- table_close(rel, NoLock);-
/* If this is the top-level index, we're done. */
if (!OidIsValid(parentIndexId))
pgstat_progress_end_command();
@@ -1387,11 +1400,6 @@ DefineIndex(Oid relationId,
return address;
}
- /* save lockrelid and locktag for below, then close rel */- 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
* visible to other transactions before we start to build the index. That
diff --git a/src/test/regress/expected/indexing.out b/src/test/regress/expected/indexing.outindex f78865ef81..17fa77e317 100644--- a/src/test/regress/expected/indexing.out+++ b/src/test/regress/expected/indexing.out@@ -50,11 +50,21 @@ 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+\d idxpart1+ Table "public.idxpart1"+ Column | Type | Collation | Nullable | Default +--------+---------+-----------+----------+---------+ a | integer | | | + b | integer | | | + c | text | | | +Partition of: idxpart FOR VALUES FROM (0) TO (10)+Indexes:+ "idxpart1_a_idx" btree (a)+
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.sqlindex 35d159f41b..19a21eba9d 100644--- a/src/test/regress/sql/indexing.sql+++ b/src/test/regress/sql/indexing.sql@@ -29,10 +29,11 @@ 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);
+\d idxpart1
drop table idxpart;
-- Verify bugfix with query on indexed partitioned table with no partitions
--
2.17.0
v2-0002-Implement-CLUSTER-of-partitioned-table.patch (text/x-diff, 8.9K) download
From 44a55a91146a97239cf2a0af4aa6be2fb2d8300f Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryzbyj@telsasoft.com>
Date: Sun, 7 Jun 2020 16:58:42 -0500
Subject: [PATCH v2 2/3] Implement CLUSTER of partitioned table..
This requires either specification of a partitioned index on which to cluster,
or that an partitioned index was previously set clustered.
---
src/backend/commands/cluster.c | 139 +++++++++++++++++++-------
src/bin/psql/tab-complete.c | 1 +
src/test/regress/expected/cluster.out | 16 ++-
src/test/regress/sql/cluster.sql | 5 +-
4 files changed, 119 insertions(+), 42 deletions(-)
diff --git a/src/backend/commands/cluster.c b/src/backend/commands/cluster.cindex 04d12a7ece..7409c17b0b 100644--- a/src/backend/commands/cluster.c+++ b/src/backend/commands/cluster.c@@ -33,6 +33,7 @@
#include "catalog/namespace.h"
#include "catalog/objectaccess.h"
#include "catalog/pg_am.h"
+#include "catalog/pg_inherits.h"
#include "catalog/toasting.h"
#include "commands/cluster.h"
#include "commands/progress.h"
@@ -72,6 +73,9 @@ static void copy_table_data(Oid OIDNewHeap, Oid OIDOldHeap, Oid OIDOldIndex,
bool verbose, bool *pSwapToastByContent,
TransactionId *pFreezeXid, MultiXactId *pCutoffMulti);
static List *get_tables_to_cluster(MemoryContext cluster_context);
+static List *get_tables_to_cluster_partitioned(MemoryContext cluster_context,+ Oid indexOid);+static void cluster_multiple_rels(List *rvs, int options);
/*---------------------------------------------------------------------------
@@ -124,14 +128,6 @@ cluster(ClusterStmt *stmt, bool isTopLevel)
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
errmsg("cannot cluster temporary tables of other sessions")));
- /*- * Reject clustering a partitioned table.- */- if (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)- ereport(ERROR,- (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),- errmsg("cannot cluster a partitioned table")));-
if (stmt->indexname == NULL)
{
ListCell *index;
@@ -169,8 +165,37 @@ cluster(ClusterStmt *stmt, bool isTopLevel)
/* close relation, keep lock till commit */
table_close(rel, NoLock);
- /* Do the job. */- cluster_rel(tableOid, indexOid, stmt->options);+ if (rel->rd_rel->relkind != RELKIND_PARTITIONED_TABLE)+ {+ /* Do the job. */+ cluster_rel(tableOid, indexOid, stmt->options);+ } else {+ List *rvs;+ MemoryContext cluster_context;++ /* Check index directly since cluster_rel isn't called for partitioned table */+ check_index_is_clusterable(rel, indexOid, true, AccessExclusiveLock);++ /* Refuse to hold strong locks in a user transaction */+ PreventInTransactionBlock(isTopLevel, "CLUSTER");++ cluster_context = AllocSetContextCreate(PortalContext,+ "Cluster",+ ALLOCSET_DEFAULT_SIZES);++ rvs = get_tables_to_cluster_partitioned(cluster_context, indexOid);+ cluster_multiple_rels(rvs, stmt->options);++ /* Start a new transaction for the cleanup work. */+ StartTransactionCommand();++ rel = table_open(tableOid, ShareUpdateExclusiveLock);+ mark_index_clustered(rel, indexOid, true);+ table_close(rel, NoLock);++ /* Clean up working storage */+ MemoryContextDelete(cluster_context);+ }
}
else
{
@@ -180,7 +205,6 @@ cluster(ClusterStmt *stmt, bool isTopLevel)
*/
MemoryContext cluster_context;
List *rvs;
- ListCell *rv;
/*
* We cannot run this form of CLUSTER inside a user transaction block;
@@ -204,25 +228,7 @@ cluster(ClusterStmt *stmt, bool isTopLevel)
*/
rvs = get_tables_to_cluster(cluster_context);
- /* Commit to get out of starting transaction */- PopActiveSnapshot();- CommitTransactionCommand();-- /* Ok, now that we've got them all, cluster them one by one */- foreach(rv, rvs)- {- RelToCluster *rvtc = (RelToCluster *) lfirst(rv);-- /* Start a new transaction for each relation. */- StartTransactionCommand();- /* functions in indexes may want a snapshot set */- PushActiveSnapshot(GetTransactionSnapshot());- /* Do the job. */- cluster_rel(rvtc->tableOid, rvtc->indexOid,- stmt->options | CLUOPT_RECHECK);- PopActiveSnapshot();- CommitTransactionCommand();- }+ cluster_multiple_rels(rvs, stmt->options);
/* Start a new transaction for the cleanup work. */
StartTransactionCommand();
@@ -483,12 +489,6 @@ mark_index_clustered(Relation rel, Oid indexOid, bool is_internal)
Relation pg_index;
ListCell *index;
- /* Disallow applying to a partitioned table */- if (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)- ereport(ERROR,- (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),- errmsg("cannot mark index clustered in partitioned table")));-
/*
* If the index is already marked clustered, no need to do anything.
*/
@@ -1557,3 +1557,70 @@ get_tables_to_cluster(MemoryContext cluster_context)
return rvs;
}
++/*+ * Return a List of tables and associated index, where each index is a+ * partition of the given index+ */+static List *+get_tables_to_cluster_partitioned(MemoryContext cluster_context, Oid indexOid)+{+ List *inhoids;+ ListCell *lc;+ List *rvs = NIL;++ MemoryContext old_context = MemoryContextSwitchTo(cluster_context);++ inhoids = find_all_inheritors(indexOid, NoLock, NULL);+ foreach(lc, inhoids)+ {+ Oid indexrelid = lfirst_oid(lc);+ Oid relid = IndexGetRelation(indexrelid, false);+ RelToCluster *rvtc;++ /*+ * We have a full list of direct and indirect children, so skip+ * partitioned tables and just handle their children.+ */+ if (get_rel_relkind(relid) == RELKIND_PARTITIONED_TABLE)+ continue;++ rvtc = (RelToCluster *) palloc(sizeof(RelToCluster));+ rvtc->tableOid = relid;+ rvtc->indexOid = indexrelid;+ rvs = lappend(rvs, rvtc);+ }++ MemoryContextSwitchTo(old_context);+ return rvs;+}++/* Cluster each relation in a separate transaction */+static void+cluster_multiple_rels(List *rvs, int options)+{+ ListCell *lc;++ /* Commit to get out of starting transaction */+ PopActiveSnapshot();+ CommitTransactionCommand();++ /* Ok, now that we've got them all, cluster them one by one */+ foreach(lc, rvs)+ {+ RelToCluster *rvtc = (RelToCluster *) lfirst(lc);++ /* Start a new transaction for each relation. */+ StartTransactionCommand();++ /* functions in indexes may want a snapshot set */+ PushActiveSnapshot(GetTransactionSnapshot());++ /* Do the job. */+ cluster_rel(rvtc->tableOid, rvtc->indexOid,+ options | CLUOPT_RECHECK);++ PopActiveSnapshot();+ CommitTransactionCommand();+ }+}diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.cindex eb018854a5..d6a7ef2c30 100644--- a/src/bin/psql/tab-complete.c+++ b/src/bin/psql/tab-complete.c@@ -578,6 +578,7 @@ static const SchemaQuery Query_for_list_of_vacuumables = {
.catname = "pg_catalog.pg_class c",
.selcondition =
"c.relkind IN (" CppAsString2(RELKIND_RELATION) ", "
+ CppAsString2(RELKIND_PARTITIONED_TABLE) ", "
CppAsString2(RELKIND_MATVIEW) ")",
.viscondition = "pg_catalog.pg_table_is_visible(c.oid)",
.namespace = "c.relnamespace",
diff --git a/src/test/regress/expected/cluster.out b/src/test/regress/expected/cluster.outindex bdae8fe00c..161c09ee42 100644--- a/src/test/regress/expected/cluster.out+++ b/src/test/regress/expected/cluster.out@@ -439,13 +439,21 @@ select * from clstr_temp;
drop table clstr_temp;
RESET SESSION AUTHORIZATION;
--- Check that partitioned tables cannot be clustered+-- Check that partitioned tables can be clustered
CREATE TABLE clstrpart (a int) PARTITION BY RANGE (a);
CREATE INDEX clstrpart_idx ON clstrpart (a);
-ALTER TABLE clstrpart CLUSTER ON clstrpart_idx;-ERROR: cannot mark index clustered in partitioned table+ALTER TABLE clstrpart SET WITHOUT CLUSTER;
CLUSTER clstrpart USING clstrpart_idx;
-ERROR: cannot cluster a partitioned table+\d clstrpart+ Partitioned table "public.clstrpart"+ Column | Type | Collation | Nullable | Default +--------+---------+-----------+----------+---------+ a | integer | | | +Partition key: RANGE (a)+Indexes:+ "clstrpart_idx" btree (a) CLUSTER+Number of partitions: 0+
DROP TABLE clstrpart;
-- Test CLUSTER with external tuplesorting
create table clstr_4 as select * from tenk1;
diff --git a/src/test/regress/sql/cluster.sql b/src/test/regress/sql/cluster.sqlindex 188183647c..db3c271706 100644--- a/src/test/regress/sql/cluster.sql+++ b/src/test/regress/sql/cluster.sql@@ -196,11 +196,12 @@ drop table clstr_temp;
RESET SESSION AUTHORIZATION;
--- Check that partitioned tables cannot be clustered+-- Check that partitioned tables can be clustered
CREATE TABLE clstrpart (a int) PARTITION BY RANGE (a);
CREATE INDEX clstrpart_idx ON clstrpart (a);
-ALTER TABLE clstrpart CLUSTER ON clstrpart_idx;+ALTER TABLE clstrpart SET WITHOUT CLUSTER;
CLUSTER clstrpart USING clstrpart_idx;
+\d clstrpart
DROP TABLE clstrpart;
-- Test CLUSTER with external tuplesorting
--
2.17.0
v2-0003-Implement-REINDEX-of-partitioned-tables-indexes.patch (text/x-diff, 12.8K) download
From 3a7e1307e27773a6401055ef56750c1264aa8613 Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryzbyj@telsasoft.com>
Date: Sun, 7 Jun 2020 15:59:54 -0500
Subject: [PATCH v2 3/3] Implement REINDEX of partitioned tables/indexes
---
doc/src/sgml/ref/reindex.sgml | 5 -
src/backend/catalog/index.c | 7 +-
src/backend/commands/indexcmds.c | 119 +++++++++++++--------
src/backend/tcop/utility.c | 6 +-
src/include/commands/defrem.h | 6 +-
src/test/regress/expected/create_index.out | 10 +-
src/test/regress/sql/create_index.sql | 4 +-
7 files changed, 89 insertions(+), 68 deletions(-)
diff --git a/doc/src/sgml/ref/reindex.sgml b/doc/src/sgml/ref/reindex.sgmlindex aac5d5be23..e2e32b3ba0 100644--- a/doc/src/sgml/ref/reindex.sgml+++ b/doc/src/sgml/ref/reindex.sgml@@ -258,11 +258,6 @@ REINDEX [ ( <replaceable class="parameter">option</replaceable> [, ...] ) ] { IN
can always reindex anything.
</para>
- <para>- Reindexing partitioned tables or partitioned indexes is not supported.- Each individual partition can be reindexed separately instead.- </para>-
<refsect2 id="sql-reindex-concurrently" xreflabel="Rebuilding Indexes Concurrently">
<title>Rebuilding Indexes Concurrently</title>
diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.cindex cdc01c49c9..b82d122353 100644--- a/src/backend/catalog/index.c+++ b/src/backend/catalog/index.c@@ -3688,12 +3688,7 @@ reindex_relation(Oid relid, int flags, int options)
*/
rel = table_open(relid, ShareLock);
- /*- * This may be useful when implemented someday; but that day is not today.- * For now, avoid erroring out when called in a multi-table context- * (REINDEX SCHEMA) and happen to come across a partitioned table. The- * partitions may be reindexed on their own anyway.- */+ /* Avoid erroring out */
if (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)
{
ereport(WARNING,
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.cindex 344cabaf52..66be1c9c8c 100644--- a/src/backend/commands/indexcmds.c+++ b/src/backend/commands/indexcmds.c@@ -88,7 +88,8 @@ static List *ChooseIndexColumnNames(List *indexElems);
static void RangeVarCallbackForReindexIndex(const RangeVar *relation,
Oid relId, Oid oldRelId, void *arg);
static bool ReindexRelationConcurrently(Oid relationOid, int options);
-static void ReindexPartitionedIndex(Relation parentIdx);+static void ReindexPartitionedRel(Oid reloid, int options, bool concurrent,+ bool isTopLevel);
static void update_relispartition(Oid relationId, bool newval);
static bool CompareOpclassOptions(Datum *opts1, Datum *opts2, int natts);
@@ -2428,11 +2429,10 @@ ChooseIndexColumnNames(List *indexElems)
* Recreate a specific index.
*/
void
-ReindexIndex(RangeVar *indexRelation, int options, bool concurrent)+ReindexIndex(RangeVar *indexRelation, int options, bool concurrent, bool isTopLevel)
{
struct ReindexIndexCallbackState state;
Oid indOid;
- Relation irel;
char persistence;
/*
@@ -2453,22 +2453,10 @@ ReindexIndex(RangeVar *indexRelation, int options, bool concurrent)
RangeVarCallbackForReindexIndex,
&state);
- /*- * Obtain the current persistence of the existing index. We already hold- * lock on the index.- */- irel = index_open(indOid, NoLock);-- if (irel->rd_rel->relkind == RELKIND_PARTITIONED_INDEX)- {- ReindexPartitionedIndex(irel);- return;- }-- persistence = irel->rd_rel->relpersistence;- index_close(irel, NoLock);-- if (concurrent && persistence != RELPERSISTENCE_TEMP)+ persistence = get_rel_persistence(indOid);+ if (get_rel_relkind(indOid) == RELKIND_PARTITIONED_INDEX)+ ReindexPartitionedRel(indOid, options, concurrent, isTopLevel);+ else if (concurrent && persistence != RELPERSISTENCE_TEMP)
ReindexRelationConcurrently(indOid, options);
else
reindex_index(indOid, false, persistence,
@@ -2550,7 +2538,7 @@ RangeVarCallbackForReindexIndex(const RangeVar *relation,
* Recreate all indexes of a table (and of its toast table, if any)
*/
Oid
-ReindexTable(RangeVar *relation, int options, bool concurrent)+ReindexTable(RangeVar *relation, int options, bool concurrent, bool isTopLevel)
{
Oid heapOid;
bool result;
@@ -2568,7 +2556,9 @@ ReindexTable(RangeVar *relation, int options, bool concurrent)
0,
RangeVarCallbackOwnsTable, NULL);
- if (concurrent && get_rel_persistence(heapOid) != RELPERSISTENCE_TEMP)+ if (get_rel_relkind(heapOid) == RELKIND_PARTITIONED_TABLE)+ ReindexPartitionedRel(heapOid, options, concurrent, isTopLevel);+ else if (concurrent && get_rel_persistence(heapOid) != RELPERSISTENCE_TEMP)
{
result = ReindexRelationConcurrently(heapOid, options);
@@ -2696,11 +2686,8 @@ ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind,
* Only regular tables and matviews can have indexes, so ignore any
* other kind of relation.
*
- * It is tempting to also consider partitioned tables here, but that- * has the problem that if the children are in the same schema, they- * would be processed twice. Maybe we could have a separate list of- * partitioned tables, and expand that afterwards into relids,- * ignoring any duplicates.+ * Partitioned tables/indexes are skipped but matching leaf+ * partitions are processed.
*/
if (classtuple->relkind != RELKIND_RELATION &&
classtuple->relkind != RELKIND_MATVIEW)
@@ -2813,8 +2800,7 @@ ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind,
* view. For tables and materialized views, all its indexes will be rebuilt,
* excluding invalid indexes and any indexes used in exclusion constraints,
* but including its associated toast table indexes. For indexes, the index
- * itself will be rebuilt. If 'relationOid' belongs to a partitioned table- * then we issue a warning to mention these are not yet supported.+ * itself will be rebuilt.
*
* The locks taken on parent tables and involved indexes are kept until the
* transaction is committed, at which point a session lock is taken on each
@@ -3018,13 +3004,6 @@ ReindexRelationConcurrently(Oid relationOid, int options)
MemoryContextSwitchTo(oldcontext);
break;
}
- case RELKIND_PARTITIONED_TABLE:- /* see reindex_relation() */- ereport(WARNING,- (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),- errmsg("REINDEX of partitioned tables is not yet implemented, skipping \"%s\"",- get_rel_name(relationOid))));- return false;
default:
/* Return error if type of relation is not supported */
ereport(ERROR,
@@ -3486,17 +3465,71 @@ ReindexRelationConcurrently(Oid relationOid, int options)
}
/*
- * ReindexPartitionedIndex- * Reindex each child of the given partitioned index.- *- * Not yet implemented.+ * ReindexPartitionedRel+ * Reindex each child of the given partitioned relation.
*/
static void
-ReindexPartitionedIndex(Relation parentIdx)+ReindexPartitionedRel(Oid reloid, int options, bool concurrent, bool isTopLevel)
{
- ereport(ERROR,- (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),- errmsg("REINDEX is not yet implemented for partitioned indexes")));+ MemoryContext oldcontext, reindex_context;+ List *inhoids;+ ListCell *lc;++ /*+ * This cannot run inside a user transaction block; if+ * we were inside a transaction, then its commit- and+ * start-transaction-command calls would not have the+ * intended effect!+ */+ PreventInTransactionBlock(isTopLevel,+ "REINDEX of partitioned relations"); // XXX++ /*+ * Create list of children in longlived context, since we process each+ * child in a separate transaction+ */+ reindex_context = AllocSetContextCreate(PortalContext, "Reindex",+ ALLOCSET_DEFAULT_SIZES);+ oldcontext = MemoryContextSwitchTo(reindex_context);+ inhoids = find_all_inheritors(reloid, NoLock, NULL);+ MemoryContextSwitchTo(oldcontext);++ foreach (lc, inhoids)+ {+ Oid inhrelid = lfirst_oid(lc);++ PopActiveSnapshot();+ CommitTransactionCommand();++ StartTransactionCommand();+ /* functions in indexes may want a snapshot set */+ PushActiveSnapshot(GetTransactionSnapshot());++ switch (get_rel_relkind(inhrelid))+ {+ case RELKIND_PARTITIONED_INDEX:+ case RELKIND_PARTITIONED_TABLE:+ /*+ * We have a full list of direct and indirect children, so skip+ * partitioned relations and just handle their children.+ */+ continue;+ case RELKIND_INDEX:+ reindex_index(inhrelid, false, get_rel_persistence(inhrelid),+ options | REINDEXOPT_REPORT_PROGRESS);+ break;+ case RELKIND_RELATION:+ (void) reindex_relation(inhrelid,+ REINDEX_REL_PROCESS_TOAST |+ REINDEX_REL_CHECK_CONSTRAINTS,+ options | REINDEXOPT_REPORT_PROGRESS);+ break;+ }+ }++ PopActiveSnapshot();+ CommitTransactionCommand();+ StartTransactionCommand();
}
/*
diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.cindex 97cbaa3072..e72bbbb425 100644--- a/src/backend/tcop/utility.c+++ b/src/backend/tcop/utility.c@@ -926,10 +926,12 @@ standard_ProcessUtility(PlannedStmt *pstmt,
switch (stmt->kind)
{
case REINDEX_OBJECT_INDEX:
- ReindexIndex(stmt->relation, stmt->options, stmt->concurrent);+ ReindexIndex(stmt->relation, stmt->options,+ stmt->concurrent, isTopLevel);
break;
case REINDEX_OBJECT_TABLE:
- ReindexTable(stmt->relation, stmt->options, stmt->concurrent);+ ReindexTable(stmt->relation, stmt->options,+ stmt->concurrent, isTopLevel);
break;
case REINDEX_OBJECT_SCHEMA:
case REINDEX_OBJECT_SYSTEM:
diff --git a/src/include/commands/defrem.h b/src/include/commands/defrem.hindex c77c9a6ed5..eea6b01791 100644--- a/src/include/commands/defrem.h+++ b/src/include/commands/defrem.h@@ -34,8 +34,10 @@ extern ObjectAddress DefineIndex(Oid relationId,
bool check_not_in_use,
bool skip_build,
bool quiet);
-extern void ReindexIndex(RangeVar *indexRelation, int options, bool concurrent);-extern Oid ReindexTable(RangeVar *relation, int options, bool concurrent);+extern void ReindexIndex(RangeVar *indexRelation, int options, bool concurrent,+ bool isTopLevel);+extern Oid ReindexTable(RangeVar *relation, int options, bool concurrent,+ bool isTopLevel);
extern void ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind,
int options, bool concurrent);
extern char *makeObjectName(const char *name1, const char *name2,
diff --git a/src/test/regress/expected/create_index.out b/src/test/regress/expected/create_index.outindex e3e6634d7e..8ad103d80b 100644--- a/src/test/regress/expected/create_index.out+++ b/src/test/regress/expected/create_index.out@@ -2196,18 +2196,12 @@ SELECT relid, parentrelid, level FROM pg_partition_tree('concur_reindex_part_ind
concur_reindex_part_index_0_2 | concur_reindex_part_index_0 | 2
(5 rows)
--- REINDEX fails for partitioned indexes+-- REINDEX for partitioned indexes
REINDEX INDEX concur_reindex_part_index_10;
-ERROR: REINDEX is not yet implemented for partitioned indexes
REINDEX INDEX CONCURRENTLY concur_reindex_part_index_10;
-ERROR: REINDEX is not yet implemented for partitioned indexes--- REINDEX is a no-op for partitioned tables+-- REINDEX for partitioned tables
REINDEX TABLE concur_reindex_part_10;
-WARNING: REINDEX of partitioned tables is not yet implemented, skipping "concur_reindex_part_10"-NOTICE: table "concur_reindex_part_10" has no indexes to reindex
REINDEX TABLE CONCURRENTLY concur_reindex_part_10;
-WARNING: REINDEX of partitioned tables is not yet implemented, skipping "concur_reindex_part_10"-NOTICE: table "concur_reindex_part_10" has no indexes that can be reindexed concurrently
SELECT relid, parentrelid, level FROM pg_partition_tree('concur_reindex_part_index')
ORDER BY relid, level;
relid | parentrelid | level
diff --git a/src/test/regress/sql/create_index.sql b/src/test/regress/sql/create_index.sqlindex f3667bacdc..cd7f13b307 100644--- a/src/test/regress/sql/create_index.sql+++ b/src/test/regress/sql/create_index.sql@@ -903,10 +903,10 @@ CREATE INDEX concur_reindex_part_index_0_2 ON ONLY concur_reindex_part_0_2 (c1);
ALTER INDEX concur_reindex_part_index_0 ATTACH PARTITION concur_reindex_part_index_0_2;
SELECT relid, parentrelid, level FROM pg_partition_tree('concur_reindex_part_index')
ORDER BY relid, level;
--- REINDEX fails for partitioned indexes+-- REINDEX for partitioned indexes
REINDEX INDEX concur_reindex_part_index_10;
REINDEX INDEX CONCURRENTLY concur_reindex_part_index_10;
--- REINDEX is a no-op for partitioned tables+-- REINDEX for partitioned tables
REINDEX TABLE concur_reindex_part_10;
REINDEX TABLE CONCURRENTLY concur_reindex_part_10;
SELECT relid, parentrelid, level FROM pg_partition_tree('concur_reindex_part_index')
--
2.17.0
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: pryzby@telsasoft.com, adger.lj@alibaba-inc.com, pgsql-hackers@lists.postgresql.org, wenjing.zwj@alibaba-inc.com, alvherre@2ndquadrant.com
Subject: Re: how to create index concurrently on partitioned table
In-Reply-To: <20200611153502.GT14879@telsasoft.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox