Skip to content

Commit 3f19e17

Browse files
alvherrealvherre
committed
Have CLUSTER ignore partitions not owned by caller
If a partitioned table has partitions owned by roles other than the owner of the partitioned table, don't include them in the to-be- clustered list. This is similar to what VACUUM FULL does (except we do it sooner, because there is no reason to postpone it). Add a simple test to verify that only owned partitions are clustered. While at it, change memory context switch-and-back to occur once per partition instead of outside of the loop. Author: Justin Pryzby <pryzby@telsasoft.com> Reviewed-by: Zhihong Yu <zyu@yugabyte.com> Reviewed-by: Michael Paquier <michael@paquier.xyz> Discussion: https://postgr.es/m/20220411140609.GF26620@telsasoft.com
1 parent 275e719 commit 3f19e17

File tree

3 files changed

+56
-7
lines changed

3 files changed

+56
-7
lines changed

‎src/backend/commands/cluster.c

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1647,10 +1647,8 @@ get_tables_to_cluster(MemoryContext cluster_context)
16471647
* Given an index on a partitioned table, return a list of RelToCluster for
16481648
* all the children leaves tables/indexes.
16491649
*
1650-
* Caller must hold lock on the table containing the index.
1651-
*
16521650
* Like expand_vacuum_rel, but here caller must hold AccessExclusiveLock
1653-
* on the table already.
1651+
* on the table containing the index.
16541652
*/
16551653
static List *
16561654
get_tables_to_cluster_partitioned(MemoryContext cluster_context, Oid indexOid)
@@ -1663,9 +1661,6 @@ get_tables_to_cluster_partitioned(MemoryContext cluster_context, Oid indexOid)
16631661
/* Do not lock the children until they're processed */
16641662
inhoids = find_all_inheritors(indexOid, NoLock, NULL);
16651663

1666-
/* Use a permanent memory context for the result list */
1667-
old_context = MemoryContextSwitchTo(cluster_context);
1668-
16691664
foreach(lc, inhoids)
16701665
{
16711666
Oid indexrelid = lfirst_oid(lc);
@@ -1676,12 +1671,22 @@ get_tables_to_cluster_partitioned(MemoryContext cluster_context, Oid indexOid)
16761671
if (get_rel_relkind(indexrelid) != RELKIND_INDEX)
16771672
continue;
16781673

1674+
/* Silently skip partitions which the user has no access to. */
1675+
if (!pg_class_ownercheck(relid, GetUserId()) &&
1676+
(!pg_database_ownercheck(MyDatabaseId, GetUserId()) ||
1677+
IsSharedRelation(relid)))
1678+
continue;
1679+
1680+
/* Use a permanent memory context for the result list */
1681+
old_context = MemoryContextSwitchTo(cluster_context);
1682+
16791683
rtc = (RelToCluster *) palloc(sizeof(RelToCluster));
16801684
rtc->tableOid = relid;
16811685
rtc->indexOid = indexrelid;
16821686
rtcs = lappend(rtcs, rtc);
1687+
1688+
MemoryContextSwitchTo(old_context);
16831689
}
16841690

1685-
MemoryContextSwitchTo(old_context);
16861691
return rtcs;
16871692
}

‎src/test/regress/expected/cluster.out

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -493,6 +493,31 @@ ERROR: cannot mark index clustered in partitioned table
493493
ALTER TABLE clstrpart CLUSTER ON clstrpart_idx;
494494
ERROR: cannot mark index clustered in partitioned table
495495
DROP TABLE clstrpart;
496+
-- Ownership of partitions is checked
497+
CREATE TABLE ptnowner(i int unique) PARTITION BY LIST (i);
498+
CREATE INDEX ptnowner_i_idx ON ptnowner(i);
499+
CREATE TABLE ptnowner1 PARTITION OF ptnowner FOR VALUES IN (1);
500+
CREATE ROLE regress_ptnowner;
501+
CREATE TABLE ptnowner2 PARTITION OF ptnowner FOR VALUES IN (2);
502+
ALTER TABLE ptnowner1 OWNER TO regress_ptnowner;
503+
ALTER TABLE ptnowner OWNER TO regress_ptnowner;
504+
CREATE TEMP TABLE ptnowner_oldnodes AS
505+
SELECT oid, relname, relfilenode FROM pg_partition_tree('ptnowner') AS tree
506+
JOIN pg_class AS c ON c.oid=tree.relid;
507+
SET SESSION AUTHORIZATION regress_ptnowner;
508+
CLUSTER ptnowner USING ptnowner_i_idx;
509+
RESET SESSION AUTHORIZATION;
510+
SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a
511+
JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C";
512+
relname | ?column?
513+
-----------+----------
514+
ptnowner | t
515+
ptnowner1 | f
516+
ptnowner2 | t
517+
(3 rows)
518+
519+
DROP TABLE ptnowner;
520+
DROP ROLE regress_ptnowner;
496521
-- Test CLUSTER with external tuplesorting
497522
create table clstr_4 as select * from tenk1;
498523
create index cluster_sort on clstr_4 (hundred, thousand, tenthous);

‎src/test/regress/sql/cluster.sql

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -229,6 +229,25 @@ ALTER TABLE clstrpart SET WITHOUT CLUSTER;
229229
ALTER TABLE clstrpart CLUSTER ON clstrpart_idx;
230230
DROP TABLE clstrpart;
231231

232+
-- Ownership of partitions is checked
233+
CREATE TABLE ptnowner(i int unique) PARTITION BY LIST (i);
234+
CREATE INDEX ptnowner_i_idx ON ptnowner(i);
235+
CREATE TABLE ptnowner1 PARTITION OF ptnowner FOR VALUES IN (1);
236+
CREATE ROLE regress_ptnowner;
237+
CREATE TABLE ptnowner2 PARTITION OF ptnowner FOR VALUES IN (2);
238+
ALTER TABLE ptnowner1 OWNER TO regress_ptnowner;
239+
ALTER TABLE ptnowner OWNER TO regress_ptnowner;
240+
CREATE TEMP TABLE ptnowner_oldnodes AS
241+
SELECT oid, relname, relfilenode FROM pg_partition_tree('ptnowner') AS tree
242+
JOIN pg_class AS c ON c.oid=tree.relid;
243+
SET SESSION AUTHORIZATION regress_ptnowner;
244+
CLUSTER ptnowner USING ptnowner_i_idx;
245+
RESET SESSION AUTHORIZATION;
246+
SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a
247+
JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C";
248+
DROP TABLE ptnowner;
249+
DROP ROLE regress_ptnowner;
250+
232251
-- Test CLUSTER with external tuplesorting
233252

234253
create table clstr_4 as select * from tenk1;

0 commit comments

Comments
 (0)