Re: 回复:how to create index concurrently on partitioned table

From: Michael Paquier <michael(at)paquier(dot)xyz>
To: Justin Pryzby <pryzby(at)telsasoft(dot)com>
Cc: 李杰(慎追) <adger(dot)lj(at)alibaba-inc(dot)com>, pgsql-hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, 曾文旌(义从) <wenjing(dot)zwj(at)alibaba-inc(dot)com>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>
Subject: Re: 回复:how to create index concurrently on partitioned table
Date: 2020-08-12 13:37:08
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On Wed, Aug 12, 2020 at 12:28:20AM -0500, Justin Pryzby wrote:
> On Wed, Aug 12, 2020 at 01:54:38PM +0900, Michael Paquier wrote:
>> +++ b/src/backend/catalog/index.c
>> @@ -3661,20 +3662,12 @@ reindex_relation(Oid relid, int flags, int options)
>> + elog(ERROR, "unsupported relation kind for relation \"%s\"",
>> + RelationGetRelationName(rel));
> I guess it should show the relkind(%c) in the message, like these:

Sure, but I don't see much the point in adding the relkind here
knowing that we know which one it is.

> ISTM reindex_index is missing that, too:
> 8b08f7d4820fd7a8ef6152a9dd8c6e3cb01e5f99
> + if (iRel->rd_rel->relkind == RELKIND_PARTITIONED_INDEX)
> + elog(ERROR, "unsupported relation kind for index \"%s\"",
> + RelationGetRelationName(iRel));

The error string does not follow the usual project style either, so I
have updated both.

>> <para>
>> - Reindexing partitioned tables or partitioned indexes is not supported.
>> - Each individual partition can be reindexed separately instead.
>> + Reindexing partitioned indexes or partitioned tables is supported
>> + with respectively <command>REINDEX INDEX</command> or
>> + <command>REINDEX TABLE</command>.
> Should say "..with REINDEX INDEX or REINDEX TABLE, respectively".
>> + Each partition of the partitioned
>> + relation defined is rebuilt in its own transaction.
> => Each partition of the specified partitioned relation is reindexed in a
> separate transaction.

Thanks, good idea.

I have been able to work more on this patch today, and finally added
an error context for the transaction block check, as that's cleaner.
In my manual tests, I have also bumped into a case that failed with
the original patch (where there were no session locks), and created
an isolation test based on it: drop of a partition leaf concurrently
to REINDEX done on the parent. One last thing I have spotted is that
we failed to discard properly foreign tables defined as leaves of a
partition tree, causing a reindex to fail, so reindex_partitions()
ought to just use RELKIND_HAS_STORAGE() to do its filtering work. I
am leaving this patch alone for a couple of days now, and I'll try to
come back to it after and potentially commit it. The attached has
been indented by the way.

Attachment Content-Type Size
reindex-part-v7.patch text/x-diff 32.9 KB

In response to


Browse pgsql-hackers by date

  From Date Subject
Next Message Bharath Rupireddy 2020-08-12 15:32:35 Parallel query hangs after a smart shutdown is issued
Previous Message Ashutosh Sharma 2020-08-12 13:26:52 Re: recovering from "found xmin ... from before relfrozenxid ..."