|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|
|Views:||Raw Message | Whole Thread | Download mbox | Resend email|
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:
> + 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.
>> - 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.
|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 ..."|