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
- v11-0001-Implement-CLUSTER-of-partitioned-table.patch (text/x-diff)
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
-
Add some isolation tests for CLUSTER
- 21a10368eb3f 15.0 landed
-
Have CLUSTER ignore partitions not owned by caller
- 3f19e176ae0f 15.0 landed
-
Remove "recheck" argument from check_index_is_clusterable()
- b940918dc888 15.0 landed
-
Allow CLUSTER on partitioned tables
- cfdd03f45e6a 15.0 landed
-
doc: Add backlinks to progress reporting documentation
- 8a8f4d8ede28 14.0 cited