From: | Nathan Bossart <nathandbossart(at)gmail(dot)com> |
---|---|
To: | Michael Paquier <michael(at)paquier(dot)xyz> |
Cc: | Jeff Davis <pgsql(at)j-davis(dot)com>, Ted Yu <yuzhihong(at)gmail(dot)com>, Pavel Luzanov <p(dot)luzanov(at)postgrespro(dot)ru>, Justin Pryzby <pryzby(at)telsasoft(dot)com>, pgsql-hackers(at)postgresql(dot)org |
Subject: | Re: allow granting CLUSTER, REFRESH MATERIALIZED VIEW, and REINDEX |
Date: | 2023-06-22 03:06:06 |
Message-ID: | 20230622030606.GA1074545@nathanxps13 |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Thu, Jun 22, 2023 at 10:46:41AM +0900, Michael Paquier wrote:
> On Wed, Jun 21, 2023 at 10:16:24AM -0700, Nathan Bossart wrote:
>>> I think that there is a testing gap with the coverage of CLUSTER.
>>> "Ownership of partitions is checked" is a test that looks for the case
>>> where regress_ptnowner owns the partitioned table and one of its
>>> partitions, checking that the leaf not owned is skipped, but we don't
>>> have a test where we attempt a CLUSTER on the partitioned table with
>>> regress_ptnowner *not* owning the partitioned table, only one or more
>>> of its partitions owned by regress_ptnowner. In this case, the
>>> command would fail.
>>
>> We could add something for this, but it'd really just exercise the checks
>> in RangeVarCallbackMaintainsTable(), which already has a decent amount of
>> coverage.
>
> It seems to me that this has some value for the CLUSTER path, so I
> would add a small thing for it.
Done.
> - /*
> - * We already checked that the user has privileges to CLUSTER the
> - * partitioned table when we locked it earlier, so there's no need to
> - * check the privileges again here.
> - */
> + if (!cluster_is_permitted_for_relation(relid, GetUserId()))
> + continue;
> I would add a comment here that this ACL recheck for the leaves is an
> important thing to keep around as it impacts the case where the leaves
> have a different owner than the parent, and the owner of the parent
> clusters it. The only place in the tests where this has an influence
> is the isolation test cluster-conflict-partition.
Done.
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
Attachment | Content-Type | Size |
---|---|---|
v9-0001-partial-revert-of-ff9618e82a.patch | text/x-diff | 23.0 KB |
v9-0002-simplify-privilege-related-documentation-for-main.patch | text/x-diff | 8.3 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Amit Kapila | 2023-06-22 03:46:40 | Re: Assert while autovacuum was executing |
Previous Message | Masahiro Ikeda | 2023-06-22 03:06:04 | Re: Support to define custom wait events for extensions |