public inbox for [email protected]help / color / mirror / Atom feed
[PATCH 1/2] Add tests for ICU collation customization 10+ messages / 6 participants [nested] [flat]
* [PATCH 1/2] Add tests for ICU collation customization @ 2019-03-11 15:10 Peter Eisentraut <[email protected]> 0 siblings, 0 replies; 10+ messages in thread From: Peter Eisentraut @ 2019-03-11 15:10 UTC (permalink / raw) --- .../regress/expected/collate.icu.utf8.out | 39 +++++++++++++++++++ src/test/regress/sql/collate.icu.utf8.sql | 21 ++++++++++ 2 files changed, 60 insertions(+) diff --git a/src/test/regress/expected/collate.icu.utf8.out b/src/test/regress/expected/collate.icu.utf8.out index f95d165288..4b94921cf8 100644 --- a/src/test/regress/expected/collate.icu.utf8.out +++ b/src/test/regress/expected/collate.icu.utf8.out @@ -1100,6 +1100,45 @@ select textrange_en_us('A','Z') @> 'b'::text; drop type textrange_c; drop type textrange_en_us; +-- test ICU collation customization +CREATE COLLATION testcoll_ignore_accents (provider = icu, locale = '@colStrength=primary;colCaseLevel=yes'); +SELECT 'aaá' > 'AAA' COLLATE "und-x-icu", 'aaá' < 'AAA' COLLATE testcoll_ignore_accents; + ?column? | ?column? +----------+---------- + t | t +(1 row) + +CREATE COLLATION testcoll_backwards (provider = icu, locale = '@colBackwards=yes'); +SELECT 'coté' < 'côte' COLLATE "und-x-icu", 'coté' > 'côte' COLLATE testcoll_backwards; + ?column? | ?column? +----------+---------- + t | t +(1 row) + +CREATE COLLATION testcoll_lower_first (provider = icu, locale = '@colCaseFirst=lower'); +CREATE COLLATION testcoll_upper_first (provider = icu, locale = '@colCaseFirst=upper'); +SELECT 'aaa' < 'AAA' COLLATE testcoll_lower_first, 'aaa' > 'AAA' COLLATE testcoll_upper_first; + ?column? | ?column? +----------+---------- + t | t +(1 row) + +CREATE COLLATION testcoll_shifted (provider = icu, locale = '@colAlternate=shifted'); +SELECT 'de-luge' < 'deanza' COLLATE "und-x-icu", 'de-luge' > 'deanza' COLLATE testcoll_shifted; + ?column? | ?column? +----------+---------- + t | t +(1 row) + +CREATE COLLATION testcoll_numeric (provider = icu, locale = '@colNumeric=yes'); +SELECT 'A-21' > 'A-123' COLLATE "und-x-icu", 'A-21' < 'A-123' COLLATE testcoll_numeric; + ?column? | ?column? +----------+---------- + t | t +(1 row) + +CREATE COLLATION testcoll_error1 (provider = icu, locale = '@colNumeric=lower'); +ERROR: could not open collator for locale "@colNumeric=lower": U_ILLEGAL_ARGUMENT_ERROR -- cleanup SET client_min_messages TO warning; DROP SCHEMA collate_tests CASCADE; diff --git a/src/test/regress/sql/collate.icu.utf8.sql b/src/test/regress/sql/collate.icu.utf8.sql index 0aeba3e202..73fb1232a7 100644 --- a/src/test/regress/sql/collate.icu.utf8.sql +++ b/src/test/regress/sql/collate.icu.utf8.sql @@ -425,6 +425,27 @@ CREATE INDEX collate_dep_test4i ON collate_dep_test4t (b COLLATE test0); drop type textrange_en_us; +-- test ICU collation customization + +CREATE COLLATION testcoll_ignore_accents (provider = icu, locale = '@colStrength=primary;colCaseLevel=yes'); +SELECT 'aaá' > 'AAA' COLLATE "und-x-icu", 'aaá' < 'AAA' COLLATE testcoll_ignore_accents; + +CREATE COLLATION testcoll_backwards (provider = icu, locale = '@colBackwards=yes'); +SELECT 'coté' < 'côte' COLLATE "und-x-icu", 'coté' > 'côte' COLLATE testcoll_backwards; + +CREATE COLLATION testcoll_lower_first (provider = icu, locale = '@colCaseFirst=lower'); +CREATE COLLATION testcoll_upper_first (provider = icu, locale = '@colCaseFirst=upper'); +SELECT 'aaa' < 'AAA' COLLATE testcoll_lower_first, 'aaa' > 'AAA' COLLATE testcoll_upper_first; + +CREATE COLLATION testcoll_shifted (provider = icu, locale = '@colAlternate=shifted'); +SELECT 'de-luge' < 'deanza' COLLATE "und-x-icu", 'de-luge' > 'deanza' COLLATE testcoll_shifted; + +CREATE COLLATION testcoll_numeric (provider = icu, locale = '@colNumeric=yes'); +SELECT 'A-21' > 'A-123' COLLATE "und-x-icu", 'A-21' < 'A-123' COLLATE testcoll_numeric; + +CREATE COLLATION testcoll_error1 (provider = icu, locale = '@colNumeric=lower'); + + -- cleanup SET client_min_messages TO warning; DROP SCHEMA collate_tests CASCADE; -- 2.21.0 --------------FE4A217DFA47D74F9A255A4B Content-Type: text/plain; charset=UTF-8; x-mac-type="0"; x-mac-creator="0"; name="0002-Add-support-for-collation-attributes-on-older-ICU-ve.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename*0="0002-Add-support-for-collation-attributes-on-older-ICU-ve.pa"; filename*1="tch" ^ permalink raw reply [nested|flat] 10+ messages in thread
* fix and document CLUSTER privileges @ 2022-12-07 22:39 Nathan Bossart <[email protected]> 0 siblings, 2 replies; 10+ messages in thread From: Nathan Bossart @ 2022-12-07 22:39 UTC (permalink / raw) To: pgsql-hackers Hi hackers, While looking into other opportunities for per-table permissions, I noticed a weird discrepancy in CLUSTER. When evaluating whether the current user has permission to CLUSTER a table, we ordinarily just check for ownership. However, the database owner is also allowed to CLUSTER all partitions that are not shared. This was added in 3f19e17, and I didn't see any discussion about it in the corresponding thread [0]. My first instinct is that we should just remove the database ownership check, which is what I've done in the attached patch. I don't see any strong reason to complicate matters with special database-owner-but-not-shared checks like other commands (e.g., VACUUM). But perhaps we should do so just for consistency's sake. Thoughts? It was also noted elsewhere [1] that the privilege requirements for CLUSTER are not documented. The attached patch adds such documentation. [0] https://postgr.es/m/20220411140609.GF26620%40telsasoft.com [1] https://postgr.es/m/661148f4-c7f1-dec1-2bc8-29f3bd58e242%40postgrespro.ru -- Nathan Bossart Amazon Web Services: https://aws.amazon.com Attachments: [text/x-diff] fix_cluster_privs.patch (1.8K, ../../20221207223924.GA4182184@nathanxps13/2-fix_cluster_privs.patch) download | inline diff: diff --git a/doc/src/sgml/ref/cluster.sgml b/doc/src/sgml/ref/cluster.sgml index c37f4236f1..feb61e6ec2 100644 --- a/doc/src/sgml/ref/cluster.sgml +++ b/doc/src/sgml/ref/cluster.sgml @@ -67,7 +67,8 @@ CLUSTER [VERBOSE] </para> <para> - <command>CLUSTER</command> without any parameter reclusters all the + <command>CLUSTER</command> without a + <replaceable class="parameter">table_name</replaceable> reclusters all the previously-clustered tables in the current database that the calling user owns, or all such tables if called by a superuser. This form of <command>CLUSTER</command> cannot be executed inside a transaction @@ -132,6 +133,13 @@ CLUSTER [VERBOSE] <refsect1> <title>Notes</title> + <para> + To cluster a table, one must be the table's owner or a superuser. + Database-wide clusters and clusters on partitioned tables will silently + skip over any tables that the calling user does not have permission to + cluster. + </para> + <para> In cases where you are accessing single rows randomly within a table, the actual order of the data in the diff --git a/src/backend/commands/cluster.c b/src/backend/commands/cluster.c index 07e091bb87..a8a51cc674 100644 --- a/src/backend/commands/cluster.c +++ b/src/backend/commands/cluster.c @@ -1693,9 +1693,7 @@ get_tables_to_cluster_partitioned(MemoryContext cluster_context, Oid indexOid) continue; /* Silently skip partitions which the user has no access to. */ - if (!object_ownercheck(RelationRelationId, relid, GetUserId()) && - (!object_ownercheck(DatabaseRelationId, MyDatabaseId, GetUserId()) || - IsSharedRelation(relid))) + if (!object_ownercheck(RelationRelationId, relid, GetUserId())) continue; /* Use a permanent memory context for the result list */ ^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: fix and document CLUSTER privileges @ 2022-12-08 02:25 Justin Pryzby <[email protected]> parent: Nathan Bossart <[email protected]> 1 sibling, 1 reply; 10+ messages in thread From: Justin Pryzby @ 2022-12-08 02:25 UTC (permalink / raw) To: Nathan Bossart <[email protected]>; +Cc: pgsql-hackers On Wed, Dec 07, 2022 at 02:39:24PM -0800, Nathan Bossart wrote: > Hi hackers, > > While looking into other opportunities for per-table permissions, I noticed > a weird discrepancy in CLUSTER. When evaluating whether the current user > has permission to CLUSTER a table, we ordinarily just check for ownership. > However, the database owner is also allowed to CLUSTER all partitions that > are not shared. This was added in 3f19e17, and I didn't see any discussion > about it in the corresponding thread [0]. > > My first instinct is that we should just remove the database ownership > check, which is what I've done in the attached patch. I don't see any > strong reason to complicate matters with special > database-owner-but-not-shared checks like other commands (e.g., VACUUM). > But perhaps we should do so just for consistency's sake. Thoughts? Your patch makes it inconsistent with vacuum full, which is strange because vacuum full calls cluster. postgres=> VACUUM FULL t; VACUUM postgres=> CLUSTER t; ERROR: must be owner of table t BTW, it'd be helpful to copy the relevant parties on this kind of message, especially if there's a new thread dedicated just to this. -- Justin ^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: fix and document CLUSTER privileges @ 2022-12-08 04:13 Nathan Bossart <[email protected]> parent: Justin Pryzby <[email protected]> 0 siblings, 1 reply; 10+ messages in thread From: Nathan Bossart @ 2022-12-08 04:13 UTC (permalink / raw) To: Justin Pryzby <[email protected]>; +Cc: pgsql-hackers On Wed, Dec 07, 2022 at 08:25:59PM -0600, Justin Pryzby wrote: > Your patch makes it inconsistent with vacuum full, which is strange > because vacuum full calls cluster. > > postgres=> VACUUM FULL t; > VACUUM > postgres=> CLUSTER t; > ERROR: must be owner of table t This is the existing behavior on HEAD. I think it has been this way for a while. Granted, that doesn't mean it's ideal, but AFAICT it's intentional. Looking closer, the database ownership check in get_tables_to_cluster_partitioned() appears to have no meaningful effect. In this code path, cluster_rel() will always be called with CLUOPT_RECHECK, and that function only checks for table ownership. Anything gathered in get_tables_to_cluster_partitioned() that the user doesn't own will be skipped. So, I don't think my patch changes the behavior in any meaningful way, but I still think it's worthwhile to make the checks consistent. -- Nathan Bossart Amazon Web Services: https://aws.amazon.com ^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: fix and document CLUSTER privileges @ 2022-12-08 11:15 Pavel Luzanov <[email protected]> parent: Nathan Bossart <[email protected]> 1 sibling, 0 replies; 10+ messages in thread From: Pavel Luzanov @ 2022-12-08 11:15 UTC (permalink / raw) To: Nathan Bossart <[email protected]>; pgsql-hackers On 08.12.2022 01:39, Nathan Bossart wrote: > It was also noted elsewhere [1] that the privilege requirements for CLUSTER > are not documented. The attached patch adds such documentation. > [1] https://postgr.es/m/661148f4-c7f1-dec1-2bc8-29f3bd58e242%40postgrespro.ru Thanks for the patch. It correctly states the existing behavior. But perhaps we should wait for the decision in discussion [1] (link above), since this decision may affect the documentation on the CLUSTER command. -- Pavel Luzanov Postgres Professional: https://postgrespro.com ^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: fix and document CLUSTER privileges @ 2022-12-08 12:20 Andrew Dunstan <[email protected]> parent: Nathan Bossart <[email protected]> 0 siblings, 1 reply; 10+ messages in thread From: Andrew Dunstan @ 2022-12-08 12:20 UTC (permalink / raw) To: Nathan Bossart <[email protected]>; Justin Pryzby <[email protected]>; +Cc: pgsql-hackers On 2022-12-07 We 23:13, Nathan Bossart wrote: > On Wed, Dec 07, 2022 at 08:25:59PM -0600, Justin Pryzby wrote: >> Your patch makes it inconsistent with vacuum full, which is strange >> because vacuum full calls cluster. >> >> postgres=> VACUUM FULL t; >> VACUUM >> postgres=> CLUSTER t; >> ERROR: must be owner of table t > This is the existing behavior on HEAD. I think it has been this way for a > while. Granted, that doesn't mean it's ideal, but AFAICT it's intentional. > > Looking closer, the database ownership check in > get_tables_to_cluster_partitioned() appears to have no meaningful effect. > In this code path, cluster_rel() will always be called with CLUOPT_RECHECK, > and that function only checks for table ownership. Anything gathered in > get_tables_to_cluster_partitioned() that the user doesn't own will be > skipped. So, I don't think my patch changes the behavior in any meaningful > way, but I still think it's worthwhile to make the checks consistent. We should probably talk about what the privileges should be, though. I think there's a case to be made that CLUSTER should be governed by the VACUUM privileges, given how VACUUM FULL is now implemented. cheers andrew -- Andrew Dunstan EDB: https://www.enterprisedb.com ^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: fix and document CLUSTER privileges @ 2022-12-08 18:13 Nathan Bossart <[email protected]> parent: Andrew Dunstan <[email protected]> 0 siblings, 1 reply; 10+ messages in thread From: Nathan Bossart @ 2022-12-08 18:13 UTC (permalink / raw) To: Andrew Dunstan <[email protected]>; +Cc: Justin Pryzby <[email protected]>; pgsql-hackers On Thu, Dec 08, 2022 at 07:20:28AM -0500, Andrew Dunstan wrote: > We should probably talk about what the privileges should be, though. I > think there's a case to be made that CLUSTER should be governed by the > VACUUM privileges, given how VACUUM FULL is now implemented. Currently, CLUSTER, REFRESH MATERIALIZED VIEW, and REINDEX (minus REINDEX SCHEMA|DATABASE|SYSTEM) require ownership of the relation or superuser. In fact, all three use the same RangeVarCallbackOwnsTable() callback function. My current thinking is that this is good enough. I don't sense any strong demand for allowing database owners to run these commands on all non-shared relations, and there's ongoing work to break out the privileges to GRANT and predefined roles. However, I don't have a strong opinion about this. If we do want to change the permissions model for CLUSTER, it might make sense to change all three of the aforementioned commands to look more like VACUUM/ANALYZE. -- Nathan Bossart Amazon Web Services: https://aws.amazon.com ^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: fix and document CLUSTER privileges @ 2022-12-08 21:08 Robert Haas <[email protected]> parent: Nathan Bossart <[email protected]> 0 siblings, 1 reply; 10+ messages in thread From: Robert Haas @ 2022-12-08 21:08 UTC (permalink / raw) To: Nathan Bossart <[email protected]>; +Cc: Andrew Dunstan <[email protected]>; Justin Pryzby <[email protected]>; pgsql-hackers On Thu, Dec 8, 2022 at 1:13 PM Nathan Bossart <[email protected]> wrote: > On Thu, Dec 08, 2022 at 07:20:28AM -0500, Andrew Dunstan wrote: > > We should probably talk about what the privileges should be, though. I > > think there's a case to be made that CLUSTER should be governed by the > > VACUUM privileges, given how VACUUM FULL is now implemented. > > Currently, CLUSTER, REFRESH MATERIALIZED VIEW, and REINDEX (minus REINDEX > SCHEMA|DATABASE|SYSTEM) require ownership of the relation or superuser. In > fact, all three use the same RangeVarCallbackOwnsTable() callback function. > My current thinking is that this is good enough. I don't sense any strong > demand for allowing database owners to run these commands on all non-shared > relations, and there's ongoing work to break out the privileges to GRANT > and predefined roles. +1. I don't see why being the database owner should give you the right to run a random subset of commands on any table in the database. Tables have their own system for access privileges; we should use that, or extend it as required. -- Robert Haas EDB: http://www.enterprisedb.com ^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: fix and document CLUSTER privileges @ 2022-12-14 17:34 Nathan Bossart <[email protected]> parent: Robert Haas <[email protected]> 0 siblings, 1 reply; 10+ messages in thread From: Nathan Bossart @ 2022-12-14 17:34 UTC (permalink / raw) To: Robert Haas <[email protected]>; +Cc: Andrew Dunstan <[email protected]>; Justin Pryzby <[email protected]>; pgsql-hackers On Thu, Dec 08, 2022 at 04:08:40PM -0500, Robert Haas wrote: > On Thu, Dec 8, 2022 at 1:13 PM Nathan Bossart <[email protected]> wrote: >> Currently, CLUSTER, REFRESH MATERIALIZED VIEW, and REINDEX (minus REINDEX >> SCHEMA|DATABASE|SYSTEM) require ownership of the relation or superuser. In >> fact, all three use the same RangeVarCallbackOwnsTable() callback function. >> My current thinking is that this is good enough. I don't sense any strong >> demand for allowing database owners to run these commands on all non-shared >> relations, and there's ongoing work to break out the privileges to GRANT >> and predefined roles. > > +1. > > I don't see why being the database owner should give you the right to > run a random subset of commands on any table in the database. Tables > have their own system for access privileges; we should use that, or > extend it as required. Here is a rebased version of the patch. -- Nathan Bossart Amazon Web Services: https://aws.amazon.com Attachments: [text/x-diff] fix_cluster_privs_v2.patch (2.0K, ../../20221214173435.GA690225@nathanxps13/2-fix_cluster_privs_v2.patch) download | inline diff: diff --git a/doc/src/sgml/ref/cluster.sgml b/doc/src/sgml/ref/cluster.sgml index 145101e6a5..d6b2651657 100644 --- a/doc/src/sgml/ref/cluster.sgml +++ b/doc/src/sgml/ref/cluster.sgml @@ -67,7 +67,8 @@ CLUSTER [VERBOSE] </para> <para> - <command>CLUSTER</command> without any parameter reclusters all the + <command>CLUSTER</command> without a + <replaceable class="parameter">table_name</replaceable> reclusters all the previously-clustered tables in the current database that the calling user owns or has the <literal>MAINTAIN</literal> privilege for, or all such tables if called by a superuser or a role with privileges of the @@ -134,6 +135,16 @@ CLUSTER [VERBOSE] <refsect1> <title>Notes</title> + <para> + To cluster a table, one must have the <literal>MAINTAIN</literal> privilege + on the table or be the table's owner, a superuser, or a role with + privileges of the + <link linkend="predefined-roles-table"><literal>pg_maintain</literal></link> + role. Database-wide clusters and clusters on partitioned tables will + silently skip over any tables that the calling user does not have + permission to cluster. + </para> + <para> In cases where you are accessing single rows randomly within a table, the actual order of the data in the diff --git a/src/backend/commands/cluster.c b/src/backend/commands/cluster.c index 8966b75bd1..8140a90699 100644 --- a/src/backend/commands/cluster.c +++ b/src/backend/commands/cluster.c @@ -1697,9 +1697,7 @@ get_tables_to_cluster_partitioned(MemoryContext cluster_context, Oid indexOid) /* Silently skip partitions which the user has no access to. */ if (!object_ownercheck(RelationRelationId, relid, GetUserId()) && - pg_class_aclcheck(relid, GetUserId(), ACL_MAINTAIN) != ACLCHECK_OK && - (!object_ownercheck(DatabaseRelationId, MyDatabaseId, GetUserId()) || - IsSharedRelation(relid))) + pg_class_aclcheck(relid, GetUserId(), ACL_MAINTAIN) != ACLCHECK_OK) continue; /* Use a permanent memory context for the result list */ ^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: fix and document CLUSTER privileges @ 2022-12-16 04:57 Nathan Bossart <[email protected]> parent: Nathan Bossart <[email protected]> 0 siblings, 0 replies; 10+ messages in thread From: Nathan Bossart @ 2022-12-16 04:57 UTC (permalink / raw) To: Robert Haas <[email protected]>; +Cc: Andrew Dunstan <[email protected]>; Justin Pryzby <[email protected]>; pgsql-hackers Here is a new version of the patch. I've moved the privilege checks to a new function, and I added a note in the docs about clustering partitioned tables in a transaction block (it's not allowed). -- Nathan Bossart Amazon Web Services: https://aws.amazon.com Attachments: [text/x-diff] fix_cluster_privs_v3.patch (4.1K, ../../20221216045700.GA804311@nathanxps13/2-fix_cluster_privs_v3.patch) download | inline diff: diff --git a/doc/src/sgml/ref/cluster.sgml b/doc/src/sgml/ref/cluster.sgml index 145101e6a5..9ca66cd2ee 100644 --- a/doc/src/sgml/ref/cluster.sgml +++ b/doc/src/sgml/ref/cluster.sgml @@ -67,7 +67,8 @@ CLUSTER [VERBOSE] </para> <para> - <command>CLUSTER</command> without any parameter reclusters all the + <command>CLUSTER</command> without a + <replaceable class="parameter">table_name</replaceable> reclusters all the previously-clustered tables in the current database that the calling user owns or has the <literal>MAINTAIN</literal> privilege for, or all such tables if called by a superuser or a role with privileges of the @@ -134,6 +135,16 @@ CLUSTER [VERBOSE] <refsect1> <title>Notes</title> + <para> + To cluster a table, one must have the <literal>MAINTAIN</literal> privilege + on the table or be the table's owner, a superuser, or a role with + privileges of the + <link linkend="predefined-roles-table"><literal>pg_maintain</literal></link> + role. Database-wide clusters and clusters on partitioned tables will + silently skip over any tables that the calling user does not have + permission to cluster. + </para> + <para> In cases where you are accessing single rows randomly within a table, the actual order of the data in the @@ -202,7 +213,8 @@ CLUSTER [VERBOSE] <para> Clustering a partitioned table clusters each of its partitions using the partition of the specified partitioned index. When clustering a partitioned - table, the index may not be omitted. + table, the index may not be omitted. <command>CLUSTER</command> on a + partitioned table cannot be executed inside a transaction block. </para> </refsect1> diff --git a/src/backend/commands/cluster.c b/src/backend/commands/cluster.c index 8966b75bd1..14328abfa6 100644 --- a/src/backend/commands/cluster.c +++ b/src/backend/commands/cluster.c @@ -80,6 +80,7 @@ static void copy_table_data(Oid OIDNewHeap, Oid OIDOldHeap, Oid OIDOldIndex, static List *get_tables_to_cluster(MemoryContext cluster_context); static List *get_tables_to_cluster_partitioned(MemoryContext cluster_context, Oid indexOid); +static bool cluster_is_permitted_for_relation(Oid relid, Oid userid); /*--------------------------------------------------------------------------- @@ -366,8 +367,7 @@ cluster_rel(Oid tableOid, Oid indexOid, ClusterParams *params) if (recheck) { /* Check that the user still has privileges for the relation */ - if (!object_ownercheck(RelationRelationId, tableOid, save_userid) && - pg_class_aclcheck(tableOid, save_userid, ACL_MAINTAIN) != ACLCHECK_OK) + if (!cluster_is_permitted_for_relation(tableOid, save_userid)) { relation_close(OldHeap, AccessExclusiveLock); goto out; @@ -1646,8 +1646,7 @@ get_tables_to_cluster(MemoryContext cluster_context) index = (Form_pg_index) GETSTRUCT(indexTuple); - if (!object_ownercheck(RelationRelationId, index->indrelid, GetUserId()) && - pg_class_aclcheck(index->indrelid, GetUserId(), ACL_MAINTAIN) != ACLCHECK_OK) + if (!cluster_is_permitted_for_relation(index->indrelid, GetUserId())) continue; /* Use a permanent memory context for the result list */ @@ -1696,10 +1695,7 @@ get_tables_to_cluster_partitioned(MemoryContext cluster_context, Oid indexOid) continue; /* Silently skip partitions which the user has no access to. */ - if (!object_ownercheck(RelationRelationId, relid, GetUserId()) && - pg_class_aclcheck(relid, GetUserId(), ACL_MAINTAIN) != ACLCHECK_OK && - (!object_ownercheck(DatabaseRelationId, MyDatabaseId, GetUserId()) || - IsSharedRelation(relid))) + if (!cluster_is_permitted_for_relation(relid, GetUserId())) continue; /* Use a permanent memory context for the result list */ @@ -1715,3 +1711,10 @@ get_tables_to_cluster_partitioned(MemoryContext cluster_context, Oid indexOid) return rtcs; } + +static bool +cluster_is_permitted_for_relation(Oid relid, Oid userid) +{ + return object_ownercheck(RelationRelationId, relid, userid) || + pg_class_aclcheck(relid, userid, ACL_MAINTAIN) == ACLCHECK_OK; +} ^ permalink raw reply [nested|flat] 10+ messages in thread
end of thread, other threads:[~2022-12-16 04:57 UTC | newest] Thread overview: 10+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2019-03-11 15:10 [PATCH 1/2] Add tests for ICU collation customization Peter Eisentraut <[email protected]> 2022-12-07 22:39 fix and document CLUSTER privileges Nathan Bossart <[email protected]> 2022-12-08 02:25 ` Re: fix and document CLUSTER privileges Justin Pryzby <[email protected]> 2022-12-08 04:13 ` Re: fix and document CLUSTER privileges Nathan Bossart <[email protected]> 2022-12-08 12:20 ` Re: fix and document CLUSTER privileges Andrew Dunstan <[email protected]> 2022-12-08 18:13 ` Re: fix and document CLUSTER privileges Nathan Bossart <[email protected]> 2022-12-08 21:08 ` Re: fix and document CLUSTER privileges Robert Haas <[email protected]> 2022-12-14 17:34 ` Re: fix and document CLUSTER privileges Nathan Bossart <[email protected]> 2022-12-16 04:57 ` Re: fix and document CLUSTER privileges Nathan Bossart <[email protected]> 2022-12-08 11:15 ` Re: fix and document CLUSTER privileges Pavel Luzanov <[email protected]>
This inbox is served by agora; see mirroring instructions for how to clone and mirror all data and code used for this inbox