Re: allow granting CLUSTER, REFRESH MATERIALIZED VIEW, and REINDEX

Поиск
Список
Период
Сортировка
От Michael Paquier
Тема Re: allow granting CLUSTER, REFRESH MATERIALIZED VIEW, and REINDEX
Дата
Msg-id ZJJQgBkTEvL8RpwP@paquier.xyz
обсуждение исходный текст
Ответ на Re: allow granting CLUSTER, REFRESH MATERIALIZED VIEW, and REINDEX  (Nathan Bossart <nathandbossart@gmail.com>)
Ответы Re: allow granting CLUSTER, REFRESH MATERIALIZED VIEW, and REINDEX  (Nathan Bossart <nathandbossart@gmail.com>)
Список pgsql-hackers
On Tue, Jun 20, 2023 at 03:52:57PM -0700, Nathan Bossart wrote:
> However, I do agree that it feels inconsistent.  Besides the options you
> proposed, we might also consider making REINDEX work a bit more like VACUUM
> and ANALYZE and emit a WARNING for any relations that the user is not
> permitted to process.  But this probably deserves its own thread, and it
> might even need to wait until v17.

Looking at 0001..

-step s2_auth           { SET ROLE regress_cluster_part; }
+step s2_auth           { SET ROLE regress_cluster_part; SET client_min_messages = ERROR; }

Is this change necessary because the ordering of the WARNING messages
generated for denied permissions is not guaranteed?

From the generated vacuum.out:
-- Only one partition owned by other user.
ALTER TABLE vacowned_parted OWNER TO CURRENT_USER;
SET ROLE regress_vacuum;
VACUUM vacowned_parted;
WARNING:  permission denied to vacuum "vacowned_parted", skipping it
WARNING:  permission denied to vacuum "vacowned_part2", skipping it

This is interesting.  In this case, regress_vacuum owns only one
partition, but we would be able to vacuum it even when querying
vacowned_parted.  Seeing from [1], this is intentional as per the
argument that VACUUM/ANALYZE can take multiple relations.  Am I
getting that right?  That's different from CLUSTER or REINDEX, where
not owning the partitioned table fails immediately.

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.

-   privilege on the catalog.  If a role has permission to
-   <command>REINDEX</command> a partitioned table, it is also permitted to
-   <command>REINDEX</command> each of its partitions, regardless of whether the
-   role has the aforementioned privileges on the partition.  Of course,
-   superusers can always reindex anything.
+   privilege on the catalog.  Of course, superusers can always reindex anything.

With 0001 applied, if a user is marked as an owner of a partitioned
table, all the partitions are reindexed even if this user does not own
a portion of them, making this change incorrect while the former is
more correct?

[1]: https://www.postgresql.org/message-id/20221216011926.GA771496@nathanxps13
--
Michael

Вложения

В списке pgsql-hackers по дате отправления:

Предыдущее
От: James Coleman
Дата:
Сообщение: Re: path->param_info only set for lateral?
Следующее
От: Peter Geoghegan
Дата:
Сообщение: Re: Adding further hardening to nbtree page deletion