Re: CLUSTER on partitioned index

Justin Pryzby <pryzby@telsasoft.com>

From: Justin Pryzby <pryzby@telsasoft.com>
To: Michael Paquier <michael@paquier.xyz>
Cc: Matthias van de Meent <boekewurm+postgres@gmail.com>, Alvaro Herrera <alvherre@2ndquadrant.com>, 李杰(慎追) <adger.lj@alibaba-inc.com>, pgsql-hackers@lists.postgresql.org, 曾文旌(义从) <wenjing.zwj@alibaba-inc.com>, Zhihong Yu <zyu@yugabyte.com>
Date: 2022-01-08T19:55:28Z
Lists: pgsql-hackers

Attachments

On Fri, Dec 03, 2021 at 10:16:24AM +0900, Michael Paquier wrote:
> On Thu, Sep 23, 2021 at 06:56:26PM -0500, Justin Pryzby wrote:
> > On Thu, Sep 23, 2021 at 08:18:41PM +0200, Matthias van de Meent wrote:
> >> Note: The following review is based on the assumption that this v11
> >> revision was meant to contain only one patch. I put this up as a note,
> >> because it seemed quite limited when compared to earlier versions of
> >> the patchset.
> > 
> > Alvaro's critique was that the patchset was too complicated for what was
> > claimed to be a marginal feature.  My response was to rearrange the patchset to
> > its minimal form, supporting CLUSTER without marking the index as clustered.
> >
> > My goal is to present a minimal patch and avoid any nonessential complexity.
> 
> FWIW, my opinion on the matter is similar to Alvaro's, and an extra
> read of the patch gives me the same impression.  Let's see if others
> have an opinion on the matter.

You and Alvaro thought the patch was too complicated for its value, so I
reduced the scope to its essential form.

CLUSTER was claimed to be of marginal utility, since the table is locked.
But locking one partition at a time would be less disruptive than locking an
equivalent non-partitioned table.

There's only about a dozen, other remaining restrictions/limitations on
partitioned tables, (AMs, triggers, identity, generated, exclusion, CIC,
FREEZE).  

Since it's supported to VACUUM (including VACUUM FULL) and REINDEX a
partitioned table, I'm still suprised there's much hesitation to support
CLUSTER (which is used by vacuum full).

> > Thanks for looking.  I'm going to see about updating comments based on
> > corresponding parts of vacuum and on this message itself.
> 
> It doesn't feel right to just discard the patch at this stage, and it
> needs an update, so I have moved it to the next CF for now, waiting on
> author.  If this does not really move on, my suggestion is to discard
> the patch at the end of next CF, aka 2022-01.

This includes minor updates based on Mathias review (commit message and test
case).

-- 
Justin

Commits

  1. Add some isolation tests for CLUSTER

  2. Have CLUSTER ignore partitions not owned by caller

  3. Remove "recheck" argument from check_index_is_clusterable()

  4. Allow CLUSTER on partitioned tables

  5. doc: Add backlinks to progress reporting documentation