public inbox for [email protected]
help / color / mirror / Atom feedCREATE INDEX CONCURRENTLY on partitioned index
22+ messages / 7 participants
[nested] [flat]
* CREATE INDEX CONCURRENTLY on partitioned index
@ 2020-10-31 06:31 Justin Pryzby <[email protected]>
2020-11-29 20:22 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Justin Pryzby <[email protected]>
0 siblings, 1 reply; 22+ messages in thread
From: Justin Pryzby @ 2020-10-31 06:31 UTC (permalink / raw)
To: Michael Paquier <[email protected]>; +Cc: 李杰(慎追) <[email protected]>; pgsql-hackers <[email protected]>; 曾文旌(义从) <[email protected]>; Alvaro Herrera <[email protected]>
Forking this thread, since the existing CFs have been closed.
https://www.postgresql.org/message-id/flat/20200914143102.GX18552%40telsasoft.com#58b1056488451f8594...
The strategy is to create catalog entries for all tables with indisvalid=false,
and then process them like REINDEX CONCURRENTLY. If it's interrupted, it
leaves INVALID indexes, which can be cleaned up with DROP or REINDEX, same as
CIC on a plain table.
On Sat, Aug 08, 2020 at 01:37:44AM -0500, Justin Pryzby wrote:
> On Mon, Jun 15, 2020 at 09:37:42PM +0900, Michael Paquier wrote:
> > On Mon, Jun 15, 2020 at 08:15:05PM +0800, 李杰(慎追) wrote:
> > > As shown above, an error occurred while creating an index in the second partition.
> > > It can be clearly seen that the index of the partitioned table is invalid
> > > and the index of the first partition is normal, the second partition is invalid,
> > > and the Third Partition index does not exist at all.
> >
> > That's a problem. I really think that we should make the steps of the
> > concurrent operation consistent across all relations, meaning that all
> > the indexes should be created as invalid for all the parts of the
> > partition tree, including partitioned tables as well as their
> > partitions, in the same transaction. Then a second new transaction
> > gets used for the index build, followed by a third one for the
> > validation that switches the indexes to become valid.
>
> Note that the mentioned problem wasn't serious: there was missing index on
> child table, therefor the parent index was invalid, as intended. However I
> agree that it's not nice that the command can fail so easily and leave behind
> some indexes created successfully and some failed some not created at all.
>
> But I took your advice initially creating invalid inds.
...
> That gave me the idea to layer CIC on top of Reindex, since I think it does
> exactly what's needed.
On Sat, Sep 26, 2020 at 02:56:55PM -0500, Justin Pryzby wrote:
> On Thu, Sep 24, 2020 at 05:11:03PM +0900, Michael Paquier wrote:
> > It would be good also to check if
> > we have a partition index tree that maps partially with a partition
> > table tree (aka no all table partitions have a partition index), where
> > these don't get clustered because there is no index to work on.
>
> This should not happen, since a incomplete partitioned index is "invalid".
--
Justin
^ permalink raw reply [nested|flat] 22+ messages in thread
* Re: CREATE INDEX CONCURRENTLY on partitioned index
2020-10-31 06:31 CREATE INDEX CONCURRENTLY on partitioned index Justin Pryzby <[email protected]>
@ 2020-11-29 20:22 ` Justin Pryzby <[email protected]>
2021-01-28 12:51 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Masahiko Sawada <[email protected]>
0 siblings, 1 reply; 22+ messages in thread
From: Justin Pryzby @ 2020-11-29 20:22 UTC (permalink / raw)
To: Michael Paquier <[email protected]>; +Cc: 李杰(慎追) <[email protected]>; pgsql-hackers <[email protected]>; 曾文旌(义从) <[email protected]>; Alvaro Herrera <[email protected]>
On Sat, Oct 31, 2020 at 01:31:17AM -0500, Justin Pryzby wrote:
> Forking this thread, since the existing CFs have been closed.
> https://www.postgresql.org/message-id/flat/20200914143102.GX18552%40telsasoft.com#58b1056488451f8594...
>
> The strategy is to create catalog entries for all tables with indisvalid=false,
> and then process them like REINDEX CONCURRENTLY. If it's interrupted, it
> leaves INVALID indexes, which can be cleaned up with DROP or REINDEX, same as
> CIC on a plain table.
>
> On Sat, Aug 08, 2020 at 01:37:44AM -0500, Justin Pryzby wrote:
> > On Mon, Jun 15, 2020 at 09:37:42PM +0900, Michael Paquier wrote:
> > > On Mon, Jun 15, 2020 at 08:15:05PM +0800, 李杰(慎追) wrote:
> > > > As shown above, an error occurred while creating an index in the second partition.
> > > > It can be clearly seen that the index of the partitioned table is invalid
> > > > and the index of the first partition is normal, the second partition is invalid,
> > > > and the Third Partition index does not exist at all.
> > >
> > > That's a problem. I really think that we should make the steps of the
> > > concurrent operation consistent across all relations, meaning that all
> > > the indexes should be created as invalid for all the parts of the
> > > partition tree, including partitioned tables as well as their
> > > partitions, in the same transaction. Then a second new transaction
> > > gets used for the index build, followed by a third one for the
> > > validation that switches the indexes to become valid.
> >
> > Note that the mentioned problem wasn't serious: there was missing index on
> > child table, therefor the parent index was invalid, as intended. However I
> > agree that it's not nice that the command can fail so easily and leave behind
> > some indexes created successfully and some failed some not created at all.
> >
> > But I took your advice initially creating invalid inds.
> ...
> > That gave me the idea to layer CIC on top of Reindex, since I think it does
> > exactly what's needed.
>
> On Sat, Sep 26, 2020 at 02:56:55PM -0500, Justin Pryzby wrote:
> > On Thu, Sep 24, 2020 at 05:11:03PM +0900, Michael Paquier wrote:
> > > It would be good also to check if
> > > we have a partition index tree that maps partially with a partition
> > > table tree (aka no all table partitions have a partition index), where
> > > these don't get clustered because there is no index to work on.
> >
> > This should not happen, since a incomplete partitioned index is "invalid".
@cfbot: rebased over recent changes to indexcmds.c
--
Justin
^ permalink raw reply [nested|flat] 22+ messages in thread
* Re: CREATE INDEX CONCURRENTLY on partitioned index
2020-10-31 06:31 CREATE INDEX CONCURRENTLY on partitioned index Justin Pryzby <[email protected]>
2020-11-29 20:22 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Justin Pryzby <[email protected]>
@ 2021-01-28 12:51 ` Masahiko Sawada <[email protected]>
2021-01-28 14:30 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Justin Pryzby <[email protected]>
0 siblings, 1 reply; 22+ messages in thread
From: Masahiko Sawada @ 2021-01-28 12:51 UTC (permalink / raw)
To: Justin Pryzby <[email protected]>; +Cc: Michael Paquier <[email protected]>; 李杰(慎追) <[email protected]>; pgsql-hackers <[email protected]>; 曾文旌(义从) <[email protected]>; Alvaro Herrera <[email protected]>
On Mon, Nov 30, 2020 at 5:22 AM Justin Pryzby <[email protected]> wrote:
>
> On Sat, Oct 31, 2020 at 01:31:17AM -0500, Justin Pryzby wrote:
> > Forking this thread, since the existing CFs have been closed.
> > https://www.postgresql.org/message-id/flat/20200914143102.GX18552%40telsasoft.com#58b1056488451f8594...
> >
> > The strategy is to create catalog entries for all tables with indisvalid=false,
> > and then process them like REINDEX CONCURRENTLY. If it's interrupted, it
> > leaves INVALID indexes, which can be cleaned up with DROP or REINDEX, same as
> > CIC on a plain table.
> >
> > On Sat, Aug 08, 2020 at 01:37:44AM -0500, Justin Pryzby wrote:
> > > On Mon, Jun 15, 2020 at 09:37:42PM +0900, Michael Paquier wrote:
> > > > On Mon, Jun 15, 2020 at 08:15:05PM +0800, 李杰(慎追) wrote:
> > > > > As shown above, an error occurred while creating an index in the second partition.
> > > > > It can be clearly seen that the index of the partitioned table is invalid
> > > > > and the index of the first partition is normal, the second partition is invalid,
> > > > > and the Third Partition index does not exist at all.
> > > >
> > > > That's a problem. I really think that we should make the steps of the
> > > > concurrent operation consistent across all relations, meaning that all
> > > > the indexes should be created as invalid for all the parts of the
> > > > partition tree, including partitioned tables as well as their
> > > > partitions, in the same transaction. Then a second new transaction
> > > > gets used for the index build, followed by a third one for the
> > > > validation that switches the indexes to become valid.
> > >
> > > Note that the mentioned problem wasn't serious: there was missing index on
> > > child table, therefor the parent index was invalid, as intended. However I
> > > agree that it's not nice that the command can fail so easily and leave behind
> > > some indexes created successfully and some failed some not created at all.
> > >
> > > But I took your advice initially creating invalid inds.
> > ...
> > > That gave me the idea to layer CIC on top of Reindex, since I think it does
> > > exactly what's needed.
> >
> > On Sat, Sep 26, 2020 at 02:56:55PM -0500, Justin Pryzby wrote:
> > > On Thu, Sep 24, 2020 at 05:11:03PM +0900, Michael Paquier wrote:
> > > > It would be good also to check if
> > > > we have a partition index tree that maps partially with a partition
> > > > table tree (aka no all table partitions have a partition index), where
> > > > these don't get clustered because there is no index to work on.
> > >
> > > This should not happen, since a incomplete partitioned index is "invalid".
>
> @cfbot: rebased over recent changes to indexcmds.c
Status update for a commitfest entry.
This patch has not been updated and "Waiting on Author" status since
Nov 30. Are you still planning to work on this, Justin? If no, I'm
going to set this entry to "Returned with Feedback" barring
objections.
Regards,
--
Masahiko Sawada
EDB: https://www.enterprisedb.com/
^ permalink raw reply [nested|flat] 22+ messages in thread
* Re: CREATE INDEX CONCURRENTLY on partitioned index
2020-10-31 06:31 CREATE INDEX CONCURRENTLY on partitioned index Justin Pryzby <[email protected]>
2020-11-29 20:22 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Justin Pryzby <[email protected]>
2021-01-28 12:51 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Masahiko Sawada <[email protected]>
@ 2021-01-28 14:30 ` Justin Pryzby <[email protected]>
2021-02-15 19:06 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Anastasia Lubennikova <[email protected]>
2021-02-15 19:07 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Anastasia Lubennikova <[email protected]>
0 siblings, 2 replies; 22+ messages in thread
From: Justin Pryzby @ 2021-01-28 14:30 UTC (permalink / raw)
To: Masahiko Sawada <[email protected]>; +Cc: Michael Paquier <[email protected]>; 李杰(慎追) <[email protected]>; pgsql-hackers <[email protected]>; 曾文旌(义从) <[email protected]>; Alvaro Herrera <[email protected]>
On Thu, Jan 28, 2021 at 09:51:51PM +0900, Masahiko Sawada wrote:
> On Mon, Nov 30, 2020 at 5:22 AM Justin Pryzby <[email protected]> wrote:
> > On Sat, Oct 31, 2020 at 01:31:17AM -0500, Justin Pryzby wrote:
> > > Forking this thread, since the existing CFs have been closed.
> > > https://www.postgresql.org/message-id/flat/20200914143102.GX18552%40telsasoft.com#58b1056488451f8594...
> > >
> > > The strategy is to create catalog entries for all tables with indisvalid=false,
> > > and then process them like REINDEX CONCURRENTLY. If it's interrupted, it
> > > leaves INVALID indexes, which can be cleaned up with DROP or REINDEX, same as
> > > CIC on a plain table.
> > >
> > > On Sat, Aug 08, 2020 at 01:37:44AM -0500, Justin Pryzby wrote:
> > > > On Mon, Jun 15, 2020 at 09:37:42PM +0900, Michael Paquier wrote:
> > > > Note that the mentioned problem wasn't serious: there was missing index on
> > > > child table, therefor the parent index was invalid, as intended. However I
> > > > agree that it's not nice that the command can fail so easily and leave behind
> > > > some indexes created successfully and some failed some not created at all.
> > > >
> > > > But I took your advice initially creating invalid inds.
> > > ...
> > > > That gave me the idea to layer CIC on top of Reindex, since I think it does
> > > > exactly what's needed.
> > >
> > > On Sat, Sep 26, 2020 at 02:56:55PM -0500, Justin Pryzby wrote:
> > > > On Thu, Sep 24, 2020 at 05:11:03PM +0900, Michael Paquier wrote:
> > > > > It would be good also to check if
> > > > > we have a partition index tree that maps partially with a partition
> > > > > table tree (aka no all table partitions have a partition index), where
> > > > > these don't get clustered because there is no index to work on.
> > > >
> > > > This should not happen, since a incomplete partitioned index is "invalid".
> >
> > @cfbot: rebased over recent changes to indexcmds.c
>
> Status update for a commitfest entry.
>
> This patch has not been updated and "Waiting on Author" status since
> Nov 30. Are you still planning to work on this, Justin? If no, I'm
> going to set this entry to "Returned with Feedback" barring
> objections.
I had been waiting to rebase since there hasn't been any review comments and I
expected additional, future conflicts.
--
Justin
^ permalink raw reply [nested|flat] 22+ messages in thread
* Re: CREATE INDEX CONCURRENTLY on partitioned index
2020-10-31 06:31 CREATE INDEX CONCURRENTLY on partitioned index Justin Pryzby <[email protected]>
2020-11-29 20:22 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Justin Pryzby <[email protected]>
2021-01-28 12:51 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Masahiko Sawada <[email protected]>
2021-01-28 14:30 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Justin Pryzby <[email protected]>
@ 2021-02-15 19:06 ` Anastasia Lubennikova <[email protected]>
2021-02-15 19:33 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Justin Pryzby <[email protected]>
1 sibling, 1 reply; 22+ messages in thread
From: Anastasia Lubennikova @ 2021-02-15 19:06 UTC (permalink / raw)
To: Justin Pryzby <[email protected]>; Masahiko Sawada <[email protected]>; +Cc: Michael Paquier <[email protected]>; 李杰(慎追) <[email protected]>; pgsql-hackers <[email protected]>; 曾文旌(义从) <[email protected]>; Alvaro Herrera <[email protected]>
On 28.01.2021 17:30, Justin Pryzby wrote:
> On Thu, Jan 28, 2021 at 09:51:51PM +0900, Masahiko Sawada wrote:
>> On Mon, Nov 30, 2020 at 5:22 AM Justin Pryzby <[email protected]> wrote:
>>> On Sat, Oct 31, 2020 at 01:31:17AM -0500, Justin Pryzby wrote:
>>>> Forking this thread, since the existing CFs have been closed.
>>>> https://www.postgresql.org/message-id/flat/20200914143102.GX18552%40telsasoft.com#58b1056488451f8594...
>>>>
>>>> The strategy is to create catalog entries for all tables with indisvalid=false,
>>>> and then process them like REINDEX CONCURRENTLY. If it's interrupted, it
>>>> leaves INVALID indexes, which can be cleaned up with DROP or REINDEX, same as
>>>> CIC on a plain table.
>>>>
>>>> On Sat, Aug 08, 2020 at 01:37:44AM -0500, Justin Pryzby wrote:
>>>>> On Mon, Jun 15, 2020 at 09:37:42PM +0900, Michael Paquier wrote:
>>>>> Note that the mentioned problem wasn't serious: there was missing index on
>>>>> child table, therefor the parent index was invalid, as intended. However I
>>>>> agree that it's not nice that the command can fail so easily and leave behind
>>>>> some indexes created successfully and some failed some not created at all.
>>>>>
>>>>> But I took your advice initially creating invalid inds.
>>>> ...
>>>>> That gave me the idea to layer CIC on top of Reindex, since I think it does
>>>>> exactly what's needed.
>>>> On Sat, Sep 26, 2020 at 02:56:55PM -0500, Justin Pryzby wrote:
>>>>> On Thu, Sep 24, 2020 at 05:11:03PM +0900, Michael Paquier wrote:
>>>>>> It would be good also to check if
>>>>>> we have a partition index tree that maps partially with a partition
>>>>>> table tree (aka no all table partitions have a partition index), where
>>>>>> these don't get clustered because there is no index to work on.
>>>>> This should not happen, since a incomplete partitioned index is "invalid".
>>>>> I had been waiting to rebase since there hasn't been any review comments and I
>>>>> expected additional, future conflicts.
>>>>>
I attempted to review this feature, but the last patch conflicts with
the recent refactoring, so I wasn't able to test it properly.
Could you please send a new version?
Meanwhile, here are my questions about the patch:
1) I don't see a reason to change the logic here. We don't skip counting
existing indexes when create parent index. Why should we skip them in
CONCURRENTLY mode?
// If concurrent, maybe this should be done after excluding
indexes which already exist ?
pgstat_progress_update_param(PROGRESS_CREATEIDX_PARTITIONS_TOTAL,
nparts);
2) Here we access relation field after closing the relation. Is it safe?
/* save lockrelid and locktag for below */
heaprelid = rel->rd_lockInfo.lockRelId;
3) leaf_partitions() function only handles indexes, so I suggest to name
it more specifically and add a comment about meaning of 'options' parameter.
4) I don't quite understand the idea of the regression test. Why do we
expect to see invalid indexes there?
+ "idxpart_a_idx1" UNIQUE, btree (a) INVALID
5) Speaking of documentation, I think we need to add a paragraph about
CIC on partitioned indexes which will explain that invalid indexes may
appear and what user should do to fix them.
6) ReindexIndexesConcurrently() needs some code cleanup.
--
Anastasia Lubennikova
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company
^ permalink raw reply [nested|flat] 22+ messages in thread
* Re: CREATE INDEX CONCURRENTLY on partitioned index
2020-10-31 06:31 CREATE INDEX CONCURRENTLY on partitioned index Justin Pryzby <[email protected]>
2020-11-29 20:22 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Justin Pryzby <[email protected]>
2021-01-28 12:51 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Masahiko Sawada <[email protected]>
2021-01-28 14:30 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Justin Pryzby <[email protected]>
2021-02-15 19:06 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Anastasia Lubennikova <[email protected]>
@ 2021-02-15 19:33 ` Justin Pryzby <[email protected]>
2021-02-15 21:37 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Zhihong Yu <[email protected]>
0 siblings, 1 reply; 22+ messages in thread
From: Justin Pryzby @ 2021-02-15 19:33 UTC (permalink / raw)
To: Anastasia Lubennikova <[email protected]>; +Cc: Masahiko Sawada <[email protected]>; Michael Paquier <[email protected]>; 李杰(慎追) <[email protected]>; pgsql-hackers <[email protected]>; 曾文旌(义从) <[email protected]>; Alvaro Herrera <[email protected]>
On Mon, Feb 15, 2021 at 10:06:47PM +0300, Anastasia Lubennikova wrote:
> On 28.01.2021 17:30, Justin Pryzby wrote:
> > On Thu, Jan 28, 2021 at 09:51:51PM +0900, Masahiko Sawada wrote:
> > > On Mon, Nov 30, 2020 at 5:22 AM Justin Pryzby <[email protected]> wrote:
> > > > On Sat, Oct 31, 2020 at 01:31:17AM -0500, Justin Pryzby wrote:
> > > > > Forking this thread, since the existing CFs have been closed.
> > > > > https://www.postgresql.org/message-id/flat/20200914143102.GX18552%40telsasoft.com#58b1056488451f8594...
> > > > >
> > > > > The strategy is to create catalog entries for all tables with indisvalid=false,
> > > > > and then process them like REINDEX CONCURRENTLY. If it's interrupted, it
> > > > > leaves INVALID indexes, which can be cleaned up with DROP or REINDEX, same as
> > > > > CIC on a plain table.
> > > > >
> > > > > On Sat, Aug 08, 2020 at 01:37:44AM -0500, Justin Pryzby wrote:
> > > > > > On Mon, Jun 15, 2020 at 09:37:42PM +0900, Michael Paquier wrote:
> > > > > > Note that the mentioned problem wasn't serious: there was missing index on
> > > > > > child table, therefor the parent index was invalid, as intended. However I
> > > > > > agree that it's not nice that the command can fail so easily and leave behind
> > > > > > some indexes created successfully and some failed some not created at all.
> > > > > >
> > > > > > But I took your advice initially creating invalid inds.
> > > > > ...
> > > > > > That gave me the idea to layer CIC on top of Reindex, since I think it does
> > > > > > exactly what's needed.
> > > > > On Sat, Sep 26, 2020 at 02:56:55PM -0500, Justin Pryzby wrote:
> > > > > > On Thu, Sep 24, 2020 at 05:11:03PM +0900, Michael Paquier wrote:
> > > > > > > It would be good also to check if
> > > > > > > we have a partition index tree that maps partially with a partition
> > > > > > > table tree (aka no all table partitions have a partition index), where
> > > > > > > these don't get clustered because there is no index to work on.
> > > > > > This should not happen, since a incomplete partitioned index is "invalid".
>
> > > > > > I had been waiting to rebase since there hasn't been any review comments and I
> > > > > > expected additional, future conflicts.
> > > > > >
>
> I attempted to review this feature, but the last patch conflicts with the
> recent refactoring, so I wasn't able to test it properly.
> Could you please send a new version?
I rebased this yesterday, so here's my latest.
> 2) Here we access relation field after closing the relation. Is it safe?
> /* save lockrelid and locktag for below */
> heaprelid = rel->rd_lockInfo.lockRelId;
Thanks, fixed this just now.
> 3) leaf_partitions() function only handles indexes, so I suggest to name it
> more specifically and add a comment about meaning of 'options' parameter.
>
> 4) I don't quite understand the idea of the regression test. Why do we
> expect to see invalid indexes there?
> + "idxpart_a_idx1" UNIQUE, btree (a) INVALID
Because of the unique failure:
+create unique index concurrently on idxpart (a); -- partitioned, unique failure
+ERROR: could not create unique index "idxpart2_a_idx2_ccnew"
+DETAIL: Key (a)=(10) is duplicated.
+\d idxpart
This shows that CIC first creates catalog-only INVALID indexes, and then
reindexes them to "validate".
--
Justin
^ permalink raw reply [nested|flat] 22+ messages in thread
* Re: CREATE INDEX CONCURRENTLY on partitioned index
2020-10-31 06:31 CREATE INDEX CONCURRENTLY on partitioned index Justin Pryzby <[email protected]>
2020-11-29 20:22 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Justin Pryzby <[email protected]>
2021-01-28 12:51 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Masahiko Sawada <[email protected]>
2021-01-28 14:30 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Justin Pryzby <[email protected]>
2021-02-15 19:06 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Anastasia Lubennikova <[email protected]>
2021-02-15 19:33 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Justin Pryzby <[email protected]>
@ 2021-02-15 21:37 ` Zhihong Yu <[email protected]>
0 siblings, 0 replies; 22+ messages in thread
From: Zhihong Yu @ 2021-02-15 21:37 UTC (permalink / raw)
To: Justin Pryzby <[email protected]>; +Cc: Anastasia Lubennikova <[email protected]>; Masahiko Sawada <[email protected]>; Michael Paquier <[email protected]>; 李杰(慎追) <[email protected]>; pgsql-hackers <[email protected]>; 曾文旌(义从) <[email protected]>; Alvaro Herrera <[email protected]>
Hi,
For v13-0006-More-refactoring.patch :
+ /* It's not a shared catalog, so refuse to move it to shared tablespace
*/
+ if (params->tablespaceOid == GLOBALTABLESPACE_OID && false)
+ ereport(ERROR,
Do you intend to remove the ineffective check ?
+ else
+ heapRelation = table_open(heapId,
+ ShareUpdateExclusiveLock);
+ table_close(heapRelation, NoLock);
The table_open() seems to be unnecessary since there is no check after the
open.
+ // heapRelationIds = list_make1_oid(heapId);
If the code is not needed, you can remove the above.
For v13-0005-Refactor-to-allow-reindexing-all-index-partition.patch :
+ /* Skip invalid indexes, if requested */
+ if ((options & REINDEXOPT_SKIPVALID) != 0 &&
+ get_index_isvalid(partoid))
The comment seems to diverge from the name of the flag (which says skip
valid index).
Cheers
On Mon, Feb 15, 2021 at 11:34 AM Justin Pryzby <[email protected]> wrote:
> On Mon, Feb 15, 2021 at 10:06:47PM +0300, Anastasia Lubennikova wrote:
> > On 28.01.2021 17:30, Justin Pryzby wrote:
> > > On Thu, Jan 28, 2021 at 09:51:51PM +0900, Masahiko Sawada wrote:
> > > > On Mon, Nov 30, 2020 at 5:22 AM Justin Pryzby <[email protected]>
> wrote:
> > > > > On Sat, Oct 31, 2020 at 01:31:17AM -0500, Justin Pryzby wrote:
> > > > > > Forking this thread, since the existing CFs have been closed.
> > > > > >
> https://www.postgresql.org/message-id/flat/20200914143102.GX18552%40telsasoft.com#58b1056488451f8594...
> > > > > >
> > > > > > The strategy is to create catalog entries for all tables with
> indisvalid=false,
> > > > > > and then process them like REINDEX CONCURRENTLY. If it's
> interrupted, it
> > > > > > leaves INVALID indexes, which can be cleaned up with DROP or
> REINDEX, same as
> > > > > > CIC on a plain table.
> > > > > >
> > > > > > On Sat, Aug 08, 2020 at 01:37:44AM -0500, Justin Pryzby wrote:
> > > > > > > On Mon, Jun 15, 2020 at 09:37:42PM +0900, Michael Paquier
> wrote:
> > > > > > > Note that the mentioned problem wasn't serious: there was
> missing index on
> > > > > > > child table, therefor the parent index was invalid, as
> intended. However I
> > > > > > > agree that it's not nice that the command can fail so easily
> and leave behind
> > > > > > > some indexes created successfully and some failed some not
> created at all.
> > > > > > >
> > > > > > > But I took your advice initially creating invalid inds.
> > > > > > ...
> > > > > > > That gave me the idea to layer CIC on top of Reindex, since I
> think it does
> > > > > > > exactly what's needed.
> > > > > > On Sat, Sep 26, 2020 at 02:56:55PM -0500, Justin Pryzby wrote:
> > > > > > > On Thu, Sep 24, 2020 at 05:11:03PM +0900, Michael Paquier
> wrote:
> > > > > > > > It would be good also to check if
> > > > > > > > we have a partition index tree that maps partially with a
> partition
> > > > > > > > table tree (aka no all table partitions have a partition
> index), where
> > > > > > > > these don't get clustered because there is no index to work
> on.
> > > > > > > This should not happen, since a incomplete partitioned index
> is "invalid".
> >
> > > > > > > I had been waiting to rebase since there hasn't been any
> review comments and I
> > > > > > > expected additional, future conflicts.
> > > > > > >
> >
> > I attempted to review this feature, but the last patch conflicts with the
> > recent refactoring, so I wasn't able to test it properly.
> > Could you please send a new version?
>
> I rebased this yesterday, so here's my latest.
>
> > 2) Here we access relation field after closing the relation. Is it safe?
> > /* save lockrelid and locktag for below */
> > heaprelid = rel->rd_lockInfo.lockRelId;
>
> Thanks, fixed this just now.
>
> > 3) leaf_partitions() function only handles indexes, so I suggest to name
> it
> > more specifically and add a comment about meaning of 'options' parameter.
> >
> > 4) I don't quite understand the idea of the regression test. Why do we
> > expect to see invalid indexes there?
> > + "idxpart_a_idx1" UNIQUE, btree (a) INVALID
>
> Because of the unique failure:
> +create unique index concurrently on idxpart (a); -- partitioned, unique
> failure
> +ERROR: could not create unique index "idxpart2_a_idx2_ccnew"
> +DETAIL: Key (a)=(10) is duplicated.
> +\d idxpart
>
> This shows that CIC first creates catalog-only INVALID indexes, and then
> reindexes them to "validate".
>
> --
> Justin
>
^ permalink raw reply [nested|flat] 22+ messages in thread
* Re: CREATE INDEX CONCURRENTLY on partitioned index
2020-10-31 06:31 CREATE INDEX CONCURRENTLY on partitioned index Justin Pryzby <[email protected]>
2020-11-29 20:22 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Justin Pryzby <[email protected]>
2021-01-28 12:51 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Masahiko Sawada <[email protected]>
2021-01-28 14:30 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Justin Pryzby <[email protected]>
@ 2021-02-15 19:07 ` Anastasia Lubennikova <[email protected]>
2021-02-26 18:20 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Justin Pryzby <[email protected]>
1 sibling, 1 reply; 22+ messages in thread
From: Anastasia Lubennikova @ 2021-02-15 19:07 UTC (permalink / raw)
To: Justin Pryzby <[email protected]>; Masahiko Sawada <[email protected]>; +Cc: Michael Paquier <[email protected]>; 李杰(慎追) <[email protected]>; pgsql-hackers <[email protected]>; 曾文旌(义从) <[email protected]>; Alvaro Herrera <[email protected]>
On 28.01.2021 17:30, Justin Pryzby wrote:
> On Thu, Jan 28, 2021 at 09:51:51PM +0900, Masahiko Sawada wrote:
>> On Mon, Nov 30, 2020 at 5:22 AM Justin Pryzby <[email protected]> wrote:
>>> On Sat, Oct 31, 2020 at 01:31:17AM -0500, Justin Pryzby wrote:
>>>> Forking this thread, since the existing CFs have been closed.
>>>> https://www.postgresql.org/message-id/flat/20200914143102.GX18552%40telsasoft.com#58b1056488451f8594...
>>>>
>>>> The strategy is to create catalog entries for all tables with indisvalid=false,
>>>> and then process them like REINDEX CONCURRENTLY. If it's interrupted, it
>>>> leaves INVALID indexes, which can be cleaned up with DROP or REINDEX, same as
>>>> CIC on a plain table.
>>>>
>>>> On Sat, Aug 08, 2020 at 01:37:44AM -0500, Justin Pryzby wrote:
>>>>> On Mon, Jun 15, 2020 at 09:37:42PM +0900, Michael Paquier wrote:
>>>>> Note that the mentioned problem wasn't serious: there was missing index on
>>>>> child table, therefor the parent index was invalid, as intended. However I
>>>>> agree that it's not nice that the command can fail so easily and leave behind
>>>>> some indexes created successfully and some failed some not created at all.
>>>>>
>>>>> But I took your advice initially creating invalid inds.
>>>> ...
>>>>> That gave me the idea to layer CIC on top of Reindex, since I think it does
>>>>> exactly what's needed.
>>>> On Sat, Sep 26, 2020 at 02:56:55PM -0500, Justin Pryzby wrote:
>>>>> On Thu, Sep 24, 2020 at 05:11:03PM +0900, Michael Paquier wrote:
>>>>>> It would be good also to check if
>>>>>> we have a partition index tree that maps partially with a partition
>>>>>> table tree (aka no all table partitions have a partition index), where
>>>>>> these don't get clustered because there is no index to work on.
>>>>> This should not happen, since a incomplete partitioned index is "invalid".
>>>>> I had been waiting to rebase since there hasn't been any review comments and I
>>>>> expected additional, future conflicts.
>>>>>
I attempted to review this feature, but the last patch conflicts with
the recent refactoring, so I wasn't able to test it properly.
Could you please send a new version?
Meanwhile, here are my questions about the patch:
1) I don't see a reason to change the logic here. We don't skip counting
existing indexes when create parent index. Why should we skip them in
CONCURRENTLY mode?
// If concurrent, maybe this should be done after excluding
indexes which already exist ?
pgstat_progress_update_param(PROGRESS_CREATEIDX_PARTITIONS_TOTAL,
nparts);
2) Here we access relation field after closing the relation. Is it safe?
/* save lockrelid and locktag for below */
heaprelid = rel->rd_lockInfo.lockRelId;
3) leaf_partitions() function only handles indexes, so I suggest to name
it more specifically and add a comment about meaning of 'options' parameter.
4) I don't quite understand the idea of the regression test. Why do we
expect to see invalid indexes there?
+ "idxpart_a_idx1" UNIQUE, btree (a) INVALID
5) Speaking of documentation, I think we need to add a paragraph about
CIC on partitioned indexes which will explain that invalid indexes may
appear and what user should do to fix them.
6) ReindexIndexesConcurrently() needs some code cleanup.
--
Anastasia Lubennikova
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company
^ permalink raw reply [nested|flat] 22+ messages in thread
* Re: CREATE INDEX CONCURRENTLY on partitioned index
2020-10-31 06:31 CREATE INDEX CONCURRENTLY on partitioned index Justin Pryzby <[email protected]>
2020-11-29 20:22 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Justin Pryzby <[email protected]>
2021-01-28 12:51 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Masahiko Sawada <[email protected]>
2021-01-28 14:30 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Justin Pryzby <[email protected]>
2021-02-15 19:07 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Anastasia Lubennikova <[email protected]>
@ 2021-02-26 18:20 ` Justin Pryzby <[email protected]>
2022-03-25 05:05 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Greg Stark <[email protected]>
2022-06-28 18:33 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Justin Pryzby <[email protected]>
0 siblings, 2 replies; 22+ messages in thread
From: Justin Pryzby @ 2021-02-26 18:20 UTC (permalink / raw)
To: Anastasia Lubennikova <[email protected]>; +Cc: Masahiko Sawada <[email protected]>; Michael Paquier <[email protected]>; 李杰(慎追) <[email protected]>; [email protected]; 曾文旌(义从) <[email protected]>; Alvaro Herrera <[email protected]>
On Mon, Feb 15, 2021 at 10:07:05PM +0300, Anastasia Lubennikova wrote:
> 5) Speaking of documentation, I think we need to add a paragraph about CIC
> on partitioned indexes which will explain that invalid indexes may appear
> and what user should do to fix them.
I'm not sure about that - it's already documented in general, for
nonpartitioned indexes.
--
Justin
^ permalink raw reply [nested|flat] 22+ messages in thread
* Re: CREATE INDEX CONCURRENTLY on partitioned index
2020-10-31 06:31 CREATE INDEX CONCURRENTLY on partitioned index Justin Pryzby <[email protected]>
2020-11-29 20:22 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Justin Pryzby <[email protected]>
2021-01-28 12:51 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Masahiko Sawada <[email protected]>
2021-01-28 14:30 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Justin Pryzby <[email protected]>
2021-02-15 19:07 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Anastasia Lubennikova <[email protected]>
2021-02-26 18:20 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Justin Pryzby <[email protected]>
@ 2022-03-25 05:05 ` Greg Stark <[email protected]>
2022-03-25 13:13 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Justin Pryzby <[email protected]>
1 sibling, 1 reply; 22+ messages in thread
From: Greg Stark @ 2022-03-25 05:05 UTC (permalink / raw)
To: Alexander Pyhalov <[email protected]>; +Cc: Justin Pryzby <[email protected]>; Masahiko Sawada <[email protected]>; Michael Paquier <[email protected]>; 李杰(慎追) <[email protected]>; PostgreSQL Hackers <[email protected]>; 曾文旌(义从) <[email protected]>; Alvaro Herrera <[email protected]>
This patch is marked "waiting on author" in the CF. However the most
recent emails have patches and it's not clear to me what's left from
previous reviews that might not be addressed yet. Should this patch be
marked "Needs Review"?
Anastasia and Alexander are marked as reviewers. Are you still able to
review it or are there still pending issues that need to be resolved
from previous reviews?
^ permalink raw reply [nested|flat] 22+ messages in thread
* Re: CREATE INDEX CONCURRENTLY on partitioned index
2020-10-31 06:31 CREATE INDEX CONCURRENTLY on partitioned index Justin Pryzby <[email protected]>
2020-11-29 20:22 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Justin Pryzby <[email protected]>
2021-01-28 12:51 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Masahiko Sawada <[email protected]>
2021-01-28 14:30 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Justin Pryzby <[email protected]>
2021-02-15 19:07 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Anastasia Lubennikova <[email protected]>
2021-02-26 18:20 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Justin Pryzby <[email protected]>
2022-03-25 05:05 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Greg Stark <[email protected]>
@ 2022-03-25 13:13 ` Justin Pryzby <[email protected]>
0 siblings, 0 replies; 22+ messages in thread
From: Justin Pryzby @ 2022-03-25 13:13 UTC (permalink / raw)
To: Greg Stark <[email protected]>; +Cc: Alexander Pyhalov <[email protected]>; Masahiko Sawada <[email protected]>; Michael Paquier <[email protected]>; 李杰(慎追) <[email protected]>; [email protected]; 曾文旌(义从) <[email protected]>; Alvaro Herrera <[email protected]>
On Fri, Mar 25, 2022 at 01:05:49AM -0400, Greg Stark wrote:
> This patch is marked "waiting on author" in the CF. However the most
> recent emails have patches and it's not clear to me what's left from
> previous reviews that might not be addressed yet. Should this patch be
> marked "Needs Review"?
>
> Anastasia and Alexander are marked as reviewers. Are you still able to
> review it or are there still pending issues that need to be resolved
> from previous reviews?
I still haven't responded to Alexander's feedback, so I need to do that.
(Sorry).
However, since the patch attracted no attention for 50 some weeks last year, so
now is a weird time to shift attention to it. As such, I will move it to the
next CF.
https://www.postgresql.org/message-id/flat/[email protected]#da169a0a518bf8121604...
--
Justin
^ permalink raw reply [nested|flat] 22+ messages in thread
* Re: CREATE INDEX CONCURRENTLY on partitioned index
2020-10-31 06:31 CREATE INDEX CONCURRENTLY on partitioned index Justin Pryzby <[email protected]>
2020-11-29 20:22 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Justin Pryzby <[email protected]>
2021-01-28 12:51 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Masahiko Sawada <[email protected]>
2021-01-28 14:30 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Justin Pryzby <[email protected]>
2021-02-15 19:07 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Anastasia Lubennikova <[email protected]>
2021-02-26 18:20 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Justin Pryzby <[email protected]>
@ 2022-06-28 18:33 ` Justin Pryzby <[email protected]>
2022-06-29 06:45 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Alexander Pyhalov <[email protected]>
2022-06-29 08:01 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Alexander Pyhalov <[email protected]>
1 sibling, 2 replies; 22+ messages in thread
From: Justin Pryzby @ 2022-06-28 18:33 UTC (permalink / raw)
To: Alexander Pyhalov <[email protected]>; +Cc: Anastasia Lubennikova <[email protected]>; Masahiko Sawada <[email protected]>; Michael Paquier <[email protected]>; 李杰(慎追) <[email protected]>; [email protected]; 曾文旌(义从) <[email protected]>; Alvaro Herrera <[email protected]>
Hi,
On Thu, Feb 10, 2022 at 06:07:08PM +0300, Alexander Pyhalov wrote:
> I've rebased patches and tried to fix issues I've seen. I've fixed reference
> after table_close() in the first patch (can be seen while building with
> CPPFLAGS='-DRELCACHE_FORCE_RELEASE').
Thanks for finding that.
The patches other than 0001 are more experimental, and need someone to check if
it's even a good approach to use, so I kept them separate from the essential
patch.
Your latest 0005 patch (mark intermediate partitioned indexes as valid) is
probably fixing a bug in my SKIPVALID patch, right ? I'm not sure whether the
SKIPVALID patch should be merged into 0001, and I've been awaiting feedback on
the main patch before handling progress reporting.
Sorry for not responding sooner. The patch saw no activity for ~11 months so I
wasn't prepared to pick it up in March, at least not without guidance from a
committer.
Would you want to take over this patch ? I wrote it following someone's
question, but don't expect that I'd use the feature myself. I can help review
it or try to clarify the organization of my existing patches (but still haven't
managed to work my way through your amendments to my patches).
Thanks for caring about partitioned DDL ;)
--
Justin
^ permalink raw reply [nested|flat] 22+ messages in thread
* Re: CREATE INDEX CONCURRENTLY on partitioned index
2020-10-31 06:31 CREATE INDEX CONCURRENTLY on partitioned index Justin Pryzby <[email protected]>
2020-11-29 20:22 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Justin Pryzby <[email protected]>
2021-01-28 12:51 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Masahiko Sawada <[email protected]>
2021-01-28 14:30 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Justin Pryzby <[email protected]>
2021-02-15 19:07 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Anastasia Lubennikova <[email protected]>
2021-02-26 18:20 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Justin Pryzby <[email protected]>
2022-06-28 18:33 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Justin Pryzby <[email protected]>
@ 2022-06-29 06:45 ` Alexander Pyhalov <[email protected]>
1 sibling, 0 replies; 22+ messages in thread
From: Alexander Pyhalov @ 2022-06-29 06:45 UTC (permalink / raw)
To: Justin Pryzby <[email protected]>; +Cc: Masahiko Sawada <[email protected]>; Michael Paquier <[email protected]>; 李杰(慎追) <[email protected]>; [email protected]; 曾文旌(义从) <[email protected]>; Alvaro Herrera <[email protected]>
Justin Pryzby писал 2022-06-28 21:33:
> Hi,
>
> On Thu, Feb 10, 2022 at 06:07:08PM +0300, Alexander Pyhalov wrote:
>> I've rebased patches and tried to fix issues I've seen. I've fixed
>> reference
>> after table_close() in the first patch (can be seen while building
>> with
>> CPPFLAGS='-DRELCACHE_FORCE_RELEASE').
>
> Thanks for finding that.
>
> The patches other than 0001 are more experimental, and need someone to
> check if
> it's even a good approach to use, so I kept them separate from the
> essential
> patch.
>
> Your latest 0005 patch (mark intermediate partitioned indexes as valid)
> is
> probably fixing a bug in my SKIPVALID patch, right ? I'm not sure
> whether the
> SKIPVALID patch should be merged into 0001, and I've been awaiting
> feedback on
> the main patch before handling progress reporting.
Hi. I think it's more about fixing ReindexPartitions-to-set-indisvalid
patch, as
we also should mark intermediate indexes as valid when reindex succeeds.
> Sorry for not responding sooner. The patch saw no activity for ~11
> months so I
> wasn't prepared to pick it up in March, at least not without guidance
> from a
> committer.
>
> Would you want to take over this patch ? I wrote it following
> someone's
> question, but don't expect that I'd use the feature myself. I can help
> review
> it or try to clarify the organization of my existing patches (but still
> haven't
> managed to work my way through your amendments to my patches).
>
Yes, I'm glad to work on the patches, as this for us this is a very
important feature.
--
Best regards,
Alexander Pyhalov,
Postgres Professional
^ permalink raw reply [nested|flat] 22+ messages in thread
* Re: CREATE INDEX CONCURRENTLY on partitioned index
2020-10-31 06:31 CREATE INDEX CONCURRENTLY on partitioned index Justin Pryzby <[email protected]>
2020-11-29 20:22 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Justin Pryzby <[email protected]>
2021-01-28 12:51 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Masahiko Sawada <[email protected]>
2021-01-28 14:30 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Justin Pryzby <[email protected]>
2021-02-15 19:07 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Anastasia Lubennikova <[email protected]>
2021-02-26 18:20 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Justin Pryzby <[email protected]>
2022-06-28 18:33 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Justin Pryzby <[email protected]>
@ 2022-06-29 08:01 ` Alexander Pyhalov <[email protected]>
2022-11-21 03:00 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Justin Pryzby <[email protected]>
1 sibling, 1 reply; 22+ messages in thread
From: Alexander Pyhalov @ 2022-06-29 08:01 UTC (permalink / raw)
To: [email protected]; +Cc: Masahiko Sawada <[email protected]>; Michael Paquier <[email protected]>; 李杰(慎追) <[email protected]>; 曾文旌(义从) <[email protected]>; Alvaro Herrera <[email protected]>; Justin Pryzby <[email protected]>
Justin Pryzby писал 2022-06-28 21:33:
> Hi,
>
> On Thu, Feb 10, 2022 at 06:07:08PM +0300, Alexander Pyhalov wrote:
>> I've rebased patches and tried to fix issues I've seen. I've fixed
>> reference
>> after table_close() in the first patch (can be seen while building
>> with
>> CPPFLAGS='-DRELCACHE_FORCE_RELEASE').
>
Rebased patches on the current master.
They still require proper review.
--
Best regards,
Alexander Pyhalov,
Postgres Professional
Attachments:
[text/x-diff] v3-0005-Mark-intermediate-partitioned-indexes-as-valid.patch (8.3K, ../../[email protected]/2-v3-0005-Mark-intermediate-partitioned-indexes-as-valid.patch)
download | inline diff:
From 5c11849ceb2a1feb0e44dbdf30cc27de0282a659 Mon Sep 17 00:00:00 2001
From: Alexander Pyhalov <[email protected]>
Date: Mon, 28 Feb 2022 10:50:58 +0300
Subject: [PATCH 5/5] Mark intermediate partitioned indexes as valid
---
src/backend/commands/indexcmds.c | 33 ++++++++++-
src/test/regress/expected/indexing.out | 80 +++++++++++++++++++++++++-
src/test/regress/sql/indexing.sql | 8 +++
3 files changed, 118 insertions(+), 3 deletions(-)
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index d09f0390413..d3ced6265b6 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -3139,6 +3139,7 @@ static void
ReindexPartitions(Oid relid, ReindexParams *params, bool isTopLevel)
{
List *partitions = NIL;
+ List *inhpartindexes = NIL;
char relkind = get_rel_relkind(relid);
char *relname = get_rel_name(relid);
char *relnamespace = get_namespace_name(get_rel_namespace(relid));
@@ -3193,6 +3194,17 @@ ReindexPartitions(Oid relid, ReindexParams *params, bool isTopLevel)
char partkind = get_rel_relkind(partoid);
MemoryContext old_context;
+ /* Create a list of invalid inherited partitioned indexes */
+ if (partkind == RELKIND_PARTITIONED_INDEX)
+ {
+ if (partoid == relid || get_index_isvalid(partoid))
+ continue;
+
+ old_context = MemoryContextSwitchTo(reindex_context);
+ inhpartindexes = lappend_oid(inhpartindexes, partoid);
+ MemoryContextSwitchTo(old_context);
+ }
+
/*
* This discards partitioned tables, partitioned indexes and foreign
* tables.
@@ -3237,9 +3249,28 @@ ReindexPartitions(Oid relid, ReindexParams *params, bool isTopLevel)
Oid tableoid = IndexGetRelation(relid, false);
List *child_tables = find_all_inheritors(tableoid, ShareLock, NULL);
- /* Both lists include their parent relation as well as any intermediate partitioned rels */
+ /*
+ * Both lists include their parent relation as well as any
+ * intermediate partitioned rels
+ */
if (list_length(inhoids) == list_length(child_tables))
+ {
index_set_state_flags(relid, INDEX_CREATE_SET_VALID);
+
+ /* Mark any intermediate partitioned index as valid */
+ foreach(lc, inhpartindexes)
+ {
+ Oid partoid = lfirst_oid(lc);
+
+ Assert(get_rel_relkind(partoid) == RELKIND_PARTITIONED_INDEX);
+ Assert(!get_index_isvalid(partoid));
+
+ /* Can't mark an index valid without marking it ready */
+ index_set_state_flags(partoid, INDEX_CREATE_SET_READY);
+ CommandCounterIncrement();
+ index_set_state_flags(partoid, INDEX_CREATE_SET_VALID);
+ }
+ }
}
/*
diff --git a/src/test/regress/expected/indexing.out b/src/test/regress/expected/indexing.out
index a4ccae50de3..b4f1aea6fca 100644
--- a/src/test/regress/expected/indexing.out
+++ b/src/test/regress/expected/indexing.out
@@ -57,6 +57,8 @@ create table idxpart11 partition of idxpart1 for values from (0) to (10) partiti
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 idxpart (a); -- partitioned
create index concurrently on idxpart1 (a); -- partitioned and partition
@@ -76,7 +78,7 @@ Partition key: RANGE (a)
Indexes:
"idxpart_a_idx" btree (a)
"idxpart_a_idx1" UNIQUE, btree (a) INVALID
-Number of partitions: 2 (Use \d+ to list them.)
+Number of partitions: 3 (Use \d+ to list them.)
\d idxpart1
Partitioned table "public.idxpart1"
@@ -88,11 +90,59 @@ Number of partitions: 2 (Use \d+ to list them.)
Partition of: idxpart FOR VALUES FROM (0) TO (10)
Partition key: RANGE (a)
Indexes:
- "idxpart1_a_idx" btree (a) INVALID
+ "idxpart1_a_idx" btree (a)
"idxpart1_a_idx1" btree (a)
"idxpart1_a_idx2" 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" btree (a)
+ "idxpart11_a_idx2" btree (a)
+ "idxpart11_a_idx3" 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" btree (a)
+ "idxpart111_a_idx2" btree (a)
+ "idxpart111_a_idx3" 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" btree (a)
+ "idxpart1111_a_idx2" btree (a)
+ "idxpart1111_a_idx3" UNIQUE, btree (a) INVALID
+Number of partitions: 0
+
\d idxpart2
Table "public.idxpart2"
Column | Type | Collation | Nullable | Default
@@ -107,6 +157,32 @@ Indexes:
"idxpart2_a_idx2" UNIQUE, btree (a) INVALID
"idxpart2_a_idx2_ccnew" 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/[email protected]
diff --git a/src/test/regress/sql/indexing.sql b/src/test/regress/sql/indexing.sql
index 3d4b6e9bc95..06673c15199 100644
--- a/src/test/regress/sql/indexing.sql
+++ b/src/test/regress/sql/indexing.sql
@@ -36,6 +36,9 @@ create table idxpart11 partition of idxpart1 for values from (0) to (10) partiti
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 idxpart (a); -- partitioned
create index concurrently on idxpart1 (a); -- partitioned and partition
@@ -44,7 +47,12 @@ create index concurrently on idxpart2 (a); -- leaf
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.34.1
[text/x-diff] v3-0004-ReindexPartitions-to-set-indisvalid.patch (2.4K, ../../[email protected]/3-v3-0004-ReindexPartitions-to-set-indisvalid.patch)
download | inline diff:
From 71838e0146e5150013c48818710d899e69786dc0 Mon Sep 17 00:00:00 2001
From: Justin Pryzby <[email protected]>
Date: Mon, 7 Feb 2022 10:39:59 +0300
Subject: [PATCH 4/5] ReindexPartitions() to set indisvalid
0004-ReindexPartitions-to-set-indisvalid.patch from
https://www.postgresql.org/message-id/[email protected]
---
src/backend/commands/indexcmds.c | 23 ++++++++++++++++++-----
1 file changed, 18 insertions(+), 5 deletions(-)
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index b231a05b8c9..d09f0390413 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -1716,8 +1716,6 @@ reindex_invalid_child_indexes(Oid indexRelationId)
* CIC needs to mark a partitioned index as VALID, which itself
* requires setting READY, which is unset for CIC (even though
* it's meaningless for an index without storage).
- * This must be done only while holding a lock which precludes adding
- * partitions.
*/
CommandCounterIncrement();
index_set_state_flags(indexRelationId, INDEX_CREATE_SET_READY);
@@ -1729,9 +1727,6 @@ reindex_invalid_child_indexes(Oid indexRelationId)
* this commits and then starts a new transaction immediately.
*/
ReindexPartitions(indexRelationId, ¶ms, true);
-
- CommandCounterIncrement();
- index_set_state_flags(indexRelationId, INDEX_CREATE_SET_VALID);
}
/*
@@ -3229,6 +3224,24 @@ ReindexPartitions(Oid relid, ReindexParams *params, bool isTopLevel)
*/
ReindexMultipleInternal(partitions, params, relid, npart);
+ /*
+ * If indexes exist on all of the partitioned table's children, and we
+ * just reindexed them, then we know they're valid, and so can mark the
+ * parent index as valid.
+ * This handles the case of CREATE INDEX CONCURRENTLY.
+ * See also: validatePartitionedIndex().
+ */
+ if (get_rel_relkind(relid) == RELKIND_PARTITIONED_INDEX
+ && !get_index_isvalid(relid))
+ {
+ Oid tableoid = IndexGetRelation(relid, false);
+ List *child_tables = find_all_inheritors(tableoid, ShareLock, NULL);
+
+ /* Both lists include their parent relation as well as any intermediate partitioned rels */
+ if (list_length(inhoids) == list_length(child_tables))
+ index_set_state_flags(relid, INDEX_CREATE_SET_VALID);
+ }
+
/*
* Clean up working storage --- note we must do this after
* StartTransactionCommand, else we might be trying to delete the active
--
2.34.1
[text/x-diff] v3-0003-Try-to-fix-create-index-progress-report.patch (7.7K, ../../[email protected]/4-v3-0003-Try-to-fix-create-index-progress-report.patch)
download | inline diff:
From f5b8afe1aafb78c53a527d379b978ce9f1fe06d6 Mon Sep 17 00:00:00 2001
From: Alexander Pyhalov <[email protected]>
Date: Tue, 8 Feb 2022 21:15:05 +0300
Subject: [PATCH 3/5] Try to fix create index progress report
---
src/backend/commands/indexcmds.c | 67 ++++++++++++++++++++++++++------
src/include/catalog/index.h | 1 +
2 files changed, 57 insertions(+), 11 deletions(-)
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index 149235780b9..b231a05b8c9 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -102,11 +102,14 @@ static void reindex_error_callback(void *args);
static void ReindexPartitions(Oid relid, ReindexParams *params,
bool isTopLevel);
static void ReindexMultipleInternal(List *relids,
- ReindexParams *params);
+ ReindexParams *params,
+ Oid parent,
+ int npart);
static bool ReindexRelationConcurrently(Oid relationOid,
ReindexParams *params);
static void update_relispartition(Oid relationId, bool newval);
static inline void set_indexsafe_procflags(void);
+static void report_create_partition_index_done(Oid parent, int npart);
/*
* callback argument type for RangeVarCallbackForReindexIndex()
@@ -1220,6 +1223,7 @@ DefineIndex(Oid relationId,
Oid *opfamOids;
char *relname;
+
pgstat_progress_update_param(PROGRESS_CREATEIDX_PARTITIONS_TOTAL,
nparts);
@@ -1705,7 +1709,7 @@ static void
reindex_invalid_child_indexes(Oid indexRelationId)
{
ReindexParams params = {
- .options = REINDEXOPT_CONCURRENTLY | REINDEXOPT_SKIPVALID
+ .options = REINDEXOPT_CONCURRENTLY | REINDEXOPT_SKIPVALID | REINDEXOPT_REPORT_CREATE_PART
};
/*
@@ -1718,6 +1722,8 @@ reindex_invalid_child_indexes(Oid indexRelationId)
CommandCounterIncrement();
index_set_state_flags(indexRelationId, INDEX_CREATE_SET_READY);
+ pgstat_progress_update_param(PROGRESS_CREATEIDX_PHASE, PROGRESS_CREATEIDX_PHASE_VALIDATE_IDXSCAN);
+
/*
* Process each partition listed in a separate transaction. Note that
* this commits and then starts a new transaction immediately.
@@ -3105,7 +3111,7 @@ ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind,
* Process each relation listed in a separate transaction. Note that this
* commits and then starts a new transaction immediately.
*/
- ReindexMultipleInternal(relids, params);
+ ReindexMultipleInternal(relids, params, InvalidOid, 0);
MemoryContextDelete(private_context);
}
@@ -3141,6 +3147,7 @@ ReindexPartitions(Oid relid, ReindexParams *params, bool isTopLevel)
char relkind = get_rel_relkind(relid);
char *relname = get_rel_name(relid);
char *relnamespace = get_namespace_name(get_rel_namespace(relid));
+ int npart = 1;
MemoryContext reindex_context;
List *inhoids;
ListCell *lc;
@@ -3201,7 +3208,11 @@ ReindexPartitions(Oid relid, ReindexParams *params, bool isTopLevel)
/* Skip valid indexes, if requested */
if ((params->options & REINDEXOPT_SKIPVALID) != 0 &&
get_index_isvalid(partoid))
+ {
+ if (params->options & REINDEXOPT_REPORT_CREATE_PART)
+ report_create_partition_index_done(relid, npart++);
continue;
+ }
Assert(partkind == RELKIND_INDEX ||
partkind == RELKIND_RELATION);
@@ -3216,7 +3227,7 @@ ReindexPartitions(Oid relid, ReindexParams *params, bool isTopLevel)
* Process each partition listed in a separate transaction. Note that
* this commits and then starts a new transaction immediately.
*/
- ReindexMultipleInternal(partitions, params);
+ ReindexMultipleInternal(partitions, params, relid, npart);
/*
* Clean up working storage --- note we must do this after
@@ -3234,7 +3245,7 @@ ReindexPartitions(Oid relid, ReindexParams *params, bool isTopLevel)
* and starts a new transaction when finished.
*/
static void
-ReindexMultipleInternal(List *relids, ReindexParams *params)
+ReindexMultipleInternal(List *relids, ReindexParams *params, Oid parent, int npart)
{
ListCell *l;
@@ -3328,6 +3339,9 @@ ReindexMultipleInternal(List *relids, ReindexParams *params)
}
CommitTransactionCommand();
+
+ if (params->options & REINDEXOPT_REPORT_CREATE_PART)
+ report_create_partition_index_done(parent, npart++);
}
StartTransactionCommand();
@@ -3723,7 +3737,9 @@ ReindexRelationConcurrently(Oid relationOid, ReindexParams *params)
if (indexRel->rd_rel->relpersistence == RELPERSISTENCE_TEMP)
elog(ERROR, "cannot reindex a temporary table concurrently");
- pgstat_progress_start_command(PROGRESS_COMMAND_CREATE_INDEX,
+ /* Don't overwrite CREATE INDEX command */
+ if (!(params->options & REINDEXOPT_REPORT_CREATE_PART))
+ pgstat_progress_start_command(PROGRESS_COMMAND_CREATE_INDEX,
idx->tableId);
progress_vals[0] = PROGRESS_CREATEIDX_COMMAND_REINDEX_CONCURRENTLY;
@@ -3883,9 +3899,11 @@ ReindexRelationConcurrently(Oid relationOid, ReindexParams *params)
/*
* Update progress for the index to build, with the correct parent
- * table involved.
+ * table involved. Don't overwrite CREATE INDEX command.
*/
- pgstat_progress_start_command(PROGRESS_COMMAND_CREATE_INDEX, newidx->tableId);
+ if (!(params->options & REINDEXOPT_REPORT_CREATE_PART))
+ pgstat_progress_start_command(PROGRESS_COMMAND_CREATE_INDEX, newidx->tableId);
+
progress_vals[0] = PROGRESS_CREATEIDX_COMMAND_REINDEX_CONCURRENTLY;
progress_vals[1] = PROGRESS_CREATEIDX_PHASE_BUILD;
progress_vals[2] = newidx->indexId;
@@ -3947,10 +3965,12 @@ ReindexRelationConcurrently(Oid relationOid, ReindexParams *params)
/*
* Update progress for the index to build, with the correct parent
- * table involved.
+ * table involved. Don't overwrite CREATE INDEX command.
*/
- pgstat_progress_start_command(PROGRESS_COMMAND_CREATE_INDEX,
+ if (!(params->options & REINDEXOPT_REPORT_CREATE_PART))
+ pgstat_progress_start_command(PROGRESS_COMMAND_CREATE_INDEX,
newidx->tableId);
+
progress_vals[0] = PROGRESS_CREATEIDX_COMMAND_REINDEX_CONCURRENTLY;
progress_vals[1] = PROGRESS_CREATEIDX_PHASE_VALIDATE_IDXSCAN;
progress_vals[2] = newidx->indexId;
@@ -4185,7 +4205,9 @@ ReindexRelationConcurrently(Oid relationOid, ReindexParams *params)
MemoryContextDelete(private_context);
- pgstat_progress_end_command();
+ /* Don't overwrite CREATE INDEX command. */
+ if (!(params->options & REINDEXOPT_REPORT_CREATE_PART))
+ pgstat_progress_end_command();
return true;
}
@@ -4321,6 +4343,29 @@ IndexSetParentIndex(Relation partitionIdx, Oid parentOid)
}
}
+/*
+ * Update pgstat progress report to indicate that create index on
+ * partition was finished.
+ */
+static void
+report_create_partition_index_done(Oid index, int npart)
+{
+ const int progress_cols[] = {
+ PROGRESS_CREATEIDX_COMMAND,
+ PROGRESS_CREATEIDX_INDEX_OID,
+ PROGRESS_CREATEIDX_PHASE,
+ PROGRESS_CREATEIDX_PARTITIONS_DONE
+ };
+ const int64 progress_vals[] = {
+ PROGRESS_CREATEIDX_COMMAND_CREATE_CONCURRENTLY,
+ index,
+ PROGRESS_CREATEIDX_PHASE_VALIDATE_IDXSCAN,
+ npart
+ };
+
+ pgstat_progress_update_multi_param(4, progress_cols, progress_vals);
+}
+
/*
* Subroutine of IndexSetParentIndex to update the relispartition flag of the
* given index to the given value.
diff --git a/src/include/catalog/index.h b/src/include/catalog/index.h
index c31b66ad0b9..b5b0a71e7d4 100644
--- a/src/include/catalog/index.h
+++ b/src/include/catalog/index.h
@@ -43,6 +43,7 @@ typedef struct ReindexParams
#define REINDEXOPT_MISSING_OK 0x04 /* skip missing relations */
#define REINDEXOPT_CONCURRENTLY 0x08 /* concurrent mode */
#define REINDEXOPT_SKIPVALID 0x10 /* skip valid indexes */
+#define REINDEXOPT_REPORT_CREATE_PART 0x20 /* report that index was created for partition */
/* state info for validate_index bulkdelete callback */
typedef struct ValidateIndexState
--
2.34.1
[text/x-diff] v3-0002-Add-SKIPVALID-flag-for-more-integration.patch (3.9K, ../../[email protected]/5-v3-0002-Add-SKIPVALID-flag-for-more-integration.patch)
download | inline diff:
From 4f16e729a835dc8123a8ce593e6ae47542b82c83 Mon Sep 17 00:00:00 2001
From: Justin Pryzby <[email protected]>
Date: Mon, 7 Feb 2022 10:31:40 +0300
Subject: [PATCH 2/5] Add SKIPVALID flag for more integration
Combined
0002-f-progress-reporting.patch and
0003-WIP-Add-SKIPVALID-flag-for-more-integration.patch from
https://www.postgresql.org/message-id/[email protected]
---
src/backend/commands/indexcmds.c | 57 ++++++++++----------------------
src/include/catalog/index.h | 1 +
2 files changed, 19 insertions(+), 39 deletions(-)
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index 784f6d9eb87..149235780b9 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -1697,59 +1697,33 @@ DefineIndex(Oid relationId,
return address;
}
-/* Reindex invalid child indexes created earlier */
+/*
+ * Reindex invalid child indexes created earlier thereby validating
+ * the parent index.
+ */
static void
reindex_invalid_child_indexes(Oid indexRelationId)
{
- ListCell *lc;
- int npart = 0;
ReindexParams params = {
- .options = REINDEXOPT_CONCURRENTLY
+ .options = REINDEXOPT_CONCURRENTLY | REINDEXOPT_SKIPVALID
};
- MemoryContext ind_context = AllocSetContextCreate(PortalContext, "CREATE INDEX",
- ALLOCSET_DEFAULT_SIZES);
- MemoryContext oldcontext;
- List *childs = find_inheritance_children(indexRelationId, ShareLock);
- List *partitions = NIL;
-
- PreventInTransactionBlock(true, "REINDEX INDEX");
-
- foreach (lc, childs)
- {
- Oid partoid = lfirst_oid(lc);
-
- /* XXX: need to retrofit progress reporting into it */
- // pgstat_progress_update_param(PROGRESS_CREATEIDX_PARTITIONS_DONE,
- // npart++);
-
- if (get_index_isvalid(partoid) ||
- !RELKIND_HAS_STORAGE(get_rel_relkind(partoid)))
- continue;
-
- /* Save partition OID */
- oldcontext = MemoryContextSwitchTo(ind_context);
- partitions = lappend_oid(partitions, partoid);
- MemoryContextSwitchTo(oldcontext);
- }
-
- /*
- * Process each partition listed in a separate transaction. Note that
- * this commits and then starts a new transaction immediately.
- * XXX: since this is done in 2*N transactions, it could just as well
- * call ReindexRelationConcurrently directly
- */
- ReindexMultipleInternal(partitions, ¶ms);
-
/*
* CIC needs to mark a partitioned index as VALID, which itself
* requires setting READY, which is unset for CIC (even though
* it's meaningless for an index without storage).
* This must be done only while holding a lock which precludes adding
* partitions.
- * See also: validatePartitionedIndex().
*/
+ CommandCounterIncrement();
index_set_state_flags(indexRelationId, INDEX_CREATE_SET_READY);
+
+ /*
+ * Process each partition listed in a separate transaction. Note that
+ * this commits and then starts a new transaction immediately.
+ */
+ ReindexPartitions(indexRelationId, ¶ms, true);
+
CommandCounterIncrement();
index_set_state_flags(indexRelationId, INDEX_CREATE_SET_VALID);
}
@@ -3224,6 +3198,11 @@ ReindexPartitions(Oid relid, ReindexParams *params, bool isTopLevel)
if (!RELKIND_HAS_STORAGE(partkind))
continue;
+ /* Skip valid indexes, if requested */
+ if ((params->options & REINDEXOPT_SKIPVALID) != 0 &&
+ get_index_isvalid(partoid))
+ continue;
+
Assert(partkind == RELKIND_INDEX ||
partkind == RELKIND_RELATION);
diff --git a/src/include/catalog/index.h b/src/include/catalog/index.h
index a1d6e3b645f..c31b66ad0b9 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_SKIPVALID 0x10 /* skip valid indexes */
/* state info for validate_index bulkdelete callback */
typedef struct ValidateIndexState
--
2.34.1
[text/x-diff] v3-0001-Allow-CREATE-INDEX-CONCURRENTLY-on-partitioned-table.patch (15.6K, ../../[email protected]/6-v3-0001-Allow-CREATE-INDEX-CONCURRENTLY-on-partitioned-table.patch)
download | inline diff:
From f4933625541b030fabcb15d1426f496598949898 Mon Sep 17 00:00:00 2001
From: Justin Pryzby <[email protected]>
Date: Mon, 7 Feb 2022 10:28:42 +0300
Subject: [PATCH 1/5] Allow CREATE INDEX CONCURRENTLY on partitioned table
0001-Allow-CREATE-INDEX-CONCURRENTLY-on-partitioned-table.patch from
https://www.postgresql.org/message-id/[email protected]
Fixes:
- rel was used after table_close();
- it seems childidxs shouldn't live in ind_context;
- updated doc.
---
doc/src/sgml/ref/create_index.sgml | 14 +--
src/backend/commands/indexcmds.c | 151 ++++++++++++++++++-------
src/test/regress/expected/indexing.out | 60 +++++++++-
src/test/regress/sql/indexing.sql | 18 ++-
4 files changed, 186 insertions(+), 57 deletions(-)
diff --git a/doc/src/sgml/ref/create_index.sgml b/doc/src/sgml/ref/create_index.sgml
index 9ffcdc629e6..2040b27b685 100644
--- a/doc/src/sgml/ref/create_index.sgml
+++ b/doc/src/sgml/ref/create_index.sgml
@@ -642,7 +642,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 creating 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>:
@@ -689,15 +692,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 99f5ab83c32..784f6d9eb87 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -68,6 +68,7 @@
/* non-export function prototypes */
+static void reindex_invalid_child_indexes(Oid indexRelationId);
static bool CompareOpclassOptions(Datum *opts1, Datum *opts2, int natts);
static void CheckPredicate(Expr *predicate);
static void ComputeIndexAttrs(IndexInfo *indexInfo,
@@ -695,17 +696,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),
@@ -1145,6 +1135,11 @@ DefineIndex(Oid relationId,
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;
@@ -1210,18 +1205,30 @@ DefineIndex(Oid relationId,
partdesc = RelationGetPartitionDesc(rel, true);
if ((!stmt->relation || stmt->relation->inh) && partdesc->nparts > 0)
{
+ /*
+ * Need to close the relation before recursing into children, so
+ * copy needed data into a longlived context.
+ */
+
+ MemoryContext ind_context = AllocSetContextCreate(PortalContext, "CREATE INDEX",
+ ALLOCSET_DEFAULT_SIZES);
+ MemoryContext oldcontext = MemoryContextSwitchTo(ind_context);
int nparts = partdesc->nparts;
Oid *part_oids = palloc(sizeof(Oid) * nparts);
bool invalidate_parent = false;
TupleDesc parentDesc;
Oid *opfamOids;
+ char *relname;
pgstat_progress_update_param(PROGRESS_CREATEIDX_PARTITIONS_TOTAL,
nparts);
memcpy(part_oids, partdesc->oids, sizeof(Oid) * nparts);
+ parentDesc = CreateTupleDescCopy(RelationGetDescr(rel));
+ relname = pstrdup(RelationGetRelationName(rel));
+ table_close(rel, NoLock);
+ MemoryContextSwitchTo(oldcontext);
- parentDesc = RelationGetDescr(rel);
opfamOids = palloc(sizeof(Oid) * numberOfKeyAttributes);
for (i = 0; i < numberOfKeyAttributes; i++)
opfamOids[i] = get_opclass_family(classObjectId[i]);
@@ -1265,9 +1272,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)));
AtEOXact_GUC(false, child_save_nestlevel);
SetUserIdAndSecContext(child_save_userid,
@@ -1277,9 +1284,12 @@ DefineIndex(Oid relationId,
}
childidxs = RelationGetIndexList(childrel);
+
+ oldcontext = MemoryContextSwitchTo(ind_context);
attmap =
build_attrmap_by_name(RelationGetDescr(childrel),
parentDesc);
+ MemoryContextSwitchTo(oldcontext);
foreach(cell, childidxs)
{
@@ -1353,10 +1363,14 @@ DefineIndex(Oid relationId,
*/
if (!found)
{
- IndexStmt *childStmt = copyObject(stmt);
+ IndexStmt *childStmt;
bool found_whole_row;
ListCell *lc;
+ oldcontext = MemoryContextSwitchTo(ind_context);
+ childStmt = copyObject(stmt);
+ MemoryContextSwitchTo(oldcontext);
+
/*
* We can't use the same index name for the child index,
* so clear idxname to let the recursive invocation choose
@@ -1417,12 +1431,21 @@ DefineIndex(Oid relationId,
skip_build, quiet);
SetUserIdAndSecContext(child_save_userid,
child_save_sec_context);
+ if (concurrent)
+ {
+ PopActiveSnapshot();
+ PushActiveSnapshot(GetTransactionSnapshot());
+ invalidate_parent = true;
+ }
}
- pgstat_progress_update_param(PROGRESS_CREATEIDX_PARTITIONS_DONE,
- i + 1);
+ /* For concurrent build, this is a catalog-only stage */
+ if (!concurrent)
+ pgstat_progress_update_param(PROGRESS_CREATEIDX_PARTITIONS_DONE,
+ i + 1);
free_attrmap(attmap);
}
+ pfree(relname);
/*
* The pg_index row we inserted for this index was marked
@@ -1430,24 +1453,9 @@ DefineIndex(Oid relationId,
* invalid, this is incorrect, so update our row to invalid too.
*/
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);
+ } else
+ table_close(rel, NoLock);
/*
* Indexes on partitioned tables are not themselves built, so we're
@@ -1455,21 +1463,28 @@ DefineIndex(Oid relationId,
*/
AtEOXact_GUC(false, root_save_nestlevel);
SetUserIdAndSecContext(root_save_userid, root_save_sec_context);
- table_close(rel, NoLock);
if (!OidIsValid(parentIndexId))
+ {
+ if (concurrent)
+ reindex_invalid_child_indexes(indexRelationId);
+
pgstat_progress_end_command();
+ }
+
return address;
}
AtEOXact_GUC(false, root_save_nestlevel);
SetUserIdAndSecContext(root_save_userid, root_save_sec_context);
- if (!concurrent)
+ if (!concurrent || OidIsValid(parentIndexId))
{
- /* Close the heap and we're done, in the non-concurrent case */
- table_close(rel, NoLock);
+ /*
+ * We're done if this is the top-level index,
+ * or the catalog-only phase of a partition built concurrently
+ */
- /* If this is the top-level index, we're done. */
+ table_close(rel, NoLock);
if (!OidIsValid(parentIndexId))
pgstat_progress_end_command();
@@ -1682,6 +1697,62 @@ DefineIndex(Oid relationId,
return address;
}
+/* Reindex invalid child indexes created earlier */
+static void
+reindex_invalid_child_indexes(Oid indexRelationId)
+{
+ ListCell *lc;
+ int npart = 0;
+ ReindexParams params = {
+ .options = REINDEXOPT_CONCURRENTLY
+ };
+
+ MemoryContext ind_context = AllocSetContextCreate(PortalContext, "CREATE INDEX",
+ ALLOCSET_DEFAULT_SIZES);
+ MemoryContext oldcontext;
+ List *childs = find_inheritance_children(indexRelationId, ShareLock);
+ List *partitions = NIL;
+
+ PreventInTransactionBlock(true, "REINDEX INDEX");
+
+ foreach (lc, childs)
+ {
+ Oid partoid = lfirst_oid(lc);
+
+ /* XXX: need to retrofit progress reporting into it */
+ // pgstat_progress_update_param(PROGRESS_CREATEIDX_PARTITIONS_DONE,
+ // npart++);
+
+ if (get_index_isvalid(partoid) ||
+ !RELKIND_HAS_STORAGE(get_rel_relkind(partoid)))
+ continue;
+
+ /* Save partition OID */
+ oldcontext = MemoryContextSwitchTo(ind_context);
+ partitions = lappend_oid(partitions, partoid);
+ MemoryContextSwitchTo(oldcontext);
+ }
+
+ /*
+ * Process each partition listed in a separate transaction. Note that
+ * this commits and then starts a new transaction immediately.
+ * XXX: since this is done in 2*N transactions, it could just as well
+ * call ReindexRelationConcurrently directly
+ */
+ ReindexMultipleInternal(partitions, ¶ms);
+
+ /*
+ * CIC needs to mark a partitioned index as VALID, which itself
+ * requires setting READY, which is unset for CIC (even though
+ * it's meaningless for an index without storage).
+ * This must be done only while holding a lock which precludes adding
+ * partitions.
+ * See also: validatePartitionedIndex().
+ */
+ index_set_state_flags(indexRelationId, INDEX_CREATE_SET_READY);
+ CommandCounterIncrement();
+ index_set_state_flags(indexRelationId, INDEX_CREATE_SET_VALID);
+}
/*
* CheckMutability
diff --git a/src/test/regress/expected/indexing.out b/src/test/regress/expected/indexing.out
index 193f7801912..a4ccae50de3 100644
--- a/src/test/regress/expected/indexing.out
+++ b/src/test/regress/expected/indexing.out
@@ -50,11 +50,63 @@ 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);
+insert into idxpart2 values(10),(10); -- not unique
+create index concurrently on idxpart (a); -- partitioned
+create index concurrently on idxpart1 (a); -- partitioned and partition
+create index concurrently on idxpart11 (a); -- partitioned and partition, with no leaves
+create index concurrently on idxpart2 (a); -- leaf
+create unique index concurrently on idxpart (a); -- partitioned, unique failure
+ERROR: could not create unique index "idxpart2_a_idx2_ccnew"
+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: 2 (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) INVALID
+ "idxpart1_a_idx1" btree (a)
+ "idxpart1_a_idx2" UNIQUE, btree (a) INVALID
+Number of partitions: 1 (Use \d+ to list them.)
+
+\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" btree (a)
+ "idxpart2_a_idx2" UNIQUE, btree (a) INVALID
+ "idxpart2_a_idx2_ccnew" UNIQUE, btree (a) INVALID
+
drop table idxpart;
-- Verify bugfix with query on indexed partitioned table with no partitions
-- https://postgr.es/m/[email protected]
diff --git a/src/test/regress/sql/indexing.sql b/src/test/regress/sql/indexing.sql
index 42f398b67c2..3d4b6e9bc95 100644
--- a/src/test/regress/sql/indexing.sql
+++ b/src/test/regress/sql/indexing.sql
@@ -29,10 +29,22 @@ 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);
+insert into idxpart2 values(10),(10); -- not unique
+create index concurrently on idxpart (a); -- partitioned
+create index concurrently on idxpart1 (a); -- partitioned and partition
+create index concurrently on idxpart11 (a); -- partitioned and partition, with no leaves
+create index concurrently on idxpart2 (a); -- leaf
+create unique index concurrently on idxpart (a); -- partitioned, unique failure
+\d idxpart
+\d idxpart1
+\d idxpart2
drop table idxpart;
-- Verify bugfix with query on indexed partitioned table with no partitions
--
2.34.1
^ permalink raw reply [nested|flat] 22+ messages in thread
* Re: CREATE INDEX CONCURRENTLY on partitioned index
2020-10-31 06:31 CREATE INDEX CONCURRENTLY on partitioned index Justin Pryzby <[email protected]>
2020-11-29 20:22 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Justin Pryzby <[email protected]>
2021-01-28 12:51 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Masahiko Sawada <[email protected]>
2021-01-28 14:30 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Justin Pryzby <[email protected]>
2021-02-15 19:07 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Anastasia Lubennikova <[email protected]>
2021-02-26 18:20 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Justin Pryzby <[email protected]>
2022-06-28 18:33 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Justin Pryzby <[email protected]>
2022-06-29 08:01 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Alexander Pyhalov <[email protected]>
@ 2022-11-21 03:00 ` Justin Pryzby <[email protected]>
2022-11-21 14:44 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Alexander Pyhalov <[email protected]>
2022-12-03 15:13 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Ilya Gladyshev <[email protected]>
0 siblings, 2 replies; 22+ messages in thread
From: Justin Pryzby @ 2022-11-21 03:00 UTC (permalink / raw)
To: Alexander Pyhalov <[email protected]>; +Cc: [email protected]; Masahiko Sawada <[email protected]>; Michael Paquier <[email protected]>; 李杰(慎追) <[email protected]>; 曾文旌(义从) <[email protected]>; Alvaro Herrera <[email protected]>
I finally found time to digest and integrate your changes into my local
branch. This fixes the three issues you reported: FORCE_RELEASE, issue
with INVALID partitions issue (for which I adapted your patch into an
earlier patch in my series), and progress reporting. And rebased.
--
Justin
^ permalink raw reply [nested|flat] 22+ messages in thread
* Re: CREATE INDEX CONCURRENTLY on partitioned index
2020-10-31 06:31 CREATE INDEX CONCURRENTLY on partitioned index Justin Pryzby <[email protected]>
2020-11-29 20:22 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Justin Pryzby <[email protected]>
2021-01-28 12:51 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Masahiko Sawada <[email protected]>
2021-01-28 14:30 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Justin Pryzby <[email protected]>
2021-02-15 19:07 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Anastasia Lubennikova <[email protected]>
2021-02-26 18:20 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Justin Pryzby <[email protected]>
2022-06-28 18:33 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Justin Pryzby <[email protected]>
2022-06-29 08:01 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Alexander Pyhalov <[email protected]>
2022-11-21 03:00 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Justin Pryzby <[email protected]>
@ 2022-11-21 14:44 ` Alexander Pyhalov <[email protected]>
1 sibling, 0 replies; 22+ messages in thread
From: Alexander Pyhalov @ 2022-11-21 14:44 UTC (permalink / raw)
To: Justin Pryzby <[email protected]>; +Cc: [email protected]; Masahiko Sawada <[email protected]>; Michael Paquier <[email protected]>; 李杰(慎追) <[email protected]>; 曾文旌(义从) <[email protected]>; Alvaro Herrera <[email protected]>
Justin Pryzby писал 2022-11-21 06:00:
> I finally found time to digest and integrate your changes into my local
> branch. This fixes the three issues you reported: FORCE_RELEASE, issue
> with INVALID partitions issue (for which I adapted your patch into an
> earlier patch in my series), and progress reporting. And rebased.
Hi.
Thank you for the effort.
I've looked through and tested new patch a bit. Overall it looks good to
me.
The question I have is whether we should update
pg_stat_progress_create_index in reindex_invalid_child_indexes(), when
we skip valid indexes?
--
Best regards,
Alexander Pyhalov,
Postgres Professional
^ permalink raw reply [nested|flat] 22+ messages in thread
* Re: CREATE INDEX CONCURRENTLY on partitioned index
2020-10-31 06:31 CREATE INDEX CONCURRENTLY on partitioned index Justin Pryzby <[email protected]>
2020-11-29 20:22 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Justin Pryzby <[email protected]>
2021-01-28 12:51 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Masahiko Sawada <[email protected]>
2021-01-28 14:30 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Justin Pryzby <[email protected]>
2021-02-15 19:07 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Anastasia Lubennikova <[email protected]>
2021-02-26 18:20 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Justin Pryzby <[email protected]>
2022-06-28 18:33 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Justin Pryzby <[email protected]>
2022-06-29 08:01 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Alexander Pyhalov <[email protected]>
2022-11-21 03:00 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Justin Pryzby <[email protected]>
@ 2022-12-03 15:13 ` Ilya Gladyshev <[email protected]>
2022-12-04 19:09 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Justin Pryzby <[email protected]>
1 sibling, 1 reply; 22+ messages in thread
From: Ilya Gladyshev @ 2022-12-03 15:13 UTC (permalink / raw)
To: Justin Pryzby <[email protected]>; Alexander Pyhalov <[email protected]>; +Cc: [email protected]; Masahiko Sawada <[email protected]>; Michael Paquier <[email protected]>; 李杰(慎追) <[email protected]>; 曾文旌(义从) <[email protected]>; Alvaro Herrera <[email protected]>
Hi,
Thank you Justin and Alexander for working on this, I have reviewed and
tested the latest patch, it works well, the problems mentioned
previously are all fixed. I like the idea of sharing code of reindex
and index, but I have noticed some peculiarities as a user.
The reporting is somewhat confusing as it switches to reporting for
reindex concurrently while building child indexes, this should be fixed
with the simple patch I have attached. Another thing that I have
noticed is that REINDEX, which is used under the hood, creates new
indexes with suffix _ccnew, and if the index building fails, the
indexes that could not be build will have the name with _ccnew suffix.
This can actually be seen in your test:
ERROR: could not create unique index "idxpart2_a_idx2_ccnew"
I find it quite confusing and I don't think that this the expected
behavior (if it is, I think it should be documented, like it is for
REINDEX). As an example of problems that it might entail, DROP INDEX
will not drop all the invalid indexes in the inheritance tree, because
it will leave _ccnew indexes in place, which is ok for reindex
concurrently, but that's not how C-I-C works now. I think that fixing
this problem requires some heavy code rewrite and I'm not quite sure
how to go about it, if you have any ideas, I will be happy to try them
out.
Thanks,
Ilya
Attachments:
[text/x-patch] 0001-turn-off-reindex-reporting-for-create.patch (3.4K, ../../[email protected]/2-0001-turn-off-reindex-reporting-for-create.patch)
download | inline diff:
From 8eb9fd7ce7d34c5c323c47b60a7f883f360ef090 Mon Sep 17 00:00:00 2001
From: Ilya Gladyshev <[email protected]>
Date: Sat, 3 Dec 2022 18:20:03 +0400
Subject: [PATCH] turn off reindex reporting for create
---
src/backend/commands/indexcmds.c | 36 +++++++++++++++++++-------------
1 file changed, 21 insertions(+), 15 deletions(-)
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index a2775931e2..b3c713037f 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -3804,14 +3804,16 @@ ReindexRelationConcurrently(Oid relationOid, ReindexParams *params)
/* Don't overwrite CREATE INDEX command */
if (!(params->options & REINDEXOPT_REPORT_CREATE_PART))
+ {
pgstat_progress_start_command(PROGRESS_COMMAND_CREATE_INDEX,
idx->tableId);
- progress_vals[0] = PROGRESS_CREATEIDX_COMMAND_REINDEX_CONCURRENTLY;
- progress_vals[1] = 0; /* initializing */
- progress_vals[2] = idx->indexId;
- progress_vals[3] = idx->amId;
- pgstat_progress_update_multi_param(4, progress_index, progress_vals);
+ progress_vals[0] = PROGRESS_CREATEIDX_COMMAND_REINDEX_CONCURRENTLY;
+ progress_vals[1] = 0; /* initializing */
+ progress_vals[2] = idx->indexId;
+ progress_vals[3] = idx->amId;
+ pgstat_progress_update_multi_param(4, progress_index, progress_vals);
+ }
/* Choose a temporary relation name for the new index */
concurrentName = ChooseRelationName(get_rel_name(idx->indexId),
@@ -3967,13 +3969,15 @@ ReindexRelationConcurrently(Oid relationOid, ReindexParams *params)
* table involved. Don't overwrite CREATE INDEX command.
*/
if (!(params->options & REINDEXOPT_REPORT_CREATE_PART))
+ {
pgstat_progress_start_command(PROGRESS_COMMAND_CREATE_INDEX, newidx->tableId);
- progress_vals[0] = PROGRESS_CREATEIDX_COMMAND_REINDEX_CONCURRENTLY;
- progress_vals[1] = PROGRESS_CREATEIDX_PHASE_BUILD;
- progress_vals[2] = newidx->indexId;
- progress_vals[3] = newidx->amId;
- pgstat_progress_update_multi_param(4, progress_index, progress_vals);
+ progress_vals[0] = PROGRESS_CREATEIDX_COMMAND_REINDEX_CONCURRENTLY;
+ progress_vals[1] = PROGRESS_CREATEIDX_PHASE_BUILD;
+ progress_vals[2] = newidx->indexId;
+ progress_vals[3] = newidx->amId;
+ pgstat_progress_update_multi_param(4, progress_index, progress_vals);
+ }
/* Perform concurrent build of new index */
index_concurrently_build(newidx->tableId, newidx->indexId);
@@ -4033,14 +4037,16 @@ ReindexRelationConcurrently(Oid relationOid, ReindexParams *params)
* table involved. Don't overwrite CREATE INDEX command.
*/
if (!(params->options & REINDEXOPT_REPORT_CREATE_PART))
+ {
pgstat_progress_start_command(PROGRESS_COMMAND_CREATE_INDEX,
newidx->tableId);
- progress_vals[0] = PROGRESS_CREATEIDX_COMMAND_REINDEX_CONCURRENTLY;
- progress_vals[1] = PROGRESS_CREATEIDX_PHASE_VALIDATE_IDXSCAN;
- progress_vals[2] = newidx->indexId;
- progress_vals[3] = newidx->amId;
- pgstat_progress_update_multi_param(4, progress_index, progress_vals);
+ progress_vals[0] = PROGRESS_CREATEIDX_COMMAND_REINDEX_CONCURRENTLY;
+ progress_vals[1] = PROGRESS_CREATEIDX_PHASE_VALIDATE_IDXSCAN;
+ progress_vals[2] = newidx->indexId;
+ progress_vals[3] = newidx->amId;
+ pgstat_progress_update_multi_param(4, progress_index, progress_vals);
+ }
validate_index(newidx->tableId, newidx->indexId, snapshot);
--
2.30.2
^ permalink raw reply [nested|flat] 22+ messages in thread
* Re: CREATE INDEX CONCURRENTLY on partitioned index
2020-10-31 06:31 CREATE INDEX CONCURRENTLY on partitioned index Justin Pryzby <[email protected]>
2020-11-29 20:22 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Justin Pryzby <[email protected]>
2021-01-28 12:51 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Masahiko Sawada <[email protected]>
2021-01-28 14:30 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Justin Pryzby <[email protected]>
2021-02-15 19:07 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Anastasia Lubennikova <[email protected]>
2021-02-26 18:20 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Justin Pryzby <[email protected]>
2022-06-28 18:33 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Justin Pryzby <[email protected]>
2022-06-29 08:01 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Alexander Pyhalov <[email protected]>
2022-11-21 03:00 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Justin Pryzby <[email protected]>
2022-12-03 15:13 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Ilya Gladyshev <[email protected]>
@ 2022-12-04 19:09 ` Justin Pryzby <[email protected]>
2022-12-09 13:53 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Ilya Gladyshev <[email protected]>
2024-01-29 12:43 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Alexander Pyhalov <[email protected]>
0 siblings, 2 replies; 22+ messages in thread
From: Justin Pryzby @ 2022-12-04 19:09 UTC (permalink / raw)
To: Ilya Gladyshev <[email protected]>; +Cc: Alexander Pyhalov <[email protected]>; [email protected]; Masahiko Sawada <[email protected]>; Michael Paquier <[email protected]>; 李杰(慎追) <[email protected]>; 曾文旌(义从) <[email protected]>; Alvaro Herrera <[email protected]>
On Sat, Dec 03, 2022 at 07:13:30PM +0400, Ilya Gladyshev wrote:
> Hi,
>
> Thank you Justin and Alexander for working on this, I have reviewed and
> tested the latest patch, it works well, the problems mentioned
> previously are all fixed. I like the idea of sharing code of reindex
> and index, but I have noticed some peculiarities as a user.
>
> The reporting is somewhat confusing as it switches to reporting for
> reindex concurrently while building child indexes, this should be fixed
> with the simple patch I have attached. Another thing that I have
> noticed is that REINDEX, which is used under the hood, creates new
> indexes with suffix _ccnew, and if the index building fails, the
> indexes that could not be build will have the name with _ccnew suffix.
> This can actually be seen in your test:
>
> ERROR: could not create unique index "idxpart2_a_idx2_ccnew"
> I find it quite confusing and I don't think that this the expected
> behavior (if it is, I think it should be documented, like it is for
> REINDEX). As an example of problems that it might entail, DROP INDEX
> will not drop all the invalid indexes in the inheritance tree, because
> it will leave _ccnew indexes in place, which is ok for reindex
> concurrently, but that's not how C-I-C works now. I think that fixing
> this problem requires some heavy code rewrite and I'm not quite sure
This beavior is fixed. I re-factored and re-implented to use
DefineIndex() for building indexes concurrently rather than reindexing.
That makes the patch smaller, actually, and has the added benefit of
splitting off the "Concurrently" part of DefineIndex() into a separate
function.
This currently handles partitions with a loop around the whole CIC
implementation, which means that things like WaitForLockers() happen
once for each index, the same as REINDEX CONCURRENTLY on a partitioned
table. Contrast that with ReindexRelationConcurrently(), which handles
all the indexes on a table in one pass by looping around indexes within
each phase.
BTW, it causes the patch to fail to apply in cfbot when you send an
additional (002) supplementary patch without including the original
(001) patch. You can name it *.txt to avoid the issue.
https://wiki.postgresql.org/wiki/Cfbot#Which_attachments_are_considered_to_be_patches.3F
Thanks for looking.
--
Justin
^ permalink raw reply [nested|flat] 22+ messages in thread
* Re: CREATE INDEX CONCURRENTLY on partitioned index
2020-10-31 06:31 CREATE INDEX CONCURRENTLY on partitioned index Justin Pryzby <[email protected]>
2020-11-29 20:22 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Justin Pryzby <[email protected]>
2021-01-28 12:51 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Masahiko Sawada <[email protected]>
2021-01-28 14:30 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Justin Pryzby <[email protected]>
2021-02-15 19:07 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Anastasia Lubennikova <[email protected]>
2021-02-26 18:20 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Justin Pryzby <[email protected]>
2022-06-28 18:33 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Justin Pryzby <[email protected]>
2022-06-29 08:01 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Alexander Pyhalov <[email protected]>
2022-11-21 03:00 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Justin Pryzby <[email protected]>
2022-12-03 15:13 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Ilya Gladyshev <[email protected]>
2022-12-04 19:09 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Justin Pryzby <[email protected]>
@ 2022-12-09 13:53 ` Ilya Gladyshev <[email protected]>
1 sibling, 0 replies; 22+ messages in thread
From: Ilya Gladyshev @ 2022-12-09 13:53 UTC (permalink / raw)
To: Justin Pryzby <[email protected]>; +Cc: Alexander Pyhalov <[email protected]>; [email protected]; Masahiko Sawada <[email protected]>; Michael Paquier <[email protected]>; 李杰(慎追) <[email protected]>; 曾文旌(义从) <[email protected]>; Alvaro Herrera <[email protected]>
On Sun, 2022-12-04 at 13:09 -0600, Justin Pryzby wrote:
>
> This beavior is fixed. I re-factored and re-implented to use
> DefineIndex() for building indexes concurrently rather than
> reindexing.
> That makes the patch smaller, actually, and has the added benefit of
> splitting off the "Concurrently" part of DefineIndex() into a
> separate
> function.
Nice, I think it turned out pretty concise. I played around with the
patch quite a bit, didn't find any major problems, the only minor thing
that I can note is that we should skip the top parent index itself in
the loop not to increment the pg_stat counter, something like this:
diff --git a/src/backend/commands/indexcmds.c
b/src/backend/commands/indexcmds.c
index cfab45b999..9049540b5b 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -1515,6 +1515,9 @@ DefineIndex(Oid relationId,
Oid indrelid =
lfirst_oid(lc);
Oid tabrelid =
IndexGetRelation(indrelid, false);
+ if (indrelid == indexRelationId)
+ continue;
+
if
(RELKIND_HAS_STORAGE(get_rel_relkind(indrelid)) &&
!get_index_isvalid(indrelid))
{
>
> BTW, it causes the patch to fail to apply in cfbot when you send an
> additional (002) supplementary patch without including the original
> (001) patch. You can name it *.txt to avoid the issue.
>
> https://wiki.postgresql.org/wiki/Cfbot#Which_attachments_are_considered_to_be_patches.3F
>
> Thanks for looking.
>
My bad, didn't know about this, thanks for the link.
On a side note, I noticed that reindex behaviour is strange on
partitioned tables, it doesn't mark partitioned tables as valid after
reindexing children, as I could understand from the code and mailing
lists, this is the intended behaviour, but I can't quite understand the
rationale for it, do you know why it is done this way?
^ permalink raw reply [nested|flat] 22+ messages in thread
* Re: CREATE INDEX CONCURRENTLY on partitioned index
2020-10-31 06:31 CREATE INDEX CONCURRENTLY on partitioned index Justin Pryzby <[email protected]>
2020-11-29 20:22 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Justin Pryzby <[email protected]>
2021-01-28 12:51 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Masahiko Sawada <[email protected]>
2021-01-28 14:30 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Justin Pryzby <[email protected]>
2021-02-15 19:07 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Anastasia Lubennikova <[email protected]>
2021-02-26 18:20 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Justin Pryzby <[email protected]>
2022-06-28 18:33 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Justin Pryzby <[email protected]>
2022-06-29 08:01 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Alexander Pyhalov <[email protected]>
2022-11-21 03:00 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Justin Pryzby <[email protected]>
2022-12-03 15:13 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Ilya Gladyshev <[email protected]>
2022-12-04 19:09 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Justin Pryzby <[email protected]>
@ 2024-01-29 12:43 ` Alexander Pyhalov <[email protected]>
2025-07-01 06:43 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Alexander Pyhalov <[email protected]>
1 sibling, 1 reply; 22+ messages in thread
From: Alexander Pyhalov @ 2024-01-29 12:43 UTC (permalink / raw)
To: Justin Pryzby <[email protected]>; +Cc: Ilya Gladyshev <[email protected]>; [email protected]; Masahiko Sawada <[email protected]>; Michael Paquier <[email protected]>; 李杰(慎追) <[email protected]>; 曾文旌(义从) <[email protected]>; Alvaro Herrera <[email protected]>
Hi.
I've rebased patch on master and it'seems to me there's one more issue -
when we call DefineIndexConcurrentInternal() in partitioned case, it
waits for transactions, locking tableId, not tabrelid - heaprelid
LockRelId is constructed for parent index relation, not for child index
relation.
Attaching fixed version.
Also I'm not sure what to do with locking of child relations. If we
don't do anything, you can drop one of the partitioned table childs
while CIC is in progress, and get error
ERROR: cache lookup failed for index 16399
If you try to lock all child tables in CIC session, you'll get
deadlocks.
--
Best regards,
Alexander Pyhalov,
Postgres Professional
Attachments:
[text/x-diff] 0001-Allow-CREATE-INDEX-CONCURRENTLY-on-partitioned-table.patch (19.6K, ../../[email protected]/2-0001-Allow-CREATE-INDEX-CONCURRENTLY-on-partitioned-table.patch)
download | inline diff:
From 37a13b7fa1c3277b9d038b7a0c75399ff05b28a7 Mon Sep 17 00:00:00 2001
From: Justin Pryzby <[email protected]>
Date: Mon, 29 Jan 2024 10:41:01 +0300
Subject: [PATCH] Allow CREATE INDEX CONCURRENTLY on partitioned table
---
doc/src/sgml/ddl.sgml | 4 +-
doc/src/sgml/ref/create_index.sgml | 14 +-
src/backend/commands/indexcmds.c | 200 ++++++++++++++++++-------
src/test/regress/expected/indexing.out | 127 +++++++++++++++-
src/test/regress/sql/indexing.sql | 26 +++-
5 files changed, 296 insertions(+), 75 deletions(-)
diff --git a/doc/src/sgml/ddl.sgml b/doc/src/sgml/ddl.sgml
index 075ff329912..8ee80c40e3b 100644
--- a/doc/src/sgml/ddl.sgml
+++ b/doc/src/sgml/ddl.sgml
@@ -4194,9 +4194,7 @@ ALTER TABLE measurement ATTACH PARTITION measurement_y2008m02
so that they are applied automatically to the entire hierarchy.
This is very
convenient, as not only will the existing partitions become indexed, but
- also any partitions that are created in the future will. One limitation is
- that it's not possible to use the <literal>CONCURRENTLY</literal>
- qualifier when creating such a partitioned index. To avoid long lock
+ also any partitions that are created in the future will. To avoid long lock
times, it is possible to use <command>CREATE INDEX ON ONLY</command>
the partitioned table; such an index is marked invalid, and the partitions
do not get the index applied automatically. The indexes on partitions can
diff --git a/doc/src/sgml/ref/create_index.sgml b/doc/src/sgml/ref/create_index.sgml
index 40986aa502f..b05102efdaf 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 ab8b81b3020..65477aeb3a8 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -93,6 +93,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 RangeVar *indexRelation, const ReindexParams *params,
bool isTopLevel);
static void RangeVarCallbackForReindexIndex(const RangeVar *relation,
@@ -554,7 +559,6 @@ DefineIndex(Oid tableId,
bool amissummarizing;
amoptions_function amoptions;
bool partitioned;
- bool safe_index;
Datum reloptions;
int16 *coloptions;
IndexInfo *indexInfo;
@@ -562,12 +566,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;
@@ -697,20 +699,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.
@@ -1100,10 +1088,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)
@@ -1168,6 +1152,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;
@@ -1516,21 +1505,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
@@ -1542,37 +1517,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;
@@ -1583,6 +1570,113 @@ 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, ShareLock, 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;
+ }
+
+ tabrelid = IndexGetRelation(indrelid, false);
+ rel = table_open(tabrelid, ShareUpdateExclusiveLock);
+ heaprelid = rel->rd_lockInfo.lockRelId;
+ table_close(rel, ShareUpdateExclusiveLock);
+ 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);
+
+ index_set_state_flags(indrelid, INDEX_CREATE_SET_READY);
+ CommandCounterIncrement();
+ index_set_state_flags(indrelid, INDEX_CREATE_SET_VALID);
+ }
+
+ 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
@@ -1778,10 +1872,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 087f955b1e6..5d529454993 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/[email protected]
diff --git a/src/test/regress/sql/indexing.sql b/src/test/regress/sql/indexing.sql
index 44f6788915c..96c63615e9a 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.34.1
^ permalink raw reply [nested|flat] 22+ messages in thread
* Re: CREATE INDEX CONCURRENTLY on partitioned index
2020-10-31 06:31 CREATE INDEX CONCURRENTLY on partitioned index Justin Pryzby <[email protected]>
2020-11-29 20:22 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Justin Pryzby <[email protected]>
2021-01-28 12:51 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Masahiko Sawada <[email protected]>
2021-01-28 14:30 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Justin Pryzby <[email protected]>
2021-02-15 19:07 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Anastasia Lubennikova <[email protected]>
2021-02-26 18:20 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Justin Pryzby <[email protected]>
2022-06-28 18:33 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Justin Pryzby <[email protected]>
2022-06-29 08:01 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Alexander Pyhalov <[email protected]>
2022-11-21 03:00 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Justin Pryzby <[email protected]>
2022-12-03 15:13 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Ilya Gladyshev <[email protected]>
2022-12-04 19:09 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Justin Pryzby <[email protected]>
2024-01-29 12:43 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Alexander Pyhalov <[email protected]>
@ 2025-07-01 06:43 ` Alexander Pyhalov <[email protected]>
2026-05-28 13:55 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Alexander Pyhalov <[email protected]>
0 siblings, 1 reply; 22+ messages in thread
From: Alexander Pyhalov @ 2025-07-01 06:43 UTC (permalink / raw)
To: Ilya Gladyshev <[email protected]>; +Cc: Michael Paquier <[email protected]>; Justin Pryzby <[email protected]>; [email protected]; Masahiko Sawada <[email protected]>
Ilya Gladyshev писал(а) 2024-07-13 01:17:
> On 12.07.2024 01:01, Michael Paquier wrote:
>
>> Please let's move this point to its own thread and deal with it with
>> an independent patch. Hiding that in a thread that's already quite
>> long is not a good idea. This needs proper review, and a separate
>> thread with a good subject to describe the problem will attract a
>> better audience to deal with the problem you are seeing.
>>
>> I was not paying much attention, until you've mentioned that this
>> was
>> an issue with HEAD.
>> --
>> Michael
>
> Sure, created a separate thread [1]. Please disregard the second patch
> in this thread. Duplicating the last version of the relevant patch
> here to avoid any confusion.
>
> [1]
> https://www.postgresql.org/message-id/b72f2d89-820a-4fa2-9058-b155cf646f4f%40gmail.com
Hi. We've found an issue with this patch. CIC can pop last active
snapshot when operating on partitioned table without leaves. This seems
to be similar to the issue, fixed by
commit c426f7c2b36a5efd9bcef2a2dfcc559f7879cd84
Author: Michael Paquier <[email protected]>
Date: Thu Dec 7 08:31:02 2023 +0900
Fix assertion failure with REINDEX and event triggers
Attaching rebased patch with the fix.
--
Best regards,
Alexander Pyhalov,
Postgres Professional
Attachments:
[text/x-diff] v6-0001-Allow-CREATE-INDEX-CONCURRENTLY-on-partitioned-ta.patch (28.9K, ../../[email protected]/2-v6-0001-Allow-CREATE-INDEX-CONCURRENTLY-on-partitioned-ta.patch)
download | inline diff:
From a29654340a50f29cb31a665899bf4635fee295b8 Mon Sep 17 00:00:00 2001
From: Ilya Gladyshev <[email protected]>
Date: Tue, 1 Jul 2025 08:59:02 +0300
Subject: [PATCH] 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 | 231 +++++++++++++-----
.../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 | 132 +++++++++-
src/test/regress/sql/indexing.sql | 42 +++-
8 files changed, 543 insertions(+), 79 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 65bc070d2e5..85c53f93cd5 100644
--- a/doc/src/sgml/ddl.sgml
+++ b/doc/src/sgml/ddl.sgml
@@ -4418,14 +4418,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 b9c679c41e8..253e62e7348 100644
--- a/doc/src/sgml/ref/create_index.sgml
+++ b/doc/src/sgml/ref/create_index.sgml
@@ -651,7 +651,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>:
@@ -698,15 +701,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 6f753ab6d7a..c39b3b18ea8 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -98,6 +98,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,
@@ -573,7 +578,6 @@ DefineIndex(Oid tableId,
amoptions_function amoptions;
bool exclusion;
bool partitioned;
- bool safe_index;
Datum reloptions;
int16 *coloptions;
IndexInfo *indexInfo;
@@ -581,12 +585,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;
@@ -724,20 +726,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.
@@ -1166,10 +1154,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)
@@ -1234,6 +1218,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;
@@ -1551,21 +1540,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
@@ -1577,37 +1552,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;
@@ -1618,6 +1605,144 @@ 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;
+ bool pushed_snapshot = false;
+
+ /* 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());
+ pushed_snapshot = true;
+ 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();
+ if (pushed_snapshot)
+ 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
@@ -1821,10 +1946,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 00000000000..b66acc6f6a2
--- /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 e3c669a29c7..270efefdd71 100644
--- a/src/test/isolation/isolation_schedule
+++ b/src/test/isolation/isolation_schedule
@@ -116,3 +116,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 00000000000..95f0bb2b47a
--- /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 bcf1db11d73..ff448e0f988 100644
--- a/src/test/regress/expected/indexing.out
+++ b/src/test/regress/expected/indexing.out
@@ -50,12 +50,136 @@ 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)
+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)
+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)
+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)
+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)
+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)
+
+\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)
+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)
+
+drop table idxpart;
+-- CIC should preserve active snapshot
+create function access_snapshot() returns event_trigger language plpgsql as $$
+begin
+perform 1;
+end
+$$;
+create table idxpart (col1 int) partition by range (col1);
+create event trigger check_trigger on ddl_command_end when tag in ('CREATE INDEX')
+execute procedure access_snapshot();
+create index concurrently on idxpart (col1);
+drop event trigger check_trigger;
drop table idxpart;
+drop function access_snapshot();
-- Verify bugfix with query on indexed partitioned table with no partitions
-- https://postgr.es/m/[email protected]
CREATE TABLE idxpart (col1 INT) PARTITION BY RANGE (col1);
diff --git a/src/test/regress/sql/indexing.sql b/src/test/regress/sql/indexing.sql
index b5cb01c2d70..b6b76424e76 100644
--- a/src/test/regress/sql/indexing.sql
+++ b/src/test/regress/sql/indexing.sql
@@ -29,11 +29,45 @@ 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);
-drop table idxpart;
+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;
+-- CIC should preserve active snapshot
+create function access_snapshot() returns event_trigger language plpgsql as $$
+begin
+perform 1;
+end
+$$;
+create table idxpart (col1 int) partition by range (col1);
+create event trigger check_trigger on ddl_command_end when tag in ('CREATE INDEX')
+execute procedure access_snapshot();
+create index concurrently on idxpart (col1);
+drop event trigger check_trigger;
+drop table idxpart;
+drop function access_snapshot();
+
-- Verify bugfix with query on indexed partitioned table with no partitions
-- https://postgr.es/m/[email protected]
--
2.43.0
^ permalink raw reply [nested|flat] 22+ messages in thread
* Re: CREATE INDEX CONCURRENTLY on partitioned index
2020-10-31 06:31 CREATE INDEX CONCURRENTLY on partitioned index Justin Pryzby <[email protected]>
2020-11-29 20:22 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Justin Pryzby <[email protected]>
2021-01-28 12:51 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Masahiko Sawada <[email protected]>
2021-01-28 14:30 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Justin Pryzby <[email protected]>
2021-02-15 19:07 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Anastasia Lubennikova <[email protected]>
2021-02-26 18:20 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Justin Pryzby <[email protected]>
2022-06-28 18:33 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Justin Pryzby <[email protected]>
2022-06-29 08:01 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Alexander Pyhalov <[email protected]>
2022-11-21 03:00 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Justin Pryzby <[email protected]>
2022-12-03 15:13 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Ilya Gladyshev <[email protected]>
2022-12-04 19:09 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Justin Pryzby <[email protected]>
2024-01-29 12:43 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Alexander Pyhalov <[email protected]>
2025-07-01 06:43 ` Re: CREATE INDEX CONCURRENTLY on partitioned index Alexander Pyhalov <[email protected]>
@ 2026-05-28 13:55 ` Alexander Pyhalov <[email protected]>
0 siblings, 0 replies; 22+ messages in thread
From: Alexander Pyhalov @ 2026-05-28 13:55 UTC (permalink / raw)
To: Ilya Gladyshev <[email protected]>; +Cc: Michael Paquier <[email protected]>; Justin Pryzby <[email protected]>; [email protected]; Masahiko Sawada <[email protected]>
Alexander Pyhalov писал(а) 2025-07-01 09:43:
> Ilya Gladyshev писал(а) 2024-07-13 01:17:
>> On 12.07.2024 01:01, Michael Paquier wrote:
>>
>>> Please let's move this point to its own thread and deal with it with
>>> an independent patch. Hiding that in a thread that's already quite
>>> long is not a good idea. This needs proper review, and a separate
>>> thread with a good subject to describe the problem will attract a
>>> better audience to deal with the problem you are seeing.
>>>
>>> I was not paying much attention, until you've mentioned that this
>>> was
>>> an issue with HEAD.
>>> --
>>> Michael
>>
>> Sure, created a separate thread [1]. Please disregard the second patch
>> in this thread. Duplicating the last version of the relevant patch
>> here to avoid any confusion.
>>
>> [1]
>> https://www.postgresql.org/message-id/b72f2d89-820a-4fa2-9058-b155cf646f4f%40gmail.com
>
> Hi. We've found an issue with this patch. CIC can pop last active
> snapshot when operating on partitioned table without leaves. This seems
> to be similar to the issue, fixed by
...
Hi.
The patch evidently fails to do concurrent index creation on master, as
childStmt looses "concurrent" property.
Attaching fixed patch.
--
Best regards,
Alexander Pyhalov,
Postgres Professional
Attachments:
[text/x-diff] v7-0001-Allow-CREATE-INDEX-CONCURRENTLY-on-partitioned-table.patch (29.6K, ../../[email protected]/2-v7-0001-Allow-CREATE-INDEX-CONCURRENTLY-on-partitioned-table.patch)
download | inline diff:
From 35d5256b066cc58ad93f7c89f00bd360c4f40afc Mon Sep 17 00:00:00 2001
From: Alexander Pyhalov <[email protected]>
Date: Thu, 28 May 2026 16:47:03 +0300
Subject: [PATCH] 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 | 233 +++++++++++++-----
.../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 | 140 ++++++++++-
src/test/regress/sql/indexing.sql | 40 ++-
8 files changed, 552 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 747f929aee3..aa9bf6d037b 100644
--- a/doc/src/sgml/ddl.sgml
+++ b/doc/src/sgml/ddl.sgml
@@ -4722,14 +4722,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 bb7505d171b..e6b62b73fd9 100644
--- a/doc/src/sgml/ref/create_index.sgml
+++ b/doc/src/sgml/ref/create_index.sgml
@@ -651,7 +651,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>:
@@ -698,15 +701,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 9ab74c8df0a..7635cc10425 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -99,6 +99,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,
@@ -577,7 +582,6 @@ DefineIndex(ParseState *pstate,
amoptions_function amoptions;
bool exclusion;
bool partitioned;
- bool safe_index;
Datum reloptions;
int16 *coloptions;
IndexInfo *indexInfo;
@@ -585,12 +589,10 @@ DefineIndex(ParseState *pstate,
uint16 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;
@@ -728,20 +730,6 @@ DefineIndex(ParseState *pstate,
* 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.
@@ -1175,10 +1163,6 @@ DefineIndex(ParseState *pstate,
}
}
- /* 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)
@@ -1243,6 +1227,11 @@ DefineIndex(ParseState *pstate,
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;
@@ -1519,6 +1508,8 @@ DefineIndex(ParseState *pstate,
parentIndex,
attmap,
NULL);
+ /* Preserve concurrent property from the original statement. */
+ childStmt->concurrent = stmt->concurrent;
/*
* Recurse as the starting user ID. Callee will use that
@@ -1561,21 +1552,7 @@ DefineIndex(ParseState *pstate,
*/
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
@@ -1587,37 +1564,49 @@ DefineIndex(ParseState *pstate,
/*
* 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;
@@ -1628,6 +1617,144 @@ DefineIndex(ParseState *pstate,
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;
+ bool pushed_snapshot = false;
+
+ /* 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());
+ pushed_snapshot = true;
+ 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();
+ if (pushed_snapshot)
+ 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
@@ -1832,10 +1959,6 @@ DefineIndex(ParseState *pstate,
* 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 00000000000..b66acc6f6a2
--- /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 15c33fad4c5..772e2e3cd41 100644
--- a/src/test/isolation/isolation_schedule
+++ b/src/test/isolation/isolation_schedule
@@ -127,3 +127,4 @@ test: matview-write-skew
test: lock-nowait
test: for-portion-of
test: ddl-dependency-locking
+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 00000000000..95f0bb2b47a
--- /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 929feda6fa3..e39420242e1 100644
--- a/src/test/regress/expected/indexing.out
+++ b/src/test/regress/expected/indexing.out
@@ -50,12 +50,144 @@ 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;
+-- CIC should preserve active snapshot
+create function access_snapshot() returns event_trigger language plpgsql as $$
+begin
+perform 1;
+end
+$$;
+create table idxpart (col1 int) partition by range (col1);
+create event trigger check_trigger on ddl_command_end when tag in ('CREATE INDEX')
+execute procedure access_snapshot();
+create index concurrently on idxpart (col1);
+drop event trigger check_trigger;
drop table idxpart;
+drop function access_snapshot();
-- Verify bugfix with query on indexed partitioned table with no partitions
-- https://postgr.es/m/[email protected]
CREATE TABLE idxpart (col1 INT) PARTITION BY RANGE (col1);
diff --git a/src/test/regress/sql/indexing.sql b/src/test/regress/sql/indexing.sql
index 3d43af3323c..86d2aa15ba3 100644
--- a/src/test/regress/sql/indexing.sql
+++ b/src/test/regress/sql/indexing.sql
@@ -29,11 +29,45 @@ 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;
+-- CIC should preserve active snapshot
+create function access_snapshot() returns event_trigger language plpgsql as $$
+begin
+perform 1;
+end
+$$;
+create table idxpart (col1 int) partition by range (col1);
+create event trigger check_trigger on ddl_command_end when tag in ('CREATE INDEX')
+execute procedure access_snapshot();
+create index concurrently on idxpart (col1);
+drop event trigger check_trigger;
drop table idxpart;
+drop function access_snapshot();
+
-- Verify bugfix with query on indexed partitioned table with no partitions
-- https://postgr.es/m/[email protected]
--
2.43.0
^ permalink raw reply [nested|flat] 22+ messages in thread
end of thread, other threads:[~2026-05-28 13:55 UTC | newest]
Thread overview: 22+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2020-10-31 06:31 CREATE INDEX CONCURRENTLY on partitioned index Justin Pryzby <[email protected]>
2020-11-29 20:22 ` Justin Pryzby <[email protected]>
2021-01-28 12:51 ` Masahiko Sawada <[email protected]>
2021-01-28 14:30 ` Justin Pryzby <[email protected]>
2021-02-15 19:06 ` Anastasia Lubennikova <[email protected]>
2021-02-15 19:33 ` Justin Pryzby <[email protected]>
2021-02-15 21:37 ` Zhihong Yu <[email protected]>
2021-02-15 19:07 ` Anastasia Lubennikova <[email protected]>
2021-02-26 18:20 ` Justin Pryzby <[email protected]>
2022-03-25 05:05 ` Greg Stark <[email protected]>
2022-03-25 13:13 ` Justin Pryzby <[email protected]>
2022-06-28 18:33 ` Justin Pryzby <[email protected]>
2022-06-29 06:45 ` Alexander Pyhalov <[email protected]>
2022-06-29 08:01 ` Alexander Pyhalov <[email protected]>
2022-11-21 03:00 ` Justin Pryzby <[email protected]>
2022-11-21 14:44 ` Alexander Pyhalov <[email protected]>
2022-12-03 15:13 ` Ilya Gladyshev <[email protected]>
2022-12-04 19:09 ` Justin Pryzby <[email protected]>
2022-12-09 13:53 ` Ilya Gladyshev <[email protected]>
2024-01-29 12:43 ` Alexander Pyhalov <[email protected]>
2025-07-01 06:43 ` Alexander Pyhalov <[email protected]>
2026-05-28 13:55 ` Alexander Pyhalov <[email protected]>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox