Refactor ReindexStmt and its "concurrent" boolean

Started by Michael Paquierover 5 years ago4 messageshackers
Jump to latest
#1Michael Paquier
michael@paquier.xyz

Hi all,

$subject has been mentioned a couple of times, including today:
/messages/by-id/20200902010012.GE1489@paquier.xyz

We have a boolean argument in ReindexStmt to control a concurrent
run, and we also have in parallel of that a bitmask to control the
options of the statement, which feels like a duplicate. Attached is a
patch to refactor the whole, adding CONCURRENTLY as a member of the
available options. This simplifies a bit the code.

Any thoughts?
--
Michael

Attachments:

reindex-concurrent-refactor-v1.patchtext/x-diff; charset=us-asciiDownload+36-27
#2Julien Rouhaud
rjuju123@gmail.com
In reply to: Michael Paquier (#1)
Re: Refactor ReindexStmt and its "concurrent" boolean

On Wed, Sep 2, 2020 at 1:03 PM Michael Paquier <michael@paquier.xyz> wrote:

Hi all,

$subject has been mentioned a couple of times, including today:
/messages/by-id/20200902010012.GE1489@paquier.xyz

We have a boolean argument in ReindexStmt to control a concurrent
run, and we also have in parallel of that a bitmask to control the
options of the statement, which feels like a duplicate. Attached is a
patch to refactor the whole, adding CONCURRENTLY as a member of the
available options. This simplifies a bit the code.

Any thoughts?

+1

 struct ReindexIndexCallbackState
 {
- bool concurrent; /* flag from statement */
+ bool options; /* flag from statement */
  Oid locked_table_oid; /* tracks previously locked table */
 };

Shouldn't options be an int? The rest of the patch looks good to me.

#3Michael Paquier
michael@paquier.xyz
In reply to: Julien Rouhaud (#2)
Re: Refactor ReindexStmt and its "concurrent" boolean

On Wed, Sep 02, 2020 at 01:17:32PM +0200, Julien Rouhaud wrote:

Shouldn't options be an int? The rest of the patch looks good to me.

It should, thanks for looking at it. Let's wait a couple of days and
see if others have any comments. If there are no objections, I'll try
to commit this one.
--
Michael

Attachments:

reindex-concurrent-refactor-v2.patchtext/x-diff; charset=us-asciiDownload+36-27
#4Michael Paquier
michael@paquier.xyz
In reply to: Michael Paquier (#3)
Re: Refactor ReindexStmt and its "concurrent" boolean

On Thu, Sep 03, 2020 at 11:14:38AM +0900, Michael Paquier wrote:

It should, thanks for looking at it. Let's wait a couple of days and
see if others have any comments. If there are no objections, I'll try
to commit this one.

And applied.
--
Michael