agora inbox for [email protected]help / color / mirror / Atom feed
[PATCH v4 3/7] Row pattern recognition patch (planner). 41+ messages / 2 participants [nested] [flat]
* [PATCH v4 3/7] Row pattern recognition patch (planner). @ 2023-08-09 07:56 Tatsuo Ishii <[email protected]> 0 siblings, 0 replies; 41+ messages in thread From: Tatsuo Ishii @ 2023-08-09 07:56 UTC (permalink / raw) --- src/backend/optimizer/plan/createplan.c | 23 ++++++++++++++++++----- src/backend/optimizer/plan/setrefs.c | 23 +++++++++++++++++++++++ src/include/nodes/plannodes.h | 15 +++++++++++++++ 3 files changed, 56 insertions(+), 5 deletions(-) diff --git a/src/backend/optimizer/plan/createplan.c b/src/backend/optimizer/plan/createplan.c index af48109058..828d8d5b62 100644 --- a/src/backend/optimizer/plan/createplan.c +++ b/src/backend/optimizer/plan/createplan.c @@ -286,9 +286,10 @@ static WindowAgg *make_windowagg(List *tlist, Index winref, int ordNumCols, AttrNumber *ordColIdx, Oid *ordOperators, Oid *ordCollations, int frameOptions, Node *startOffset, Node *endOffset, Oid startInRangeFunc, Oid endInRangeFunc, - Oid inRangeColl, bool inRangeAsc, bool inRangeNullsFirst, - List *runCondition, List *qual, bool topWindow, - Plan *lefttree); + Oid inRangeColl, bool inRangeAsc, bool inRangeNullsFirst, List *runCondition, + RPSkipTo rpSkipTo, List *patternVariable, List *patternRegexp, List *defineClause, + List *defineInitial, + List *qual, bool topWindow, Plan *lefttree); static Group *make_group(List *tlist, List *qual, int numGroupCols, AttrNumber *grpColIdx, Oid *grpOperators, Oid *grpCollations, Plan *lefttree); @@ -2679,6 +2680,11 @@ create_windowagg_plan(PlannerInfo *root, WindowAggPath *best_path) wc->inRangeAsc, wc->inRangeNullsFirst, wc->runCondition, + wc->rpSkipTo, + wc->patternVariable, + wc->patternRegexp, + wc->defineClause, + wc->defineInitial, best_path->qual, best_path->topwindow, subplan); @@ -6582,8 +6588,10 @@ make_windowagg(List *tlist, Index winref, int ordNumCols, AttrNumber *ordColIdx, Oid *ordOperators, Oid *ordCollations, int frameOptions, Node *startOffset, Node *endOffset, Oid startInRangeFunc, Oid endInRangeFunc, - Oid inRangeColl, bool inRangeAsc, bool inRangeNullsFirst, - List *runCondition, List *qual, bool topWindow, Plan *lefttree) + Oid inRangeColl, bool inRangeAsc, bool inRangeNullsFirst, List *runCondition, + RPSkipTo rpSkipTo, List *patternVariable, List *patternRegexp, List *defineClause, + List *defineInitial, + List *qual, bool topWindow, Plan *lefttree) { WindowAgg *node = makeNode(WindowAgg); Plan *plan = &node->plan; @@ -6609,6 +6617,11 @@ make_windowagg(List *tlist, Index winref, node->inRangeAsc = inRangeAsc; node->inRangeNullsFirst = inRangeNullsFirst; node->topWindow = topWindow; + node->rpSkipTo = rpSkipTo, + node->patternVariable = patternVariable; + node->patternRegexp = patternRegexp; + node->defineClause = defineClause; + node->defineInitial = defineInitial; plan->targetlist = tlist; plan->lefttree = lefttree; diff --git a/src/backend/optimizer/plan/setrefs.c b/src/backend/optimizer/plan/setrefs.c index 97fa561e4e..2ed00b5d41 100644 --- a/src/backend/optimizer/plan/setrefs.c +++ b/src/backend/optimizer/plan/setrefs.c @@ -2456,6 +2456,29 @@ set_upper_references(PlannerInfo *root, Plan *plan, int rtoffset) NRM_EQUAL, NUM_EXEC_QUAL(plan)); + /* + * Modifies an expression tree in each DEFINE clause so that all Var + * nodes reference outputs of a subplan. + */ + if (IsA(plan, WindowAgg)) + { + WindowAgg *wplan = (WindowAgg *) plan; + + foreach(l, wplan->defineClause) + { + TargetEntry *tle = (TargetEntry *) lfirst(l); + + tle->expr = (Expr *) + fix_upper_expr(root, + (Node *) tle->expr, + subplan_itlist, + OUTER_VAR, + rtoffset, + NRM_EQUAL, + NUM_EXEC_QUAL(plan)); + } + } + pfree(subplan_itlist); } diff --git a/src/include/nodes/plannodes.h b/src/include/nodes/plannodes.h index 1b787fe031..19815a98bb 100644 --- a/src/include/nodes/plannodes.h +++ b/src/include/nodes/plannodes.h @@ -1096,6 +1096,21 @@ typedef struct WindowAgg /* nulls sort first for in_range tests? */ bool inRangeNullsFirst; + /* Row Pattern Recognition AFTER MACH SKIP clause */ + RPSkipTo rpSkipTo; /* Row Pattern Skip To type */ + + /* Row Pattern PATTERN variable name (list of String) */ + List *patternVariable; + + /* Row Pattern RPATTERN regular expression quantifier ('+' or ''. list of String) */ + List *patternRegexp; + + /* Row Pattern DEFINE clause (list of TargetEntry) */ + List *defineClause; + + /* Row Pattern DEFINE variable initial names (list of String) */ + List *defineInitial; + /* * false for all apart from the WindowAgg that's closest to the root of * the plan -- 2.25.1 ----Next_Part(Wed_Aug__9_17_41_12_2023_134)-- Content-Type: Text/X-Patch; charset=us-ascii Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename="v4-0004-Row-pattern-recognition-patch-executor.patch" ^ permalink raw reply [nested|flat] 41+ messages in thread
* [PATCH 1/2] REPACK: do not require the user to have REPLICATION @ 2026-04-20 09:38 Álvaro Herrera <[email protected]> 0 siblings, 0 replies; 41+ messages in thread From: Álvaro Herrera @ 2026-04-20 09:38 UTC (permalink / raw) Although REPACK (CONCURRENTLY) uses replication slots, there is no concern that the slot will leak data of other users, because the MAINTAIN privilege on the table is required anyway; requiring REPLICATION is user-unfriendly without providing any actual protection. A related aspect is that the REPLICATION attribute is not needed to prevent REPACK from stealing slots from logical replication, since commit e76d8c749c31 made REPACK use a separate pool of replication slots. Because there are now successful concurrent repack runs in the regression tests, we're forced to run test_plan_advice under wal_level=replica. Author: Antonin Houska <[email protected]> Reported-by: Justin Pryzby <[email protected]> Reviewed-by: Chao Li <[email protected]> Discussion: https://postgr.es/m/aeJHPNmL4vVy3oPw@pryzbyj2023 --- src/backend/commands/repack_worker.c | 1 - .../test_plan_advice/t/001_replan_regress.pl | 1 + src/test/regress/expected/cluster.out | 20 +++++++++++++++++-- src/test/regress/sql/cluster.sql | 11 ++++++++-- 4 files changed, 28 insertions(+), 5 deletions(-) diff --git a/src/backend/commands/repack_worker.c b/src/backend/commands/repack_worker.c index b17edd771e2..e4a4860805b 100644 --- a/src/backend/commands/repack_worker.c +++ b/src/backend/commands/repack_worker.c @@ -214,7 +214,6 @@ repack_setup_logical_decoding(Oid relid) /* * Make sure we can use logical decoding. */ - CheckSlotPermissions(); CheckLogicalDecodingRequirements(true); /* diff --git a/src/test/modules/test_plan_advice/t/001_replan_regress.pl b/src/test/modules/test_plan_advice/t/001_replan_regress.pl index 38ffa4d11ae..452b179a665 100644 --- a/src/test/modules/test_plan_advice/t/001_replan_regress.pl +++ b/src/test/modules/test_plan_advice/t/001_replan_regress.pl @@ -18,6 +18,7 @@ $node->init(); # Set up our desired configuration. $node->append_conf('postgresql.conf', <<EOM); shared_preload_libraries='test_plan_advice' +wal_level=replica pg_plan_advice.always_explain_supplied_advice=false pg_plan_advice.feedback_warnings=true EOM diff --git a/src/test/regress/expected/cluster.out b/src/test/regress/expected/cluster.out index 6127b215a86..e17bc91fae1 100644 --- a/src/test/regress/expected/cluster.out +++ b/src/test/regress/expected/cluster.out @@ -543,15 +543,17 @@ ERROR: REPACK (CONCURRENTLY) is not supported for partitioned tables HINT: Consider running the command on individual partitions. DROP TABLE clstrpart; -- Ownership of partitions is checked -CREATE TABLE ptnowner(i int unique) PARTITION BY LIST (i); +CREATE TABLE ptnowner(i int unique not null) PARTITION BY LIST (i); CREATE INDEX ptnowner_i_idx ON ptnowner(i); CREATE TABLE ptnowner1 PARTITION OF ptnowner FOR VALUES IN (1); -CREATE ROLE regress_ptnowner; +CREATE ROLE regress_ptnowner LOGIN; CREATE TABLE ptnowner2 PARTITION OF ptnowner FOR VALUES IN (2); ALTER TABLE ptnowner1 OWNER TO regress_ptnowner; SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; ERROR: permission denied for table ptnowner +ALTER TABLE ptnowner1 REPLICA IDENTITY USING INDEX ptnowner1_i_key; +REPACK (CONCURRENTLY) ptnowner1; RESET SESSION AUTHORIZATION; ALTER TABLE ptnowner OWNER TO regress_ptnowner; CREATE TEMP TABLE ptnowner_oldnodes AS @@ -560,6 +562,11 @@ CREATE TEMP TABLE ptnowner_oldnodes AS SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; WARNING: permission denied to execute CLUSTER on "ptnowner2", skipping it +-- still can't repack without a replica identity +ALTER TABLE ptnowner1 REPLICA IDENTITY DEFAULT; +REPACK (CONCURRENTLY) ptnowner1; +ERROR: cannot process relation "ptnowner1" +HINT: Relation "ptnowner1" has no identity index. RESET SESSION AUTHORIZATION; SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; @@ -570,6 +577,15 @@ SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a ptnowner2 | t (3 rows) +SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a + JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; + relname | ?column? +-----------+---------- + ptnowner | t + ptnowner1 | f + ptnowner2 | t +(3 rows) + DROP TABLE ptnowner; DROP ROLE regress_ptnowner; -- Test CLUSTER with external tuplesorting diff --git a/src/test/regress/sql/cluster.sql b/src/test/regress/sql/cluster.sql index d14063a9683..1f471a8821a 100644 --- a/src/test/regress/sql/cluster.sql +++ b/src/test/regress/sql/cluster.sql @@ -254,14 +254,16 @@ REPACK (CONCURRENTLY) clstrpart; DROP TABLE clstrpart; -- Ownership of partitions is checked -CREATE TABLE ptnowner(i int unique) PARTITION BY LIST (i); +CREATE TABLE ptnowner(i int unique not null) PARTITION BY LIST (i); CREATE INDEX ptnowner_i_idx ON ptnowner(i); CREATE TABLE ptnowner1 PARTITION OF ptnowner FOR VALUES IN (1); -CREATE ROLE regress_ptnowner; +CREATE ROLE regress_ptnowner LOGIN; CREATE TABLE ptnowner2 PARTITION OF ptnowner FOR VALUES IN (2); ALTER TABLE ptnowner1 OWNER TO regress_ptnowner; SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; +ALTER TABLE ptnowner1 REPLICA IDENTITY USING INDEX ptnowner1_i_key; +REPACK (CONCURRENTLY) ptnowner1; RESET SESSION AUTHORIZATION; ALTER TABLE ptnowner OWNER TO regress_ptnowner; CREATE TEMP TABLE ptnowner_oldnodes AS @@ -269,7 +271,12 @@ CREATE TEMP TABLE ptnowner_oldnodes AS JOIN pg_class AS c ON c.oid=tree.relid; SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; +-- still can't repack without a replica identity +ALTER TABLE ptnowner1 REPLICA IDENTITY DEFAULT; +REPACK (CONCURRENTLY) ptnowner1; RESET SESSION AUTHORIZATION; +SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a + JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; DROP TABLE ptnowner; -- 2.47.3 --m6zy3l65ushq557m Content-Type: text/x-diff; charset=utf-8 Content-Disposition: attachment; filename="0002-REPACK-do-not-require-LOGIN-privileges.patch" ^ permalink raw reply [nested|flat] 41+ messages in thread
* [PATCH 1/2] REPACK: do not require the user to have REPLICATION @ 2026-04-20 09:38 Álvaro Herrera <[email protected]> 0 siblings, 0 replies; 41+ messages in thread From: Álvaro Herrera @ 2026-04-20 09:38 UTC (permalink / raw) Although REPACK (CONCURRENTLY) uses replication slots, there is no concern that the slot will leak data of other users, because the MAINTAIN privilege on the table is required anyway; requiring REPLICATION is user-unfriendly without providing any actual protection. A related aspect is that the REPLICATION attribute is not needed to prevent REPACK from stealing slots from logical replication, since commit e76d8c749c31 made REPACK use a separate pool of replication slots. Because there are now successful concurrent repack runs in the regression tests, we're forced to run test_plan_advice under wal_level=replica. Author: Antonin Houska <[email protected]> Reported-by: Justin Pryzby <[email protected]> Reviewed-by: Chao Li <[email protected]> Discussion: https://postgr.es/m/aeJHPNmL4vVy3oPw@pryzbyj2023 --- src/backend/commands/repack_worker.c | 1 - .../test_plan_advice/t/001_replan_regress.pl | 1 + src/test/regress/expected/cluster.out | 20 +++++++++++++++++-- src/test/regress/sql/cluster.sql | 11 ++++++++-- 4 files changed, 28 insertions(+), 5 deletions(-) diff --git a/src/backend/commands/repack_worker.c b/src/backend/commands/repack_worker.c index b17edd771e2..e4a4860805b 100644 --- a/src/backend/commands/repack_worker.c +++ b/src/backend/commands/repack_worker.c @@ -214,7 +214,6 @@ repack_setup_logical_decoding(Oid relid) /* * Make sure we can use logical decoding. */ - CheckSlotPermissions(); CheckLogicalDecodingRequirements(true); /* diff --git a/src/test/modules/test_plan_advice/t/001_replan_regress.pl b/src/test/modules/test_plan_advice/t/001_replan_regress.pl index 38ffa4d11ae..452b179a665 100644 --- a/src/test/modules/test_plan_advice/t/001_replan_regress.pl +++ b/src/test/modules/test_plan_advice/t/001_replan_regress.pl @@ -18,6 +18,7 @@ $node->init(); # Set up our desired configuration. $node->append_conf('postgresql.conf', <<EOM); shared_preload_libraries='test_plan_advice' +wal_level=replica pg_plan_advice.always_explain_supplied_advice=false pg_plan_advice.feedback_warnings=true EOM diff --git a/src/test/regress/expected/cluster.out b/src/test/regress/expected/cluster.out index 6127b215a86..e17bc91fae1 100644 --- a/src/test/regress/expected/cluster.out +++ b/src/test/regress/expected/cluster.out @@ -543,15 +543,17 @@ ERROR: REPACK (CONCURRENTLY) is not supported for partitioned tables HINT: Consider running the command on individual partitions. DROP TABLE clstrpart; -- Ownership of partitions is checked -CREATE TABLE ptnowner(i int unique) PARTITION BY LIST (i); +CREATE TABLE ptnowner(i int unique not null) PARTITION BY LIST (i); CREATE INDEX ptnowner_i_idx ON ptnowner(i); CREATE TABLE ptnowner1 PARTITION OF ptnowner FOR VALUES IN (1); -CREATE ROLE regress_ptnowner; +CREATE ROLE regress_ptnowner LOGIN; CREATE TABLE ptnowner2 PARTITION OF ptnowner FOR VALUES IN (2); ALTER TABLE ptnowner1 OWNER TO regress_ptnowner; SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; ERROR: permission denied for table ptnowner +ALTER TABLE ptnowner1 REPLICA IDENTITY USING INDEX ptnowner1_i_key; +REPACK (CONCURRENTLY) ptnowner1; RESET SESSION AUTHORIZATION; ALTER TABLE ptnowner OWNER TO regress_ptnowner; CREATE TEMP TABLE ptnowner_oldnodes AS @@ -560,6 +562,11 @@ CREATE TEMP TABLE ptnowner_oldnodes AS SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; WARNING: permission denied to execute CLUSTER on "ptnowner2", skipping it +-- still can't repack without a replica identity +ALTER TABLE ptnowner1 REPLICA IDENTITY DEFAULT; +REPACK (CONCURRENTLY) ptnowner1; +ERROR: cannot process relation "ptnowner1" +HINT: Relation "ptnowner1" has no identity index. RESET SESSION AUTHORIZATION; SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; @@ -570,6 +577,15 @@ SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a ptnowner2 | t (3 rows) +SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a + JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; + relname | ?column? +-----------+---------- + ptnowner | t + ptnowner1 | f + ptnowner2 | t +(3 rows) + DROP TABLE ptnowner; DROP ROLE regress_ptnowner; -- Test CLUSTER with external tuplesorting diff --git a/src/test/regress/sql/cluster.sql b/src/test/regress/sql/cluster.sql index d14063a9683..1f471a8821a 100644 --- a/src/test/regress/sql/cluster.sql +++ b/src/test/regress/sql/cluster.sql @@ -254,14 +254,16 @@ REPACK (CONCURRENTLY) clstrpart; DROP TABLE clstrpart; -- Ownership of partitions is checked -CREATE TABLE ptnowner(i int unique) PARTITION BY LIST (i); +CREATE TABLE ptnowner(i int unique not null) PARTITION BY LIST (i); CREATE INDEX ptnowner_i_idx ON ptnowner(i); CREATE TABLE ptnowner1 PARTITION OF ptnowner FOR VALUES IN (1); -CREATE ROLE regress_ptnowner; +CREATE ROLE regress_ptnowner LOGIN; CREATE TABLE ptnowner2 PARTITION OF ptnowner FOR VALUES IN (2); ALTER TABLE ptnowner1 OWNER TO regress_ptnowner; SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; +ALTER TABLE ptnowner1 REPLICA IDENTITY USING INDEX ptnowner1_i_key; +REPACK (CONCURRENTLY) ptnowner1; RESET SESSION AUTHORIZATION; ALTER TABLE ptnowner OWNER TO regress_ptnowner; CREATE TEMP TABLE ptnowner_oldnodes AS @@ -269,7 +271,12 @@ CREATE TEMP TABLE ptnowner_oldnodes AS JOIN pg_class AS c ON c.oid=tree.relid; SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; +-- still can't repack without a replica identity +ALTER TABLE ptnowner1 REPLICA IDENTITY DEFAULT; +REPACK (CONCURRENTLY) ptnowner1; RESET SESSION AUTHORIZATION; +SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a + JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; DROP TABLE ptnowner; -- 2.47.3 --m6zy3l65ushq557m Content-Type: text/x-diff; charset=utf-8 Content-Disposition: attachment; filename="0002-REPACK-do-not-require-LOGIN-privileges.patch" ^ permalink raw reply [nested|flat] 41+ messages in thread
* [PATCH 1/2] REPACK: do not require the user to have REPLICATION @ 2026-04-20 09:38 Álvaro Herrera <[email protected]> 0 siblings, 0 replies; 41+ messages in thread From: Álvaro Herrera @ 2026-04-20 09:38 UTC (permalink / raw) Although REPACK (CONCURRENTLY) uses replication slots, there is no concern that the slot will leak data of other users, because the MAINTAIN privilege on the table is required anyway; requiring REPLICATION is user-unfriendly without providing any actual protection. A related aspect is that the REPLICATION attribute is not needed to prevent REPACK from stealing slots from logical replication, since commit e76d8c749c31 made REPACK use a separate pool of replication slots. Because there are now successful concurrent repack runs in the regression tests, we're forced to run test_plan_advice under wal_level=replica. Author: Antonin Houska <[email protected]> Reported-by: Justin Pryzby <[email protected]> Reviewed-by: Chao Li <[email protected]> Discussion: https://postgr.es/m/aeJHPNmL4vVy3oPw@pryzbyj2023 --- src/backend/commands/repack_worker.c | 1 - .../test_plan_advice/t/001_replan_regress.pl | 1 + src/test/regress/expected/cluster.out | 20 +++++++++++++++++-- src/test/regress/sql/cluster.sql | 11 ++++++++-- 4 files changed, 28 insertions(+), 5 deletions(-) diff --git a/src/backend/commands/repack_worker.c b/src/backend/commands/repack_worker.c index b17edd771e2..e4a4860805b 100644 --- a/src/backend/commands/repack_worker.c +++ b/src/backend/commands/repack_worker.c @@ -214,7 +214,6 @@ repack_setup_logical_decoding(Oid relid) /* * Make sure we can use logical decoding. */ - CheckSlotPermissions(); CheckLogicalDecodingRequirements(true); /* diff --git a/src/test/modules/test_plan_advice/t/001_replan_regress.pl b/src/test/modules/test_plan_advice/t/001_replan_regress.pl index 38ffa4d11ae..452b179a665 100644 --- a/src/test/modules/test_plan_advice/t/001_replan_regress.pl +++ b/src/test/modules/test_plan_advice/t/001_replan_regress.pl @@ -18,6 +18,7 @@ $node->init(); # Set up our desired configuration. $node->append_conf('postgresql.conf', <<EOM); shared_preload_libraries='test_plan_advice' +wal_level=replica pg_plan_advice.always_explain_supplied_advice=false pg_plan_advice.feedback_warnings=true EOM diff --git a/src/test/regress/expected/cluster.out b/src/test/regress/expected/cluster.out index 6127b215a86..e17bc91fae1 100644 --- a/src/test/regress/expected/cluster.out +++ b/src/test/regress/expected/cluster.out @@ -543,15 +543,17 @@ ERROR: REPACK (CONCURRENTLY) is not supported for partitioned tables HINT: Consider running the command on individual partitions. DROP TABLE clstrpart; -- Ownership of partitions is checked -CREATE TABLE ptnowner(i int unique) PARTITION BY LIST (i); +CREATE TABLE ptnowner(i int unique not null) PARTITION BY LIST (i); CREATE INDEX ptnowner_i_idx ON ptnowner(i); CREATE TABLE ptnowner1 PARTITION OF ptnowner FOR VALUES IN (1); -CREATE ROLE regress_ptnowner; +CREATE ROLE regress_ptnowner LOGIN; CREATE TABLE ptnowner2 PARTITION OF ptnowner FOR VALUES IN (2); ALTER TABLE ptnowner1 OWNER TO regress_ptnowner; SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; ERROR: permission denied for table ptnowner +ALTER TABLE ptnowner1 REPLICA IDENTITY USING INDEX ptnowner1_i_key; +REPACK (CONCURRENTLY) ptnowner1; RESET SESSION AUTHORIZATION; ALTER TABLE ptnowner OWNER TO regress_ptnowner; CREATE TEMP TABLE ptnowner_oldnodes AS @@ -560,6 +562,11 @@ CREATE TEMP TABLE ptnowner_oldnodes AS SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; WARNING: permission denied to execute CLUSTER on "ptnowner2", skipping it +-- still can't repack without a replica identity +ALTER TABLE ptnowner1 REPLICA IDENTITY DEFAULT; +REPACK (CONCURRENTLY) ptnowner1; +ERROR: cannot process relation "ptnowner1" +HINT: Relation "ptnowner1" has no identity index. RESET SESSION AUTHORIZATION; SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; @@ -570,6 +577,15 @@ SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a ptnowner2 | t (3 rows) +SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a + JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; + relname | ?column? +-----------+---------- + ptnowner | t + ptnowner1 | f + ptnowner2 | t +(3 rows) + DROP TABLE ptnowner; DROP ROLE regress_ptnowner; -- Test CLUSTER with external tuplesorting diff --git a/src/test/regress/sql/cluster.sql b/src/test/regress/sql/cluster.sql index d14063a9683..1f471a8821a 100644 --- a/src/test/regress/sql/cluster.sql +++ b/src/test/regress/sql/cluster.sql @@ -254,14 +254,16 @@ REPACK (CONCURRENTLY) clstrpart; DROP TABLE clstrpart; -- Ownership of partitions is checked -CREATE TABLE ptnowner(i int unique) PARTITION BY LIST (i); +CREATE TABLE ptnowner(i int unique not null) PARTITION BY LIST (i); CREATE INDEX ptnowner_i_idx ON ptnowner(i); CREATE TABLE ptnowner1 PARTITION OF ptnowner FOR VALUES IN (1); -CREATE ROLE regress_ptnowner; +CREATE ROLE regress_ptnowner LOGIN; CREATE TABLE ptnowner2 PARTITION OF ptnowner FOR VALUES IN (2); ALTER TABLE ptnowner1 OWNER TO regress_ptnowner; SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; +ALTER TABLE ptnowner1 REPLICA IDENTITY USING INDEX ptnowner1_i_key; +REPACK (CONCURRENTLY) ptnowner1; RESET SESSION AUTHORIZATION; ALTER TABLE ptnowner OWNER TO regress_ptnowner; CREATE TEMP TABLE ptnowner_oldnodes AS @@ -269,7 +271,12 @@ CREATE TEMP TABLE ptnowner_oldnodes AS JOIN pg_class AS c ON c.oid=tree.relid; SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; +-- still can't repack without a replica identity +ALTER TABLE ptnowner1 REPLICA IDENTITY DEFAULT; +REPACK (CONCURRENTLY) ptnowner1; RESET SESSION AUTHORIZATION; +SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a + JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; DROP TABLE ptnowner; -- 2.47.3 --m6zy3l65ushq557m Content-Type: text/x-diff; charset=utf-8 Content-Disposition: attachment; filename="0002-REPACK-do-not-require-LOGIN-privileges.patch" ^ permalink raw reply [nested|flat] 41+ messages in thread
* [PATCH 1/2] REPACK: do not require the user to have REPLICATION @ 2026-04-20 09:38 Álvaro Herrera <[email protected]> 0 siblings, 0 replies; 41+ messages in thread From: Álvaro Herrera @ 2026-04-20 09:38 UTC (permalink / raw) Although REPACK (CONCURRENTLY) uses replication slots, there is no concern that the slot will leak data of other users, because the MAINTAIN privilege on the table is required anyway; requiring REPLICATION is user-unfriendly without providing any actual protection. A related aspect is that the REPLICATION attribute is not needed to prevent REPACK from stealing slots from logical replication, since commit e76d8c749c31 made REPACK use a separate pool of replication slots. Because there are now successful concurrent repack runs in the regression tests, we're forced to run test_plan_advice under wal_level=replica. Author: Antonin Houska <[email protected]> Reported-by: Justin Pryzby <[email protected]> Reviewed-by: Chao Li <[email protected]> Discussion: https://postgr.es/m/aeJHPNmL4vVy3oPw@pryzbyj2023 --- src/backend/commands/repack_worker.c | 1 - .../test_plan_advice/t/001_replan_regress.pl | 1 + src/test/regress/expected/cluster.out | 20 +++++++++++++++++-- src/test/regress/sql/cluster.sql | 11 ++++++++-- 4 files changed, 28 insertions(+), 5 deletions(-) diff --git a/src/backend/commands/repack_worker.c b/src/backend/commands/repack_worker.c index b17edd771e2..e4a4860805b 100644 --- a/src/backend/commands/repack_worker.c +++ b/src/backend/commands/repack_worker.c @@ -214,7 +214,6 @@ repack_setup_logical_decoding(Oid relid) /* * Make sure we can use logical decoding. */ - CheckSlotPermissions(); CheckLogicalDecodingRequirements(true); /* diff --git a/src/test/modules/test_plan_advice/t/001_replan_regress.pl b/src/test/modules/test_plan_advice/t/001_replan_regress.pl index 38ffa4d11ae..452b179a665 100644 --- a/src/test/modules/test_plan_advice/t/001_replan_regress.pl +++ b/src/test/modules/test_plan_advice/t/001_replan_regress.pl @@ -18,6 +18,7 @@ $node->init(); # Set up our desired configuration. $node->append_conf('postgresql.conf', <<EOM); shared_preload_libraries='test_plan_advice' +wal_level=replica pg_plan_advice.always_explain_supplied_advice=false pg_plan_advice.feedback_warnings=true EOM diff --git a/src/test/regress/expected/cluster.out b/src/test/regress/expected/cluster.out index 6127b215a86..e17bc91fae1 100644 --- a/src/test/regress/expected/cluster.out +++ b/src/test/regress/expected/cluster.out @@ -543,15 +543,17 @@ ERROR: REPACK (CONCURRENTLY) is not supported for partitioned tables HINT: Consider running the command on individual partitions. DROP TABLE clstrpart; -- Ownership of partitions is checked -CREATE TABLE ptnowner(i int unique) PARTITION BY LIST (i); +CREATE TABLE ptnowner(i int unique not null) PARTITION BY LIST (i); CREATE INDEX ptnowner_i_idx ON ptnowner(i); CREATE TABLE ptnowner1 PARTITION OF ptnowner FOR VALUES IN (1); -CREATE ROLE regress_ptnowner; +CREATE ROLE regress_ptnowner LOGIN; CREATE TABLE ptnowner2 PARTITION OF ptnowner FOR VALUES IN (2); ALTER TABLE ptnowner1 OWNER TO regress_ptnowner; SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; ERROR: permission denied for table ptnowner +ALTER TABLE ptnowner1 REPLICA IDENTITY USING INDEX ptnowner1_i_key; +REPACK (CONCURRENTLY) ptnowner1; RESET SESSION AUTHORIZATION; ALTER TABLE ptnowner OWNER TO regress_ptnowner; CREATE TEMP TABLE ptnowner_oldnodes AS @@ -560,6 +562,11 @@ CREATE TEMP TABLE ptnowner_oldnodes AS SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; WARNING: permission denied to execute CLUSTER on "ptnowner2", skipping it +-- still can't repack without a replica identity +ALTER TABLE ptnowner1 REPLICA IDENTITY DEFAULT; +REPACK (CONCURRENTLY) ptnowner1; +ERROR: cannot process relation "ptnowner1" +HINT: Relation "ptnowner1" has no identity index. RESET SESSION AUTHORIZATION; SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; @@ -570,6 +577,15 @@ SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a ptnowner2 | t (3 rows) +SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a + JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; + relname | ?column? +-----------+---------- + ptnowner | t + ptnowner1 | f + ptnowner2 | t +(3 rows) + DROP TABLE ptnowner; DROP ROLE regress_ptnowner; -- Test CLUSTER with external tuplesorting diff --git a/src/test/regress/sql/cluster.sql b/src/test/regress/sql/cluster.sql index d14063a9683..1f471a8821a 100644 --- a/src/test/regress/sql/cluster.sql +++ b/src/test/regress/sql/cluster.sql @@ -254,14 +254,16 @@ REPACK (CONCURRENTLY) clstrpart; DROP TABLE clstrpart; -- Ownership of partitions is checked -CREATE TABLE ptnowner(i int unique) PARTITION BY LIST (i); +CREATE TABLE ptnowner(i int unique not null) PARTITION BY LIST (i); CREATE INDEX ptnowner_i_idx ON ptnowner(i); CREATE TABLE ptnowner1 PARTITION OF ptnowner FOR VALUES IN (1); -CREATE ROLE regress_ptnowner; +CREATE ROLE regress_ptnowner LOGIN; CREATE TABLE ptnowner2 PARTITION OF ptnowner FOR VALUES IN (2); ALTER TABLE ptnowner1 OWNER TO regress_ptnowner; SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; +ALTER TABLE ptnowner1 REPLICA IDENTITY USING INDEX ptnowner1_i_key; +REPACK (CONCURRENTLY) ptnowner1; RESET SESSION AUTHORIZATION; ALTER TABLE ptnowner OWNER TO regress_ptnowner; CREATE TEMP TABLE ptnowner_oldnodes AS @@ -269,7 +271,12 @@ CREATE TEMP TABLE ptnowner_oldnodes AS JOIN pg_class AS c ON c.oid=tree.relid; SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; +-- still can't repack without a replica identity +ALTER TABLE ptnowner1 REPLICA IDENTITY DEFAULT; +REPACK (CONCURRENTLY) ptnowner1; RESET SESSION AUTHORIZATION; +SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a + JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; DROP TABLE ptnowner; -- 2.47.3 --m6zy3l65ushq557m Content-Type: text/x-diff; charset=utf-8 Content-Disposition: attachment; filename="0002-REPACK-do-not-require-LOGIN-privileges.patch" ^ permalink raw reply [nested|flat] 41+ messages in thread
* [PATCH 1/2] REPACK: do not require the user to have REPLICATION @ 2026-04-20 09:38 Álvaro Herrera <[email protected]> 0 siblings, 0 replies; 41+ messages in thread From: Álvaro Herrera @ 2026-04-20 09:38 UTC (permalink / raw) Although REPACK (CONCURRENTLY) uses replication slots, there is no concern that the slot will leak data of other users, because the MAINTAIN privilege on the table is required anyway; requiring REPLICATION is user-unfriendly without providing any actual protection. A related aspect is that the REPLICATION attribute is not needed to prevent REPACK from stealing slots from logical replication, since commit e76d8c749c31 made REPACK use a separate pool of replication slots. Because there are now successful concurrent repack runs in the regression tests, we're forced to run test_plan_advice under wal_level=replica. Author: Antonin Houska <[email protected]> Reported-by: Justin Pryzby <[email protected]> Reviewed-by: Chao Li <[email protected]> Discussion: https://postgr.es/m/aeJHPNmL4vVy3oPw@pryzbyj2023 --- src/backend/commands/repack_worker.c | 1 - .../test_plan_advice/t/001_replan_regress.pl | 1 + src/test/regress/expected/cluster.out | 20 +++++++++++++++++-- src/test/regress/sql/cluster.sql | 11 ++++++++-- 4 files changed, 28 insertions(+), 5 deletions(-) diff --git a/src/backend/commands/repack_worker.c b/src/backend/commands/repack_worker.c index b17edd771e2..e4a4860805b 100644 --- a/src/backend/commands/repack_worker.c +++ b/src/backend/commands/repack_worker.c @@ -214,7 +214,6 @@ repack_setup_logical_decoding(Oid relid) /* * Make sure we can use logical decoding. */ - CheckSlotPermissions(); CheckLogicalDecodingRequirements(true); /* diff --git a/src/test/modules/test_plan_advice/t/001_replan_regress.pl b/src/test/modules/test_plan_advice/t/001_replan_regress.pl index 38ffa4d11ae..452b179a665 100644 --- a/src/test/modules/test_plan_advice/t/001_replan_regress.pl +++ b/src/test/modules/test_plan_advice/t/001_replan_regress.pl @@ -18,6 +18,7 @@ $node->init(); # Set up our desired configuration. $node->append_conf('postgresql.conf', <<EOM); shared_preload_libraries='test_plan_advice' +wal_level=replica pg_plan_advice.always_explain_supplied_advice=false pg_plan_advice.feedback_warnings=true EOM diff --git a/src/test/regress/expected/cluster.out b/src/test/regress/expected/cluster.out index 6127b215a86..e17bc91fae1 100644 --- a/src/test/regress/expected/cluster.out +++ b/src/test/regress/expected/cluster.out @@ -543,15 +543,17 @@ ERROR: REPACK (CONCURRENTLY) is not supported for partitioned tables HINT: Consider running the command on individual partitions. DROP TABLE clstrpart; -- Ownership of partitions is checked -CREATE TABLE ptnowner(i int unique) PARTITION BY LIST (i); +CREATE TABLE ptnowner(i int unique not null) PARTITION BY LIST (i); CREATE INDEX ptnowner_i_idx ON ptnowner(i); CREATE TABLE ptnowner1 PARTITION OF ptnowner FOR VALUES IN (1); -CREATE ROLE regress_ptnowner; +CREATE ROLE regress_ptnowner LOGIN; CREATE TABLE ptnowner2 PARTITION OF ptnowner FOR VALUES IN (2); ALTER TABLE ptnowner1 OWNER TO regress_ptnowner; SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; ERROR: permission denied for table ptnowner +ALTER TABLE ptnowner1 REPLICA IDENTITY USING INDEX ptnowner1_i_key; +REPACK (CONCURRENTLY) ptnowner1; RESET SESSION AUTHORIZATION; ALTER TABLE ptnowner OWNER TO regress_ptnowner; CREATE TEMP TABLE ptnowner_oldnodes AS @@ -560,6 +562,11 @@ CREATE TEMP TABLE ptnowner_oldnodes AS SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; WARNING: permission denied to execute CLUSTER on "ptnowner2", skipping it +-- still can't repack without a replica identity +ALTER TABLE ptnowner1 REPLICA IDENTITY DEFAULT; +REPACK (CONCURRENTLY) ptnowner1; +ERROR: cannot process relation "ptnowner1" +HINT: Relation "ptnowner1" has no identity index. RESET SESSION AUTHORIZATION; SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; @@ -570,6 +577,15 @@ SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a ptnowner2 | t (3 rows) +SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a + JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; + relname | ?column? +-----------+---------- + ptnowner | t + ptnowner1 | f + ptnowner2 | t +(3 rows) + DROP TABLE ptnowner; DROP ROLE regress_ptnowner; -- Test CLUSTER with external tuplesorting diff --git a/src/test/regress/sql/cluster.sql b/src/test/regress/sql/cluster.sql index d14063a9683..1f471a8821a 100644 --- a/src/test/regress/sql/cluster.sql +++ b/src/test/regress/sql/cluster.sql @@ -254,14 +254,16 @@ REPACK (CONCURRENTLY) clstrpart; DROP TABLE clstrpart; -- Ownership of partitions is checked -CREATE TABLE ptnowner(i int unique) PARTITION BY LIST (i); +CREATE TABLE ptnowner(i int unique not null) PARTITION BY LIST (i); CREATE INDEX ptnowner_i_idx ON ptnowner(i); CREATE TABLE ptnowner1 PARTITION OF ptnowner FOR VALUES IN (1); -CREATE ROLE regress_ptnowner; +CREATE ROLE regress_ptnowner LOGIN; CREATE TABLE ptnowner2 PARTITION OF ptnowner FOR VALUES IN (2); ALTER TABLE ptnowner1 OWNER TO regress_ptnowner; SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; +ALTER TABLE ptnowner1 REPLICA IDENTITY USING INDEX ptnowner1_i_key; +REPACK (CONCURRENTLY) ptnowner1; RESET SESSION AUTHORIZATION; ALTER TABLE ptnowner OWNER TO regress_ptnowner; CREATE TEMP TABLE ptnowner_oldnodes AS @@ -269,7 +271,12 @@ CREATE TEMP TABLE ptnowner_oldnodes AS JOIN pg_class AS c ON c.oid=tree.relid; SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; +-- still can't repack without a replica identity +ALTER TABLE ptnowner1 REPLICA IDENTITY DEFAULT; +REPACK (CONCURRENTLY) ptnowner1; RESET SESSION AUTHORIZATION; +SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a + JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; DROP TABLE ptnowner; -- 2.47.3 --m6zy3l65ushq557m Content-Type: text/x-diff; charset=utf-8 Content-Disposition: attachment; filename="0002-REPACK-do-not-require-LOGIN-privileges.patch" ^ permalink raw reply [nested|flat] 41+ messages in thread
* [PATCH 1/2] REPACK: do not require the user to have REPLICATION @ 2026-04-20 09:38 Álvaro Herrera <[email protected]> 0 siblings, 0 replies; 41+ messages in thread From: Álvaro Herrera @ 2026-04-20 09:38 UTC (permalink / raw) Although REPACK (CONCURRENTLY) uses replication slots, there is no concern that the slot will leak data of other users, because the MAINTAIN privilege on the table is required anyway; requiring REPLICATION is user-unfriendly without providing any actual protection. A related aspect is that the REPLICATION attribute is not needed to prevent REPACK from stealing slots from logical replication, since commit e76d8c749c31 made REPACK use a separate pool of replication slots. Because there are now successful concurrent repack runs in the regression tests, we're forced to run test_plan_advice under wal_level=replica. Author: Antonin Houska <[email protected]> Reported-by: Justin Pryzby <[email protected]> Reviewed-by: Chao Li <[email protected]> Discussion: https://postgr.es/m/aeJHPNmL4vVy3oPw@pryzbyj2023 --- src/backend/commands/repack_worker.c | 1 - .../test_plan_advice/t/001_replan_regress.pl | 1 + src/test/regress/expected/cluster.out | 20 +++++++++++++++++-- src/test/regress/sql/cluster.sql | 11 ++++++++-- 4 files changed, 28 insertions(+), 5 deletions(-) diff --git a/src/backend/commands/repack_worker.c b/src/backend/commands/repack_worker.c index b17edd771e2..e4a4860805b 100644 --- a/src/backend/commands/repack_worker.c +++ b/src/backend/commands/repack_worker.c @@ -214,7 +214,6 @@ repack_setup_logical_decoding(Oid relid) /* * Make sure we can use logical decoding. */ - CheckSlotPermissions(); CheckLogicalDecodingRequirements(true); /* diff --git a/src/test/modules/test_plan_advice/t/001_replan_regress.pl b/src/test/modules/test_plan_advice/t/001_replan_regress.pl index 38ffa4d11ae..452b179a665 100644 --- a/src/test/modules/test_plan_advice/t/001_replan_regress.pl +++ b/src/test/modules/test_plan_advice/t/001_replan_regress.pl @@ -18,6 +18,7 @@ $node->init(); # Set up our desired configuration. $node->append_conf('postgresql.conf', <<EOM); shared_preload_libraries='test_plan_advice' +wal_level=replica pg_plan_advice.always_explain_supplied_advice=false pg_plan_advice.feedback_warnings=true EOM diff --git a/src/test/regress/expected/cluster.out b/src/test/regress/expected/cluster.out index 6127b215a86..e17bc91fae1 100644 --- a/src/test/regress/expected/cluster.out +++ b/src/test/regress/expected/cluster.out @@ -543,15 +543,17 @@ ERROR: REPACK (CONCURRENTLY) is not supported for partitioned tables HINT: Consider running the command on individual partitions. DROP TABLE clstrpart; -- Ownership of partitions is checked -CREATE TABLE ptnowner(i int unique) PARTITION BY LIST (i); +CREATE TABLE ptnowner(i int unique not null) PARTITION BY LIST (i); CREATE INDEX ptnowner_i_idx ON ptnowner(i); CREATE TABLE ptnowner1 PARTITION OF ptnowner FOR VALUES IN (1); -CREATE ROLE regress_ptnowner; +CREATE ROLE regress_ptnowner LOGIN; CREATE TABLE ptnowner2 PARTITION OF ptnowner FOR VALUES IN (2); ALTER TABLE ptnowner1 OWNER TO regress_ptnowner; SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; ERROR: permission denied for table ptnowner +ALTER TABLE ptnowner1 REPLICA IDENTITY USING INDEX ptnowner1_i_key; +REPACK (CONCURRENTLY) ptnowner1; RESET SESSION AUTHORIZATION; ALTER TABLE ptnowner OWNER TO regress_ptnowner; CREATE TEMP TABLE ptnowner_oldnodes AS @@ -560,6 +562,11 @@ CREATE TEMP TABLE ptnowner_oldnodes AS SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; WARNING: permission denied to execute CLUSTER on "ptnowner2", skipping it +-- still can't repack without a replica identity +ALTER TABLE ptnowner1 REPLICA IDENTITY DEFAULT; +REPACK (CONCURRENTLY) ptnowner1; +ERROR: cannot process relation "ptnowner1" +HINT: Relation "ptnowner1" has no identity index. RESET SESSION AUTHORIZATION; SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; @@ -570,6 +577,15 @@ SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a ptnowner2 | t (3 rows) +SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a + JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; + relname | ?column? +-----------+---------- + ptnowner | t + ptnowner1 | f + ptnowner2 | t +(3 rows) + DROP TABLE ptnowner; DROP ROLE regress_ptnowner; -- Test CLUSTER with external tuplesorting diff --git a/src/test/regress/sql/cluster.sql b/src/test/regress/sql/cluster.sql index d14063a9683..1f471a8821a 100644 --- a/src/test/regress/sql/cluster.sql +++ b/src/test/regress/sql/cluster.sql @@ -254,14 +254,16 @@ REPACK (CONCURRENTLY) clstrpart; DROP TABLE clstrpart; -- Ownership of partitions is checked -CREATE TABLE ptnowner(i int unique) PARTITION BY LIST (i); +CREATE TABLE ptnowner(i int unique not null) PARTITION BY LIST (i); CREATE INDEX ptnowner_i_idx ON ptnowner(i); CREATE TABLE ptnowner1 PARTITION OF ptnowner FOR VALUES IN (1); -CREATE ROLE regress_ptnowner; +CREATE ROLE regress_ptnowner LOGIN; CREATE TABLE ptnowner2 PARTITION OF ptnowner FOR VALUES IN (2); ALTER TABLE ptnowner1 OWNER TO regress_ptnowner; SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; +ALTER TABLE ptnowner1 REPLICA IDENTITY USING INDEX ptnowner1_i_key; +REPACK (CONCURRENTLY) ptnowner1; RESET SESSION AUTHORIZATION; ALTER TABLE ptnowner OWNER TO regress_ptnowner; CREATE TEMP TABLE ptnowner_oldnodes AS @@ -269,7 +271,12 @@ CREATE TEMP TABLE ptnowner_oldnodes AS JOIN pg_class AS c ON c.oid=tree.relid; SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; +-- still can't repack without a replica identity +ALTER TABLE ptnowner1 REPLICA IDENTITY DEFAULT; +REPACK (CONCURRENTLY) ptnowner1; RESET SESSION AUTHORIZATION; +SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a + JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; DROP TABLE ptnowner; -- 2.47.3 --m6zy3l65ushq557m Content-Type: text/x-diff; charset=utf-8 Content-Disposition: attachment; filename="0002-REPACK-do-not-require-LOGIN-privileges.patch" ^ permalink raw reply [nested|flat] 41+ messages in thread
* [PATCH 1/2] REPACK: do not require the user to have REPLICATION @ 2026-04-20 09:38 Álvaro Herrera <[email protected]> 0 siblings, 0 replies; 41+ messages in thread From: Álvaro Herrera @ 2026-04-20 09:38 UTC (permalink / raw) Although REPACK (CONCURRENTLY) uses replication slots, there is no concern that the slot will leak data of other users, because the MAINTAIN privilege on the table is required anyway; requiring REPLICATION is user-unfriendly without providing any actual protection. A related aspect is that the REPLICATION attribute is not needed to prevent REPACK from stealing slots from logical replication, since commit e76d8c749c31 made REPACK use a separate pool of replication slots. Because there are now successful concurrent repack runs in the regression tests, we're forced to run test_plan_advice under wal_level=replica. Author: Antonin Houska <[email protected]> Reported-by: Justin Pryzby <[email protected]> Reviewed-by: Chao Li <[email protected]> Discussion: https://postgr.es/m/aeJHPNmL4vVy3oPw@pryzbyj2023 --- src/backend/commands/repack_worker.c | 1 - .../test_plan_advice/t/001_replan_regress.pl | 1 + src/test/regress/expected/cluster.out | 20 +++++++++++++++++-- src/test/regress/sql/cluster.sql | 11 ++++++++-- 4 files changed, 28 insertions(+), 5 deletions(-) diff --git a/src/backend/commands/repack_worker.c b/src/backend/commands/repack_worker.c index b17edd771e2..e4a4860805b 100644 --- a/src/backend/commands/repack_worker.c +++ b/src/backend/commands/repack_worker.c @@ -214,7 +214,6 @@ repack_setup_logical_decoding(Oid relid) /* * Make sure we can use logical decoding. */ - CheckSlotPermissions(); CheckLogicalDecodingRequirements(true); /* diff --git a/src/test/modules/test_plan_advice/t/001_replan_regress.pl b/src/test/modules/test_plan_advice/t/001_replan_regress.pl index 38ffa4d11ae..452b179a665 100644 --- a/src/test/modules/test_plan_advice/t/001_replan_regress.pl +++ b/src/test/modules/test_plan_advice/t/001_replan_regress.pl @@ -18,6 +18,7 @@ $node->init(); # Set up our desired configuration. $node->append_conf('postgresql.conf', <<EOM); shared_preload_libraries='test_plan_advice' +wal_level=replica pg_plan_advice.always_explain_supplied_advice=false pg_plan_advice.feedback_warnings=true EOM diff --git a/src/test/regress/expected/cluster.out b/src/test/regress/expected/cluster.out index 6127b215a86..e17bc91fae1 100644 --- a/src/test/regress/expected/cluster.out +++ b/src/test/regress/expected/cluster.out @@ -543,15 +543,17 @@ ERROR: REPACK (CONCURRENTLY) is not supported for partitioned tables HINT: Consider running the command on individual partitions. DROP TABLE clstrpart; -- Ownership of partitions is checked -CREATE TABLE ptnowner(i int unique) PARTITION BY LIST (i); +CREATE TABLE ptnowner(i int unique not null) PARTITION BY LIST (i); CREATE INDEX ptnowner_i_idx ON ptnowner(i); CREATE TABLE ptnowner1 PARTITION OF ptnowner FOR VALUES IN (1); -CREATE ROLE regress_ptnowner; +CREATE ROLE regress_ptnowner LOGIN; CREATE TABLE ptnowner2 PARTITION OF ptnowner FOR VALUES IN (2); ALTER TABLE ptnowner1 OWNER TO regress_ptnowner; SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; ERROR: permission denied for table ptnowner +ALTER TABLE ptnowner1 REPLICA IDENTITY USING INDEX ptnowner1_i_key; +REPACK (CONCURRENTLY) ptnowner1; RESET SESSION AUTHORIZATION; ALTER TABLE ptnowner OWNER TO regress_ptnowner; CREATE TEMP TABLE ptnowner_oldnodes AS @@ -560,6 +562,11 @@ CREATE TEMP TABLE ptnowner_oldnodes AS SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; WARNING: permission denied to execute CLUSTER on "ptnowner2", skipping it +-- still can't repack without a replica identity +ALTER TABLE ptnowner1 REPLICA IDENTITY DEFAULT; +REPACK (CONCURRENTLY) ptnowner1; +ERROR: cannot process relation "ptnowner1" +HINT: Relation "ptnowner1" has no identity index. RESET SESSION AUTHORIZATION; SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; @@ -570,6 +577,15 @@ SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a ptnowner2 | t (3 rows) +SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a + JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; + relname | ?column? +-----------+---------- + ptnowner | t + ptnowner1 | f + ptnowner2 | t +(3 rows) + DROP TABLE ptnowner; DROP ROLE regress_ptnowner; -- Test CLUSTER with external tuplesorting diff --git a/src/test/regress/sql/cluster.sql b/src/test/regress/sql/cluster.sql index d14063a9683..1f471a8821a 100644 --- a/src/test/regress/sql/cluster.sql +++ b/src/test/regress/sql/cluster.sql @@ -254,14 +254,16 @@ REPACK (CONCURRENTLY) clstrpart; DROP TABLE clstrpart; -- Ownership of partitions is checked -CREATE TABLE ptnowner(i int unique) PARTITION BY LIST (i); +CREATE TABLE ptnowner(i int unique not null) PARTITION BY LIST (i); CREATE INDEX ptnowner_i_idx ON ptnowner(i); CREATE TABLE ptnowner1 PARTITION OF ptnowner FOR VALUES IN (1); -CREATE ROLE regress_ptnowner; +CREATE ROLE regress_ptnowner LOGIN; CREATE TABLE ptnowner2 PARTITION OF ptnowner FOR VALUES IN (2); ALTER TABLE ptnowner1 OWNER TO regress_ptnowner; SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; +ALTER TABLE ptnowner1 REPLICA IDENTITY USING INDEX ptnowner1_i_key; +REPACK (CONCURRENTLY) ptnowner1; RESET SESSION AUTHORIZATION; ALTER TABLE ptnowner OWNER TO regress_ptnowner; CREATE TEMP TABLE ptnowner_oldnodes AS @@ -269,7 +271,12 @@ CREATE TEMP TABLE ptnowner_oldnodes AS JOIN pg_class AS c ON c.oid=tree.relid; SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; +-- still can't repack without a replica identity +ALTER TABLE ptnowner1 REPLICA IDENTITY DEFAULT; +REPACK (CONCURRENTLY) ptnowner1; RESET SESSION AUTHORIZATION; +SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a + JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; DROP TABLE ptnowner; -- 2.47.3 --m6zy3l65ushq557m Content-Type: text/x-diff; charset=utf-8 Content-Disposition: attachment; filename="0002-REPACK-do-not-require-LOGIN-privileges.patch" ^ permalink raw reply [nested|flat] 41+ messages in thread
* [PATCH 1/2] REPACK: do not require the user to have REPLICATION @ 2026-04-20 09:38 Álvaro Herrera <[email protected]> 0 siblings, 0 replies; 41+ messages in thread From: Álvaro Herrera @ 2026-04-20 09:38 UTC (permalink / raw) Although REPACK (CONCURRENTLY) uses replication slots, there is no concern that the slot will leak data of other users, because the MAINTAIN privilege on the table is required anyway; requiring REPLICATION is user-unfriendly without providing any actual protection. A related aspect is that the REPLICATION attribute is not needed to prevent REPACK from stealing slots from logical replication, since commit e76d8c749c31 made REPACK use a separate pool of replication slots. Because there are now successful concurrent repack runs in the regression tests, we're forced to run test_plan_advice under wal_level=replica. Author: Antonin Houska <[email protected]> Reported-by: Justin Pryzby <[email protected]> Reviewed-by: Chao Li <[email protected]> Discussion: https://postgr.es/m/aeJHPNmL4vVy3oPw@pryzbyj2023 --- src/backend/commands/repack_worker.c | 1 - .../test_plan_advice/t/001_replan_regress.pl | 1 + src/test/regress/expected/cluster.out | 20 +++++++++++++++++-- src/test/regress/sql/cluster.sql | 11 ++++++++-- 4 files changed, 28 insertions(+), 5 deletions(-) diff --git a/src/backend/commands/repack_worker.c b/src/backend/commands/repack_worker.c index b17edd771e2..e4a4860805b 100644 --- a/src/backend/commands/repack_worker.c +++ b/src/backend/commands/repack_worker.c @@ -214,7 +214,6 @@ repack_setup_logical_decoding(Oid relid) /* * Make sure we can use logical decoding. */ - CheckSlotPermissions(); CheckLogicalDecodingRequirements(true); /* diff --git a/src/test/modules/test_plan_advice/t/001_replan_regress.pl b/src/test/modules/test_plan_advice/t/001_replan_regress.pl index 38ffa4d11ae..452b179a665 100644 --- a/src/test/modules/test_plan_advice/t/001_replan_regress.pl +++ b/src/test/modules/test_plan_advice/t/001_replan_regress.pl @@ -18,6 +18,7 @@ $node->init(); # Set up our desired configuration. $node->append_conf('postgresql.conf', <<EOM); shared_preload_libraries='test_plan_advice' +wal_level=replica pg_plan_advice.always_explain_supplied_advice=false pg_plan_advice.feedback_warnings=true EOM diff --git a/src/test/regress/expected/cluster.out b/src/test/regress/expected/cluster.out index 6127b215a86..e17bc91fae1 100644 --- a/src/test/regress/expected/cluster.out +++ b/src/test/regress/expected/cluster.out @@ -543,15 +543,17 @@ ERROR: REPACK (CONCURRENTLY) is not supported for partitioned tables HINT: Consider running the command on individual partitions. DROP TABLE clstrpart; -- Ownership of partitions is checked -CREATE TABLE ptnowner(i int unique) PARTITION BY LIST (i); +CREATE TABLE ptnowner(i int unique not null) PARTITION BY LIST (i); CREATE INDEX ptnowner_i_idx ON ptnowner(i); CREATE TABLE ptnowner1 PARTITION OF ptnowner FOR VALUES IN (1); -CREATE ROLE regress_ptnowner; +CREATE ROLE regress_ptnowner LOGIN; CREATE TABLE ptnowner2 PARTITION OF ptnowner FOR VALUES IN (2); ALTER TABLE ptnowner1 OWNER TO regress_ptnowner; SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; ERROR: permission denied for table ptnowner +ALTER TABLE ptnowner1 REPLICA IDENTITY USING INDEX ptnowner1_i_key; +REPACK (CONCURRENTLY) ptnowner1; RESET SESSION AUTHORIZATION; ALTER TABLE ptnowner OWNER TO regress_ptnowner; CREATE TEMP TABLE ptnowner_oldnodes AS @@ -560,6 +562,11 @@ CREATE TEMP TABLE ptnowner_oldnodes AS SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; WARNING: permission denied to execute CLUSTER on "ptnowner2", skipping it +-- still can't repack without a replica identity +ALTER TABLE ptnowner1 REPLICA IDENTITY DEFAULT; +REPACK (CONCURRENTLY) ptnowner1; +ERROR: cannot process relation "ptnowner1" +HINT: Relation "ptnowner1" has no identity index. RESET SESSION AUTHORIZATION; SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; @@ -570,6 +577,15 @@ SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a ptnowner2 | t (3 rows) +SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a + JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; + relname | ?column? +-----------+---------- + ptnowner | t + ptnowner1 | f + ptnowner2 | t +(3 rows) + DROP TABLE ptnowner; DROP ROLE regress_ptnowner; -- Test CLUSTER with external tuplesorting diff --git a/src/test/regress/sql/cluster.sql b/src/test/regress/sql/cluster.sql index d14063a9683..1f471a8821a 100644 --- a/src/test/regress/sql/cluster.sql +++ b/src/test/regress/sql/cluster.sql @@ -254,14 +254,16 @@ REPACK (CONCURRENTLY) clstrpart; DROP TABLE clstrpart; -- Ownership of partitions is checked -CREATE TABLE ptnowner(i int unique) PARTITION BY LIST (i); +CREATE TABLE ptnowner(i int unique not null) PARTITION BY LIST (i); CREATE INDEX ptnowner_i_idx ON ptnowner(i); CREATE TABLE ptnowner1 PARTITION OF ptnowner FOR VALUES IN (1); -CREATE ROLE regress_ptnowner; +CREATE ROLE regress_ptnowner LOGIN; CREATE TABLE ptnowner2 PARTITION OF ptnowner FOR VALUES IN (2); ALTER TABLE ptnowner1 OWNER TO regress_ptnowner; SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; +ALTER TABLE ptnowner1 REPLICA IDENTITY USING INDEX ptnowner1_i_key; +REPACK (CONCURRENTLY) ptnowner1; RESET SESSION AUTHORIZATION; ALTER TABLE ptnowner OWNER TO regress_ptnowner; CREATE TEMP TABLE ptnowner_oldnodes AS @@ -269,7 +271,12 @@ CREATE TEMP TABLE ptnowner_oldnodes AS JOIN pg_class AS c ON c.oid=tree.relid; SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; +-- still can't repack without a replica identity +ALTER TABLE ptnowner1 REPLICA IDENTITY DEFAULT; +REPACK (CONCURRENTLY) ptnowner1; RESET SESSION AUTHORIZATION; +SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a + JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; DROP TABLE ptnowner; -- 2.47.3 --m6zy3l65ushq557m Content-Type: text/x-diff; charset=utf-8 Content-Disposition: attachment; filename="0002-REPACK-do-not-require-LOGIN-privileges.patch" ^ permalink raw reply [nested|flat] 41+ messages in thread
* [PATCH 1/2] REPACK: do not require the user to have REPLICATION @ 2026-04-20 09:38 Álvaro Herrera <[email protected]> 0 siblings, 0 replies; 41+ messages in thread From: Álvaro Herrera @ 2026-04-20 09:38 UTC (permalink / raw) Although REPACK (CONCURRENTLY) uses replication slots, there is no concern that the slot will leak data of other users, because the MAINTAIN privilege on the table is required anyway; requiring REPLICATION is user-unfriendly without providing any actual protection. A related aspect is that the REPLICATION attribute is not needed to prevent REPACK from stealing slots from logical replication, since commit e76d8c749c31 made REPACK use a separate pool of replication slots. Because there are now successful concurrent repack runs in the regression tests, we're forced to run test_plan_advice under wal_level=replica. Author: Antonin Houska <[email protected]> Reported-by: Justin Pryzby <[email protected]> Reviewed-by: Chao Li <[email protected]> Discussion: https://postgr.es/m/aeJHPNmL4vVy3oPw@pryzbyj2023 --- src/backend/commands/repack_worker.c | 1 - .../test_plan_advice/t/001_replan_regress.pl | 1 + src/test/regress/expected/cluster.out | 20 +++++++++++++++++-- src/test/regress/sql/cluster.sql | 11 ++++++++-- 4 files changed, 28 insertions(+), 5 deletions(-) diff --git a/src/backend/commands/repack_worker.c b/src/backend/commands/repack_worker.c index b17edd771e2..e4a4860805b 100644 --- a/src/backend/commands/repack_worker.c +++ b/src/backend/commands/repack_worker.c @@ -214,7 +214,6 @@ repack_setup_logical_decoding(Oid relid) /* * Make sure we can use logical decoding. */ - CheckSlotPermissions(); CheckLogicalDecodingRequirements(true); /* diff --git a/src/test/modules/test_plan_advice/t/001_replan_regress.pl b/src/test/modules/test_plan_advice/t/001_replan_regress.pl index 38ffa4d11ae..452b179a665 100644 --- a/src/test/modules/test_plan_advice/t/001_replan_regress.pl +++ b/src/test/modules/test_plan_advice/t/001_replan_regress.pl @@ -18,6 +18,7 @@ $node->init(); # Set up our desired configuration. $node->append_conf('postgresql.conf', <<EOM); shared_preload_libraries='test_plan_advice' +wal_level=replica pg_plan_advice.always_explain_supplied_advice=false pg_plan_advice.feedback_warnings=true EOM diff --git a/src/test/regress/expected/cluster.out b/src/test/regress/expected/cluster.out index 6127b215a86..e17bc91fae1 100644 --- a/src/test/regress/expected/cluster.out +++ b/src/test/regress/expected/cluster.out @@ -543,15 +543,17 @@ ERROR: REPACK (CONCURRENTLY) is not supported for partitioned tables HINT: Consider running the command on individual partitions. DROP TABLE clstrpart; -- Ownership of partitions is checked -CREATE TABLE ptnowner(i int unique) PARTITION BY LIST (i); +CREATE TABLE ptnowner(i int unique not null) PARTITION BY LIST (i); CREATE INDEX ptnowner_i_idx ON ptnowner(i); CREATE TABLE ptnowner1 PARTITION OF ptnowner FOR VALUES IN (1); -CREATE ROLE regress_ptnowner; +CREATE ROLE regress_ptnowner LOGIN; CREATE TABLE ptnowner2 PARTITION OF ptnowner FOR VALUES IN (2); ALTER TABLE ptnowner1 OWNER TO regress_ptnowner; SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; ERROR: permission denied for table ptnowner +ALTER TABLE ptnowner1 REPLICA IDENTITY USING INDEX ptnowner1_i_key; +REPACK (CONCURRENTLY) ptnowner1; RESET SESSION AUTHORIZATION; ALTER TABLE ptnowner OWNER TO regress_ptnowner; CREATE TEMP TABLE ptnowner_oldnodes AS @@ -560,6 +562,11 @@ CREATE TEMP TABLE ptnowner_oldnodes AS SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; WARNING: permission denied to execute CLUSTER on "ptnowner2", skipping it +-- still can't repack without a replica identity +ALTER TABLE ptnowner1 REPLICA IDENTITY DEFAULT; +REPACK (CONCURRENTLY) ptnowner1; +ERROR: cannot process relation "ptnowner1" +HINT: Relation "ptnowner1" has no identity index. RESET SESSION AUTHORIZATION; SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; @@ -570,6 +577,15 @@ SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a ptnowner2 | t (3 rows) +SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a + JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; + relname | ?column? +-----------+---------- + ptnowner | t + ptnowner1 | f + ptnowner2 | t +(3 rows) + DROP TABLE ptnowner; DROP ROLE regress_ptnowner; -- Test CLUSTER with external tuplesorting diff --git a/src/test/regress/sql/cluster.sql b/src/test/regress/sql/cluster.sql index d14063a9683..1f471a8821a 100644 --- a/src/test/regress/sql/cluster.sql +++ b/src/test/regress/sql/cluster.sql @@ -254,14 +254,16 @@ REPACK (CONCURRENTLY) clstrpart; DROP TABLE clstrpart; -- Ownership of partitions is checked -CREATE TABLE ptnowner(i int unique) PARTITION BY LIST (i); +CREATE TABLE ptnowner(i int unique not null) PARTITION BY LIST (i); CREATE INDEX ptnowner_i_idx ON ptnowner(i); CREATE TABLE ptnowner1 PARTITION OF ptnowner FOR VALUES IN (1); -CREATE ROLE regress_ptnowner; +CREATE ROLE regress_ptnowner LOGIN; CREATE TABLE ptnowner2 PARTITION OF ptnowner FOR VALUES IN (2); ALTER TABLE ptnowner1 OWNER TO regress_ptnowner; SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; +ALTER TABLE ptnowner1 REPLICA IDENTITY USING INDEX ptnowner1_i_key; +REPACK (CONCURRENTLY) ptnowner1; RESET SESSION AUTHORIZATION; ALTER TABLE ptnowner OWNER TO regress_ptnowner; CREATE TEMP TABLE ptnowner_oldnodes AS @@ -269,7 +271,12 @@ CREATE TEMP TABLE ptnowner_oldnodes AS JOIN pg_class AS c ON c.oid=tree.relid; SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; +-- still can't repack without a replica identity +ALTER TABLE ptnowner1 REPLICA IDENTITY DEFAULT; +REPACK (CONCURRENTLY) ptnowner1; RESET SESSION AUTHORIZATION; +SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a + JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; DROP TABLE ptnowner; -- 2.47.3 --m6zy3l65ushq557m Content-Type: text/x-diff; charset=utf-8 Content-Disposition: attachment; filename="0002-REPACK-do-not-require-LOGIN-privileges.patch" ^ permalink raw reply [nested|flat] 41+ messages in thread
* [PATCH 1/2] REPACK: do not require the user to have REPLICATION @ 2026-04-20 09:38 Álvaro Herrera <[email protected]> 0 siblings, 0 replies; 41+ messages in thread From: Álvaro Herrera @ 2026-04-20 09:38 UTC (permalink / raw) Although REPACK (CONCURRENTLY) uses replication slots, there is no concern that the slot will leak data of other users, because the MAINTAIN privilege on the table is required anyway; requiring REPLICATION is user-unfriendly without providing any actual protection. A related aspect is that the REPLICATION attribute is not needed to prevent REPACK from stealing slots from logical replication, since commit e76d8c749c31 made REPACK use a separate pool of replication slots. Because there are now successful concurrent repack runs in the regression tests, we're forced to run test_plan_advice under wal_level=replica. Author: Antonin Houska <[email protected]> Reported-by: Justin Pryzby <[email protected]> Reviewed-by: Chao Li <[email protected]> Discussion: https://postgr.es/m/aeJHPNmL4vVy3oPw@pryzbyj2023 --- src/backend/commands/repack_worker.c | 1 - .../test_plan_advice/t/001_replan_regress.pl | 1 + src/test/regress/expected/cluster.out | 20 +++++++++++++++++-- src/test/regress/sql/cluster.sql | 11 ++++++++-- 4 files changed, 28 insertions(+), 5 deletions(-) diff --git a/src/backend/commands/repack_worker.c b/src/backend/commands/repack_worker.c index b17edd771e2..e4a4860805b 100644 --- a/src/backend/commands/repack_worker.c +++ b/src/backend/commands/repack_worker.c @@ -214,7 +214,6 @@ repack_setup_logical_decoding(Oid relid) /* * Make sure we can use logical decoding. */ - CheckSlotPermissions(); CheckLogicalDecodingRequirements(true); /* diff --git a/src/test/modules/test_plan_advice/t/001_replan_regress.pl b/src/test/modules/test_plan_advice/t/001_replan_regress.pl index 38ffa4d11ae..452b179a665 100644 --- a/src/test/modules/test_plan_advice/t/001_replan_regress.pl +++ b/src/test/modules/test_plan_advice/t/001_replan_regress.pl @@ -18,6 +18,7 @@ $node->init(); # Set up our desired configuration. $node->append_conf('postgresql.conf', <<EOM); shared_preload_libraries='test_plan_advice' +wal_level=replica pg_plan_advice.always_explain_supplied_advice=false pg_plan_advice.feedback_warnings=true EOM diff --git a/src/test/regress/expected/cluster.out b/src/test/regress/expected/cluster.out index 6127b215a86..e17bc91fae1 100644 --- a/src/test/regress/expected/cluster.out +++ b/src/test/regress/expected/cluster.out @@ -543,15 +543,17 @@ ERROR: REPACK (CONCURRENTLY) is not supported for partitioned tables HINT: Consider running the command on individual partitions. DROP TABLE clstrpart; -- Ownership of partitions is checked -CREATE TABLE ptnowner(i int unique) PARTITION BY LIST (i); +CREATE TABLE ptnowner(i int unique not null) PARTITION BY LIST (i); CREATE INDEX ptnowner_i_idx ON ptnowner(i); CREATE TABLE ptnowner1 PARTITION OF ptnowner FOR VALUES IN (1); -CREATE ROLE regress_ptnowner; +CREATE ROLE regress_ptnowner LOGIN; CREATE TABLE ptnowner2 PARTITION OF ptnowner FOR VALUES IN (2); ALTER TABLE ptnowner1 OWNER TO regress_ptnowner; SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; ERROR: permission denied for table ptnowner +ALTER TABLE ptnowner1 REPLICA IDENTITY USING INDEX ptnowner1_i_key; +REPACK (CONCURRENTLY) ptnowner1; RESET SESSION AUTHORIZATION; ALTER TABLE ptnowner OWNER TO regress_ptnowner; CREATE TEMP TABLE ptnowner_oldnodes AS @@ -560,6 +562,11 @@ CREATE TEMP TABLE ptnowner_oldnodes AS SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; WARNING: permission denied to execute CLUSTER on "ptnowner2", skipping it +-- still can't repack without a replica identity +ALTER TABLE ptnowner1 REPLICA IDENTITY DEFAULT; +REPACK (CONCURRENTLY) ptnowner1; +ERROR: cannot process relation "ptnowner1" +HINT: Relation "ptnowner1" has no identity index. RESET SESSION AUTHORIZATION; SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; @@ -570,6 +577,15 @@ SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a ptnowner2 | t (3 rows) +SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a + JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; + relname | ?column? +-----------+---------- + ptnowner | t + ptnowner1 | f + ptnowner2 | t +(3 rows) + DROP TABLE ptnowner; DROP ROLE regress_ptnowner; -- Test CLUSTER with external tuplesorting diff --git a/src/test/regress/sql/cluster.sql b/src/test/regress/sql/cluster.sql index d14063a9683..1f471a8821a 100644 --- a/src/test/regress/sql/cluster.sql +++ b/src/test/regress/sql/cluster.sql @@ -254,14 +254,16 @@ REPACK (CONCURRENTLY) clstrpart; DROP TABLE clstrpart; -- Ownership of partitions is checked -CREATE TABLE ptnowner(i int unique) PARTITION BY LIST (i); +CREATE TABLE ptnowner(i int unique not null) PARTITION BY LIST (i); CREATE INDEX ptnowner_i_idx ON ptnowner(i); CREATE TABLE ptnowner1 PARTITION OF ptnowner FOR VALUES IN (1); -CREATE ROLE regress_ptnowner; +CREATE ROLE regress_ptnowner LOGIN; CREATE TABLE ptnowner2 PARTITION OF ptnowner FOR VALUES IN (2); ALTER TABLE ptnowner1 OWNER TO regress_ptnowner; SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; +ALTER TABLE ptnowner1 REPLICA IDENTITY USING INDEX ptnowner1_i_key; +REPACK (CONCURRENTLY) ptnowner1; RESET SESSION AUTHORIZATION; ALTER TABLE ptnowner OWNER TO regress_ptnowner; CREATE TEMP TABLE ptnowner_oldnodes AS @@ -269,7 +271,12 @@ CREATE TEMP TABLE ptnowner_oldnodes AS JOIN pg_class AS c ON c.oid=tree.relid; SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; +-- still can't repack without a replica identity +ALTER TABLE ptnowner1 REPLICA IDENTITY DEFAULT; +REPACK (CONCURRENTLY) ptnowner1; RESET SESSION AUTHORIZATION; +SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a + JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; DROP TABLE ptnowner; -- 2.47.3 --m6zy3l65ushq557m Content-Type: text/x-diff; charset=utf-8 Content-Disposition: attachment; filename="0002-REPACK-do-not-require-LOGIN-privileges.patch" ^ permalink raw reply [nested|flat] 41+ messages in thread
* [PATCH 1/2] REPACK: do not require the user to have REPLICATION @ 2026-04-20 09:38 Álvaro Herrera <[email protected]> 0 siblings, 0 replies; 41+ messages in thread From: Álvaro Herrera @ 2026-04-20 09:38 UTC (permalink / raw) Although REPACK (CONCURRENTLY) uses replication slots, there is no concern that the slot will leak data of other users, because the MAINTAIN privilege on the table is required anyway; requiring REPLICATION is user-unfriendly without providing any actual protection. A related aspect is that the REPLICATION attribute is not needed to prevent REPACK from stealing slots from logical replication, since commit e76d8c749c31 made REPACK use a separate pool of replication slots. Because there are now successful concurrent repack runs in the regression tests, we're forced to run test_plan_advice under wal_level=replica. Author: Antonin Houska <[email protected]> Reported-by: Justin Pryzby <[email protected]> Reviewed-by: Chao Li <[email protected]> Discussion: https://postgr.es/m/aeJHPNmL4vVy3oPw@pryzbyj2023 --- src/backend/commands/repack_worker.c | 1 - .../test_plan_advice/t/001_replan_regress.pl | 1 + src/test/regress/expected/cluster.out | 20 +++++++++++++++++-- src/test/regress/sql/cluster.sql | 11 ++++++++-- 4 files changed, 28 insertions(+), 5 deletions(-) diff --git a/src/backend/commands/repack_worker.c b/src/backend/commands/repack_worker.c index b17edd771e2..e4a4860805b 100644 --- a/src/backend/commands/repack_worker.c +++ b/src/backend/commands/repack_worker.c @@ -214,7 +214,6 @@ repack_setup_logical_decoding(Oid relid) /* * Make sure we can use logical decoding. */ - CheckSlotPermissions(); CheckLogicalDecodingRequirements(true); /* diff --git a/src/test/modules/test_plan_advice/t/001_replan_regress.pl b/src/test/modules/test_plan_advice/t/001_replan_regress.pl index 38ffa4d11ae..452b179a665 100644 --- a/src/test/modules/test_plan_advice/t/001_replan_regress.pl +++ b/src/test/modules/test_plan_advice/t/001_replan_regress.pl @@ -18,6 +18,7 @@ $node->init(); # Set up our desired configuration. $node->append_conf('postgresql.conf', <<EOM); shared_preload_libraries='test_plan_advice' +wal_level=replica pg_plan_advice.always_explain_supplied_advice=false pg_plan_advice.feedback_warnings=true EOM diff --git a/src/test/regress/expected/cluster.out b/src/test/regress/expected/cluster.out index 6127b215a86..e17bc91fae1 100644 --- a/src/test/regress/expected/cluster.out +++ b/src/test/regress/expected/cluster.out @@ -543,15 +543,17 @@ ERROR: REPACK (CONCURRENTLY) is not supported for partitioned tables HINT: Consider running the command on individual partitions. DROP TABLE clstrpart; -- Ownership of partitions is checked -CREATE TABLE ptnowner(i int unique) PARTITION BY LIST (i); +CREATE TABLE ptnowner(i int unique not null) PARTITION BY LIST (i); CREATE INDEX ptnowner_i_idx ON ptnowner(i); CREATE TABLE ptnowner1 PARTITION OF ptnowner FOR VALUES IN (1); -CREATE ROLE regress_ptnowner; +CREATE ROLE regress_ptnowner LOGIN; CREATE TABLE ptnowner2 PARTITION OF ptnowner FOR VALUES IN (2); ALTER TABLE ptnowner1 OWNER TO regress_ptnowner; SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; ERROR: permission denied for table ptnowner +ALTER TABLE ptnowner1 REPLICA IDENTITY USING INDEX ptnowner1_i_key; +REPACK (CONCURRENTLY) ptnowner1; RESET SESSION AUTHORIZATION; ALTER TABLE ptnowner OWNER TO regress_ptnowner; CREATE TEMP TABLE ptnowner_oldnodes AS @@ -560,6 +562,11 @@ CREATE TEMP TABLE ptnowner_oldnodes AS SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; WARNING: permission denied to execute CLUSTER on "ptnowner2", skipping it +-- still can't repack without a replica identity +ALTER TABLE ptnowner1 REPLICA IDENTITY DEFAULT; +REPACK (CONCURRENTLY) ptnowner1; +ERROR: cannot process relation "ptnowner1" +HINT: Relation "ptnowner1" has no identity index. RESET SESSION AUTHORIZATION; SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; @@ -570,6 +577,15 @@ SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a ptnowner2 | t (3 rows) +SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a + JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; + relname | ?column? +-----------+---------- + ptnowner | t + ptnowner1 | f + ptnowner2 | t +(3 rows) + DROP TABLE ptnowner; DROP ROLE regress_ptnowner; -- Test CLUSTER with external tuplesorting diff --git a/src/test/regress/sql/cluster.sql b/src/test/regress/sql/cluster.sql index d14063a9683..1f471a8821a 100644 --- a/src/test/regress/sql/cluster.sql +++ b/src/test/regress/sql/cluster.sql @@ -254,14 +254,16 @@ REPACK (CONCURRENTLY) clstrpart; DROP TABLE clstrpart; -- Ownership of partitions is checked -CREATE TABLE ptnowner(i int unique) PARTITION BY LIST (i); +CREATE TABLE ptnowner(i int unique not null) PARTITION BY LIST (i); CREATE INDEX ptnowner_i_idx ON ptnowner(i); CREATE TABLE ptnowner1 PARTITION OF ptnowner FOR VALUES IN (1); -CREATE ROLE regress_ptnowner; +CREATE ROLE regress_ptnowner LOGIN; CREATE TABLE ptnowner2 PARTITION OF ptnowner FOR VALUES IN (2); ALTER TABLE ptnowner1 OWNER TO regress_ptnowner; SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; +ALTER TABLE ptnowner1 REPLICA IDENTITY USING INDEX ptnowner1_i_key; +REPACK (CONCURRENTLY) ptnowner1; RESET SESSION AUTHORIZATION; ALTER TABLE ptnowner OWNER TO regress_ptnowner; CREATE TEMP TABLE ptnowner_oldnodes AS @@ -269,7 +271,12 @@ CREATE TEMP TABLE ptnowner_oldnodes AS JOIN pg_class AS c ON c.oid=tree.relid; SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; +-- still can't repack without a replica identity +ALTER TABLE ptnowner1 REPLICA IDENTITY DEFAULT; +REPACK (CONCURRENTLY) ptnowner1; RESET SESSION AUTHORIZATION; +SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a + JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; DROP TABLE ptnowner; -- 2.47.3 --m6zy3l65ushq557m Content-Type: text/x-diff; charset=utf-8 Content-Disposition: attachment; filename="0002-REPACK-do-not-require-LOGIN-privileges.patch" ^ permalink raw reply [nested|flat] 41+ messages in thread
* [PATCH 1/2] REPACK: do not require the user to have REPLICATION @ 2026-04-20 09:38 Álvaro Herrera <[email protected]> 0 siblings, 0 replies; 41+ messages in thread From: Álvaro Herrera @ 2026-04-20 09:38 UTC (permalink / raw) Although REPACK (CONCURRENTLY) uses replication slots, there is no concern that the slot will leak data of other users, because the MAINTAIN privilege on the table is required anyway; requiring REPLICATION is user-unfriendly without providing any actual protection. A related aspect is that the REPLICATION attribute is not needed to prevent REPACK from stealing slots from logical replication, since commit e76d8c749c31 made REPACK use a separate pool of replication slots. Because there are now successful concurrent repack runs in the regression tests, we're forced to run test_plan_advice under wal_level=replica. Author: Antonin Houska <[email protected]> Reported-by: Justin Pryzby <[email protected]> Reviewed-by: Chao Li <[email protected]> Discussion: https://postgr.es/m/aeJHPNmL4vVy3oPw@pryzbyj2023 --- src/backend/commands/repack_worker.c | 1 - .../test_plan_advice/t/001_replan_regress.pl | 1 + src/test/regress/expected/cluster.out | 20 +++++++++++++++++-- src/test/regress/sql/cluster.sql | 11 ++++++++-- 4 files changed, 28 insertions(+), 5 deletions(-) diff --git a/src/backend/commands/repack_worker.c b/src/backend/commands/repack_worker.c index b17edd771e2..e4a4860805b 100644 --- a/src/backend/commands/repack_worker.c +++ b/src/backend/commands/repack_worker.c @@ -214,7 +214,6 @@ repack_setup_logical_decoding(Oid relid) /* * Make sure we can use logical decoding. */ - CheckSlotPermissions(); CheckLogicalDecodingRequirements(true); /* diff --git a/src/test/modules/test_plan_advice/t/001_replan_regress.pl b/src/test/modules/test_plan_advice/t/001_replan_regress.pl index 38ffa4d11ae..452b179a665 100644 --- a/src/test/modules/test_plan_advice/t/001_replan_regress.pl +++ b/src/test/modules/test_plan_advice/t/001_replan_regress.pl @@ -18,6 +18,7 @@ $node->init(); # Set up our desired configuration. $node->append_conf('postgresql.conf', <<EOM); shared_preload_libraries='test_plan_advice' +wal_level=replica pg_plan_advice.always_explain_supplied_advice=false pg_plan_advice.feedback_warnings=true EOM diff --git a/src/test/regress/expected/cluster.out b/src/test/regress/expected/cluster.out index 6127b215a86..e17bc91fae1 100644 --- a/src/test/regress/expected/cluster.out +++ b/src/test/regress/expected/cluster.out @@ -543,15 +543,17 @@ ERROR: REPACK (CONCURRENTLY) is not supported for partitioned tables HINT: Consider running the command on individual partitions. DROP TABLE clstrpart; -- Ownership of partitions is checked -CREATE TABLE ptnowner(i int unique) PARTITION BY LIST (i); +CREATE TABLE ptnowner(i int unique not null) PARTITION BY LIST (i); CREATE INDEX ptnowner_i_idx ON ptnowner(i); CREATE TABLE ptnowner1 PARTITION OF ptnowner FOR VALUES IN (1); -CREATE ROLE regress_ptnowner; +CREATE ROLE regress_ptnowner LOGIN; CREATE TABLE ptnowner2 PARTITION OF ptnowner FOR VALUES IN (2); ALTER TABLE ptnowner1 OWNER TO regress_ptnowner; SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; ERROR: permission denied for table ptnowner +ALTER TABLE ptnowner1 REPLICA IDENTITY USING INDEX ptnowner1_i_key; +REPACK (CONCURRENTLY) ptnowner1; RESET SESSION AUTHORIZATION; ALTER TABLE ptnowner OWNER TO regress_ptnowner; CREATE TEMP TABLE ptnowner_oldnodes AS @@ -560,6 +562,11 @@ CREATE TEMP TABLE ptnowner_oldnodes AS SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; WARNING: permission denied to execute CLUSTER on "ptnowner2", skipping it +-- still can't repack without a replica identity +ALTER TABLE ptnowner1 REPLICA IDENTITY DEFAULT; +REPACK (CONCURRENTLY) ptnowner1; +ERROR: cannot process relation "ptnowner1" +HINT: Relation "ptnowner1" has no identity index. RESET SESSION AUTHORIZATION; SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; @@ -570,6 +577,15 @@ SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a ptnowner2 | t (3 rows) +SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a + JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; + relname | ?column? +-----------+---------- + ptnowner | t + ptnowner1 | f + ptnowner2 | t +(3 rows) + DROP TABLE ptnowner; DROP ROLE regress_ptnowner; -- Test CLUSTER with external tuplesorting diff --git a/src/test/regress/sql/cluster.sql b/src/test/regress/sql/cluster.sql index d14063a9683..1f471a8821a 100644 --- a/src/test/regress/sql/cluster.sql +++ b/src/test/regress/sql/cluster.sql @@ -254,14 +254,16 @@ REPACK (CONCURRENTLY) clstrpart; DROP TABLE clstrpart; -- Ownership of partitions is checked -CREATE TABLE ptnowner(i int unique) PARTITION BY LIST (i); +CREATE TABLE ptnowner(i int unique not null) PARTITION BY LIST (i); CREATE INDEX ptnowner_i_idx ON ptnowner(i); CREATE TABLE ptnowner1 PARTITION OF ptnowner FOR VALUES IN (1); -CREATE ROLE regress_ptnowner; +CREATE ROLE regress_ptnowner LOGIN; CREATE TABLE ptnowner2 PARTITION OF ptnowner FOR VALUES IN (2); ALTER TABLE ptnowner1 OWNER TO regress_ptnowner; SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; +ALTER TABLE ptnowner1 REPLICA IDENTITY USING INDEX ptnowner1_i_key; +REPACK (CONCURRENTLY) ptnowner1; RESET SESSION AUTHORIZATION; ALTER TABLE ptnowner OWNER TO regress_ptnowner; CREATE TEMP TABLE ptnowner_oldnodes AS @@ -269,7 +271,12 @@ CREATE TEMP TABLE ptnowner_oldnodes AS JOIN pg_class AS c ON c.oid=tree.relid; SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; +-- still can't repack without a replica identity +ALTER TABLE ptnowner1 REPLICA IDENTITY DEFAULT; +REPACK (CONCURRENTLY) ptnowner1; RESET SESSION AUTHORIZATION; +SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a + JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; DROP TABLE ptnowner; -- 2.47.3 --m6zy3l65ushq557m Content-Type: text/x-diff; charset=utf-8 Content-Disposition: attachment; filename="0002-REPACK-do-not-require-LOGIN-privileges.patch" ^ permalink raw reply [nested|flat] 41+ messages in thread
* [PATCH 1/2] REPACK: do not require the user to have REPLICATION @ 2026-04-20 09:38 Álvaro Herrera <[email protected]> 0 siblings, 0 replies; 41+ messages in thread From: Álvaro Herrera @ 2026-04-20 09:38 UTC (permalink / raw) Although REPACK (CONCURRENTLY) uses replication slots, there is no concern that the slot will leak data of other users, because the MAINTAIN privilege on the table is required anyway; requiring REPLICATION is user-unfriendly without providing any actual protection. A related aspect is that the REPLICATION attribute is not needed to prevent REPACK from stealing slots from logical replication, since commit e76d8c749c31 made REPACK use a separate pool of replication slots. Because there are now successful concurrent repack runs in the regression tests, we're forced to run test_plan_advice under wal_level=replica. Author: Antonin Houska <[email protected]> Reported-by: Justin Pryzby <[email protected]> Reviewed-by: Chao Li <[email protected]> Discussion: https://postgr.es/m/aeJHPNmL4vVy3oPw@pryzbyj2023 --- src/backend/commands/repack_worker.c | 1 - .../test_plan_advice/t/001_replan_regress.pl | 1 + src/test/regress/expected/cluster.out | 20 +++++++++++++++++-- src/test/regress/sql/cluster.sql | 11 ++++++++-- 4 files changed, 28 insertions(+), 5 deletions(-) diff --git a/src/backend/commands/repack_worker.c b/src/backend/commands/repack_worker.c index b17edd771e2..e4a4860805b 100644 --- a/src/backend/commands/repack_worker.c +++ b/src/backend/commands/repack_worker.c @@ -214,7 +214,6 @@ repack_setup_logical_decoding(Oid relid) /* * Make sure we can use logical decoding. */ - CheckSlotPermissions(); CheckLogicalDecodingRequirements(true); /* diff --git a/src/test/modules/test_plan_advice/t/001_replan_regress.pl b/src/test/modules/test_plan_advice/t/001_replan_regress.pl index 38ffa4d11ae..452b179a665 100644 --- a/src/test/modules/test_plan_advice/t/001_replan_regress.pl +++ b/src/test/modules/test_plan_advice/t/001_replan_regress.pl @@ -18,6 +18,7 @@ $node->init(); # Set up our desired configuration. $node->append_conf('postgresql.conf', <<EOM); shared_preload_libraries='test_plan_advice' +wal_level=replica pg_plan_advice.always_explain_supplied_advice=false pg_plan_advice.feedback_warnings=true EOM diff --git a/src/test/regress/expected/cluster.out b/src/test/regress/expected/cluster.out index 6127b215a86..e17bc91fae1 100644 --- a/src/test/regress/expected/cluster.out +++ b/src/test/regress/expected/cluster.out @@ -543,15 +543,17 @@ ERROR: REPACK (CONCURRENTLY) is not supported for partitioned tables HINT: Consider running the command on individual partitions. DROP TABLE clstrpart; -- Ownership of partitions is checked -CREATE TABLE ptnowner(i int unique) PARTITION BY LIST (i); +CREATE TABLE ptnowner(i int unique not null) PARTITION BY LIST (i); CREATE INDEX ptnowner_i_idx ON ptnowner(i); CREATE TABLE ptnowner1 PARTITION OF ptnowner FOR VALUES IN (1); -CREATE ROLE regress_ptnowner; +CREATE ROLE regress_ptnowner LOGIN; CREATE TABLE ptnowner2 PARTITION OF ptnowner FOR VALUES IN (2); ALTER TABLE ptnowner1 OWNER TO regress_ptnowner; SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; ERROR: permission denied for table ptnowner +ALTER TABLE ptnowner1 REPLICA IDENTITY USING INDEX ptnowner1_i_key; +REPACK (CONCURRENTLY) ptnowner1; RESET SESSION AUTHORIZATION; ALTER TABLE ptnowner OWNER TO regress_ptnowner; CREATE TEMP TABLE ptnowner_oldnodes AS @@ -560,6 +562,11 @@ CREATE TEMP TABLE ptnowner_oldnodes AS SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; WARNING: permission denied to execute CLUSTER on "ptnowner2", skipping it +-- still can't repack without a replica identity +ALTER TABLE ptnowner1 REPLICA IDENTITY DEFAULT; +REPACK (CONCURRENTLY) ptnowner1; +ERROR: cannot process relation "ptnowner1" +HINT: Relation "ptnowner1" has no identity index. RESET SESSION AUTHORIZATION; SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; @@ -570,6 +577,15 @@ SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a ptnowner2 | t (3 rows) +SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a + JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; + relname | ?column? +-----------+---------- + ptnowner | t + ptnowner1 | f + ptnowner2 | t +(3 rows) + DROP TABLE ptnowner; DROP ROLE regress_ptnowner; -- Test CLUSTER with external tuplesorting diff --git a/src/test/regress/sql/cluster.sql b/src/test/regress/sql/cluster.sql index d14063a9683..1f471a8821a 100644 --- a/src/test/regress/sql/cluster.sql +++ b/src/test/regress/sql/cluster.sql @@ -254,14 +254,16 @@ REPACK (CONCURRENTLY) clstrpart; DROP TABLE clstrpart; -- Ownership of partitions is checked -CREATE TABLE ptnowner(i int unique) PARTITION BY LIST (i); +CREATE TABLE ptnowner(i int unique not null) PARTITION BY LIST (i); CREATE INDEX ptnowner_i_idx ON ptnowner(i); CREATE TABLE ptnowner1 PARTITION OF ptnowner FOR VALUES IN (1); -CREATE ROLE regress_ptnowner; +CREATE ROLE regress_ptnowner LOGIN; CREATE TABLE ptnowner2 PARTITION OF ptnowner FOR VALUES IN (2); ALTER TABLE ptnowner1 OWNER TO regress_ptnowner; SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; +ALTER TABLE ptnowner1 REPLICA IDENTITY USING INDEX ptnowner1_i_key; +REPACK (CONCURRENTLY) ptnowner1; RESET SESSION AUTHORIZATION; ALTER TABLE ptnowner OWNER TO regress_ptnowner; CREATE TEMP TABLE ptnowner_oldnodes AS @@ -269,7 +271,12 @@ CREATE TEMP TABLE ptnowner_oldnodes AS JOIN pg_class AS c ON c.oid=tree.relid; SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; +-- still can't repack without a replica identity +ALTER TABLE ptnowner1 REPLICA IDENTITY DEFAULT; +REPACK (CONCURRENTLY) ptnowner1; RESET SESSION AUTHORIZATION; +SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a + JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; DROP TABLE ptnowner; -- 2.47.3 --m6zy3l65ushq557m Content-Type: text/x-diff; charset=utf-8 Content-Disposition: attachment; filename="0002-REPACK-do-not-require-LOGIN-privileges.patch" ^ permalink raw reply [nested|flat] 41+ messages in thread
* [PATCH 1/2] REPACK: do not require the user to have REPLICATION @ 2026-04-20 09:38 Álvaro Herrera <[email protected]> 0 siblings, 0 replies; 41+ messages in thread From: Álvaro Herrera @ 2026-04-20 09:38 UTC (permalink / raw) Although REPACK (CONCURRENTLY) uses replication slots, there is no concern that the slot will leak data of other users, because the MAINTAIN privilege on the table is required anyway; requiring REPLICATION is user-unfriendly without providing any actual protection. A related aspect is that the REPLICATION attribute is not needed to prevent REPACK from stealing slots from logical replication, since commit e76d8c749c31 made REPACK use a separate pool of replication slots. Because there are now successful concurrent repack runs in the regression tests, we're forced to run test_plan_advice under wal_level=replica. Author: Antonin Houska <[email protected]> Reported-by: Justin Pryzby <[email protected]> Reviewed-by: Chao Li <[email protected]> Discussion: https://postgr.es/m/aeJHPNmL4vVy3oPw@pryzbyj2023 --- src/backend/commands/repack_worker.c | 1 - .../test_plan_advice/t/001_replan_regress.pl | 1 + src/test/regress/expected/cluster.out | 20 +++++++++++++++++-- src/test/regress/sql/cluster.sql | 11 ++++++++-- 4 files changed, 28 insertions(+), 5 deletions(-) diff --git a/src/backend/commands/repack_worker.c b/src/backend/commands/repack_worker.c index b17edd771e2..e4a4860805b 100644 --- a/src/backend/commands/repack_worker.c +++ b/src/backend/commands/repack_worker.c @@ -214,7 +214,6 @@ repack_setup_logical_decoding(Oid relid) /* * Make sure we can use logical decoding. */ - CheckSlotPermissions(); CheckLogicalDecodingRequirements(true); /* diff --git a/src/test/modules/test_plan_advice/t/001_replan_regress.pl b/src/test/modules/test_plan_advice/t/001_replan_regress.pl index 38ffa4d11ae..452b179a665 100644 --- a/src/test/modules/test_plan_advice/t/001_replan_regress.pl +++ b/src/test/modules/test_plan_advice/t/001_replan_regress.pl @@ -18,6 +18,7 @@ $node->init(); # Set up our desired configuration. $node->append_conf('postgresql.conf', <<EOM); shared_preload_libraries='test_plan_advice' +wal_level=replica pg_plan_advice.always_explain_supplied_advice=false pg_plan_advice.feedback_warnings=true EOM diff --git a/src/test/regress/expected/cluster.out b/src/test/regress/expected/cluster.out index 6127b215a86..e17bc91fae1 100644 --- a/src/test/regress/expected/cluster.out +++ b/src/test/regress/expected/cluster.out @@ -543,15 +543,17 @@ ERROR: REPACK (CONCURRENTLY) is not supported for partitioned tables HINT: Consider running the command on individual partitions. DROP TABLE clstrpart; -- Ownership of partitions is checked -CREATE TABLE ptnowner(i int unique) PARTITION BY LIST (i); +CREATE TABLE ptnowner(i int unique not null) PARTITION BY LIST (i); CREATE INDEX ptnowner_i_idx ON ptnowner(i); CREATE TABLE ptnowner1 PARTITION OF ptnowner FOR VALUES IN (1); -CREATE ROLE regress_ptnowner; +CREATE ROLE regress_ptnowner LOGIN; CREATE TABLE ptnowner2 PARTITION OF ptnowner FOR VALUES IN (2); ALTER TABLE ptnowner1 OWNER TO regress_ptnowner; SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; ERROR: permission denied for table ptnowner +ALTER TABLE ptnowner1 REPLICA IDENTITY USING INDEX ptnowner1_i_key; +REPACK (CONCURRENTLY) ptnowner1; RESET SESSION AUTHORIZATION; ALTER TABLE ptnowner OWNER TO regress_ptnowner; CREATE TEMP TABLE ptnowner_oldnodes AS @@ -560,6 +562,11 @@ CREATE TEMP TABLE ptnowner_oldnodes AS SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; WARNING: permission denied to execute CLUSTER on "ptnowner2", skipping it +-- still can't repack without a replica identity +ALTER TABLE ptnowner1 REPLICA IDENTITY DEFAULT; +REPACK (CONCURRENTLY) ptnowner1; +ERROR: cannot process relation "ptnowner1" +HINT: Relation "ptnowner1" has no identity index. RESET SESSION AUTHORIZATION; SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; @@ -570,6 +577,15 @@ SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a ptnowner2 | t (3 rows) +SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a + JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; + relname | ?column? +-----------+---------- + ptnowner | t + ptnowner1 | f + ptnowner2 | t +(3 rows) + DROP TABLE ptnowner; DROP ROLE regress_ptnowner; -- Test CLUSTER with external tuplesorting diff --git a/src/test/regress/sql/cluster.sql b/src/test/regress/sql/cluster.sql index d14063a9683..1f471a8821a 100644 --- a/src/test/regress/sql/cluster.sql +++ b/src/test/regress/sql/cluster.sql @@ -254,14 +254,16 @@ REPACK (CONCURRENTLY) clstrpart; DROP TABLE clstrpart; -- Ownership of partitions is checked -CREATE TABLE ptnowner(i int unique) PARTITION BY LIST (i); +CREATE TABLE ptnowner(i int unique not null) PARTITION BY LIST (i); CREATE INDEX ptnowner_i_idx ON ptnowner(i); CREATE TABLE ptnowner1 PARTITION OF ptnowner FOR VALUES IN (1); -CREATE ROLE regress_ptnowner; +CREATE ROLE regress_ptnowner LOGIN; CREATE TABLE ptnowner2 PARTITION OF ptnowner FOR VALUES IN (2); ALTER TABLE ptnowner1 OWNER TO regress_ptnowner; SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; +ALTER TABLE ptnowner1 REPLICA IDENTITY USING INDEX ptnowner1_i_key; +REPACK (CONCURRENTLY) ptnowner1; RESET SESSION AUTHORIZATION; ALTER TABLE ptnowner OWNER TO regress_ptnowner; CREATE TEMP TABLE ptnowner_oldnodes AS @@ -269,7 +271,12 @@ CREATE TEMP TABLE ptnowner_oldnodes AS JOIN pg_class AS c ON c.oid=tree.relid; SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; +-- still can't repack without a replica identity +ALTER TABLE ptnowner1 REPLICA IDENTITY DEFAULT; +REPACK (CONCURRENTLY) ptnowner1; RESET SESSION AUTHORIZATION; +SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a + JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; DROP TABLE ptnowner; -- 2.47.3 --m6zy3l65ushq557m Content-Type: text/x-diff; charset=utf-8 Content-Disposition: attachment; filename="0002-REPACK-do-not-require-LOGIN-privileges.patch" ^ permalink raw reply [nested|flat] 41+ messages in thread
* [PATCH 1/2] REPACK: do not require the user to have REPLICATION @ 2026-04-20 09:38 Álvaro Herrera <[email protected]> 0 siblings, 0 replies; 41+ messages in thread From: Álvaro Herrera @ 2026-04-20 09:38 UTC (permalink / raw) Although REPACK (CONCURRENTLY) uses replication slots, there is no concern that the slot will leak data of other users, because the MAINTAIN privilege on the table is required anyway; requiring REPLICATION is user-unfriendly without providing any actual protection. A related aspect is that the REPLICATION attribute is not needed to prevent REPACK from stealing slots from logical replication, since commit e76d8c749c31 made REPACK use a separate pool of replication slots. Because there are now successful concurrent repack runs in the regression tests, we're forced to run test_plan_advice under wal_level=replica. Author: Antonin Houska <[email protected]> Reported-by: Justin Pryzby <[email protected]> Reviewed-by: Chao Li <[email protected]> Discussion: https://postgr.es/m/aeJHPNmL4vVy3oPw@pryzbyj2023 --- src/backend/commands/repack_worker.c | 1 - .../test_plan_advice/t/001_replan_regress.pl | 1 + src/test/regress/expected/cluster.out | 20 +++++++++++++++++-- src/test/regress/sql/cluster.sql | 11 ++++++++-- 4 files changed, 28 insertions(+), 5 deletions(-) diff --git a/src/backend/commands/repack_worker.c b/src/backend/commands/repack_worker.c index b17edd771e2..e4a4860805b 100644 --- a/src/backend/commands/repack_worker.c +++ b/src/backend/commands/repack_worker.c @@ -214,7 +214,6 @@ repack_setup_logical_decoding(Oid relid) /* * Make sure we can use logical decoding. */ - CheckSlotPermissions(); CheckLogicalDecodingRequirements(true); /* diff --git a/src/test/modules/test_plan_advice/t/001_replan_regress.pl b/src/test/modules/test_plan_advice/t/001_replan_regress.pl index 38ffa4d11ae..452b179a665 100644 --- a/src/test/modules/test_plan_advice/t/001_replan_regress.pl +++ b/src/test/modules/test_plan_advice/t/001_replan_regress.pl @@ -18,6 +18,7 @@ $node->init(); # Set up our desired configuration. $node->append_conf('postgresql.conf', <<EOM); shared_preload_libraries='test_plan_advice' +wal_level=replica pg_plan_advice.always_explain_supplied_advice=false pg_plan_advice.feedback_warnings=true EOM diff --git a/src/test/regress/expected/cluster.out b/src/test/regress/expected/cluster.out index 6127b215a86..e17bc91fae1 100644 --- a/src/test/regress/expected/cluster.out +++ b/src/test/regress/expected/cluster.out @@ -543,15 +543,17 @@ ERROR: REPACK (CONCURRENTLY) is not supported for partitioned tables HINT: Consider running the command on individual partitions. DROP TABLE clstrpart; -- Ownership of partitions is checked -CREATE TABLE ptnowner(i int unique) PARTITION BY LIST (i); +CREATE TABLE ptnowner(i int unique not null) PARTITION BY LIST (i); CREATE INDEX ptnowner_i_idx ON ptnowner(i); CREATE TABLE ptnowner1 PARTITION OF ptnowner FOR VALUES IN (1); -CREATE ROLE regress_ptnowner; +CREATE ROLE regress_ptnowner LOGIN; CREATE TABLE ptnowner2 PARTITION OF ptnowner FOR VALUES IN (2); ALTER TABLE ptnowner1 OWNER TO regress_ptnowner; SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; ERROR: permission denied for table ptnowner +ALTER TABLE ptnowner1 REPLICA IDENTITY USING INDEX ptnowner1_i_key; +REPACK (CONCURRENTLY) ptnowner1; RESET SESSION AUTHORIZATION; ALTER TABLE ptnowner OWNER TO regress_ptnowner; CREATE TEMP TABLE ptnowner_oldnodes AS @@ -560,6 +562,11 @@ CREATE TEMP TABLE ptnowner_oldnodes AS SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; WARNING: permission denied to execute CLUSTER on "ptnowner2", skipping it +-- still can't repack without a replica identity +ALTER TABLE ptnowner1 REPLICA IDENTITY DEFAULT; +REPACK (CONCURRENTLY) ptnowner1; +ERROR: cannot process relation "ptnowner1" +HINT: Relation "ptnowner1" has no identity index. RESET SESSION AUTHORIZATION; SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; @@ -570,6 +577,15 @@ SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a ptnowner2 | t (3 rows) +SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a + JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; + relname | ?column? +-----------+---------- + ptnowner | t + ptnowner1 | f + ptnowner2 | t +(3 rows) + DROP TABLE ptnowner; DROP ROLE regress_ptnowner; -- Test CLUSTER with external tuplesorting diff --git a/src/test/regress/sql/cluster.sql b/src/test/regress/sql/cluster.sql index d14063a9683..1f471a8821a 100644 --- a/src/test/regress/sql/cluster.sql +++ b/src/test/regress/sql/cluster.sql @@ -254,14 +254,16 @@ REPACK (CONCURRENTLY) clstrpart; DROP TABLE clstrpart; -- Ownership of partitions is checked -CREATE TABLE ptnowner(i int unique) PARTITION BY LIST (i); +CREATE TABLE ptnowner(i int unique not null) PARTITION BY LIST (i); CREATE INDEX ptnowner_i_idx ON ptnowner(i); CREATE TABLE ptnowner1 PARTITION OF ptnowner FOR VALUES IN (1); -CREATE ROLE regress_ptnowner; +CREATE ROLE regress_ptnowner LOGIN; CREATE TABLE ptnowner2 PARTITION OF ptnowner FOR VALUES IN (2); ALTER TABLE ptnowner1 OWNER TO regress_ptnowner; SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; +ALTER TABLE ptnowner1 REPLICA IDENTITY USING INDEX ptnowner1_i_key; +REPACK (CONCURRENTLY) ptnowner1; RESET SESSION AUTHORIZATION; ALTER TABLE ptnowner OWNER TO regress_ptnowner; CREATE TEMP TABLE ptnowner_oldnodes AS @@ -269,7 +271,12 @@ CREATE TEMP TABLE ptnowner_oldnodes AS JOIN pg_class AS c ON c.oid=tree.relid; SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; +-- still can't repack without a replica identity +ALTER TABLE ptnowner1 REPLICA IDENTITY DEFAULT; +REPACK (CONCURRENTLY) ptnowner1; RESET SESSION AUTHORIZATION; +SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a + JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; DROP TABLE ptnowner; -- 2.47.3 --m6zy3l65ushq557m Content-Type: text/x-diff; charset=utf-8 Content-Disposition: attachment; filename="0002-REPACK-do-not-require-LOGIN-privileges.patch" ^ permalink raw reply [nested|flat] 41+ messages in thread
* [PATCH 1/2] REPACK: do not require the user to have REPLICATION @ 2026-04-20 09:38 Álvaro Herrera <[email protected]> 0 siblings, 0 replies; 41+ messages in thread From: Álvaro Herrera @ 2026-04-20 09:38 UTC (permalink / raw) Although REPACK (CONCURRENTLY) uses replication slots, there is no concern that the slot will leak data of other users, because the MAINTAIN privilege on the table is required anyway; requiring REPLICATION is user-unfriendly without providing any actual protection. A related aspect is that the REPLICATION attribute is not needed to prevent REPACK from stealing slots from logical replication, since commit e76d8c749c31 made REPACK use a separate pool of replication slots. Because there are now successful concurrent repack runs in the regression tests, we're forced to run test_plan_advice under wal_level=replica. Author: Antonin Houska <[email protected]> Reported-by: Justin Pryzby <[email protected]> Reviewed-by: Chao Li <[email protected]> Discussion: https://postgr.es/m/aeJHPNmL4vVy3oPw@pryzbyj2023 --- src/backend/commands/repack_worker.c | 1 - .../test_plan_advice/t/001_replan_regress.pl | 1 + src/test/regress/expected/cluster.out | 20 +++++++++++++++++-- src/test/regress/sql/cluster.sql | 11 ++++++++-- 4 files changed, 28 insertions(+), 5 deletions(-) diff --git a/src/backend/commands/repack_worker.c b/src/backend/commands/repack_worker.c index b17edd771e2..e4a4860805b 100644 --- a/src/backend/commands/repack_worker.c +++ b/src/backend/commands/repack_worker.c @@ -214,7 +214,6 @@ repack_setup_logical_decoding(Oid relid) /* * Make sure we can use logical decoding. */ - CheckSlotPermissions(); CheckLogicalDecodingRequirements(true); /* diff --git a/src/test/modules/test_plan_advice/t/001_replan_regress.pl b/src/test/modules/test_plan_advice/t/001_replan_regress.pl index 38ffa4d11ae..452b179a665 100644 --- a/src/test/modules/test_plan_advice/t/001_replan_regress.pl +++ b/src/test/modules/test_plan_advice/t/001_replan_regress.pl @@ -18,6 +18,7 @@ $node->init(); # Set up our desired configuration. $node->append_conf('postgresql.conf', <<EOM); shared_preload_libraries='test_plan_advice' +wal_level=replica pg_plan_advice.always_explain_supplied_advice=false pg_plan_advice.feedback_warnings=true EOM diff --git a/src/test/regress/expected/cluster.out b/src/test/regress/expected/cluster.out index 6127b215a86..e17bc91fae1 100644 --- a/src/test/regress/expected/cluster.out +++ b/src/test/regress/expected/cluster.out @@ -543,15 +543,17 @@ ERROR: REPACK (CONCURRENTLY) is not supported for partitioned tables HINT: Consider running the command on individual partitions. DROP TABLE clstrpart; -- Ownership of partitions is checked -CREATE TABLE ptnowner(i int unique) PARTITION BY LIST (i); +CREATE TABLE ptnowner(i int unique not null) PARTITION BY LIST (i); CREATE INDEX ptnowner_i_idx ON ptnowner(i); CREATE TABLE ptnowner1 PARTITION OF ptnowner FOR VALUES IN (1); -CREATE ROLE regress_ptnowner; +CREATE ROLE regress_ptnowner LOGIN; CREATE TABLE ptnowner2 PARTITION OF ptnowner FOR VALUES IN (2); ALTER TABLE ptnowner1 OWNER TO regress_ptnowner; SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; ERROR: permission denied for table ptnowner +ALTER TABLE ptnowner1 REPLICA IDENTITY USING INDEX ptnowner1_i_key; +REPACK (CONCURRENTLY) ptnowner1; RESET SESSION AUTHORIZATION; ALTER TABLE ptnowner OWNER TO regress_ptnowner; CREATE TEMP TABLE ptnowner_oldnodes AS @@ -560,6 +562,11 @@ CREATE TEMP TABLE ptnowner_oldnodes AS SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; WARNING: permission denied to execute CLUSTER on "ptnowner2", skipping it +-- still can't repack without a replica identity +ALTER TABLE ptnowner1 REPLICA IDENTITY DEFAULT; +REPACK (CONCURRENTLY) ptnowner1; +ERROR: cannot process relation "ptnowner1" +HINT: Relation "ptnowner1" has no identity index. RESET SESSION AUTHORIZATION; SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; @@ -570,6 +577,15 @@ SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a ptnowner2 | t (3 rows) +SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a + JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; + relname | ?column? +-----------+---------- + ptnowner | t + ptnowner1 | f + ptnowner2 | t +(3 rows) + DROP TABLE ptnowner; DROP ROLE regress_ptnowner; -- Test CLUSTER with external tuplesorting diff --git a/src/test/regress/sql/cluster.sql b/src/test/regress/sql/cluster.sql index d14063a9683..1f471a8821a 100644 --- a/src/test/regress/sql/cluster.sql +++ b/src/test/regress/sql/cluster.sql @@ -254,14 +254,16 @@ REPACK (CONCURRENTLY) clstrpart; DROP TABLE clstrpart; -- Ownership of partitions is checked -CREATE TABLE ptnowner(i int unique) PARTITION BY LIST (i); +CREATE TABLE ptnowner(i int unique not null) PARTITION BY LIST (i); CREATE INDEX ptnowner_i_idx ON ptnowner(i); CREATE TABLE ptnowner1 PARTITION OF ptnowner FOR VALUES IN (1); -CREATE ROLE regress_ptnowner; +CREATE ROLE regress_ptnowner LOGIN; CREATE TABLE ptnowner2 PARTITION OF ptnowner FOR VALUES IN (2); ALTER TABLE ptnowner1 OWNER TO regress_ptnowner; SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; +ALTER TABLE ptnowner1 REPLICA IDENTITY USING INDEX ptnowner1_i_key; +REPACK (CONCURRENTLY) ptnowner1; RESET SESSION AUTHORIZATION; ALTER TABLE ptnowner OWNER TO regress_ptnowner; CREATE TEMP TABLE ptnowner_oldnodes AS @@ -269,7 +271,12 @@ CREATE TEMP TABLE ptnowner_oldnodes AS JOIN pg_class AS c ON c.oid=tree.relid; SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; +-- still can't repack without a replica identity +ALTER TABLE ptnowner1 REPLICA IDENTITY DEFAULT; +REPACK (CONCURRENTLY) ptnowner1; RESET SESSION AUTHORIZATION; +SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a + JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; DROP TABLE ptnowner; -- 2.47.3 --m6zy3l65ushq557m Content-Type: text/x-diff; charset=utf-8 Content-Disposition: attachment; filename="0002-REPACK-do-not-require-LOGIN-privileges.patch" ^ permalink raw reply [nested|flat] 41+ messages in thread
* [PATCH 1/2] REPACK: do not require the user to have REPLICATION @ 2026-04-20 09:38 Álvaro Herrera <[email protected]> 0 siblings, 0 replies; 41+ messages in thread From: Álvaro Herrera @ 2026-04-20 09:38 UTC (permalink / raw) Although REPACK (CONCURRENTLY) uses replication slots, there is no concern that the slot will leak data of other users, because the MAINTAIN privilege on the table is required anyway; requiring REPLICATION is user-unfriendly without providing any actual protection. A related aspect is that the REPLICATION attribute is not needed to prevent REPACK from stealing slots from logical replication, since commit e76d8c749c31 made REPACK use a separate pool of replication slots. Because there are now successful concurrent repack runs in the regression tests, we're forced to run test_plan_advice under wal_level=replica. Author: Antonin Houska <[email protected]> Reported-by: Justin Pryzby <[email protected]> Reviewed-by: Chao Li <[email protected]> Discussion: https://postgr.es/m/aeJHPNmL4vVy3oPw@pryzbyj2023 --- src/backend/commands/repack_worker.c | 1 - .../test_plan_advice/t/001_replan_regress.pl | 1 + src/test/regress/expected/cluster.out | 20 +++++++++++++++++-- src/test/regress/sql/cluster.sql | 11 ++++++++-- 4 files changed, 28 insertions(+), 5 deletions(-) diff --git a/src/backend/commands/repack_worker.c b/src/backend/commands/repack_worker.c index b17edd771e2..e4a4860805b 100644 --- a/src/backend/commands/repack_worker.c +++ b/src/backend/commands/repack_worker.c @@ -214,7 +214,6 @@ repack_setup_logical_decoding(Oid relid) /* * Make sure we can use logical decoding. */ - CheckSlotPermissions(); CheckLogicalDecodingRequirements(true); /* diff --git a/src/test/modules/test_plan_advice/t/001_replan_regress.pl b/src/test/modules/test_plan_advice/t/001_replan_regress.pl index 38ffa4d11ae..452b179a665 100644 --- a/src/test/modules/test_plan_advice/t/001_replan_regress.pl +++ b/src/test/modules/test_plan_advice/t/001_replan_regress.pl @@ -18,6 +18,7 @@ $node->init(); # Set up our desired configuration. $node->append_conf('postgresql.conf', <<EOM); shared_preload_libraries='test_plan_advice' +wal_level=replica pg_plan_advice.always_explain_supplied_advice=false pg_plan_advice.feedback_warnings=true EOM diff --git a/src/test/regress/expected/cluster.out b/src/test/regress/expected/cluster.out index 6127b215a86..e17bc91fae1 100644 --- a/src/test/regress/expected/cluster.out +++ b/src/test/regress/expected/cluster.out @@ -543,15 +543,17 @@ ERROR: REPACK (CONCURRENTLY) is not supported for partitioned tables HINT: Consider running the command on individual partitions. DROP TABLE clstrpart; -- Ownership of partitions is checked -CREATE TABLE ptnowner(i int unique) PARTITION BY LIST (i); +CREATE TABLE ptnowner(i int unique not null) PARTITION BY LIST (i); CREATE INDEX ptnowner_i_idx ON ptnowner(i); CREATE TABLE ptnowner1 PARTITION OF ptnowner FOR VALUES IN (1); -CREATE ROLE regress_ptnowner; +CREATE ROLE regress_ptnowner LOGIN; CREATE TABLE ptnowner2 PARTITION OF ptnowner FOR VALUES IN (2); ALTER TABLE ptnowner1 OWNER TO regress_ptnowner; SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; ERROR: permission denied for table ptnowner +ALTER TABLE ptnowner1 REPLICA IDENTITY USING INDEX ptnowner1_i_key; +REPACK (CONCURRENTLY) ptnowner1; RESET SESSION AUTHORIZATION; ALTER TABLE ptnowner OWNER TO regress_ptnowner; CREATE TEMP TABLE ptnowner_oldnodes AS @@ -560,6 +562,11 @@ CREATE TEMP TABLE ptnowner_oldnodes AS SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; WARNING: permission denied to execute CLUSTER on "ptnowner2", skipping it +-- still can't repack without a replica identity +ALTER TABLE ptnowner1 REPLICA IDENTITY DEFAULT; +REPACK (CONCURRENTLY) ptnowner1; +ERROR: cannot process relation "ptnowner1" +HINT: Relation "ptnowner1" has no identity index. RESET SESSION AUTHORIZATION; SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; @@ -570,6 +577,15 @@ SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a ptnowner2 | t (3 rows) +SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a + JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; + relname | ?column? +-----------+---------- + ptnowner | t + ptnowner1 | f + ptnowner2 | t +(3 rows) + DROP TABLE ptnowner; DROP ROLE regress_ptnowner; -- Test CLUSTER with external tuplesorting diff --git a/src/test/regress/sql/cluster.sql b/src/test/regress/sql/cluster.sql index d14063a9683..1f471a8821a 100644 --- a/src/test/regress/sql/cluster.sql +++ b/src/test/regress/sql/cluster.sql @@ -254,14 +254,16 @@ REPACK (CONCURRENTLY) clstrpart; DROP TABLE clstrpart; -- Ownership of partitions is checked -CREATE TABLE ptnowner(i int unique) PARTITION BY LIST (i); +CREATE TABLE ptnowner(i int unique not null) PARTITION BY LIST (i); CREATE INDEX ptnowner_i_idx ON ptnowner(i); CREATE TABLE ptnowner1 PARTITION OF ptnowner FOR VALUES IN (1); -CREATE ROLE regress_ptnowner; +CREATE ROLE regress_ptnowner LOGIN; CREATE TABLE ptnowner2 PARTITION OF ptnowner FOR VALUES IN (2); ALTER TABLE ptnowner1 OWNER TO regress_ptnowner; SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; +ALTER TABLE ptnowner1 REPLICA IDENTITY USING INDEX ptnowner1_i_key; +REPACK (CONCURRENTLY) ptnowner1; RESET SESSION AUTHORIZATION; ALTER TABLE ptnowner OWNER TO regress_ptnowner; CREATE TEMP TABLE ptnowner_oldnodes AS @@ -269,7 +271,12 @@ CREATE TEMP TABLE ptnowner_oldnodes AS JOIN pg_class AS c ON c.oid=tree.relid; SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; +-- still can't repack without a replica identity +ALTER TABLE ptnowner1 REPLICA IDENTITY DEFAULT; +REPACK (CONCURRENTLY) ptnowner1; RESET SESSION AUTHORIZATION; +SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a + JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; DROP TABLE ptnowner; -- 2.47.3 --m6zy3l65ushq557m Content-Type: text/x-diff; charset=utf-8 Content-Disposition: attachment; filename="0002-REPACK-do-not-require-LOGIN-privileges.patch" ^ permalink raw reply [nested|flat] 41+ messages in thread
* [PATCH 1/2] REPACK: do not require the user to have REPLICATION @ 2026-04-20 09:38 Álvaro Herrera <[email protected]> 0 siblings, 0 replies; 41+ messages in thread From: Álvaro Herrera @ 2026-04-20 09:38 UTC (permalink / raw) Although REPACK (CONCURRENTLY) uses replication slots, there is no concern that the slot will leak data of other users, because the MAINTAIN privilege on the table is required anyway; requiring REPLICATION is user-unfriendly without providing any actual protection. A related aspect is that the REPLICATION attribute is not needed to prevent REPACK from stealing slots from logical replication, since commit e76d8c749c31 made REPACK use a separate pool of replication slots. Because there are now successful concurrent repack runs in the regression tests, we're forced to run test_plan_advice under wal_level=replica. Author: Antonin Houska <[email protected]> Reported-by: Justin Pryzby <[email protected]> Reviewed-by: Chao Li <[email protected]> Discussion: https://postgr.es/m/aeJHPNmL4vVy3oPw@pryzbyj2023 --- src/backend/commands/repack_worker.c | 1 - .../test_plan_advice/t/001_replan_regress.pl | 1 + src/test/regress/expected/cluster.out | 20 +++++++++++++++++-- src/test/regress/sql/cluster.sql | 11 ++++++++-- 4 files changed, 28 insertions(+), 5 deletions(-) diff --git a/src/backend/commands/repack_worker.c b/src/backend/commands/repack_worker.c index b17edd771e2..e4a4860805b 100644 --- a/src/backend/commands/repack_worker.c +++ b/src/backend/commands/repack_worker.c @@ -214,7 +214,6 @@ repack_setup_logical_decoding(Oid relid) /* * Make sure we can use logical decoding. */ - CheckSlotPermissions(); CheckLogicalDecodingRequirements(true); /* diff --git a/src/test/modules/test_plan_advice/t/001_replan_regress.pl b/src/test/modules/test_plan_advice/t/001_replan_regress.pl index 38ffa4d11ae..452b179a665 100644 --- a/src/test/modules/test_plan_advice/t/001_replan_regress.pl +++ b/src/test/modules/test_plan_advice/t/001_replan_regress.pl @@ -18,6 +18,7 @@ $node->init(); # Set up our desired configuration. $node->append_conf('postgresql.conf', <<EOM); shared_preload_libraries='test_plan_advice' +wal_level=replica pg_plan_advice.always_explain_supplied_advice=false pg_plan_advice.feedback_warnings=true EOM diff --git a/src/test/regress/expected/cluster.out b/src/test/regress/expected/cluster.out index 6127b215a86..e17bc91fae1 100644 --- a/src/test/regress/expected/cluster.out +++ b/src/test/regress/expected/cluster.out @@ -543,15 +543,17 @@ ERROR: REPACK (CONCURRENTLY) is not supported for partitioned tables HINT: Consider running the command on individual partitions. DROP TABLE clstrpart; -- Ownership of partitions is checked -CREATE TABLE ptnowner(i int unique) PARTITION BY LIST (i); +CREATE TABLE ptnowner(i int unique not null) PARTITION BY LIST (i); CREATE INDEX ptnowner_i_idx ON ptnowner(i); CREATE TABLE ptnowner1 PARTITION OF ptnowner FOR VALUES IN (1); -CREATE ROLE regress_ptnowner; +CREATE ROLE regress_ptnowner LOGIN; CREATE TABLE ptnowner2 PARTITION OF ptnowner FOR VALUES IN (2); ALTER TABLE ptnowner1 OWNER TO regress_ptnowner; SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; ERROR: permission denied for table ptnowner +ALTER TABLE ptnowner1 REPLICA IDENTITY USING INDEX ptnowner1_i_key; +REPACK (CONCURRENTLY) ptnowner1; RESET SESSION AUTHORIZATION; ALTER TABLE ptnowner OWNER TO regress_ptnowner; CREATE TEMP TABLE ptnowner_oldnodes AS @@ -560,6 +562,11 @@ CREATE TEMP TABLE ptnowner_oldnodes AS SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; WARNING: permission denied to execute CLUSTER on "ptnowner2", skipping it +-- still can't repack without a replica identity +ALTER TABLE ptnowner1 REPLICA IDENTITY DEFAULT; +REPACK (CONCURRENTLY) ptnowner1; +ERROR: cannot process relation "ptnowner1" +HINT: Relation "ptnowner1" has no identity index. RESET SESSION AUTHORIZATION; SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; @@ -570,6 +577,15 @@ SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a ptnowner2 | t (3 rows) +SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a + JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; + relname | ?column? +-----------+---------- + ptnowner | t + ptnowner1 | f + ptnowner2 | t +(3 rows) + DROP TABLE ptnowner; DROP ROLE regress_ptnowner; -- Test CLUSTER with external tuplesorting diff --git a/src/test/regress/sql/cluster.sql b/src/test/regress/sql/cluster.sql index d14063a9683..1f471a8821a 100644 --- a/src/test/regress/sql/cluster.sql +++ b/src/test/regress/sql/cluster.sql @@ -254,14 +254,16 @@ REPACK (CONCURRENTLY) clstrpart; DROP TABLE clstrpart; -- Ownership of partitions is checked -CREATE TABLE ptnowner(i int unique) PARTITION BY LIST (i); +CREATE TABLE ptnowner(i int unique not null) PARTITION BY LIST (i); CREATE INDEX ptnowner_i_idx ON ptnowner(i); CREATE TABLE ptnowner1 PARTITION OF ptnowner FOR VALUES IN (1); -CREATE ROLE regress_ptnowner; +CREATE ROLE regress_ptnowner LOGIN; CREATE TABLE ptnowner2 PARTITION OF ptnowner FOR VALUES IN (2); ALTER TABLE ptnowner1 OWNER TO regress_ptnowner; SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; +ALTER TABLE ptnowner1 REPLICA IDENTITY USING INDEX ptnowner1_i_key; +REPACK (CONCURRENTLY) ptnowner1; RESET SESSION AUTHORIZATION; ALTER TABLE ptnowner OWNER TO regress_ptnowner; CREATE TEMP TABLE ptnowner_oldnodes AS @@ -269,7 +271,12 @@ CREATE TEMP TABLE ptnowner_oldnodes AS JOIN pg_class AS c ON c.oid=tree.relid; SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; +-- still can't repack without a replica identity +ALTER TABLE ptnowner1 REPLICA IDENTITY DEFAULT; +REPACK (CONCURRENTLY) ptnowner1; RESET SESSION AUTHORIZATION; +SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a + JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; DROP TABLE ptnowner; -- 2.47.3 --m6zy3l65ushq557m Content-Type: text/x-diff; charset=utf-8 Content-Disposition: attachment; filename="0002-REPACK-do-not-require-LOGIN-privileges.patch" ^ permalink raw reply [nested|flat] 41+ messages in thread
* [PATCH 1/2] REPACK: do not require the user to have REPLICATION @ 2026-04-20 09:38 Álvaro Herrera <[email protected]> 0 siblings, 0 replies; 41+ messages in thread From: Álvaro Herrera @ 2026-04-20 09:38 UTC (permalink / raw) Although REPACK (CONCURRENTLY) uses replication slots, there is no concern that the slot will leak data of other users, because the MAINTAIN privilege on the table is required anyway; requiring REPLICATION is user-unfriendly without providing any actual protection. A related aspect is that the REPLICATION attribute is not needed to prevent REPACK from stealing slots from logical replication, since commit e76d8c749c31 made REPACK use a separate pool of replication slots. Because there are now successful concurrent repack runs in the regression tests, we're forced to run test_plan_advice under wal_level=replica. Author: Antonin Houska <[email protected]> Reported-by: Justin Pryzby <[email protected]> Reviewed-by: Chao Li <[email protected]> Discussion: https://postgr.es/m/aeJHPNmL4vVy3oPw@pryzbyj2023 --- src/backend/commands/repack_worker.c | 1 - .../test_plan_advice/t/001_replan_regress.pl | 1 + src/test/regress/expected/cluster.out | 20 +++++++++++++++++-- src/test/regress/sql/cluster.sql | 11 ++++++++-- 4 files changed, 28 insertions(+), 5 deletions(-) diff --git a/src/backend/commands/repack_worker.c b/src/backend/commands/repack_worker.c index b17edd771e2..e4a4860805b 100644 --- a/src/backend/commands/repack_worker.c +++ b/src/backend/commands/repack_worker.c @@ -214,7 +214,6 @@ repack_setup_logical_decoding(Oid relid) /* * Make sure we can use logical decoding. */ - CheckSlotPermissions(); CheckLogicalDecodingRequirements(true); /* diff --git a/src/test/modules/test_plan_advice/t/001_replan_regress.pl b/src/test/modules/test_plan_advice/t/001_replan_regress.pl index 38ffa4d11ae..452b179a665 100644 --- a/src/test/modules/test_plan_advice/t/001_replan_regress.pl +++ b/src/test/modules/test_plan_advice/t/001_replan_regress.pl @@ -18,6 +18,7 @@ $node->init(); # Set up our desired configuration. $node->append_conf('postgresql.conf', <<EOM); shared_preload_libraries='test_plan_advice' +wal_level=replica pg_plan_advice.always_explain_supplied_advice=false pg_plan_advice.feedback_warnings=true EOM diff --git a/src/test/regress/expected/cluster.out b/src/test/regress/expected/cluster.out index 6127b215a86..e17bc91fae1 100644 --- a/src/test/regress/expected/cluster.out +++ b/src/test/regress/expected/cluster.out @@ -543,15 +543,17 @@ ERROR: REPACK (CONCURRENTLY) is not supported for partitioned tables HINT: Consider running the command on individual partitions. DROP TABLE clstrpart; -- Ownership of partitions is checked -CREATE TABLE ptnowner(i int unique) PARTITION BY LIST (i); +CREATE TABLE ptnowner(i int unique not null) PARTITION BY LIST (i); CREATE INDEX ptnowner_i_idx ON ptnowner(i); CREATE TABLE ptnowner1 PARTITION OF ptnowner FOR VALUES IN (1); -CREATE ROLE regress_ptnowner; +CREATE ROLE regress_ptnowner LOGIN; CREATE TABLE ptnowner2 PARTITION OF ptnowner FOR VALUES IN (2); ALTER TABLE ptnowner1 OWNER TO regress_ptnowner; SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; ERROR: permission denied for table ptnowner +ALTER TABLE ptnowner1 REPLICA IDENTITY USING INDEX ptnowner1_i_key; +REPACK (CONCURRENTLY) ptnowner1; RESET SESSION AUTHORIZATION; ALTER TABLE ptnowner OWNER TO regress_ptnowner; CREATE TEMP TABLE ptnowner_oldnodes AS @@ -560,6 +562,11 @@ CREATE TEMP TABLE ptnowner_oldnodes AS SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; WARNING: permission denied to execute CLUSTER on "ptnowner2", skipping it +-- still can't repack without a replica identity +ALTER TABLE ptnowner1 REPLICA IDENTITY DEFAULT; +REPACK (CONCURRENTLY) ptnowner1; +ERROR: cannot process relation "ptnowner1" +HINT: Relation "ptnowner1" has no identity index. RESET SESSION AUTHORIZATION; SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; @@ -570,6 +577,15 @@ SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a ptnowner2 | t (3 rows) +SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a + JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; + relname | ?column? +-----------+---------- + ptnowner | t + ptnowner1 | f + ptnowner2 | t +(3 rows) + DROP TABLE ptnowner; DROP ROLE regress_ptnowner; -- Test CLUSTER with external tuplesorting diff --git a/src/test/regress/sql/cluster.sql b/src/test/regress/sql/cluster.sql index d14063a9683..1f471a8821a 100644 --- a/src/test/regress/sql/cluster.sql +++ b/src/test/regress/sql/cluster.sql @@ -254,14 +254,16 @@ REPACK (CONCURRENTLY) clstrpart; DROP TABLE clstrpart; -- Ownership of partitions is checked -CREATE TABLE ptnowner(i int unique) PARTITION BY LIST (i); +CREATE TABLE ptnowner(i int unique not null) PARTITION BY LIST (i); CREATE INDEX ptnowner_i_idx ON ptnowner(i); CREATE TABLE ptnowner1 PARTITION OF ptnowner FOR VALUES IN (1); -CREATE ROLE regress_ptnowner; +CREATE ROLE regress_ptnowner LOGIN; CREATE TABLE ptnowner2 PARTITION OF ptnowner FOR VALUES IN (2); ALTER TABLE ptnowner1 OWNER TO regress_ptnowner; SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; +ALTER TABLE ptnowner1 REPLICA IDENTITY USING INDEX ptnowner1_i_key; +REPACK (CONCURRENTLY) ptnowner1; RESET SESSION AUTHORIZATION; ALTER TABLE ptnowner OWNER TO regress_ptnowner; CREATE TEMP TABLE ptnowner_oldnodes AS @@ -269,7 +271,12 @@ CREATE TEMP TABLE ptnowner_oldnodes AS JOIN pg_class AS c ON c.oid=tree.relid; SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; +-- still can't repack without a replica identity +ALTER TABLE ptnowner1 REPLICA IDENTITY DEFAULT; +REPACK (CONCURRENTLY) ptnowner1; RESET SESSION AUTHORIZATION; +SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a + JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; DROP TABLE ptnowner; -- 2.47.3 --m6zy3l65ushq557m Content-Type: text/x-diff; charset=utf-8 Content-Disposition: attachment; filename="0002-REPACK-do-not-require-LOGIN-privileges.patch" ^ permalink raw reply [nested|flat] 41+ messages in thread
* [PATCH 1/2] REPACK: do not require the user to have REPLICATION @ 2026-04-20 09:38 Álvaro Herrera <[email protected]> 0 siblings, 0 replies; 41+ messages in thread From: Álvaro Herrera @ 2026-04-20 09:38 UTC (permalink / raw) Although REPACK (CONCURRENTLY) uses replication slots, there is no concern that the slot will leak data of other users, because the MAINTAIN privilege on the table is required anyway; requiring REPLICATION is user-unfriendly without providing any actual protection. A related aspect is that the REPLICATION attribute is not needed to prevent REPACK from stealing slots from logical replication, since commit e76d8c749c31 made REPACK use a separate pool of replication slots. Because there are now successful concurrent repack runs in the regression tests, we're forced to run test_plan_advice under wal_level=replica. Author: Antonin Houska <[email protected]> Reported-by: Justin Pryzby <[email protected]> Reviewed-by: Chao Li <[email protected]> Discussion: https://postgr.es/m/aeJHPNmL4vVy3oPw@pryzbyj2023 --- src/backend/commands/repack_worker.c | 1 - .../test_plan_advice/t/001_replan_regress.pl | 1 + src/test/regress/expected/cluster.out | 20 +++++++++++++++++-- src/test/regress/sql/cluster.sql | 11 ++++++++-- 4 files changed, 28 insertions(+), 5 deletions(-) diff --git a/src/backend/commands/repack_worker.c b/src/backend/commands/repack_worker.c index b17edd771e2..e4a4860805b 100644 --- a/src/backend/commands/repack_worker.c +++ b/src/backend/commands/repack_worker.c @@ -214,7 +214,6 @@ repack_setup_logical_decoding(Oid relid) /* * Make sure we can use logical decoding. */ - CheckSlotPermissions(); CheckLogicalDecodingRequirements(true); /* diff --git a/src/test/modules/test_plan_advice/t/001_replan_regress.pl b/src/test/modules/test_plan_advice/t/001_replan_regress.pl index 38ffa4d11ae..452b179a665 100644 --- a/src/test/modules/test_plan_advice/t/001_replan_regress.pl +++ b/src/test/modules/test_plan_advice/t/001_replan_regress.pl @@ -18,6 +18,7 @@ $node->init(); # Set up our desired configuration. $node->append_conf('postgresql.conf', <<EOM); shared_preload_libraries='test_plan_advice' +wal_level=replica pg_plan_advice.always_explain_supplied_advice=false pg_plan_advice.feedback_warnings=true EOM diff --git a/src/test/regress/expected/cluster.out b/src/test/regress/expected/cluster.out index 6127b215a86..e17bc91fae1 100644 --- a/src/test/regress/expected/cluster.out +++ b/src/test/regress/expected/cluster.out @@ -543,15 +543,17 @@ ERROR: REPACK (CONCURRENTLY) is not supported for partitioned tables HINT: Consider running the command on individual partitions. DROP TABLE clstrpart; -- Ownership of partitions is checked -CREATE TABLE ptnowner(i int unique) PARTITION BY LIST (i); +CREATE TABLE ptnowner(i int unique not null) PARTITION BY LIST (i); CREATE INDEX ptnowner_i_idx ON ptnowner(i); CREATE TABLE ptnowner1 PARTITION OF ptnowner FOR VALUES IN (1); -CREATE ROLE regress_ptnowner; +CREATE ROLE regress_ptnowner LOGIN; CREATE TABLE ptnowner2 PARTITION OF ptnowner FOR VALUES IN (2); ALTER TABLE ptnowner1 OWNER TO regress_ptnowner; SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; ERROR: permission denied for table ptnowner +ALTER TABLE ptnowner1 REPLICA IDENTITY USING INDEX ptnowner1_i_key; +REPACK (CONCURRENTLY) ptnowner1; RESET SESSION AUTHORIZATION; ALTER TABLE ptnowner OWNER TO regress_ptnowner; CREATE TEMP TABLE ptnowner_oldnodes AS @@ -560,6 +562,11 @@ CREATE TEMP TABLE ptnowner_oldnodes AS SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; WARNING: permission denied to execute CLUSTER on "ptnowner2", skipping it +-- still can't repack without a replica identity +ALTER TABLE ptnowner1 REPLICA IDENTITY DEFAULT; +REPACK (CONCURRENTLY) ptnowner1; +ERROR: cannot process relation "ptnowner1" +HINT: Relation "ptnowner1" has no identity index. RESET SESSION AUTHORIZATION; SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; @@ -570,6 +577,15 @@ SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a ptnowner2 | t (3 rows) +SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a + JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; + relname | ?column? +-----------+---------- + ptnowner | t + ptnowner1 | f + ptnowner2 | t +(3 rows) + DROP TABLE ptnowner; DROP ROLE regress_ptnowner; -- Test CLUSTER with external tuplesorting diff --git a/src/test/regress/sql/cluster.sql b/src/test/regress/sql/cluster.sql index d14063a9683..1f471a8821a 100644 --- a/src/test/regress/sql/cluster.sql +++ b/src/test/regress/sql/cluster.sql @@ -254,14 +254,16 @@ REPACK (CONCURRENTLY) clstrpart; DROP TABLE clstrpart; -- Ownership of partitions is checked -CREATE TABLE ptnowner(i int unique) PARTITION BY LIST (i); +CREATE TABLE ptnowner(i int unique not null) PARTITION BY LIST (i); CREATE INDEX ptnowner_i_idx ON ptnowner(i); CREATE TABLE ptnowner1 PARTITION OF ptnowner FOR VALUES IN (1); -CREATE ROLE regress_ptnowner; +CREATE ROLE regress_ptnowner LOGIN; CREATE TABLE ptnowner2 PARTITION OF ptnowner FOR VALUES IN (2); ALTER TABLE ptnowner1 OWNER TO regress_ptnowner; SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; +ALTER TABLE ptnowner1 REPLICA IDENTITY USING INDEX ptnowner1_i_key; +REPACK (CONCURRENTLY) ptnowner1; RESET SESSION AUTHORIZATION; ALTER TABLE ptnowner OWNER TO regress_ptnowner; CREATE TEMP TABLE ptnowner_oldnodes AS @@ -269,7 +271,12 @@ CREATE TEMP TABLE ptnowner_oldnodes AS JOIN pg_class AS c ON c.oid=tree.relid; SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; +-- still can't repack without a replica identity +ALTER TABLE ptnowner1 REPLICA IDENTITY DEFAULT; +REPACK (CONCURRENTLY) ptnowner1; RESET SESSION AUTHORIZATION; +SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a + JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; DROP TABLE ptnowner; -- 2.47.3 --m6zy3l65ushq557m Content-Type: text/x-diff; charset=utf-8 Content-Disposition: attachment; filename="0002-REPACK-do-not-require-LOGIN-privileges.patch" ^ permalink raw reply [nested|flat] 41+ messages in thread
* [PATCH 1/2] REPACK: do not require the user to have REPLICATION @ 2026-04-20 09:38 Álvaro Herrera <[email protected]> 0 siblings, 0 replies; 41+ messages in thread From: Álvaro Herrera @ 2026-04-20 09:38 UTC (permalink / raw) Although REPACK (CONCURRENTLY) uses replication slots, there is no concern that the slot will leak data of other users, because the MAINTAIN privilege on the table is required anyway; requiring REPLICATION is user-unfriendly without providing any actual protection. A related aspect is that the REPLICATION attribute is not needed to prevent REPACK from stealing slots from logical replication, since commit e76d8c749c31 made REPACK use a separate pool of replication slots. Because there are now successful concurrent repack runs in the regression tests, we're forced to run test_plan_advice under wal_level=replica. Author: Antonin Houska <[email protected]> Reported-by: Justin Pryzby <[email protected]> Reviewed-by: Chao Li <[email protected]> Discussion: https://postgr.es/m/aeJHPNmL4vVy3oPw@pryzbyj2023 --- src/backend/commands/repack_worker.c | 1 - .../test_plan_advice/t/001_replan_regress.pl | 1 + src/test/regress/expected/cluster.out | 20 +++++++++++++++++-- src/test/regress/sql/cluster.sql | 11 ++++++++-- 4 files changed, 28 insertions(+), 5 deletions(-) diff --git a/src/backend/commands/repack_worker.c b/src/backend/commands/repack_worker.c index b17edd771e2..e4a4860805b 100644 --- a/src/backend/commands/repack_worker.c +++ b/src/backend/commands/repack_worker.c @@ -214,7 +214,6 @@ repack_setup_logical_decoding(Oid relid) /* * Make sure we can use logical decoding. */ - CheckSlotPermissions(); CheckLogicalDecodingRequirements(true); /* diff --git a/src/test/modules/test_plan_advice/t/001_replan_regress.pl b/src/test/modules/test_plan_advice/t/001_replan_regress.pl index 38ffa4d11ae..452b179a665 100644 --- a/src/test/modules/test_plan_advice/t/001_replan_regress.pl +++ b/src/test/modules/test_plan_advice/t/001_replan_regress.pl @@ -18,6 +18,7 @@ $node->init(); # Set up our desired configuration. $node->append_conf('postgresql.conf', <<EOM); shared_preload_libraries='test_plan_advice' +wal_level=replica pg_plan_advice.always_explain_supplied_advice=false pg_plan_advice.feedback_warnings=true EOM diff --git a/src/test/regress/expected/cluster.out b/src/test/regress/expected/cluster.out index 6127b215a86..e17bc91fae1 100644 --- a/src/test/regress/expected/cluster.out +++ b/src/test/regress/expected/cluster.out @@ -543,15 +543,17 @@ ERROR: REPACK (CONCURRENTLY) is not supported for partitioned tables HINT: Consider running the command on individual partitions. DROP TABLE clstrpart; -- Ownership of partitions is checked -CREATE TABLE ptnowner(i int unique) PARTITION BY LIST (i); +CREATE TABLE ptnowner(i int unique not null) PARTITION BY LIST (i); CREATE INDEX ptnowner_i_idx ON ptnowner(i); CREATE TABLE ptnowner1 PARTITION OF ptnowner FOR VALUES IN (1); -CREATE ROLE regress_ptnowner; +CREATE ROLE regress_ptnowner LOGIN; CREATE TABLE ptnowner2 PARTITION OF ptnowner FOR VALUES IN (2); ALTER TABLE ptnowner1 OWNER TO regress_ptnowner; SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; ERROR: permission denied for table ptnowner +ALTER TABLE ptnowner1 REPLICA IDENTITY USING INDEX ptnowner1_i_key; +REPACK (CONCURRENTLY) ptnowner1; RESET SESSION AUTHORIZATION; ALTER TABLE ptnowner OWNER TO regress_ptnowner; CREATE TEMP TABLE ptnowner_oldnodes AS @@ -560,6 +562,11 @@ CREATE TEMP TABLE ptnowner_oldnodes AS SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; WARNING: permission denied to execute CLUSTER on "ptnowner2", skipping it +-- still can't repack without a replica identity +ALTER TABLE ptnowner1 REPLICA IDENTITY DEFAULT; +REPACK (CONCURRENTLY) ptnowner1; +ERROR: cannot process relation "ptnowner1" +HINT: Relation "ptnowner1" has no identity index. RESET SESSION AUTHORIZATION; SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; @@ -570,6 +577,15 @@ SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a ptnowner2 | t (3 rows) +SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a + JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; + relname | ?column? +-----------+---------- + ptnowner | t + ptnowner1 | f + ptnowner2 | t +(3 rows) + DROP TABLE ptnowner; DROP ROLE regress_ptnowner; -- Test CLUSTER with external tuplesorting diff --git a/src/test/regress/sql/cluster.sql b/src/test/regress/sql/cluster.sql index d14063a9683..1f471a8821a 100644 --- a/src/test/regress/sql/cluster.sql +++ b/src/test/regress/sql/cluster.sql @@ -254,14 +254,16 @@ REPACK (CONCURRENTLY) clstrpart; DROP TABLE clstrpart; -- Ownership of partitions is checked -CREATE TABLE ptnowner(i int unique) PARTITION BY LIST (i); +CREATE TABLE ptnowner(i int unique not null) PARTITION BY LIST (i); CREATE INDEX ptnowner_i_idx ON ptnowner(i); CREATE TABLE ptnowner1 PARTITION OF ptnowner FOR VALUES IN (1); -CREATE ROLE regress_ptnowner; +CREATE ROLE regress_ptnowner LOGIN; CREATE TABLE ptnowner2 PARTITION OF ptnowner FOR VALUES IN (2); ALTER TABLE ptnowner1 OWNER TO regress_ptnowner; SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; +ALTER TABLE ptnowner1 REPLICA IDENTITY USING INDEX ptnowner1_i_key; +REPACK (CONCURRENTLY) ptnowner1; RESET SESSION AUTHORIZATION; ALTER TABLE ptnowner OWNER TO regress_ptnowner; CREATE TEMP TABLE ptnowner_oldnodes AS @@ -269,7 +271,12 @@ CREATE TEMP TABLE ptnowner_oldnodes AS JOIN pg_class AS c ON c.oid=tree.relid; SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; +-- still can't repack without a replica identity +ALTER TABLE ptnowner1 REPLICA IDENTITY DEFAULT; +REPACK (CONCURRENTLY) ptnowner1; RESET SESSION AUTHORIZATION; +SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a + JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; DROP TABLE ptnowner; -- 2.47.3 --m6zy3l65ushq557m Content-Type: text/x-diff; charset=utf-8 Content-Disposition: attachment; filename="0002-REPACK-do-not-require-LOGIN-privileges.patch" ^ permalink raw reply [nested|flat] 41+ messages in thread
* [PATCH 1/2] REPACK: do not require the user to have REPLICATION @ 2026-04-20 09:38 Álvaro Herrera <[email protected]> 0 siblings, 0 replies; 41+ messages in thread From: Álvaro Herrera @ 2026-04-20 09:38 UTC (permalink / raw) Although REPACK (CONCURRENTLY) uses replication slots, there is no concern that the slot will leak data of other users, because the MAINTAIN privilege on the table is required anyway; requiring REPLICATION is user-unfriendly without providing any actual protection. A related aspect is that the REPLICATION attribute is not needed to prevent REPACK from stealing slots from logical replication, since commit e76d8c749c31 made REPACK use a separate pool of replication slots. Because there are now successful concurrent repack runs in the regression tests, we're forced to run test_plan_advice under wal_level=replica. Author: Antonin Houska <[email protected]> Reported-by: Justin Pryzby <[email protected]> Reviewed-by: Chao Li <[email protected]> Discussion: https://postgr.es/m/aeJHPNmL4vVy3oPw@pryzbyj2023 --- src/backend/commands/repack_worker.c | 1 - .../test_plan_advice/t/001_replan_regress.pl | 1 + src/test/regress/expected/cluster.out | 20 +++++++++++++++++-- src/test/regress/sql/cluster.sql | 11 ++++++++-- 4 files changed, 28 insertions(+), 5 deletions(-) diff --git a/src/backend/commands/repack_worker.c b/src/backend/commands/repack_worker.c index b17edd771e2..e4a4860805b 100644 --- a/src/backend/commands/repack_worker.c +++ b/src/backend/commands/repack_worker.c @@ -214,7 +214,6 @@ repack_setup_logical_decoding(Oid relid) /* * Make sure we can use logical decoding. */ - CheckSlotPermissions(); CheckLogicalDecodingRequirements(true); /* diff --git a/src/test/modules/test_plan_advice/t/001_replan_regress.pl b/src/test/modules/test_plan_advice/t/001_replan_regress.pl index 38ffa4d11ae..452b179a665 100644 --- a/src/test/modules/test_plan_advice/t/001_replan_regress.pl +++ b/src/test/modules/test_plan_advice/t/001_replan_regress.pl @@ -18,6 +18,7 @@ $node->init(); # Set up our desired configuration. $node->append_conf('postgresql.conf', <<EOM); shared_preload_libraries='test_plan_advice' +wal_level=replica pg_plan_advice.always_explain_supplied_advice=false pg_plan_advice.feedback_warnings=true EOM diff --git a/src/test/regress/expected/cluster.out b/src/test/regress/expected/cluster.out index 6127b215a86..e17bc91fae1 100644 --- a/src/test/regress/expected/cluster.out +++ b/src/test/regress/expected/cluster.out @@ -543,15 +543,17 @@ ERROR: REPACK (CONCURRENTLY) is not supported for partitioned tables HINT: Consider running the command on individual partitions. DROP TABLE clstrpart; -- Ownership of partitions is checked -CREATE TABLE ptnowner(i int unique) PARTITION BY LIST (i); +CREATE TABLE ptnowner(i int unique not null) PARTITION BY LIST (i); CREATE INDEX ptnowner_i_idx ON ptnowner(i); CREATE TABLE ptnowner1 PARTITION OF ptnowner FOR VALUES IN (1); -CREATE ROLE regress_ptnowner; +CREATE ROLE regress_ptnowner LOGIN; CREATE TABLE ptnowner2 PARTITION OF ptnowner FOR VALUES IN (2); ALTER TABLE ptnowner1 OWNER TO regress_ptnowner; SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; ERROR: permission denied for table ptnowner +ALTER TABLE ptnowner1 REPLICA IDENTITY USING INDEX ptnowner1_i_key; +REPACK (CONCURRENTLY) ptnowner1; RESET SESSION AUTHORIZATION; ALTER TABLE ptnowner OWNER TO regress_ptnowner; CREATE TEMP TABLE ptnowner_oldnodes AS @@ -560,6 +562,11 @@ CREATE TEMP TABLE ptnowner_oldnodes AS SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; WARNING: permission denied to execute CLUSTER on "ptnowner2", skipping it +-- still can't repack without a replica identity +ALTER TABLE ptnowner1 REPLICA IDENTITY DEFAULT; +REPACK (CONCURRENTLY) ptnowner1; +ERROR: cannot process relation "ptnowner1" +HINT: Relation "ptnowner1" has no identity index. RESET SESSION AUTHORIZATION; SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; @@ -570,6 +577,15 @@ SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a ptnowner2 | t (3 rows) +SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a + JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; + relname | ?column? +-----------+---------- + ptnowner | t + ptnowner1 | f + ptnowner2 | t +(3 rows) + DROP TABLE ptnowner; DROP ROLE regress_ptnowner; -- Test CLUSTER with external tuplesorting diff --git a/src/test/regress/sql/cluster.sql b/src/test/regress/sql/cluster.sql index d14063a9683..1f471a8821a 100644 --- a/src/test/regress/sql/cluster.sql +++ b/src/test/regress/sql/cluster.sql @@ -254,14 +254,16 @@ REPACK (CONCURRENTLY) clstrpart; DROP TABLE clstrpart; -- Ownership of partitions is checked -CREATE TABLE ptnowner(i int unique) PARTITION BY LIST (i); +CREATE TABLE ptnowner(i int unique not null) PARTITION BY LIST (i); CREATE INDEX ptnowner_i_idx ON ptnowner(i); CREATE TABLE ptnowner1 PARTITION OF ptnowner FOR VALUES IN (1); -CREATE ROLE regress_ptnowner; +CREATE ROLE regress_ptnowner LOGIN; CREATE TABLE ptnowner2 PARTITION OF ptnowner FOR VALUES IN (2); ALTER TABLE ptnowner1 OWNER TO regress_ptnowner; SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; +ALTER TABLE ptnowner1 REPLICA IDENTITY USING INDEX ptnowner1_i_key; +REPACK (CONCURRENTLY) ptnowner1; RESET SESSION AUTHORIZATION; ALTER TABLE ptnowner OWNER TO regress_ptnowner; CREATE TEMP TABLE ptnowner_oldnodes AS @@ -269,7 +271,12 @@ CREATE TEMP TABLE ptnowner_oldnodes AS JOIN pg_class AS c ON c.oid=tree.relid; SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; +-- still can't repack without a replica identity +ALTER TABLE ptnowner1 REPLICA IDENTITY DEFAULT; +REPACK (CONCURRENTLY) ptnowner1; RESET SESSION AUTHORIZATION; +SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a + JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; DROP TABLE ptnowner; -- 2.47.3 --m6zy3l65ushq557m Content-Type: text/x-diff; charset=utf-8 Content-Disposition: attachment; filename="0002-REPACK-do-not-require-LOGIN-privileges.patch" ^ permalink raw reply [nested|flat] 41+ messages in thread
* [PATCH 1/2] REPACK: do not require the user to have REPLICATION @ 2026-04-20 09:38 Álvaro Herrera <[email protected]> 0 siblings, 0 replies; 41+ messages in thread From: Álvaro Herrera @ 2026-04-20 09:38 UTC (permalink / raw) Although REPACK (CONCURRENTLY) uses replication slots, there is no concern that the slot will leak data of other users, because the MAINTAIN privilege on the table is required anyway; requiring REPLICATION is user-unfriendly without providing any actual protection. A related aspect is that the REPLICATION attribute is not needed to prevent REPACK from stealing slots from logical replication, since commit e76d8c749c31 made REPACK use a separate pool of replication slots. Because there are now successful concurrent repack runs in the regression tests, we're forced to run test_plan_advice under wal_level=replica. Author: Antonin Houska <[email protected]> Reported-by: Justin Pryzby <[email protected]> Reviewed-by: Chao Li <[email protected]> Discussion: https://postgr.es/m/aeJHPNmL4vVy3oPw@pryzbyj2023 --- src/backend/commands/repack_worker.c | 1 - .../test_plan_advice/t/001_replan_regress.pl | 1 + src/test/regress/expected/cluster.out | 20 +++++++++++++++++-- src/test/regress/sql/cluster.sql | 11 ++++++++-- 4 files changed, 28 insertions(+), 5 deletions(-) diff --git a/src/backend/commands/repack_worker.c b/src/backend/commands/repack_worker.c index b17edd771e2..e4a4860805b 100644 --- a/src/backend/commands/repack_worker.c +++ b/src/backend/commands/repack_worker.c @@ -214,7 +214,6 @@ repack_setup_logical_decoding(Oid relid) /* * Make sure we can use logical decoding. */ - CheckSlotPermissions(); CheckLogicalDecodingRequirements(true); /* diff --git a/src/test/modules/test_plan_advice/t/001_replan_regress.pl b/src/test/modules/test_plan_advice/t/001_replan_regress.pl index 38ffa4d11ae..452b179a665 100644 --- a/src/test/modules/test_plan_advice/t/001_replan_regress.pl +++ b/src/test/modules/test_plan_advice/t/001_replan_regress.pl @@ -18,6 +18,7 @@ $node->init(); # Set up our desired configuration. $node->append_conf('postgresql.conf', <<EOM); shared_preload_libraries='test_plan_advice' +wal_level=replica pg_plan_advice.always_explain_supplied_advice=false pg_plan_advice.feedback_warnings=true EOM diff --git a/src/test/regress/expected/cluster.out b/src/test/regress/expected/cluster.out index 6127b215a86..e17bc91fae1 100644 --- a/src/test/regress/expected/cluster.out +++ b/src/test/regress/expected/cluster.out @@ -543,15 +543,17 @@ ERROR: REPACK (CONCURRENTLY) is not supported for partitioned tables HINT: Consider running the command on individual partitions. DROP TABLE clstrpart; -- Ownership of partitions is checked -CREATE TABLE ptnowner(i int unique) PARTITION BY LIST (i); +CREATE TABLE ptnowner(i int unique not null) PARTITION BY LIST (i); CREATE INDEX ptnowner_i_idx ON ptnowner(i); CREATE TABLE ptnowner1 PARTITION OF ptnowner FOR VALUES IN (1); -CREATE ROLE regress_ptnowner; +CREATE ROLE regress_ptnowner LOGIN; CREATE TABLE ptnowner2 PARTITION OF ptnowner FOR VALUES IN (2); ALTER TABLE ptnowner1 OWNER TO regress_ptnowner; SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; ERROR: permission denied for table ptnowner +ALTER TABLE ptnowner1 REPLICA IDENTITY USING INDEX ptnowner1_i_key; +REPACK (CONCURRENTLY) ptnowner1; RESET SESSION AUTHORIZATION; ALTER TABLE ptnowner OWNER TO regress_ptnowner; CREATE TEMP TABLE ptnowner_oldnodes AS @@ -560,6 +562,11 @@ CREATE TEMP TABLE ptnowner_oldnodes AS SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; WARNING: permission denied to execute CLUSTER on "ptnowner2", skipping it +-- still can't repack without a replica identity +ALTER TABLE ptnowner1 REPLICA IDENTITY DEFAULT; +REPACK (CONCURRENTLY) ptnowner1; +ERROR: cannot process relation "ptnowner1" +HINT: Relation "ptnowner1" has no identity index. RESET SESSION AUTHORIZATION; SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; @@ -570,6 +577,15 @@ SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a ptnowner2 | t (3 rows) +SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a + JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; + relname | ?column? +-----------+---------- + ptnowner | t + ptnowner1 | f + ptnowner2 | t +(3 rows) + DROP TABLE ptnowner; DROP ROLE regress_ptnowner; -- Test CLUSTER with external tuplesorting diff --git a/src/test/regress/sql/cluster.sql b/src/test/regress/sql/cluster.sql index d14063a9683..1f471a8821a 100644 --- a/src/test/regress/sql/cluster.sql +++ b/src/test/regress/sql/cluster.sql @@ -254,14 +254,16 @@ REPACK (CONCURRENTLY) clstrpart; DROP TABLE clstrpart; -- Ownership of partitions is checked -CREATE TABLE ptnowner(i int unique) PARTITION BY LIST (i); +CREATE TABLE ptnowner(i int unique not null) PARTITION BY LIST (i); CREATE INDEX ptnowner_i_idx ON ptnowner(i); CREATE TABLE ptnowner1 PARTITION OF ptnowner FOR VALUES IN (1); -CREATE ROLE regress_ptnowner; +CREATE ROLE regress_ptnowner LOGIN; CREATE TABLE ptnowner2 PARTITION OF ptnowner FOR VALUES IN (2); ALTER TABLE ptnowner1 OWNER TO regress_ptnowner; SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; +ALTER TABLE ptnowner1 REPLICA IDENTITY USING INDEX ptnowner1_i_key; +REPACK (CONCURRENTLY) ptnowner1; RESET SESSION AUTHORIZATION; ALTER TABLE ptnowner OWNER TO regress_ptnowner; CREATE TEMP TABLE ptnowner_oldnodes AS @@ -269,7 +271,12 @@ CREATE TEMP TABLE ptnowner_oldnodes AS JOIN pg_class AS c ON c.oid=tree.relid; SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; +-- still can't repack without a replica identity +ALTER TABLE ptnowner1 REPLICA IDENTITY DEFAULT; +REPACK (CONCURRENTLY) ptnowner1; RESET SESSION AUTHORIZATION; +SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a + JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; DROP TABLE ptnowner; -- 2.47.3 --m6zy3l65ushq557m Content-Type: text/x-diff; charset=utf-8 Content-Disposition: attachment; filename="0002-REPACK-do-not-require-LOGIN-privileges.patch" ^ permalink raw reply [nested|flat] 41+ messages in thread
* [PATCH 1/2] REPACK: do not require the user to have REPLICATION @ 2026-04-20 09:38 Álvaro Herrera <[email protected]> 0 siblings, 0 replies; 41+ messages in thread From: Álvaro Herrera @ 2026-04-20 09:38 UTC (permalink / raw) Although REPACK (CONCURRENTLY) uses replication slots, there is no concern that the slot will leak data of other users, because the MAINTAIN privilege on the table is required anyway; requiring REPLICATION is user-unfriendly without providing any actual protection. A related aspect is that the REPLICATION attribute is not needed to prevent REPACK from stealing slots from logical replication, since commit e76d8c749c31 made REPACK use a separate pool of replication slots. Because there are now successful concurrent repack runs in the regression tests, we're forced to run test_plan_advice under wal_level=replica. Author: Antonin Houska <[email protected]> Reported-by: Justin Pryzby <[email protected]> Reviewed-by: Chao Li <[email protected]> Discussion: https://postgr.es/m/aeJHPNmL4vVy3oPw@pryzbyj2023 --- src/backend/commands/repack_worker.c | 1 - .../test_plan_advice/t/001_replan_regress.pl | 1 + src/test/regress/expected/cluster.out | 20 +++++++++++++++++-- src/test/regress/sql/cluster.sql | 11 ++++++++-- 4 files changed, 28 insertions(+), 5 deletions(-) diff --git a/src/backend/commands/repack_worker.c b/src/backend/commands/repack_worker.c index b17edd771e2..e4a4860805b 100644 --- a/src/backend/commands/repack_worker.c +++ b/src/backend/commands/repack_worker.c @@ -214,7 +214,6 @@ repack_setup_logical_decoding(Oid relid) /* * Make sure we can use logical decoding. */ - CheckSlotPermissions(); CheckLogicalDecodingRequirements(true); /* diff --git a/src/test/modules/test_plan_advice/t/001_replan_regress.pl b/src/test/modules/test_plan_advice/t/001_replan_regress.pl index 38ffa4d11ae..452b179a665 100644 --- a/src/test/modules/test_plan_advice/t/001_replan_regress.pl +++ b/src/test/modules/test_plan_advice/t/001_replan_regress.pl @@ -18,6 +18,7 @@ $node->init(); # Set up our desired configuration. $node->append_conf('postgresql.conf', <<EOM); shared_preload_libraries='test_plan_advice' +wal_level=replica pg_plan_advice.always_explain_supplied_advice=false pg_plan_advice.feedback_warnings=true EOM diff --git a/src/test/regress/expected/cluster.out b/src/test/regress/expected/cluster.out index 6127b215a86..e17bc91fae1 100644 --- a/src/test/regress/expected/cluster.out +++ b/src/test/regress/expected/cluster.out @@ -543,15 +543,17 @@ ERROR: REPACK (CONCURRENTLY) is not supported for partitioned tables HINT: Consider running the command on individual partitions. DROP TABLE clstrpart; -- Ownership of partitions is checked -CREATE TABLE ptnowner(i int unique) PARTITION BY LIST (i); +CREATE TABLE ptnowner(i int unique not null) PARTITION BY LIST (i); CREATE INDEX ptnowner_i_idx ON ptnowner(i); CREATE TABLE ptnowner1 PARTITION OF ptnowner FOR VALUES IN (1); -CREATE ROLE regress_ptnowner; +CREATE ROLE regress_ptnowner LOGIN; CREATE TABLE ptnowner2 PARTITION OF ptnowner FOR VALUES IN (2); ALTER TABLE ptnowner1 OWNER TO regress_ptnowner; SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; ERROR: permission denied for table ptnowner +ALTER TABLE ptnowner1 REPLICA IDENTITY USING INDEX ptnowner1_i_key; +REPACK (CONCURRENTLY) ptnowner1; RESET SESSION AUTHORIZATION; ALTER TABLE ptnowner OWNER TO regress_ptnowner; CREATE TEMP TABLE ptnowner_oldnodes AS @@ -560,6 +562,11 @@ CREATE TEMP TABLE ptnowner_oldnodes AS SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; WARNING: permission denied to execute CLUSTER on "ptnowner2", skipping it +-- still can't repack without a replica identity +ALTER TABLE ptnowner1 REPLICA IDENTITY DEFAULT; +REPACK (CONCURRENTLY) ptnowner1; +ERROR: cannot process relation "ptnowner1" +HINT: Relation "ptnowner1" has no identity index. RESET SESSION AUTHORIZATION; SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; @@ -570,6 +577,15 @@ SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a ptnowner2 | t (3 rows) +SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a + JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; + relname | ?column? +-----------+---------- + ptnowner | t + ptnowner1 | f + ptnowner2 | t +(3 rows) + DROP TABLE ptnowner; DROP ROLE regress_ptnowner; -- Test CLUSTER with external tuplesorting diff --git a/src/test/regress/sql/cluster.sql b/src/test/regress/sql/cluster.sql index d14063a9683..1f471a8821a 100644 --- a/src/test/regress/sql/cluster.sql +++ b/src/test/regress/sql/cluster.sql @@ -254,14 +254,16 @@ REPACK (CONCURRENTLY) clstrpart; DROP TABLE clstrpart; -- Ownership of partitions is checked -CREATE TABLE ptnowner(i int unique) PARTITION BY LIST (i); +CREATE TABLE ptnowner(i int unique not null) PARTITION BY LIST (i); CREATE INDEX ptnowner_i_idx ON ptnowner(i); CREATE TABLE ptnowner1 PARTITION OF ptnowner FOR VALUES IN (1); -CREATE ROLE regress_ptnowner; +CREATE ROLE regress_ptnowner LOGIN; CREATE TABLE ptnowner2 PARTITION OF ptnowner FOR VALUES IN (2); ALTER TABLE ptnowner1 OWNER TO regress_ptnowner; SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; +ALTER TABLE ptnowner1 REPLICA IDENTITY USING INDEX ptnowner1_i_key; +REPACK (CONCURRENTLY) ptnowner1; RESET SESSION AUTHORIZATION; ALTER TABLE ptnowner OWNER TO regress_ptnowner; CREATE TEMP TABLE ptnowner_oldnodes AS @@ -269,7 +271,12 @@ CREATE TEMP TABLE ptnowner_oldnodes AS JOIN pg_class AS c ON c.oid=tree.relid; SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; +-- still can't repack without a replica identity +ALTER TABLE ptnowner1 REPLICA IDENTITY DEFAULT; +REPACK (CONCURRENTLY) ptnowner1; RESET SESSION AUTHORIZATION; +SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a + JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; DROP TABLE ptnowner; -- 2.47.3 --m6zy3l65ushq557m Content-Type: text/x-diff; charset=utf-8 Content-Disposition: attachment; filename="0002-REPACK-do-not-require-LOGIN-privileges.patch" ^ permalink raw reply [nested|flat] 41+ messages in thread
* [PATCH 1/2] REPACK: do not require the user to have REPLICATION @ 2026-04-20 09:38 Álvaro Herrera <[email protected]> 0 siblings, 0 replies; 41+ messages in thread From: Álvaro Herrera @ 2026-04-20 09:38 UTC (permalink / raw) Although REPACK (CONCURRENTLY) uses replication slots, there is no concern that the slot will leak data of other users, because the MAINTAIN privilege on the table is required anyway; requiring REPLICATION is user-unfriendly without providing any actual protection. A related aspect is that the REPLICATION attribute is not needed to prevent REPACK from stealing slots from logical replication, since commit e76d8c749c31 made REPACK use a separate pool of replication slots. Because there are now successful concurrent repack runs in the regression tests, we're forced to run test_plan_advice under wal_level=replica. Author: Antonin Houska <[email protected]> Reported-by: Justin Pryzby <[email protected]> Reviewed-by: Chao Li <[email protected]> Discussion: https://postgr.es/m/aeJHPNmL4vVy3oPw@pryzbyj2023 --- src/backend/commands/repack_worker.c | 1 - .../test_plan_advice/t/001_replan_regress.pl | 1 + src/test/regress/expected/cluster.out | 20 +++++++++++++++++-- src/test/regress/sql/cluster.sql | 11 ++++++++-- 4 files changed, 28 insertions(+), 5 deletions(-) diff --git a/src/backend/commands/repack_worker.c b/src/backend/commands/repack_worker.c index b17edd771e2..e4a4860805b 100644 --- a/src/backend/commands/repack_worker.c +++ b/src/backend/commands/repack_worker.c @@ -214,7 +214,6 @@ repack_setup_logical_decoding(Oid relid) /* * Make sure we can use logical decoding. */ - CheckSlotPermissions(); CheckLogicalDecodingRequirements(true); /* diff --git a/src/test/modules/test_plan_advice/t/001_replan_regress.pl b/src/test/modules/test_plan_advice/t/001_replan_regress.pl index 38ffa4d11ae..452b179a665 100644 --- a/src/test/modules/test_plan_advice/t/001_replan_regress.pl +++ b/src/test/modules/test_plan_advice/t/001_replan_regress.pl @@ -18,6 +18,7 @@ $node->init(); # Set up our desired configuration. $node->append_conf('postgresql.conf', <<EOM); shared_preload_libraries='test_plan_advice' +wal_level=replica pg_plan_advice.always_explain_supplied_advice=false pg_plan_advice.feedback_warnings=true EOM diff --git a/src/test/regress/expected/cluster.out b/src/test/regress/expected/cluster.out index 6127b215a86..e17bc91fae1 100644 --- a/src/test/regress/expected/cluster.out +++ b/src/test/regress/expected/cluster.out @@ -543,15 +543,17 @@ ERROR: REPACK (CONCURRENTLY) is not supported for partitioned tables HINT: Consider running the command on individual partitions. DROP TABLE clstrpart; -- Ownership of partitions is checked -CREATE TABLE ptnowner(i int unique) PARTITION BY LIST (i); +CREATE TABLE ptnowner(i int unique not null) PARTITION BY LIST (i); CREATE INDEX ptnowner_i_idx ON ptnowner(i); CREATE TABLE ptnowner1 PARTITION OF ptnowner FOR VALUES IN (1); -CREATE ROLE regress_ptnowner; +CREATE ROLE regress_ptnowner LOGIN; CREATE TABLE ptnowner2 PARTITION OF ptnowner FOR VALUES IN (2); ALTER TABLE ptnowner1 OWNER TO regress_ptnowner; SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; ERROR: permission denied for table ptnowner +ALTER TABLE ptnowner1 REPLICA IDENTITY USING INDEX ptnowner1_i_key; +REPACK (CONCURRENTLY) ptnowner1; RESET SESSION AUTHORIZATION; ALTER TABLE ptnowner OWNER TO regress_ptnowner; CREATE TEMP TABLE ptnowner_oldnodes AS @@ -560,6 +562,11 @@ CREATE TEMP TABLE ptnowner_oldnodes AS SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; WARNING: permission denied to execute CLUSTER on "ptnowner2", skipping it +-- still can't repack without a replica identity +ALTER TABLE ptnowner1 REPLICA IDENTITY DEFAULT; +REPACK (CONCURRENTLY) ptnowner1; +ERROR: cannot process relation "ptnowner1" +HINT: Relation "ptnowner1" has no identity index. RESET SESSION AUTHORIZATION; SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; @@ -570,6 +577,15 @@ SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a ptnowner2 | t (3 rows) +SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a + JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; + relname | ?column? +-----------+---------- + ptnowner | t + ptnowner1 | f + ptnowner2 | t +(3 rows) + DROP TABLE ptnowner; DROP ROLE regress_ptnowner; -- Test CLUSTER with external tuplesorting diff --git a/src/test/regress/sql/cluster.sql b/src/test/regress/sql/cluster.sql index d14063a9683..1f471a8821a 100644 --- a/src/test/regress/sql/cluster.sql +++ b/src/test/regress/sql/cluster.sql @@ -254,14 +254,16 @@ REPACK (CONCURRENTLY) clstrpart; DROP TABLE clstrpart; -- Ownership of partitions is checked -CREATE TABLE ptnowner(i int unique) PARTITION BY LIST (i); +CREATE TABLE ptnowner(i int unique not null) PARTITION BY LIST (i); CREATE INDEX ptnowner_i_idx ON ptnowner(i); CREATE TABLE ptnowner1 PARTITION OF ptnowner FOR VALUES IN (1); -CREATE ROLE regress_ptnowner; +CREATE ROLE regress_ptnowner LOGIN; CREATE TABLE ptnowner2 PARTITION OF ptnowner FOR VALUES IN (2); ALTER TABLE ptnowner1 OWNER TO regress_ptnowner; SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; +ALTER TABLE ptnowner1 REPLICA IDENTITY USING INDEX ptnowner1_i_key; +REPACK (CONCURRENTLY) ptnowner1; RESET SESSION AUTHORIZATION; ALTER TABLE ptnowner OWNER TO regress_ptnowner; CREATE TEMP TABLE ptnowner_oldnodes AS @@ -269,7 +271,12 @@ CREATE TEMP TABLE ptnowner_oldnodes AS JOIN pg_class AS c ON c.oid=tree.relid; SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; +-- still can't repack without a replica identity +ALTER TABLE ptnowner1 REPLICA IDENTITY DEFAULT; +REPACK (CONCURRENTLY) ptnowner1; RESET SESSION AUTHORIZATION; +SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a + JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; DROP TABLE ptnowner; -- 2.47.3 --m6zy3l65ushq557m Content-Type: text/x-diff; charset=utf-8 Content-Disposition: attachment; filename="0002-REPACK-do-not-require-LOGIN-privileges.patch" ^ permalink raw reply [nested|flat] 41+ messages in thread
* [PATCH 1/2] REPACK: do not require the user to have REPLICATION @ 2026-04-20 09:38 Álvaro Herrera <[email protected]> 0 siblings, 0 replies; 41+ messages in thread From: Álvaro Herrera @ 2026-04-20 09:38 UTC (permalink / raw) Although REPACK (CONCURRENTLY) uses replication slots, there is no concern that the slot will leak data of other users, because the MAINTAIN privilege on the table is required anyway; requiring REPLICATION is user-unfriendly without providing any actual protection. A related aspect is that the REPLICATION attribute is not needed to prevent REPACK from stealing slots from logical replication, since commit e76d8c749c31 made REPACK use a separate pool of replication slots. Because there are now successful concurrent repack runs in the regression tests, we're forced to run test_plan_advice under wal_level=replica. Author: Antonin Houska <[email protected]> Reported-by: Justin Pryzby <[email protected]> Reviewed-by: Chao Li <[email protected]> Discussion: https://postgr.es/m/aeJHPNmL4vVy3oPw@pryzbyj2023 --- src/backend/commands/repack_worker.c | 1 - .../test_plan_advice/t/001_replan_regress.pl | 1 + src/test/regress/expected/cluster.out | 20 +++++++++++++++++-- src/test/regress/sql/cluster.sql | 11 ++++++++-- 4 files changed, 28 insertions(+), 5 deletions(-) diff --git a/src/backend/commands/repack_worker.c b/src/backend/commands/repack_worker.c index b17edd771e2..e4a4860805b 100644 --- a/src/backend/commands/repack_worker.c +++ b/src/backend/commands/repack_worker.c @@ -214,7 +214,6 @@ repack_setup_logical_decoding(Oid relid) /* * Make sure we can use logical decoding. */ - CheckSlotPermissions(); CheckLogicalDecodingRequirements(true); /* diff --git a/src/test/modules/test_plan_advice/t/001_replan_regress.pl b/src/test/modules/test_plan_advice/t/001_replan_regress.pl index 38ffa4d11ae..452b179a665 100644 --- a/src/test/modules/test_plan_advice/t/001_replan_regress.pl +++ b/src/test/modules/test_plan_advice/t/001_replan_regress.pl @@ -18,6 +18,7 @@ $node->init(); # Set up our desired configuration. $node->append_conf('postgresql.conf', <<EOM); shared_preload_libraries='test_plan_advice' +wal_level=replica pg_plan_advice.always_explain_supplied_advice=false pg_plan_advice.feedback_warnings=true EOM diff --git a/src/test/regress/expected/cluster.out b/src/test/regress/expected/cluster.out index 6127b215a86..e17bc91fae1 100644 --- a/src/test/regress/expected/cluster.out +++ b/src/test/regress/expected/cluster.out @@ -543,15 +543,17 @@ ERROR: REPACK (CONCURRENTLY) is not supported for partitioned tables HINT: Consider running the command on individual partitions. DROP TABLE clstrpart; -- Ownership of partitions is checked -CREATE TABLE ptnowner(i int unique) PARTITION BY LIST (i); +CREATE TABLE ptnowner(i int unique not null) PARTITION BY LIST (i); CREATE INDEX ptnowner_i_idx ON ptnowner(i); CREATE TABLE ptnowner1 PARTITION OF ptnowner FOR VALUES IN (1); -CREATE ROLE regress_ptnowner; +CREATE ROLE regress_ptnowner LOGIN; CREATE TABLE ptnowner2 PARTITION OF ptnowner FOR VALUES IN (2); ALTER TABLE ptnowner1 OWNER TO regress_ptnowner; SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; ERROR: permission denied for table ptnowner +ALTER TABLE ptnowner1 REPLICA IDENTITY USING INDEX ptnowner1_i_key; +REPACK (CONCURRENTLY) ptnowner1; RESET SESSION AUTHORIZATION; ALTER TABLE ptnowner OWNER TO regress_ptnowner; CREATE TEMP TABLE ptnowner_oldnodes AS @@ -560,6 +562,11 @@ CREATE TEMP TABLE ptnowner_oldnodes AS SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; WARNING: permission denied to execute CLUSTER on "ptnowner2", skipping it +-- still can't repack without a replica identity +ALTER TABLE ptnowner1 REPLICA IDENTITY DEFAULT; +REPACK (CONCURRENTLY) ptnowner1; +ERROR: cannot process relation "ptnowner1" +HINT: Relation "ptnowner1" has no identity index. RESET SESSION AUTHORIZATION; SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; @@ -570,6 +577,15 @@ SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a ptnowner2 | t (3 rows) +SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a + JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; + relname | ?column? +-----------+---------- + ptnowner | t + ptnowner1 | f + ptnowner2 | t +(3 rows) + DROP TABLE ptnowner; DROP ROLE regress_ptnowner; -- Test CLUSTER with external tuplesorting diff --git a/src/test/regress/sql/cluster.sql b/src/test/regress/sql/cluster.sql index d14063a9683..1f471a8821a 100644 --- a/src/test/regress/sql/cluster.sql +++ b/src/test/regress/sql/cluster.sql @@ -254,14 +254,16 @@ REPACK (CONCURRENTLY) clstrpart; DROP TABLE clstrpart; -- Ownership of partitions is checked -CREATE TABLE ptnowner(i int unique) PARTITION BY LIST (i); +CREATE TABLE ptnowner(i int unique not null) PARTITION BY LIST (i); CREATE INDEX ptnowner_i_idx ON ptnowner(i); CREATE TABLE ptnowner1 PARTITION OF ptnowner FOR VALUES IN (1); -CREATE ROLE regress_ptnowner; +CREATE ROLE regress_ptnowner LOGIN; CREATE TABLE ptnowner2 PARTITION OF ptnowner FOR VALUES IN (2); ALTER TABLE ptnowner1 OWNER TO regress_ptnowner; SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; +ALTER TABLE ptnowner1 REPLICA IDENTITY USING INDEX ptnowner1_i_key; +REPACK (CONCURRENTLY) ptnowner1; RESET SESSION AUTHORIZATION; ALTER TABLE ptnowner OWNER TO regress_ptnowner; CREATE TEMP TABLE ptnowner_oldnodes AS @@ -269,7 +271,12 @@ CREATE TEMP TABLE ptnowner_oldnodes AS JOIN pg_class AS c ON c.oid=tree.relid; SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; +-- still can't repack without a replica identity +ALTER TABLE ptnowner1 REPLICA IDENTITY DEFAULT; +REPACK (CONCURRENTLY) ptnowner1; RESET SESSION AUTHORIZATION; +SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a + JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; DROP TABLE ptnowner; -- 2.47.3 --m6zy3l65ushq557m Content-Type: text/x-diff; charset=utf-8 Content-Disposition: attachment; filename="0002-REPACK-do-not-require-LOGIN-privileges.patch" ^ permalink raw reply [nested|flat] 41+ messages in thread
* [PATCH 1/2] REPACK: do not require the user to have REPLICATION @ 2026-04-20 09:38 Álvaro Herrera <[email protected]> 0 siblings, 0 replies; 41+ messages in thread From: Álvaro Herrera @ 2026-04-20 09:38 UTC (permalink / raw) Although REPACK (CONCURRENTLY) uses replication slots, there is no concern that the slot will leak data of other users, because the MAINTAIN privilege on the table is required anyway; requiring REPLICATION is user-unfriendly without providing any actual protection. A related aspect is that the REPLICATION attribute is not needed to prevent REPACK from stealing slots from logical replication, since commit e76d8c749c31 made REPACK use a separate pool of replication slots. Because there are now successful concurrent repack runs in the regression tests, we're forced to run test_plan_advice under wal_level=replica. Author: Antonin Houska <[email protected]> Reported-by: Justin Pryzby <[email protected]> Reviewed-by: Chao Li <[email protected]> Discussion: https://postgr.es/m/aeJHPNmL4vVy3oPw@pryzbyj2023 --- src/backend/commands/repack_worker.c | 1 - .../test_plan_advice/t/001_replan_regress.pl | 1 + src/test/regress/expected/cluster.out | 20 +++++++++++++++++-- src/test/regress/sql/cluster.sql | 11 ++++++++-- 4 files changed, 28 insertions(+), 5 deletions(-) diff --git a/src/backend/commands/repack_worker.c b/src/backend/commands/repack_worker.c index b17edd771e2..e4a4860805b 100644 --- a/src/backend/commands/repack_worker.c +++ b/src/backend/commands/repack_worker.c @@ -214,7 +214,6 @@ repack_setup_logical_decoding(Oid relid) /* * Make sure we can use logical decoding. */ - CheckSlotPermissions(); CheckLogicalDecodingRequirements(true); /* diff --git a/src/test/modules/test_plan_advice/t/001_replan_regress.pl b/src/test/modules/test_plan_advice/t/001_replan_regress.pl index 38ffa4d11ae..452b179a665 100644 --- a/src/test/modules/test_plan_advice/t/001_replan_regress.pl +++ b/src/test/modules/test_plan_advice/t/001_replan_regress.pl @@ -18,6 +18,7 @@ $node->init(); # Set up our desired configuration. $node->append_conf('postgresql.conf', <<EOM); shared_preload_libraries='test_plan_advice' +wal_level=replica pg_plan_advice.always_explain_supplied_advice=false pg_plan_advice.feedback_warnings=true EOM diff --git a/src/test/regress/expected/cluster.out b/src/test/regress/expected/cluster.out index 6127b215a86..e17bc91fae1 100644 --- a/src/test/regress/expected/cluster.out +++ b/src/test/regress/expected/cluster.out @@ -543,15 +543,17 @@ ERROR: REPACK (CONCURRENTLY) is not supported for partitioned tables HINT: Consider running the command on individual partitions. DROP TABLE clstrpart; -- Ownership of partitions is checked -CREATE TABLE ptnowner(i int unique) PARTITION BY LIST (i); +CREATE TABLE ptnowner(i int unique not null) PARTITION BY LIST (i); CREATE INDEX ptnowner_i_idx ON ptnowner(i); CREATE TABLE ptnowner1 PARTITION OF ptnowner FOR VALUES IN (1); -CREATE ROLE regress_ptnowner; +CREATE ROLE regress_ptnowner LOGIN; CREATE TABLE ptnowner2 PARTITION OF ptnowner FOR VALUES IN (2); ALTER TABLE ptnowner1 OWNER TO regress_ptnowner; SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; ERROR: permission denied for table ptnowner +ALTER TABLE ptnowner1 REPLICA IDENTITY USING INDEX ptnowner1_i_key; +REPACK (CONCURRENTLY) ptnowner1; RESET SESSION AUTHORIZATION; ALTER TABLE ptnowner OWNER TO regress_ptnowner; CREATE TEMP TABLE ptnowner_oldnodes AS @@ -560,6 +562,11 @@ CREATE TEMP TABLE ptnowner_oldnodes AS SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; WARNING: permission denied to execute CLUSTER on "ptnowner2", skipping it +-- still can't repack without a replica identity +ALTER TABLE ptnowner1 REPLICA IDENTITY DEFAULT; +REPACK (CONCURRENTLY) ptnowner1; +ERROR: cannot process relation "ptnowner1" +HINT: Relation "ptnowner1" has no identity index. RESET SESSION AUTHORIZATION; SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; @@ -570,6 +577,15 @@ SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a ptnowner2 | t (3 rows) +SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a + JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; + relname | ?column? +-----------+---------- + ptnowner | t + ptnowner1 | f + ptnowner2 | t +(3 rows) + DROP TABLE ptnowner; DROP ROLE regress_ptnowner; -- Test CLUSTER with external tuplesorting diff --git a/src/test/regress/sql/cluster.sql b/src/test/regress/sql/cluster.sql index d14063a9683..1f471a8821a 100644 --- a/src/test/regress/sql/cluster.sql +++ b/src/test/regress/sql/cluster.sql @@ -254,14 +254,16 @@ REPACK (CONCURRENTLY) clstrpart; DROP TABLE clstrpart; -- Ownership of partitions is checked -CREATE TABLE ptnowner(i int unique) PARTITION BY LIST (i); +CREATE TABLE ptnowner(i int unique not null) PARTITION BY LIST (i); CREATE INDEX ptnowner_i_idx ON ptnowner(i); CREATE TABLE ptnowner1 PARTITION OF ptnowner FOR VALUES IN (1); -CREATE ROLE regress_ptnowner; +CREATE ROLE regress_ptnowner LOGIN; CREATE TABLE ptnowner2 PARTITION OF ptnowner FOR VALUES IN (2); ALTER TABLE ptnowner1 OWNER TO regress_ptnowner; SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; +ALTER TABLE ptnowner1 REPLICA IDENTITY USING INDEX ptnowner1_i_key; +REPACK (CONCURRENTLY) ptnowner1; RESET SESSION AUTHORIZATION; ALTER TABLE ptnowner OWNER TO regress_ptnowner; CREATE TEMP TABLE ptnowner_oldnodes AS @@ -269,7 +271,12 @@ CREATE TEMP TABLE ptnowner_oldnodes AS JOIN pg_class AS c ON c.oid=tree.relid; SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; +-- still can't repack without a replica identity +ALTER TABLE ptnowner1 REPLICA IDENTITY DEFAULT; +REPACK (CONCURRENTLY) ptnowner1; RESET SESSION AUTHORIZATION; +SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a + JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; DROP TABLE ptnowner; -- 2.47.3 --m6zy3l65ushq557m Content-Type: text/x-diff; charset=utf-8 Content-Disposition: attachment; filename="0002-REPACK-do-not-require-LOGIN-privileges.patch" ^ permalink raw reply [nested|flat] 41+ messages in thread
* [PATCH 1/2] REPACK: do not require the user to have REPLICATION @ 2026-04-20 09:38 Álvaro Herrera <[email protected]> 0 siblings, 0 replies; 41+ messages in thread From: Álvaro Herrera @ 2026-04-20 09:38 UTC (permalink / raw) Although REPACK (CONCURRENTLY) uses replication slots, there is no concern that the slot will leak data of other users, because the MAINTAIN privilege on the table is required anyway; requiring REPLICATION is user-unfriendly without providing any actual protection. A related aspect is that the REPLICATION attribute is not needed to prevent REPACK from stealing slots from logical replication, since commit e76d8c749c31 made REPACK use a separate pool of replication slots. Because there are now successful concurrent repack runs in the regression tests, we're forced to run test_plan_advice under wal_level=replica. Author: Antonin Houska <[email protected]> Reported-by: Justin Pryzby <[email protected]> Reviewed-by: Chao Li <[email protected]> Discussion: https://postgr.es/m/aeJHPNmL4vVy3oPw@pryzbyj2023 --- src/backend/commands/repack_worker.c | 1 - .../test_plan_advice/t/001_replan_regress.pl | 1 + src/test/regress/expected/cluster.out | 20 +++++++++++++++++-- src/test/regress/sql/cluster.sql | 11 ++++++++-- 4 files changed, 28 insertions(+), 5 deletions(-) diff --git a/src/backend/commands/repack_worker.c b/src/backend/commands/repack_worker.c index b17edd771e2..e4a4860805b 100644 --- a/src/backend/commands/repack_worker.c +++ b/src/backend/commands/repack_worker.c @@ -214,7 +214,6 @@ repack_setup_logical_decoding(Oid relid) /* * Make sure we can use logical decoding. */ - CheckSlotPermissions(); CheckLogicalDecodingRequirements(true); /* diff --git a/src/test/modules/test_plan_advice/t/001_replan_regress.pl b/src/test/modules/test_plan_advice/t/001_replan_regress.pl index 38ffa4d11ae..452b179a665 100644 --- a/src/test/modules/test_plan_advice/t/001_replan_regress.pl +++ b/src/test/modules/test_plan_advice/t/001_replan_regress.pl @@ -18,6 +18,7 @@ $node->init(); # Set up our desired configuration. $node->append_conf('postgresql.conf', <<EOM); shared_preload_libraries='test_plan_advice' +wal_level=replica pg_plan_advice.always_explain_supplied_advice=false pg_plan_advice.feedback_warnings=true EOM diff --git a/src/test/regress/expected/cluster.out b/src/test/regress/expected/cluster.out index 6127b215a86..e17bc91fae1 100644 --- a/src/test/regress/expected/cluster.out +++ b/src/test/regress/expected/cluster.out @@ -543,15 +543,17 @@ ERROR: REPACK (CONCURRENTLY) is not supported for partitioned tables HINT: Consider running the command on individual partitions. DROP TABLE clstrpart; -- Ownership of partitions is checked -CREATE TABLE ptnowner(i int unique) PARTITION BY LIST (i); +CREATE TABLE ptnowner(i int unique not null) PARTITION BY LIST (i); CREATE INDEX ptnowner_i_idx ON ptnowner(i); CREATE TABLE ptnowner1 PARTITION OF ptnowner FOR VALUES IN (1); -CREATE ROLE regress_ptnowner; +CREATE ROLE regress_ptnowner LOGIN; CREATE TABLE ptnowner2 PARTITION OF ptnowner FOR VALUES IN (2); ALTER TABLE ptnowner1 OWNER TO regress_ptnowner; SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; ERROR: permission denied for table ptnowner +ALTER TABLE ptnowner1 REPLICA IDENTITY USING INDEX ptnowner1_i_key; +REPACK (CONCURRENTLY) ptnowner1; RESET SESSION AUTHORIZATION; ALTER TABLE ptnowner OWNER TO regress_ptnowner; CREATE TEMP TABLE ptnowner_oldnodes AS @@ -560,6 +562,11 @@ CREATE TEMP TABLE ptnowner_oldnodes AS SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; WARNING: permission denied to execute CLUSTER on "ptnowner2", skipping it +-- still can't repack without a replica identity +ALTER TABLE ptnowner1 REPLICA IDENTITY DEFAULT; +REPACK (CONCURRENTLY) ptnowner1; +ERROR: cannot process relation "ptnowner1" +HINT: Relation "ptnowner1" has no identity index. RESET SESSION AUTHORIZATION; SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; @@ -570,6 +577,15 @@ SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a ptnowner2 | t (3 rows) +SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a + JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; + relname | ?column? +-----------+---------- + ptnowner | t + ptnowner1 | f + ptnowner2 | t +(3 rows) + DROP TABLE ptnowner; DROP ROLE regress_ptnowner; -- Test CLUSTER with external tuplesorting diff --git a/src/test/regress/sql/cluster.sql b/src/test/regress/sql/cluster.sql index d14063a9683..1f471a8821a 100644 --- a/src/test/regress/sql/cluster.sql +++ b/src/test/regress/sql/cluster.sql @@ -254,14 +254,16 @@ REPACK (CONCURRENTLY) clstrpart; DROP TABLE clstrpart; -- Ownership of partitions is checked -CREATE TABLE ptnowner(i int unique) PARTITION BY LIST (i); +CREATE TABLE ptnowner(i int unique not null) PARTITION BY LIST (i); CREATE INDEX ptnowner_i_idx ON ptnowner(i); CREATE TABLE ptnowner1 PARTITION OF ptnowner FOR VALUES IN (1); -CREATE ROLE regress_ptnowner; +CREATE ROLE regress_ptnowner LOGIN; CREATE TABLE ptnowner2 PARTITION OF ptnowner FOR VALUES IN (2); ALTER TABLE ptnowner1 OWNER TO regress_ptnowner; SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; +ALTER TABLE ptnowner1 REPLICA IDENTITY USING INDEX ptnowner1_i_key; +REPACK (CONCURRENTLY) ptnowner1; RESET SESSION AUTHORIZATION; ALTER TABLE ptnowner OWNER TO regress_ptnowner; CREATE TEMP TABLE ptnowner_oldnodes AS @@ -269,7 +271,12 @@ CREATE TEMP TABLE ptnowner_oldnodes AS JOIN pg_class AS c ON c.oid=tree.relid; SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; +-- still can't repack without a replica identity +ALTER TABLE ptnowner1 REPLICA IDENTITY DEFAULT; +REPACK (CONCURRENTLY) ptnowner1; RESET SESSION AUTHORIZATION; +SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a + JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; DROP TABLE ptnowner; -- 2.47.3 --m6zy3l65ushq557m Content-Type: text/x-diff; charset=utf-8 Content-Disposition: attachment; filename="0002-REPACK-do-not-require-LOGIN-privileges.patch" ^ permalink raw reply [nested|flat] 41+ messages in thread
* [PATCH 1/2] REPACK: do not require the user to have REPLICATION @ 2026-04-20 09:38 Álvaro Herrera <[email protected]> 0 siblings, 0 replies; 41+ messages in thread From: Álvaro Herrera @ 2026-04-20 09:38 UTC (permalink / raw) Although REPACK (CONCURRENTLY) uses replication slots, there is no concern that the slot will leak data of other users, because the MAINTAIN privilege on the table is required anyway; requiring REPLICATION is user-unfriendly without providing any actual protection. A related aspect is that the REPLICATION attribute is not needed to prevent REPACK from stealing slots from logical replication, since commit e76d8c749c31 made REPACK use a separate pool of replication slots. Because there are now successful concurrent repack runs in the regression tests, we're forced to run test_plan_advice under wal_level=replica. Author: Antonin Houska <[email protected]> Reported-by: Justin Pryzby <[email protected]> Reviewed-by: Chao Li <[email protected]> Discussion: https://postgr.es/m/aeJHPNmL4vVy3oPw@pryzbyj2023 --- src/backend/commands/repack_worker.c | 1 - .../test_plan_advice/t/001_replan_regress.pl | 1 + src/test/regress/expected/cluster.out | 20 +++++++++++++++++-- src/test/regress/sql/cluster.sql | 11 ++++++++-- 4 files changed, 28 insertions(+), 5 deletions(-) diff --git a/src/backend/commands/repack_worker.c b/src/backend/commands/repack_worker.c index b17edd771e2..e4a4860805b 100644 --- a/src/backend/commands/repack_worker.c +++ b/src/backend/commands/repack_worker.c @@ -214,7 +214,6 @@ repack_setup_logical_decoding(Oid relid) /* * Make sure we can use logical decoding. */ - CheckSlotPermissions(); CheckLogicalDecodingRequirements(true); /* diff --git a/src/test/modules/test_plan_advice/t/001_replan_regress.pl b/src/test/modules/test_plan_advice/t/001_replan_regress.pl index 38ffa4d11ae..452b179a665 100644 --- a/src/test/modules/test_plan_advice/t/001_replan_regress.pl +++ b/src/test/modules/test_plan_advice/t/001_replan_regress.pl @@ -18,6 +18,7 @@ $node->init(); # Set up our desired configuration. $node->append_conf('postgresql.conf', <<EOM); shared_preload_libraries='test_plan_advice' +wal_level=replica pg_plan_advice.always_explain_supplied_advice=false pg_plan_advice.feedback_warnings=true EOM diff --git a/src/test/regress/expected/cluster.out b/src/test/regress/expected/cluster.out index 6127b215a86..e17bc91fae1 100644 --- a/src/test/regress/expected/cluster.out +++ b/src/test/regress/expected/cluster.out @@ -543,15 +543,17 @@ ERROR: REPACK (CONCURRENTLY) is not supported for partitioned tables HINT: Consider running the command on individual partitions. DROP TABLE clstrpart; -- Ownership of partitions is checked -CREATE TABLE ptnowner(i int unique) PARTITION BY LIST (i); +CREATE TABLE ptnowner(i int unique not null) PARTITION BY LIST (i); CREATE INDEX ptnowner_i_idx ON ptnowner(i); CREATE TABLE ptnowner1 PARTITION OF ptnowner FOR VALUES IN (1); -CREATE ROLE regress_ptnowner; +CREATE ROLE regress_ptnowner LOGIN; CREATE TABLE ptnowner2 PARTITION OF ptnowner FOR VALUES IN (2); ALTER TABLE ptnowner1 OWNER TO regress_ptnowner; SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; ERROR: permission denied for table ptnowner +ALTER TABLE ptnowner1 REPLICA IDENTITY USING INDEX ptnowner1_i_key; +REPACK (CONCURRENTLY) ptnowner1; RESET SESSION AUTHORIZATION; ALTER TABLE ptnowner OWNER TO regress_ptnowner; CREATE TEMP TABLE ptnowner_oldnodes AS @@ -560,6 +562,11 @@ CREATE TEMP TABLE ptnowner_oldnodes AS SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; WARNING: permission denied to execute CLUSTER on "ptnowner2", skipping it +-- still can't repack without a replica identity +ALTER TABLE ptnowner1 REPLICA IDENTITY DEFAULT; +REPACK (CONCURRENTLY) ptnowner1; +ERROR: cannot process relation "ptnowner1" +HINT: Relation "ptnowner1" has no identity index. RESET SESSION AUTHORIZATION; SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; @@ -570,6 +577,15 @@ SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a ptnowner2 | t (3 rows) +SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a + JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; + relname | ?column? +-----------+---------- + ptnowner | t + ptnowner1 | f + ptnowner2 | t +(3 rows) + DROP TABLE ptnowner; DROP ROLE regress_ptnowner; -- Test CLUSTER with external tuplesorting diff --git a/src/test/regress/sql/cluster.sql b/src/test/regress/sql/cluster.sql index d14063a9683..1f471a8821a 100644 --- a/src/test/regress/sql/cluster.sql +++ b/src/test/regress/sql/cluster.sql @@ -254,14 +254,16 @@ REPACK (CONCURRENTLY) clstrpart; DROP TABLE clstrpart; -- Ownership of partitions is checked -CREATE TABLE ptnowner(i int unique) PARTITION BY LIST (i); +CREATE TABLE ptnowner(i int unique not null) PARTITION BY LIST (i); CREATE INDEX ptnowner_i_idx ON ptnowner(i); CREATE TABLE ptnowner1 PARTITION OF ptnowner FOR VALUES IN (1); -CREATE ROLE regress_ptnowner; +CREATE ROLE regress_ptnowner LOGIN; CREATE TABLE ptnowner2 PARTITION OF ptnowner FOR VALUES IN (2); ALTER TABLE ptnowner1 OWNER TO regress_ptnowner; SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; +ALTER TABLE ptnowner1 REPLICA IDENTITY USING INDEX ptnowner1_i_key; +REPACK (CONCURRENTLY) ptnowner1; RESET SESSION AUTHORIZATION; ALTER TABLE ptnowner OWNER TO regress_ptnowner; CREATE TEMP TABLE ptnowner_oldnodes AS @@ -269,7 +271,12 @@ CREATE TEMP TABLE ptnowner_oldnodes AS JOIN pg_class AS c ON c.oid=tree.relid; SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; +-- still can't repack without a replica identity +ALTER TABLE ptnowner1 REPLICA IDENTITY DEFAULT; +REPACK (CONCURRENTLY) ptnowner1; RESET SESSION AUTHORIZATION; +SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a + JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; DROP TABLE ptnowner; -- 2.47.3 --m6zy3l65ushq557m Content-Type: text/x-diff; charset=utf-8 Content-Disposition: attachment; filename="0002-REPACK-do-not-require-LOGIN-privileges.patch" ^ permalink raw reply [nested|flat] 41+ messages in thread
* [PATCH 1/2] REPACK: do not require the user to have REPLICATION @ 2026-04-20 09:38 Álvaro Herrera <[email protected]> 0 siblings, 0 replies; 41+ messages in thread From: Álvaro Herrera @ 2026-04-20 09:38 UTC (permalink / raw) Although REPACK (CONCURRENTLY) uses replication slots, there is no concern that the slot will leak data of other users, because the MAINTAIN privilege on the table is required anyway; requiring REPLICATION is user-unfriendly without providing any actual protection. A related aspect is that the REPLICATION attribute is not needed to prevent REPACK from stealing slots from logical replication, since commit e76d8c749c31 made REPACK use a separate pool of replication slots. Because there are now successful concurrent repack runs in the regression tests, we're forced to run test_plan_advice under wal_level=replica. Author: Antonin Houska <[email protected]> Reported-by: Justin Pryzby <[email protected]> Reviewed-by: Chao Li <[email protected]> Discussion: https://postgr.es/m/aeJHPNmL4vVy3oPw@pryzbyj2023 --- src/backend/commands/repack_worker.c | 1 - .../test_plan_advice/t/001_replan_regress.pl | 1 + src/test/regress/expected/cluster.out | 20 +++++++++++++++++-- src/test/regress/sql/cluster.sql | 11 ++++++++-- 4 files changed, 28 insertions(+), 5 deletions(-) diff --git a/src/backend/commands/repack_worker.c b/src/backend/commands/repack_worker.c index b17edd771e2..e4a4860805b 100644 --- a/src/backend/commands/repack_worker.c +++ b/src/backend/commands/repack_worker.c @@ -214,7 +214,6 @@ repack_setup_logical_decoding(Oid relid) /* * Make sure we can use logical decoding. */ - CheckSlotPermissions(); CheckLogicalDecodingRequirements(true); /* diff --git a/src/test/modules/test_plan_advice/t/001_replan_regress.pl b/src/test/modules/test_plan_advice/t/001_replan_regress.pl index 38ffa4d11ae..452b179a665 100644 --- a/src/test/modules/test_plan_advice/t/001_replan_regress.pl +++ b/src/test/modules/test_plan_advice/t/001_replan_regress.pl @@ -18,6 +18,7 @@ $node->init(); # Set up our desired configuration. $node->append_conf('postgresql.conf', <<EOM); shared_preload_libraries='test_plan_advice' +wal_level=replica pg_plan_advice.always_explain_supplied_advice=false pg_plan_advice.feedback_warnings=true EOM diff --git a/src/test/regress/expected/cluster.out b/src/test/regress/expected/cluster.out index 6127b215a86..e17bc91fae1 100644 --- a/src/test/regress/expected/cluster.out +++ b/src/test/regress/expected/cluster.out @@ -543,15 +543,17 @@ ERROR: REPACK (CONCURRENTLY) is not supported for partitioned tables HINT: Consider running the command on individual partitions. DROP TABLE clstrpart; -- Ownership of partitions is checked -CREATE TABLE ptnowner(i int unique) PARTITION BY LIST (i); +CREATE TABLE ptnowner(i int unique not null) PARTITION BY LIST (i); CREATE INDEX ptnowner_i_idx ON ptnowner(i); CREATE TABLE ptnowner1 PARTITION OF ptnowner FOR VALUES IN (1); -CREATE ROLE regress_ptnowner; +CREATE ROLE regress_ptnowner LOGIN; CREATE TABLE ptnowner2 PARTITION OF ptnowner FOR VALUES IN (2); ALTER TABLE ptnowner1 OWNER TO regress_ptnowner; SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; ERROR: permission denied for table ptnowner +ALTER TABLE ptnowner1 REPLICA IDENTITY USING INDEX ptnowner1_i_key; +REPACK (CONCURRENTLY) ptnowner1; RESET SESSION AUTHORIZATION; ALTER TABLE ptnowner OWNER TO regress_ptnowner; CREATE TEMP TABLE ptnowner_oldnodes AS @@ -560,6 +562,11 @@ CREATE TEMP TABLE ptnowner_oldnodes AS SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; WARNING: permission denied to execute CLUSTER on "ptnowner2", skipping it +-- still can't repack without a replica identity +ALTER TABLE ptnowner1 REPLICA IDENTITY DEFAULT; +REPACK (CONCURRENTLY) ptnowner1; +ERROR: cannot process relation "ptnowner1" +HINT: Relation "ptnowner1" has no identity index. RESET SESSION AUTHORIZATION; SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; @@ -570,6 +577,15 @@ SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a ptnowner2 | t (3 rows) +SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a + JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; + relname | ?column? +-----------+---------- + ptnowner | t + ptnowner1 | f + ptnowner2 | t +(3 rows) + DROP TABLE ptnowner; DROP ROLE regress_ptnowner; -- Test CLUSTER with external tuplesorting diff --git a/src/test/regress/sql/cluster.sql b/src/test/regress/sql/cluster.sql index d14063a9683..1f471a8821a 100644 --- a/src/test/regress/sql/cluster.sql +++ b/src/test/regress/sql/cluster.sql @@ -254,14 +254,16 @@ REPACK (CONCURRENTLY) clstrpart; DROP TABLE clstrpart; -- Ownership of partitions is checked -CREATE TABLE ptnowner(i int unique) PARTITION BY LIST (i); +CREATE TABLE ptnowner(i int unique not null) PARTITION BY LIST (i); CREATE INDEX ptnowner_i_idx ON ptnowner(i); CREATE TABLE ptnowner1 PARTITION OF ptnowner FOR VALUES IN (1); -CREATE ROLE regress_ptnowner; +CREATE ROLE regress_ptnowner LOGIN; CREATE TABLE ptnowner2 PARTITION OF ptnowner FOR VALUES IN (2); ALTER TABLE ptnowner1 OWNER TO regress_ptnowner; SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; +ALTER TABLE ptnowner1 REPLICA IDENTITY USING INDEX ptnowner1_i_key; +REPACK (CONCURRENTLY) ptnowner1; RESET SESSION AUTHORIZATION; ALTER TABLE ptnowner OWNER TO regress_ptnowner; CREATE TEMP TABLE ptnowner_oldnodes AS @@ -269,7 +271,12 @@ CREATE TEMP TABLE ptnowner_oldnodes AS JOIN pg_class AS c ON c.oid=tree.relid; SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; +-- still can't repack without a replica identity +ALTER TABLE ptnowner1 REPLICA IDENTITY DEFAULT; +REPACK (CONCURRENTLY) ptnowner1; RESET SESSION AUTHORIZATION; +SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a + JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; DROP TABLE ptnowner; -- 2.47.3 --m6zy3l65ushq557m Content-Type: text/x-diff; charset=utf-8 Content-Disposition: attachment; filename="0002-REPACK-do-not-require-LOGIN-privileges.patch" ^ permalink raw reply [nested|flat] 41+ messages in thread
* [PATCH 1/2] REPACK: do not require the user to have REPLICATION @ 2026-04-20 09:38 Álvaro Herrera <[email protected]> 0 siblings, 0 replies; 41+ messages in thread From: Álvaro Herrera @ 2026-04-20 09:38 UTC (permalink / raw) Although REPACK (CONCURRENTLY) uses replication slots, there is no concern that the slot will leak data of other users, because the MAINTAIN privilege on the table is required anyway; requiring REPLICATION is user-unfriendly without providing any actual protection. A related aspect is that the REPLICATION attribute is not needed to prevent REPACK from stealing slots from logical replication, since commit e76d8c749c31 made REPACK use a separate pool of replication slots. Because there are now successful concurrent repack runs in the regression tests, we're forced to run test_plan_advice under wal_level=replica. Author: Antonin Houska <[email protected]> Reported-by: Justin Pryzby <[email protected]> Reviewed-by: Chao Li <[email protected]> Discussion: https://postgr.es/m/aeJHPNmL4vVy3oPw@pryzbyj2023 --- src/backend/commands/repack_worker.c | 1 - .../test_plan_advice/t/001_replan_regress.pl | 1 + src/test/regress/expected/cluster.out | 20 +++++++++++++++++-- src/test/regress/sql/cluster.sql | 11 ++++++++-- 4 files changed, 28 insertions(+), 5 deletions(-) diff --git a/src/backend/commands/repack_worker.c b/src/backend/commands/repack_worker.c index b17edd771e2..e4a4860805b 100644 --- a/src/backend/commands/repack_worker.c +++ b/src/backend/commands/repack_worker.c @@ -214,7 +214,6 @@ repack_setup_logical_decoding(Oid relid) /* * Make sure we can use logical decoding. */ - CheckSlotPermissions(); CheckLogicalDecodingRequirements(true); /* diff --git a/src/test/modules/test_plan_advice/t/001_replan_regress.pl b/src/test/modules/test_plan_advice/t/001_replan_regress.pl index 38ffa4d11ae..452b179a665 100644 --- a/src/test/modules/test_plan_advice/t/001_replan_regress.pl +++ b/src/test/modules/test_plan_advice/t/001_replan_regress.pl @@ -18,6 +18,7 @@ $node->init(); # Set up our desired configuration. $node->append_conf('postgresql.conf', <<EOM); shared_preload_libraries='test_plan_advice' +wal_level=replica pg_plan_advice.always_explain_supplied_advice=false pg_plan_advice.feedback_warnings=true EOM diff --git a/src/test/regress/expected/cluster.out b/src/test/regress/expected/cluster.out index 6127b215a86..e17bc91fae1 100644 --- a/src/test/regress/expected/cluster.out +++ b/src/test/regress/expected/cluster.out @@ -543,15 +543,17 @@ ERROR: REPACK (CONCURRENTLY) is not supported for partitioned tables HINT: Consider running the command on individual partitions. DROP TABLE clstrpart; -- Ownership of partitions is checked -CREATE TABLE ptnowner(i int unique) PARTITION BY LIST (i); +CREATE TABLE ptnowner(i int unique not null) PARTITION BY LIST (i); CREATE INDEX ptnowner_i_idx ON ptnowner(i); CREATE TABLE ptnowner1 PARTITION OF ptnowner FOR VALUES IN (1); -CREATE ROLE regress_ptnowner; +CREATE ROLE regress_ptnowner LOGIN; CREATE TABLE ptnowner2 PARTITION OF ptnowner FOR VALUES IN (2); ALTER TABLE ptnowner1 OWNER TO regress_ptnowner; SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; ERROR: permission denied for table ptnowner +ALTER TABLE ptnowner1 REPLICA IDENTITY USING INDEX ptnowner1_i_key; +REPACK (CONCURRENTLY) ptnowner1; RESET SESSION AUTHORIZATION; ALTER TABLE ptnowner OWNER TO regress_ptnowner; CREATE TEMP TABLE ptnowner_oldnodes AS @@ -560,6 +562,11 @@ CREATE TEMP TABLE ptnowner_oldnodes AS SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; WARNING: permission denied to execute CLUSTER on "ptnowner2", skipping it +-- still can't repack without a replica identity +ALTER TABLE ptnowner1 REPLICA IDENTITY DEFAULT; +REPACK (CONCURRENTLY) ptnowner1; +ERROR: cannot process relation "ptnowner1" +HINT: Relation "ptnowner1" has no identity index. RESET SESSION AUTHORIZATION; SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; @@ -570,6 +577,15 @@ SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a ptnowner2 | t (3 rows) +SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a + JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; + relname | ?column? +-----------+---------- + ptnowner | t + ptnowner1 | f + ptnowner2 | t +(3 rows) + DROP TABLE ptnowner; DROP ROLE regress_ptnowner; -- Test CLUSTER with external tuplesorting diff --git a/src/test/regress/sql/cluster.sql b/src/test/regress/sql/cluster.sql index d14063a9683..1f471a8821a 100644 --- a/src/test/regress/sql/cluster.sql +++ b/src/test/regress/sql/cluster.sql @@ -254,14 +254,16 @@ REPACK (CONCURRENTLY) clstrpart; DROP TABLE clstrpart; -- Ownership of partitions is checked -CREATE TABLE ptnowner(i int unique) PARTITION BY LIST (i); +CREATE TABLE ptnowner(i int unique not null) PARTITION BY LIST (i); CREATE INDEX ptnowner_i_idx ON ptnowner(i); CREATE TABLE ptnowner1 PARTITION OF ptnowner FOR VALUES IN (1); -CREATE ROLE regress_ptnowner; +CREATE ROLE regress_ptnowner LOGIN; CREATE TABLE ptnowner2 PARTITION OF ptnowner FOR VALUES IN (2); ALTER TABLE ptnowner1 OWNER TO regress_ptnowner; SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; +ALTER TABLE ptnowner1 REPLICA IDENTITY USING INDEX ptnowner1_i_key; +REPACK (CONCURRENTLY) ptnowner1; RESET SESSION AUTHORIZATION; ALTER TABLE ptnowner OWNER TO regress_ptnowner; CREATE TEMP TABLE ptnowner_oldnodes AS @@ -269,7 +271,12 @@ CREATE TEMP TABLE ptnowner_oldnodes AS JOIN pg_class AS c ON c.oid=tree.relid; SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; +-- still can't repack without a replica identity +ALTER TABLE ptnowner1 REPLICA IDENTITY DEFAULT; +REPACK (CONCURRENTLY) ptnowner1; RESET SESSION AUTHORIZATION; +SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a + JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; DROP TABLE ptnowner; -- 2.47.3 --m6zy3l65ushq557m Content-Type: text/x-diff; charset=utf-8 Content-Disposition: attachment; filename="0002-REPACK-do-not-require-LOGIN-privileges.patch" ^ permalink raw reply [nested|flat] 41+ messages in thread
* [PATCH 1/2] REPACK: do not require the user to have REPLICATION @ 2026-04-20 09:38 Álvaro Herrera <[email protected]> 0 siblings, 0 replies; 41+ messages in thread From: Álvaro Herrera @ 2026-04-20 09:38 UTC (permalink / raw) Although REPACK (CONCURRENTLY) uses replication slots, there is no concern that the slot will leak data of other users, because the MAINTAIN privilege on the table is required anyway; requiring REPLICATION is user-unfriendly without providing any actual protection. A related aspect is that the REPLICATION attribute is not needed to prevent REPACK from stealing slots from logical replication, since commit e76d8c749c31 made REPACK use a separate pool of replication slots. Because there are now successful concurrent repack runs in the regression tests, we're forced to run test_plan_advice under wal_level=replica. Author: Antonin Houska <[email protected]> Reported-by: Justin Pryzby <[email protected]> Reviewed-by: Chao Li <[email protected]> Discussion: https://postgr.es/m/aeJHPNmL4vVy3oPw@pryzbyj2023 --- src/backend/commands/repack_worker.c | 1 - .../test_plan_advice/t/001_replan_regress.pl | 1 + src/test/regress/expected/cluster.out | 20 +++++++++++++++++-- src/test/regress/sql/cluster.sql | 11 ++++++++-- 4 files changed, 28 insertions(+), 5 deletions(-) diff --git a/src/backend/commands/repack_worker.c b/src/backend/commands/repack_worker.c index b17edd771e2..e4a4860805b 100644 --- a/src/backend/commands/repack_worker.c +++ b/src/backend/commands/repack_worker.c @@ -214,7 +214,6 @@ repack_setup_logical_decoding(Oid relid) /* * Make sure we can use logical decoding. */ - CheckSlotPermissions(); CheckLogicalDecodingRequirements(true); /* diff --git a/src/test/modules/test_plan_advice/t/001_replan_regress.pl b/src/test/modules/test_plan_advice/t/001_replan_regress.pl index 38ffa4d11ae..452b179a665 100644 --- a/src/test/modules/test_plan_advice/t/001_replan_regress.pl +++ b/src/test/modules/test_plan_advice/t/001_replan_regress.pl @@ -18,6 +18,7 @@ $node->init(); # Set up our desired configuration. $node->append_conf('postgresql.conf', <<EOM); shared_preload_libraries='test_plan_advice' +wal_level=replica pg_plan_advice.always_explain_supplied_advice=false pg_plan_advice.feedback_warnings=true EOM diff --git a/src/test/regress/expected/cluster.out b/src/test/regress/expected/cluster.out index 6127b215a86..e17bc91fae1 100644 --- a/src/test/regress/expected/cluster.out +++ b/src/test/regress/expected/cluster.out @@ -543,15 +543,17 @@ ERROR: REPACK (CONCURRENTLY) is not supported for partitioned tables HINT: Consider running the command on individual partitions. DROP TABLE clstrpart; -- Ownership of partitions is checked -CREATE TABLE ptnowner(i int unique) PARTITION BY LIST (i); +CREATE TABLE ptnowner(i int unique not null) PARTITION BY LIST (i); CREATE INDEX ptnowner_i_idx ON ptnowner(i); CREATE TABLE ptnowner1 PARTITION OF ptnowner FOR VALUES IN (1); -CREATE ROLE regress_ptnowner; +CREATE ROLE regress_ptnowner LOGIN; CREATE TABLE ptnowner2 PARTITION OF ptnowner FOR VALUES IN (2); ALTER TABLE ptnowner1 OWNER TO regress_ptnowner; SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; ERROR: permission denied for table ptnowner +ALTER TABLE ptnowner1 REPLICA IDENTITY USING INDEX ptnowner1_i_key; +REPACK (CONCURRENTLY) ptnowner1; RESET SESSION AUTHORIZATION; ALTER TABLE ptnowner OWNER TO regress_ptnowner; CREATE TEMP TABLE ptnowner_oldnodes AS @@ -560,6 +562,11 @@ CREATE TEMP TABLE ptnowner_oldnodes AS SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; WARNING: permission denied to execute CLUSTER on "ptnowner2", skipping it +-- still can't repack without a replica identity +ALTER TABLE ptnowner1 REPLICA IDENTITY DEFAULT; +REPACK (CONCURRENTLY) ptnowner1; +ERROR: cannot process relation "ptnowner1" +HINT: Relation "ptnowner1" has no identity index. RESET SESSION AUTHORIZATION; SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; @@ -570,6 +577,15 @@ SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a ptnowner2 | t (3 rows) +SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a + JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; + relname | ?column? +-----------+---------- + ptnowner | t + ptnowner1 | f + ptnowner2 | t +(3 rows) + DROP TABLE ptnowner; DROP ROLE regress_ptnowner; -- Test CLUSTER with external tuplesorting diff --git a/src/test/regress/sql/cluster.sql b/src/test/regress/sql/cluster.sql index d14063a9683..1f471a8821a 100644 --- a/src/test/regress/sql/cluster.sql +++ b/src/test/regress/sql/cluster.sql @@ -254,14 +254,16 @@ REPACK (CONCURRENTLY) clstrpart; DROP TABLE clstrpart; -- Ownership of partitions is checked -CREATE TABLE ptnowner(i int unique) PARTITION BY LIST (i); +CREATE TABLE ptnowner(i int unique not null) PARTITION BY LIST (i); CREATE INDEX ptnowner_i_idx ON ptnowner(i); CREATE TABLE ptnowner1 PARTITION OF ptnowner FOR VALUES IN (1); -CREATE ROLE regress_ptnowner; +CREATE ROLE regress_ptnowner LOGIN; CREATE TABLE ptnowner2 PARTITION OF ptnowner FOR VALUES IN (2); ALTER TABLE ptnowner1 OWNER TO regress_ptnowner; SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; +ALTER TABLE ptnowner1 REPLICA IDENTITY USING INDEX ptnowner1_i_key; +REPACK (CONCURRENTLY) ptnowner1; RESET SESSION AUTHORIZATION; ALTER TABLE ptnowner OWNER TO regress_ptnowner; CREATE TEMP TABLE ptnowner_oldnodes AS @@ -269,7 +271,12 @@ CREATE TEMP TABLE ptnowner_oldnodes AS JOIN pg_class AS c ON c.oid=tree.relid; SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; +-- still can't repack without a replica identity +ALTER TABLE ptnowner1 REPLICA IDENTITY DEFAULT; +REPACK (CONCURRENTLY) ptnowner1; RESET SESSION AUTHORIZATION; +SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a + JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; DROP TABLE ptnowner; -- 2.47.3 --m6zy3l65ushq557m Content-Type: text/x-diff; charset=utf-8 Content-Disposition: attachment; filename="0002-REPACK-do-not-require-LOGIN-privileges.patch" ^ permalink raw reply [nested|flat] 41+ messages in thread
* [PATCH 1/2] REPACK: do not require the user to have REPLICATION @ 2026-04-20 09:38 Álvaro Herrera <[email protected]> 0 siblings, 0 replies; 41+ messages in thread From: Álvaro Herrera @ 2026-04-20 09:38 UTC (permalink / raw) Although REPACK (CONCURRENTLY) uses replication slots, there is no concern that the slot will leak data of other users, because the MAINTAIN privilege on the table is required anyway; requiring REPLICATION is user-unfriendly without providing any actual protection. A related aspect is that the REPLICATION attribute is not needed to prevent REPACK from stealing slots from logical replication, since commit e76d8c749c31 made REPACK use a separate pool of replication slots. Because there are now successful concurrent repack runs in the regression tests, we're forced to run test_plan_advice under wal_level=replica. Author: Antonin Houska <[email protected]> Reported-by: Justin Pryzby <[email protected]> Reviewed-by: Chao Li <[email protected]> Discussion: https://postgr.es/m/aeJHPNmL4vVy3oPw@pryzbyj2023 --- src/backend/commands/repack_worker.c | 1 - .../test_plan_advice/t/001_replan_regress.pl | 1 + src/test/regress/expected/cluster.out | 20 +++++++++++++++++-- src/test/regress/sql/cluster.sql | 11 ++++++++-- 4 files changed, 28 insertions(+), 5 deletions(-) diff --git a/src/backend/commands/repack_worker.c b/src/backend/commands/repack_worker.c index b17edd771e2..e4a4860805b 100644 --- a/src/backend/commands/repack_worker.c +++ b/src/backend/commands/repack_worker.c @@ -214,7 +214,6 @@ repack_setup_logical_decoding(Oid relid) /* * Make sure we can use logical decoding. */ - CheckSlotPermissions(); CheckLogicalDecodingRequirements(true); /* diff --git a/src/test/modules/test_plan_advice/t/001_replan_regress.pl b/src/test/modules/test_plan_advice/t/001_replan_regress.pl index 38ffa4d11ae..452b179a665 100644 --- a/src/test/modules/test_plan_advice/t/001_replan_regress.pl +++ b/src/test/modules/test_plan_advice/t/001_replan_regress.pl @@ -18,6 +18,7 @@ $node->init(); # Set up our desired configuration. $node->append_conf('postgresql.conf', <<EOM); shared_preload_libraries='test_plan_advice' +wal_level=replica pg_plan_advice.always_explain_supplied_advice=false pg_plan_advice.feedback_warnings=true EOM diff --git a/src/test/regress/expected/cluster.out b/src/test/regress/expected/cluster.out index 6127b215a86..e17bc91fae1 100644 --- a/src/test/regress/expected/cluster.out +++ b/src/test/regress/expected/cluster.out @@ -543,15 +543,17 @@ ERROR: REPACK (CONCURRENTLY) is not supported for partitioned tables HINT: Consider running the command on individual partitions. DROP TABLE clstrpart; -- Ownership of partitions is checked -CREATE TABLE ptnowner(i int unique) PARTITION BY LIST (i); +CREATE TABLE ptnowner(i int unique not null) PARTITION BY LIST (i); CREATE INDEX ptnowner_i_idx ON ptnowner(i); CREATE TABLE ptnowner1 PARTITION OF ptnowner FOR VALUES IN (1); -CREATE ROLE regress_ptnowner; +CREATE ROLE regress_ptnowner LOGIN; CREATE TABLE ptnowner2 PARTITION OF ptnowner FOR VALUES IN (2); ALTER TABLE ptnowner1 OWNER TO regress_ptnowner; SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; ERROR: permission denied for table ptnowner +ALTER TABLE ptnowner1 REPLICA IDENTITY USING INDEX ptnowner1_i_key; +REPACK (CONCURRENTLY) ptnowner1; RESET SESSION AUTHORIZATION; ALTER TABLE ptnowner OWNER TO regress_ptnowner; CREATE TEMP TABLE ptnowner_oldnodes AS @@ -560,6 +562,11 @@ CREATE TEMP TABLE ptnowner_oldnodes AS SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; WARNING: permission denied to execute CLUSTER on "ptnowner2", skipping it +-- still can't repack without a replica identity +ALTER TABLE ptnowner1 REPLICA IDENTITY DEFAULT; +REPACK (CONCURRENTLY) ptnowner1; +ERROR: cannot process relation "ptnowner1" +HINT: Relation "ptnowner1" has no identity index. RESET SESSION AUTHORIZATION; SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; @@ -570,6 +577,15 @@ SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a ptnowner2 | t (3 rows) +SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a + JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; + relname | ?column? +-----------+---------- + ptnowner | t + ptnowner1 | f + ptnowner2 | t +(3 rows) + DROP TABLE ptnowner; DROP ROLE regress_ptnowner; -- Test CLUSTER with external tuplesorting diff --git a/src/test/regress/sql/cluster.sql b/src/test/regress/sql/cluster.sql index d14063a9683..1f471a8821a 100644 --- a/src/test/regress/sql/cluster.sql +++ b/src/test/regress/sql/cluster.sql @@ -254,14 +254,16 @@ REPACK (CONCURRENTLY) clstrpart; DROP TABLE clstrpart; -- Ownership of partitions is checked -CREATE TABLE ptnowner(i int unique) PARTITION BY LIST (i); +CREATE TABLE ptnowner(i int unique not null) PARTITION BY LIST (i); CREATE INDEX ptnowner_i_idx ON ptnowner(i); CREATE TABLE ptnowner1 PARTITION OF ptnowner FOR VALUES IN (1); -CREATE ROLE regress_ptnowner; +CREATE ROLE regress_ptnowner LOGIN; CREATE TABLE ptnowner2 PARTITION OF ptnowner FOR VALUES IN (2); ALTER TABLE ptnowner1 OWNER TO regress_ptnowner; SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; +ALTER TABLE ptnowner1 REPLICA IDENTITY USING INDEX ptnowner1_i_key; +REPACK (CONCURRENTLY) ptnowner1; RESET SESSION AUTHORIZATION; ALTER TABLE ptnowner OWNER TO regress_ptnowner; CREATE TEMP TABLE ptnowner_oldnodes AS @@ -269,7 +271,12 @@ CREATE TEMP TABLE ptnowner_oldnodes AS JOIN pg_class AS c ON c.oid=tree.relid; SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; +-- still can't repack without a replica identity +ALTER TABLE ptnowner1 REPLICA IDENTITY DEFAULT; +REPACK (CONCURRENTLY) ptnowner1; RESET SESSION AUTHORIZATION; +SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a + JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; DROP TABLE ptnowner; -- 2.47.3 --m6zy3l65ushq557m Content-Type: text/x-diff; charset=utf-8 Content-Disposition: attachment; filename="0002-REPACK-do-not-require-LOGIN-privileges.patch" ^ permalink raw reply [nested|flat] 41+ messages in thread
* [PATCH 1/2] REPACK: do not require the user to have REPLICATION @ 2026-04-20 09:38 Álvaro Herrera <[email protected]> 0 siblings, 0 replies; 41+ messages in thread From: Álvaro Herrera @ 2026-04-20 09:38 UTC (permalink / raw) Although REPACK (CONCURRENTLY) uses replication slots, there is no concern that the slot will leak data of other users, because the MAINTAIN privilege on the table is required anyway; requiring REPLICATION is user-unfriendly without providing any actual protection. A related aspect is that the REPLICATION attribute is not needed to prevent REPACK from stealing slots from logical replication, since commit e76d8c749c31 made REPACK use a separate pool of replication slots. Because there are now successful concurrent repack runs in the regression tests, we're forced to run test_plan_advice under wal_level=replica. Author: Antonin Houska <[email protected]> Reported-by: Justin Pryzby <[email protected]> Reviewed-by: Chao Li <[email protected]> Discussion: https://postgr.es/m/aeJHPNmL4vVy3oPw@pryzbyj2023 --- src/backend/commands/repack_worker.c | 1 - .../test_plan_advice/t/001_replan_regress.pl | 1 + src/test/regress/expected/cluster.out | 20 +++++++++++++++++-- src/test/regress/sql/cluster.sql | 11 ++++++++-- 4 files changed, 28 insertions(+), 5 deletions(-) diff --git a/src/backend/commands/repack_worker.c b/src/backend/commands/repack_worker.c index b17edd771e2..e4a4860805b 100644 --- a/src/backend/commands/repack_worker.c +++ b/src/backend/commands/repack_worker.c @@ -214,7 +214,6 @@ repack_setup_logical_decoding(Oid relid) /* * Make sure we can use logical decoding. */ - CheckSlotPermissions(); CheckLogicalDecodingRequirements(true); /* diff --git a/src/test/modules/test_plan_advice/t/001_replan_regress.pl b/src/test/modules/test_plan_advice/t/001_replan_regress.pl index 38ffa4d11ae..452b179a665 100644 --- a/src/test/modules/test_plan_advice/t/001_replan_regress.pl +++ b/src/test/modules/test_plan_advice/t/001_replan_regress.pl @@ -18,6 +18,7 @@ $node->init(); # Set up our desired configuration. $node->append_conf('postgresql.conf', <<EOM); shared_preload_libraries='test_plan_advice' +wal_level=replica pg_plan_advice.always_explain_supplied_advice=false pg_plan_advice.feedback_warnings=true EOM diff --git a/src/test/regress/expected/cluster.out b/src/test/regress/expected/cluster.out index 6127b215a86..e17bc91fae1 100644 --- a/src/test/regress/expected/cluster.out +++ b/src/test/regress/expected/cluster.out @@ -543,15 +543,17 @@ ERROR: REPACK (CONCURRENTLY) is not supported for partitioned tables HINT: Consider running the command on individual partitions. DROP TABLE clstrpart; -- Ownership of partitions is checked -CREATE TABLE ptnowner(i int unique) PARTITION BY LIST (i); +CREATE TABLE ptnowner(i int unique not null) PARTITION BY LIST (i); CREATE INDEX ptnowner_i_idx ON ptnowner(i); CREATE TABLE ptnowner1 PARTITION OF ptnowner FOR VALUES IN (1); -CREATE ROLE regress_ptnowner; +CREATE ROLE regress_ptnowner LOGIN; CREATE TABLE ptnowner2 PARTITION OF ptnowner FOR VALUES IN (2); ALTER TABLE ptnowner1 OWNER TO regress_ptnowner; SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; ERROR: permission denied for table ptnowner +ALTER TABLE ptnowner1 REPLICA IDENTITY USING INDEX ptnowner1_i_key; +REPACK (CONCURRENTLY) ptnowner1; RESET SESSION AUTHORIZATION; ALTER TABLE ptnowner OWNER TO regress_ptnowner; CREATE TEMP TABLE ptnowner_oldnodes AS @@ -560,6 +562,11 @@ CREATE TEMP TABLE ptnowner_oldnodes AS SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; WARNING: permission denied to execute CLUSTER on "ptnowner2", skipping it +-- still can't repack without a replica identity +ALTER TABLE ptnowner1 REPLICA IDENTITY DEFAULT; +REPACK (CONCURRENTLY) ptnowner1; +ERROR: cannot process relation "ptnowner1" +HINT: Relation "ptnowner1" has no identity index. RESET SESSION AUTHORIZATION; SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; @@ -570,6 +577,15 @@ SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a ptnowner2 | t (3 rows) +SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a + JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; + relname | ?column? +-----------+---------- + ptnowner | t + ptnowner1 | f + ptnowner2 | t +(3 rows) + DROP TABLE ptnowner; DROP ROLE regress_ptnowner; -- Test CLUSTER with external tuplesorting diff --git a/src/test/regress/sql/cluster.sql b/src/test/regress/sql/cluster.sql index d14063a9683..1f471a8821a 100644 --- a/src/test/regress/sql/cluster.sql +++ b/src/test/regress/sql/cluster.sql @@ -254,14 +254,16 @@ REPACK (CONCURRENTLY) clstrpart; DROP TABLE clstrpart; -- Ownership of partitions is checked -CREATE TABLE ptnowner(i int unique) PARTITION BY LIST (i); +CREATE TABLE ptnowner(i int unique not null) PARTITION BY LIST (i); CREATE INDEX ptnowner_i_idx ON ptnowner(i); CREATE TABLE ptnowner1 PARTITION OF ptnowner FOR VALUES IN (1); -CREATE ROLE regress_ptnowner; +CREATE ROLE regress_ptnowner LOGIN; CREATE TABLE ptnowner2 PARTITION OF ptnowner FOR VALUES IN (2); ALTER TABLE ptnowner1 OWNER TO regress_ptnowner; SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; +ALTER TABLE ptnowner1 REPLICA IDENTITY USING INDEX ptnowner1_i_key; +REPACK (CONCURRENTLY) ptnowner1; RESET SESSION AUTHORIZATION; ALTER TABLE ptnowner OWNER TO regress_ptnowner; CREATE TEMP TABLE ptnowner_oldnodes AS @@ -269,7 +271,12 @@ CREATE TEMP TABLE ptnowner_oldnodes AS JOIN pg_class AS c ON c.oid=tree.relid; SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; +-- still can't repack without a replica identity +ALTER TABLE ptnowner1 REPLICA IDENTITY DEFAULT; +REPACK (CONCURRENTLY) ptnowner1; RESET SESSION AUTHORIZATION; +SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a + JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; DROP TABLE ptnowner; -- 2.47.3 --m6zy3l65ushq557m Content-Type: text/x-diff; charset=utf-8 Content-Disposition: attachment; filename="0002-REPACK-do-not-require-LOGIN-privileges.patch" ^ permalink raw reply [nested|flat] 41+ messages in thread
* [PATCH 1/2] REPACK: do not require the user to have REPLICATION @ 2026-04-20 09:38 Álvaro Herrera <[email protected]> 0 siblings, 0 replies; 41+ messages in thread From: Álvaro Herrera @ 2026-04-20 09:38 UTC (permalink / raw) Although REPACK (CONCURRENTLY) uses replication slots, there is no concern that the slot will leak data of other users, because the MAINTAIN privilege on the table is required anyway; requiring REPLICATION is user-unfriendly without providing any actual protection. A related aspect is that the REPLICATION attribute is not needed to prevent REPACK from stealing slots from logical replication, since commit e76d8c749c31 made REPACK use a separate pool of replication slots. Because there are now successful concurrent repack runs in the regression tests, we're forced to run test_plan_advice under wal_level=replica. Author: Antonin Houska <[email protected]> Reported-by: Justin Pryzby <[email protected]> Reviewed-by: Chao Li <[email protected]> Discussion: https://postgr.es/m/aeJHPNmL4vVy3oPw@pryzbyj2023 --- src/backend/commands/repack_worker.c | 1 - .../test_plan_advice/t/001_replan_regress.pl | 1 + src/test/regress/expected/cluster.out | 20 +++++++++++++++++-- src/test/regress/sql/cluster.sql | 11 ++++++++-- 4 files changed, 28 insertions(+), 5 deletions(-) diff --git a/src/backend/commands/repack_worker.c b/src/backend/commands/repack_worker.c index b17edd771e2..e4a4860805b 100644 --- a/src/backend/commands/repack_worker.c +++ b/src/backend/commands/repack_worker.c @@ -214,7 +214,6 @@ repack_setup_logical_decoding(Oid relid) /* * Make sure we can use logical decoding. */ - CheckSlotPermissions(); CheckLogicalDecodingRequirements(true); /* diff --git a/src/test/modules/test_plan_advice/t/001_replan_regress.pl b/src/test/modules/test_plan_advice/t/001_replan_regress.pl index 38ffa4d11ae..452b179a665 100644 --- a/src/test/modules/test_plan_advice/t/001_replan_regress.pl +++ b/src/test/modules/test_plan_advice/t/001_replan_regress.pl @@ -18,6 +18,7 @@ $node->init(); # Set up our desired configuration. $node->append_conf('postgresql.conf', <<EOM); shared_preload_libraries='test_plan_advice' +wal_level=replica pg_plan_advice.always_explain_supplied_advice=false pg_plan_advice.feedback_warnings=true EOM diff --git a/src/test/regress/expected/cluster.out b/src/test/regress/expected/cluster.out index 6127b215a86..e17bc91fae1 100644 --- a/src/test/regress/expected/cluster.out +++ b/src/test/regress/expected/cluster.out @@ -543,15 +543,17 @@ ERROR: REPACK (CONCURRENTLY) is not supported for partitioned tables HINT: Consider running the command on individual partitions. DROP TABLE clstrpart; -- Ownership of partitions is checked -CREATE TABLE ptnowner(i int unique) PARTITION BY LIST (i); +CREATE TABLE ptnowner(i int unique not null) PARTITION BY LIST (i); CREATE INDEX ptnowner_i_idx ON ptnowner(i); CREATE TABLE ptnowner1 PARTITION OF ptnowner FOR VALUES IN (1); -CREATE ROLE regress_ptnowner; +CREATE ROLE regress_ptnowner LOGIN; CREATE TABLE ptnowner2 PARTITION OF ptnowner FOR VALUES IN (2); ALTER TABLE ptnowner1 OWNER TO regress_ptnowner; SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; ERROR: permission denied for table ptnowner +ALTER TABLE ptnowner1 REPLICA IDENTITY USING INDEX ptnowner1_i_key; +REPACK (CONCURRENTLY) ptnowner1; RESET SESSION AUTHORIZATION; ALTER TABLE ptnowner OWNER TO regress_ptnowner; CREATE TEMP TABLE ptnowner_oldnodes AS @@ -560,6 +562,11 @@ CREATE TEMP TABLE ptnowner_oldnodes AS SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; WARNING: permission denied to execute CLUSTER on "ptnowner2", skipping it +-- still can't repack without a replica identity +ALTER TABLE ptnowner1 REPLICA IDENTITY DEFAULT; +REPACK (CONCURRENTLY) ptnowner1; +ERROR: cannot process relation "ptnowner1" +HINT: Relation "ptnowner1" has no identity index. RESET SESSION AUTHORIZATION; SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; @@ -570,6 +577,15 @@ SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a ptnowner2 | t (3 rows) +SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a + JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; + relname | ?column? +-----------+---------- + ptnowner | t + ptnowner1 | f + ptnowner2 | t +(3 rows) + DROP TABLE ptnowner; DROP ROLE regress_ptnowner; -- Test CLUSTER with external tuplesorting diff --git a/src/test/regress/sql/cluster.sql b/src/test/regress/sql/cluster.sql index d14063a9683..1f471a8821a 100644 --- a/src/test/regress/sql/cluster.sql +++ b/src/test/regress/sql/cluster.sql @@ -254,14 +254,16 @@ REPACK (CONCURRENTLY) clstrpart; DROP TABLE clstrpart; -- Ownership of partitions is checked -CREATE TABLE ptnowner(i int unique) PARTITION BY LIST (i); +CREATE TABLE ptnowner(i int unique not null) PARTITION BY LIST (i); CREATE INDEX ptnowner_i_idx ON ptnowner(i); CREATE TABLE ptnowner1 PARTITION OF ptnowner FOR VALUES IN (1); -CREATE ROLE regress_ptnowner; +CREATE ROLE regress_ptnowner LOGIN; CREATE TABLE ptnowner2 PARTITION OF ptnowner FOR VALUES IN (2); ALTER TABLE ptnowner1 OWNER TO regress_ptnowner; SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; +ALTER TABLE ptnowner1 REPLICA IDENTITY USING INDEX ptnowner1_i_key; +REPACK (CONCURRENTLY) ptnowner1; RESET SESSION AUTHORIZATION; ALTER TABLE ptnowner OWNER TO regress_ptnowner; CREATE TEMP TABLE ptnowner_oldnodes AS @@ -269,7 +271,12 @@ CREATE TEMP TABLE ptnowner_oldnodes AS JOIN pg_class AS c ON c.oid=tree.relid; SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; +-- still can't repack without a replica identity +ALTER TABLE ptnowner1 REPLICA IDENTITY DEFAULT; +REPACK (CONCURRENTLY) ptnowner1; RESET SESSION AUTHORIZATION; +SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a + JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; DROP TABLE ptnowner; -- 2.47.3 --m6zy3l65ushq557m Content-Type: text/x-diff; charset=utf-8 Content-Disposition: attachment; filename="0002-REPACK-do-not-require-LOGIN-privileges.patch" ^ permalink raw reply [nested|flat] 41+ messages in thread
* [PATCH 1/2] REPACK: do not require the user to have REPLICATION @ 2026-04-20 09:38 Álvaro Herrera <[email protected]> 0 siblings, 0 replies; 41+ messages in thread From: Álvaro Herrera @ 2026-04-20 09:38 UTC (permalink / raw) Although REPACK (CONCURRENTLY) uses replication slots, there is no concern that the slot will leak data of other users, because the MAINTAIN privilege on the table is required anyway; requiring REPLICATION is user-unfriendly without providing any actual protection. A related aspect is that the REPLICATION attribute is not needed to prevent REPACK from stealing slots from logical replication, since commit e76d8c749c31 made REPACK use a separate pool of replication slots. Because there are now successful concurrent repack runs in the regression tests, we're forced to run test_plan_advice under wal_level=replica. Author: Antonin Houska <[email protected]> Reported-by: Justin Pryzby <[email protected]> Reviewed-by: Chao Li <[email protected]> Discussion: https://postgr.es/m/aeJHPNmL4vVy3oPw@pryzbyj2023 --- src/backend/commands/repack_worker.c | 1 - .../test_plan_advice/t/001_replan_regress.pl | 1 + src/test/regress/expected/cluster.out | 20 +++++++++++++++++-- src/test/regress/sql/cluster.sql | 11 ++++++++-- 4 files changed, 28 insertions(+), 5 deletions(-) diff --git a/src/backend/commands/repack_worker.c b/src/backend/commands/repack_worker.c index b17edd771e2..e4a4860805b 100644 --- a/src/backend/commands/repack_worker.c +++ b/src/backend/commands/repack_worker.c @@ -214,7 +214,6 @@ repack_setup_logical_decoding(Oid relid) /* * Make sure we can use logical decoding. */ - CheckSlotPermissions(); CheckLogicalDecodingRequirements(true); /* diff --git a/src/test/modules/test_plan_advice/t/001_replan_regress.pl b/src/test/modules/test_plan_advice/t/001_replan_regress.pl index 38ffa4d11ae..452b179a665 100644 --- a/src/test/modules/test_plan_advice/t/001_replan_regress.pl +++ b/src/test/modules/test_plan_advice/t/001_replan_regress.pl @@ -18,6 +18,7 @@ $node->init(); # Set up our desired configuration. $node->append_conf('postgresql.conf', <<EOM); shared_preload_libraries='test_plan_advice' +wal_level=replica pg_plan_advice.always_explain_supplied_advice=false pg_plan_advice.feedback_warnings=true EOM diff --git a/src/test/regress/expected/cluster.out b/src/test/regress/expected/cluster.out index 6127b215a86..e17bc91fae1 100644 --- a/src/test/regress/expected/cluster.out +++ b/src/test/regress/expected/cluster.out @@ -543,15 +543,17 @@ ERROR: REPACK (CONCURRENTLY) is not supported for partitioned tables HINT: Consider running the command on individual partitions. DROP TABLE clstrpart; -- Ownership of partitions is checked -CREATE TABLE ptnowner(i int unique) PARTITION BY LIST (i); +CREATE TABLE ptnowner(i int unique not null) PARTITION BY LIST (i); CREATE INDEX ptnowner_i_idx ON ptnowner(i); CREATE TABLE ptnowner1 PARTITION OF ptnowner FOR VALUES IN (1); -CREATE ROLE regress_ptnowner; +CREATE ROLE regress_ptnowner LOGIN; CREATE TABLE ptnowner2 PARTITION OF ptnowner FOR VALUES IN (2); ALTER TABLE ptnowner1 OWNER TO regress_ptnowner; SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; ERROR: permission denied for table ptnowner +ALTER TABLE ptnowner1 REPLICA IDENTITY USING INDEX ptnowner1_i_key; +REPACK (CONCURRENTLY) ptnowner1; RESET SESSION AUTHORIZATION; ALTER TABLE ptnowner OWNER TO regress_ptnowner; CREATE TEMP TABLE ptnowner_oldnodes AS @@ -560,6 +562,11 @@ CREATE TEMP TABLE ptnowner_oldnodes AS SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; WARNING: permission denied to execute CLUSTER on "ptnowner2", skipping it +-- still can't repack without a replica identity +ALTER TABLE ptnowner1 REPLICA IDENTITY DEFAULT; +REPACK (CONCURRENTLY) ptnowner1; +ERROR: cannot process relation "ptnowner1" +HINT: Relation "ptnowner1" has no identity index. RESET SESSION AUTHORIZATION; SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; @@ -570,6 +577,15 @@ SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a ptnowner2 | t (3 rows) +SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a + JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; + relname | ?column? +-----------+---------- + ptnowner | t + ptnowner1 | f + ptnowner2 | t +(3 rows) + DROP TABLE ptnowner; DROP ROLE regress_ptnowner; -- Test CLUSTER with external tuplesorting diff --git a/src/test/regress/sql/cluster.sql b/src/test/regress/sql/cluster.sql index d14063a9683..1f471a8821a 100644 --- a/src/test/regress/sql/cluster.sql +++ b/src/test/regress/sql/cluster.sql @@ -254,14 +254,16 @@ REPACK (CONCURRENTLY) clstrpart; DROP TABLE clstrpart; -- Ownership of partitions is checked -CREATE TABLE ptnowner(i int unique) PARTITION BY LIST (i); +CREATE TABLE ptnowner(i int unique not null) PARTITION BY LIST (i); CREATE INDEX ptnowner_i_idx ON ptnowner(i); CREATE TABLE ptnowner1 PARTITION OF ptnowner FOR VALUES IN (1); -CREATE ROLE regress_ptnowner; +CREATE ROLE regress_ptnowner LOGIN; CREATE TABLE ptnowner2 PARTITION OF ptnowner FOR VALUES IN (2); ALTER TABLE ptnowner1 OWNER TO regress_ptnowner; SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; +ALTER TABLE ptnowner1 REPLICA IDENTITY USING INDEX ptnowner1_i_key; +REPACK (CONCURRENTLY) ptnowner1; RESET SESSION AUTHORIZATION; ALTER TABLE ptnowner OWNER TO regress_ptnowner; CREATE TEMP TABLE ptnowner_oldnodes AS @@ -269,7 +271,12 @@ CREATE TEMP TABLE ptnowner_oldnodes AS JOIN pg_class AS c ON c.oid=tree.relid; SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; +-- still can't repack without a replica identity +ALTER TABLE ptnowner1 REPLICA IDENTITY DEFAULT; +REPACK (CONCURRENTLY) ptnowner1; RESET SESSION AUTHORIZATION; +SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a + JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; DROP TABLE ptnowner; -- 2.47.3 --m6zy3l65ushq557m Content-Type: text/x-diff; charset=utf-8 Content-Disposition: attachment; filename="0002-REPACK-do-not-require-LOGIN-privileges.patch" ^ permalink raw reply [nested|flat] 41+ messages in thread
* [PATCH 1/2] REPACK: do not require the user to have REPLICATION @ 2026-04-20 09:38 Álvaro Herrera <[email protected]> 0 siblings, 0 replies; 41+ messages in thread From: Álvaro Herrera @ 2026-04-20 09:38 UTC (permalink / raw) Although REPACK (CONCURRENTLY) uses replication slots, there is no concern that the slot will leak data of other users, because the MAINTAIN privilege on the table is required anyway; requiring REPLICATION is user-unfriendly without providing any actual protection. A related aspect is that the REPLICATION attribute is not needed to prevent REPACK from stealing slots from logical replication, since commit e76d8c749c31 made REPACK use a separate pool of replication slots. Because there are now successful concurrent repack runs in the regression tests, we're forced to run test_plan_advice under wal_level=replica. Author: Antonin Houska <[email protected]> Reported-by: Justin Pryzby <[email protected]> Reviewed-by: Chao Li <[email protected]> Discussion: https://postgr.es/m/aeJHPNmL4vVy3oPw@pryzbyj2023 --- src/backend/commands/repack_worker.c | 1 - .../test_plan_advice/t/001_replan_regress.pl | 1 + src/test/regress/expected/cluster.out | 20 +++++++++++++++++-- src/test/regress/sql/cluster.sql | 11 ++++++++-- 4 files changed, 28 insertions(+), 5 deletions(-) diff --git a/src/backend/commands/repack_worker.c b/src/backend/commands/repack_worker.c index b17edd771e2..e4a4860805b 100644 --- a/src/backend/commands/repack_worker.c +++ b/src/backend/commands/repack_worker.c @@ -214,7 +214,6 @@ repack_setup_logical_decoding(Oid relid) /* * Make sure we can use logical decoding. */ - CheckSlotPermissions(); CheckLogicalDecodingRequirements(true); /* diff --git a/src/test/modules/test_plan_advice/t/001_replan_regress.pl b/src/test/modules/test_plan_advice/t/001_replan_regress.pl index 38ffa4d11ae..452b179a665 100644 --- a/src/test/modules/test_plan_advice/t/001_replan_regress.pl +++ b/src/test/modules/test_plan_advice/t/001_replan_regress.pl @@ -18,6 +18,7 @@ $node->init(); # Set up our desired configuration. $node->append_conf('postgresql.conf', <<EOM); shared_preload_libraries='test_plan_advice' +wal_level=replica pg_plan_advice.always_explain_supplied_advice=false pg_plan_advice.feedback_warnings=true EOM diff --git a/src/test/regress/expected/cluster.out b/src/test/regress/expected/cluster.out index 6127b215a86..e17bc91fae1 100644 --- a/src/test/regress/expected/cluster.out +++ b/src/test/regress/expected/cluster.out @@ -543,15 +543,17 @@ ERROR: REPACK (CONCURRENTLY) is not supported for partitioned tables HINT: Consider running the command on individual partitions. DROP TABLE clstrpart; -- Ownership of partitions is checked -CREATE TABLE ptnowner(i int unique) PARTITION BY LIST (i); +CREATE TABLE ptnowner(i int unique not null) PARTITION BY LIST (i); CREATE INDEX ptnowner_i_idx ON ptnowner(i); CREATE TABLE ptnowner1 PARTITION OF ptnowner FOR VALUES IN (1); -CREATE ROLE regress_ptnowner; +CREATE ROLE regress_ptnowner LOGIN; CREATE TABLE ptnowner2 PARTITION OF ptnowner FOR VALUES IN (2); ALTER TABLE ptnowner1 OWNER TO regress_ptnowner; SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; ERROR: permission denied for table ptnowner +ALTER TABLE ptnowner1 REPLICA IDENTITY USING INDEX ptnowner1_i_key; +REPACK (CONCURRENTLY) ptnowner1; RESET SESSION AUTHORIZATION; ALTER TABLE ptnowner OWNER TO regress_ptnowner; CREATE TEMP TABLE ptnowner_oldnodes AS @@ -560,6 +562,11 @@ CREATE TEMP TABLE ptnowner_oldnodes AS SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; WARNING: permission denied to execute CLUSTER on "ptnowner2", skipping it +-- still can't repack without a replica identity +ALTER TABLE ptnowner1 REPLICA IDENTITY DEFAULT; +REPACK (CONCURRENTLY) ptnowner1; +ERROR: cannot process relation "ptnowner1" +HINT: Relation "ptnowner1" has no identity index. RESET SESSION AUTHORIZATION; SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; @@ -570,6 +577,15 @@ SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a ptnowner2 | t (3 rows) +SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a + JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; + relname | ?column? +-----------+---------- + ptnowner | t + ptnowner1 | f + ptnowner2 | t +(3 rows) + DROP TABLE ptnowner; DROP ROLE regress_ptnowner; -- Test CLUSTER with external tuplesorting diff --git a/src/test/regress/sql/cluster.sql b/src/test/regress/sql/cluster.sql index d14063a9683..1f471a8821a 100644 --- a/src/test/regress/sql/cluster.sql +++ b/src/test/regress/sql/cluster.sql @@ -254,14 +254,16 @@ REPACK (CONCURRENTLY) clstrpart; DROP TABLE clstrpart; -- Ownership of partitions is checked -CREATE TABLE ptnowner(i int unique) PARTITION BY LIST (i); +CREATE TABLE ptnowner(i int unique not null) PARTITION BY LIST (i); CREATE INDEX ptnowner_i_idx ON ptnowner(i); CREATE TABLE ptnowner1 PARTITION OF ptnowner FOR VALUES IN (1); -CREATE ROLE regress_ptnowner; +CREATE ROLE regress_ptnowner LOGIN; CREATE TABLE ptnowner2 PARTITION OF ptnowner FOR VALUES IN (2); ALTER TABLE ptnowner1 OWNER TO regress_ptnowner; SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; +ALTER TABLE ptnowner1 REPLICA IDENTITY USING INDEX ptnowner1_i_key; +REPACK (CONCURRENTLY) ptnowner1; RESET SESSION AUTHORIZATION; ALTER TABLE ptnowner OWNER TO regress_ptnowner; CREATE TEMP TABLE ptnowner_oldnodes AS @@ -269,7 +271,12 @@ CREATE TEMP TABLE ptnowner_oldnodes AS JOIN pg_class AS c ON c.oid=tree.relid; SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; +-- still can't repack without a replica identity +ALTER TABLE ptnowner1 REPLICA IDENTITY DEFAULT; +REPACK (CONCURRENTLY) ptnowner1; RESET SESSION AUTHORIZATION; +SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a + JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; DROP TABLE ptnowner; -- 2.47.3 --m6zy3l65ushq557m Content-Type: text/x-diff; charset=utf-8 Content-Disposition: attachment; filename="0002-REPACK-do-not-require-LOGIN-privileges.patch" ^ permalink raw reply [nested|flat] 41+ messages in thread
* [PATCH 1/2] REPACK: do not require the user to have REPLICATION @ 2026-04-20 09:38 Álvaro Herrera <[email protected]> 0 siblings, 0 replies; 41+ messages in thread From: Álvaro Herrera @ 2026-04-20 09:38 UTC (permalink / raw) Although REPACK (CONCURRENTLY) uses replication slots, there is no concern that the slot will leak data of other users, because the MAINTAIN privilege on the table is required anyway; requiring REPLICATION is user-unfriendly without providing any actual protection. A related aspect is that the REPLICATION attribute is not needed to prevent REPACK from stealing slots from logical replication, since commit e76d8c749c31 made REPACK use a separate pool of replication slots. Because there are now successful concurrent repack runs in the regression tests, we're forced to run test_plan_advice under wal_level=replica. Author: Antonin Houska <[email protected]> Reported-by: Justin Pryzby <[email protected]> Reviewed-by: Chao Li <[email protected]> Discussion: https://postgr.es/m/aeJHPNmL4vVy3oPw@pryzbyj2023 --- src/backend/commands/repack_worker.c | 1 - .../test_plan_advice/t/001_replan_regress.pl | 1 + src/test/regress/expected/cluster.out | 20 +++++++++++++++++-- src/test/regress/sql/cluster.sql | 11 ++++++++-- 4 files changed, 28 insertions(+), 5 deletions(-) diff --git a/src/backend/commands/repack_worker.c b/src/backend/commands/repack_worker.c index b17edd771e2..e4a4860805b 100644 --- a/src/backend/commands/repack_worker.c +++ b/src/backend/commands/repack_worker.c @@ -214,7 +214,6 @@ repack_setup_logical_decoding(Oid relid) /* * Make sure we can use logical decoding. */ - CheckSlotPermissions(); CheckLogicalDecodingRequirements(true); /* diff --git a/src/test/modules/test_plan_advice/t/001_replan_regress.pl b/src/test/modules/test_plan_advice/t/001_replan_regress.pl index 38ffa4d11ae..452b179a665 100644 --- a/src/test/modules/test_plan_advice/t/001_replan_regress.pl +++ b/src/test/modules/test_plan_advice/t/001_replan_regress.pl @@ -18,6 +18,7 @@ $node->init(); # Set up our desired configuration. $node->append_conf('postgresql.conf', <<EOM); shared_preload_libraries='test_plan_advice' +wal_level=replica pg_plan_advice.always_explain_supplied_advice=false pg_plan_advice.feedback_warnings=true EOM diff --git a/src/test/regress/expected/cluster.out b/src/test/regress/expected/cluster.out index 6127b215a86..e17bc91fae1 100644 --- a/src/test/regress/expected/cluster.out +++ b/src/test/regress/expected/cluster.out @@ -543,15 +543,17 @@ ERROR: REPACK (CONCURRENTLY) is not supported for partitioned tables HINT: Consider running the command on individual partitions. DROP TABLE clstrpart; -- Ownership of partitions is checked -CREATE TABLE ptnowner(i int unique) PARTITION BY LIST (i); +CREATE TABLE ptnowner(i int unique not null) PARTITION BY LIST (i); CREATE INDEX ptnowner_i_idx ON ptnowner(i); CREATE TABLE ptnowner1 PARTITION OF ptnowner FOR VALUES IN (1); -CREATE ROLE regress_ptnowner; +CREATE ROLE regress_ptnowner LOGIN; CREATE TABLE ptnowner2 PARTITION OF ptnowner FOR VALUES IN (2); ALTER TABLE ptnowner1 OWNER TO regress_ptnowner; SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; ERROR: permission denied for table ptnowner +ALTER TABLE ptnowner1 REPLICA IDENTITY USING INDEX ptnowner1_i_key; +REPACK (CONCURRENTLY) ptnowner1; RESET SESSION AUTHORIZATION; ALTER TABLE ptnowner OWNER TO regress_ptnowner; CREATE TEMP TABLE ptnowner_oldnodes AS @@ -560,6 +562,11 @@ CREATE TEMP TABLE ptnowner_oldnodes AS SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; WARNING: permission denied to execute CLUSTER on "ptnowner2", skipping it +-- still can't repack without a replica identity +ALTER TABLE ptnowner1 REPLICA IDENTITY DEFAULT; +REPACK (CONCURRENTLY) ptnowner1; +ERROR: cannot process relation "ptnowner1" +HINT: Relation "ptnowner1" has no identity index. RESET SESSION AUTHORIZATION; SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; @@ -570,6 +577,15 @@ SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a ptnowner2 | t (3 rows) +SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a + JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; + relname | ?column? +-----------+---------- + ptnowner | t + ptnowner1 | f + ptnowner2 | t +(3 rows) + DROP TABLE ptnowner; DROP ROLE regress_ptnowner; -- Test CLUSTER with external tuplesorting diff --git a/src/test/regress/sql/cluster.sql b/src/test/regress/sql/cluster.sql index d14063a9683..1f471a8821a 100644 --- a/src/test/regress/sql/cluster.sql +++ b/src/test/regress/sql/cluster.sql @@ -254,14 +254,16 @@ REPACK (CONCURRENTLY) clstrpart; DROP TABLE clstrpart; -- Ownership of partitions is checked -CREATE TABLE ptnowner(i int unique) PARTITION BY LIST (i); +CREATE TABLE ptnowner(i int unique not null) PARTITION BY LIST (i); CREATE INDEX ptnowner_i_idx ON ptnowner(i); CREATE TABLE ptnowner1 PARTITION OF ptnowner FOR VALUES IN (1); -CREATE ROLE regress_ptnowner; +CREATE ROLE regress_ptnowner LOGIN; CREATE TABLE ptnowner2 PARTITION OF ptnowner FOR VALUES IN (2); ALTER TABLE ptnowner1 OWNER TO regress_ptnowner; SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; +ALTER TABLE ptnowner1 REPLICA IDENTITY USING INDEX ptnowner1_i_key; +REPACK (CONCURRENTLY) ptnowner1; RESET SESSION AUTHORIZATION; ALTER TABLE ptnowner OWNER TO regress_ptnowner; CREATE TEMP TABLE ptnowner_oldnodes AS @@ -269,7 +271,12 @@ CREATE TEMP TABLE ptnowner_oldnodes AS JOIN pg_class AS c ON c.oid=tree.relid; SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; +-- still can't repack without a replica identity +ALTER TABLE ptnowner1 REPLICA IDENTITY DEFAULT; +REPACK (CONCURRENTLY) ptnowner1; RESET SESSION AUTHORIZATION; +SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a + JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; DROP TABLE ptnowner; -- 2.47.3 --m6zy3l65ushq557m Content-Type: text/x-diff; charset=utf-8 Content-Disposition: attachment; filename="0002-REPACK-do-not-require-LOGIN-privileges.patch" ^ permalink raw reply [nested|flat] 41+ messages in thread
* [PATCH 1/2] REPACK: do not require the user to have REPLICATION @ 2026-04-20 09:38 Álvaro Herrera <[email protected]> 0 siblings, 0 replies; 41+ messages in thread From: Álvaro Herrera @ 2026-04-20 09:38 UTC (permalink / raw) Although REPACK (CONCURRENTLY) uses replication slots, there is no concern that the slot will leak data of other users, because the MAINTAIN privilege on the table is required anyway; requiring REPLICATION is user-unfriendly without providing any actual protection. A related aspect is that the REPLICATION attribute is not needed to prevent REPACK from stealing slots from logical replication, since commit e76d8c749c31 made REPACK use a separate pool of replication slots. Because there are now successful concurrent repack runs in the regression tests, we're forced to run test_plan_advice under wal_level=replica. Author: Antonin Houska <[email protected]> Reported-by: Justin Pryzby <[email protected]> Reviewed-by: Chao Li <[email protected]> Discussion: https://postgr.es/m/aeJHPNmL4vVy3oPw@pryzbyj2023 --- src/backend/commands/repack_worker.c | 1 - .../test_plan_advice/t/001_replan_regress.pl | 1 + src/test/regress/expected/cluster.out | 20 +++++++++++++++++-- src/test/regress/sql/cluster.sql | 11 ++++++++-- 4 files changed, 28 insertions(+), 5 deletions(-) diff --git a/src/backend/commands/repack_worker.c b/src/backend/commands/repack_worker.c index b17edd771e2..e4a4860805b 100644 --- a/src/backend/commands/repack_worker.c +++ b/src/backend/commands/repack_worker.c @@ -214,7 +214,6 @@ repack_setup_logical_decoding(Oid relid) /* * Make sure we can use logical decoding. */ - CheckSlotPermissions(); CheckLogicalDecodingRequirements(true); /* diff --git a/src/test/modules/test_plan_advice/t/001_replan_regress.pl b/src/test/modules/test_plan_advice/t/001_replan_regress.pl index 38ffa4d11ae..452b179a665 100644 --- a/src/test/modules/test_plan_advice/t/001_replan_regress.pl +++ b/src/test/modules/test_plan_advice/t/001_replan_regress.pl @@ -18,6 +18,7 @@ $node->init(); # Set up our desired configuration. $node->append_conf('postgresql.conf', <<EOM); shared_preload_libraries='test_plan_advice' +wal_level=replica pg_plan_advice.always_explain_supplied_advice=false pg_plan_advice.feedback_warnings=true EOM diff --git a/src/test/regress/expected/cluster.out b/src/test/regress/expected/cluster.out index 6127b215a86..e17bc91fae1 100644 --- a/src/test/regress/expected/cluster.out +++ b/src/test/regress/expected/cluster.out @@ -543,15 +543,17 @@ ERROR: REPACK (CONCURRENTLY) is not supported for partitioned tables HINT: Consider running the command on individual partitions. DROP TABLE clstrpart; -- Ownership of partitions is checked -CREATE TABLE ptnowner(i int unique) PARTITION BY LIST (i); +CREATE TABLE ptnowner(i int unique not null) PARTITION BY LIST (i); CREATE INDEX ptnowner_i_idx ON ptnowner(i); CREATE TABLE ptnowner1 PARTITION OF ptnowner FOR VALUES IN (1); -CREATE ROLE regress_ptnowner; +CREATE ROLE regress_ptnowner LOGIN; CREATE TABLE ptnowner2 PARTITION OF ptnowner FOR VALUES IN (2); ALTER TABLE ptnowner1 OWNER TO regress_ptnowner; SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; ERROR: permission denied for table ptnowner +ALTER TABLE ptnowner1 REPLICA IDENTITY USING INDEX ptnowner1_i_key; +REPACK (CONCURRENTLY) ptnowner1; RESET SESSION AUTHORIZATION; ALTER TABLE ptnowner OWNER TO regress_ptnowner; CREATE TEMP TABLE ptnowner_oldnodes AS @@ -560,6 +562,11 @@ CREATE TEMP TABLE ptnowner_oldnodes AS SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; WARNING: permission denied to execute CLUSTER on "ptnowner2", skipping it +-- still can't repack without a replica identity +ALTER TABLE ptnowner1 REPLICA IDENTITY DEFAULT; +REPACK (CONCURRENTLY) ptnowner1; +ERROR: cannot process relation "ptnowner1" +HINT: Relation "ptnowner1" has no identity index. RESET SESSION AUTHORIZATION; SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; @@ -570,6 +577,15 @@ SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a ptnowner2 | t (3 rows) +SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a + JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; + relname | ?column? +-----------+---------- + ptnowner | t + ptnowner1 | f + ptnowner2 | t +(3 rows) + DROP TABLE ptnowner; DROP ROLE regress_ptnowner; -- Test CLUSTER with external tuplesorting diff --git a/src/test/regress/sql/cluster.sql b/src/test/regress/sql/cluster.sql index d14063a9683..1f471a8821a 100644 --- a/src/test/regress/sql/cluster.sql +++ b/src/test/regress/sql/cluster.sql @@ -254,14 +254,16 @@ REPACK (CONCURRENTLY) clstrpart; DROP TABLE clstrpart; -- Ownership of partitions is checked -CREATE TABLE ptnowner(i int unique) PARTITION BY LIST (i); +CREATE TABLE ptnowner(i int unique not null) PARTITION BY LIST (i); CREATE INDEX ptnowner_i_idx ON ptnowner(i); CREATE TABLE ptnowner1 PARTITION OF ptnowner FOR VALUES IN (1); -CREATE ROLE regress_ptnowner; +CREATE ROLE regress_ptnowner LOGIN; CREATE TABLE ptnowner2 PARTITION OF ptnowner FOR VALUES IN (2); ALTER TABLE ptnowner1 OWNER TO regress_ptnowner; SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; +ALTER TABLE ptnowner1 REPLICA IDENTITY USING INDEX ptnowner1_i_key; +REPACK (CONCURRENTLY) ptnowner1; RESET SESSION AUTHORIZATION; ALTER TABLE ptnowner OWNER TO regress_ptnowner; CREATE TEMP TABLE ptnowner_oldnodes AS @@ -269,7 +271,12 @@ CREATE TEMP TABLE ptnowner_oldnodes AS JOIN pg_class AS c ON c.oid=tree.relid; SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; +-- still can't repack without a replica identity +ALTER TABLE ptnowner1 REPLICA IDENTITY DEFAULT; +REPACK (CONCURRENTLY) ptnowner1; RESET SESSION AUTHORIZATION; +SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a + JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; DROP TABLE ptnowner; -- 2.47.3 --m6zy3l65ushq557m Content-Type: text/x-diff; charset=utf-8 Content-Disposition: attachment; filename="0002-REPACK-do-not-require-LOGIN-privileges.patch" ^ permalink raw reply [nested|flat] 41+ messages in thread
* [PATCH 1/2] REPACK: do not require the user to have REPLICATION @ 2026-04-20 09:38 Álvaro Herrera <[email protected]> 0 siblings, 0 replies; 41+ messages in thread From: Álvaro Herrera @ 2026-04-20 09:38 UTC (permalink / raw) Although REPACK (CONCURRENTLY) uses replication slots, there is no concern that the slot will leak data of other users, because the MAINTAIN privilege on the table is required anyway; requiring REPLICATION is user-unfriendly without providing any actual protection. A related aspect is that the REPLICATION attribute is not needed to prevent REPACK from stealing slots from logical replication, since commit e76d8c749c31 made REPACK use a separate pool of replication slots. Because there are now successful concurrent repack runs in the regression tests, we're forced to run test_plan_advice under wal_level=replica. Author: Antonin Houska <[email protected]> Reported-by: Justin Pryzby <[email protected]> Reviewed-by: Chao Li <[email protected]> Discussion: https://postgr.es/m/aeJHPNmL4vVy3oPw@pryzbyj2023 --- src/backend/commands/repack_worker.c | 1 - .../test_plan_advice/t/001_replan_regress.pl | 1 + src/test/regress/expected/cluster.out | 20 +++++++++++++++++-- src/test/regress/sql/cluster.sql | 11 ++++++++-- 4 files changed, 28 insertions(+), 5 deletions(-) diff --git a/src/backend/commands/repack_worker.c b/src/backend/commands/repack_worker.c index b17edd771e2..e4a4860805b 100644 --- a/src/backend/commands/repack_worker.c +++ b/src/backend/commands/repack_worker.c @@ -214,7 +214,6 @@ repack_setup_logical_decoding(Oid relid) /* * Make sure we can use logical decoding. */ - CheckSlotPermissions(); CheckLogicalDecodingRequirements(true); /* diff --git a/src/test/modules/test_plan_advice/t/001_replan_regress.pl b/src/test/modules/test_plan_advice/t/001_replan_regress.pl index 38ffa4d11ae..452b179a665 100644 --- a/src/test/modules/test_plan_advice/t/001_replan_regress.pl +++ b/src/test/modules/test_plan_advice/t/001_replan_regress.pl @@ -18,6 +18,7 @@ $node->init(); # Set up our desired configuration. $node->append_conf('postgresql.conf', <<EOM); shared_preload_libraries='test_plan_advice' +wal_level=replica pg_plan_advice.always_explain_supplied_advice=false pg_plan_advice.feedback_warnings=true EOM diff --git a/src/test/regress/expected/cluster.out b/src/test/regress/expected/cluster.out index 6127b215a86..e17bc91fae1 100644 --- a/src/test/regress/expected/cluster.out +++ b/src/test/regress/expected/cluster.out @@ -543,15 +543,17 @@ ERROR: REPACK (CONCURRENTLY) is not supported for partitioned tables HINT: Consider running the command on individual partitions. DROP TABLE clstrpart; -- Ownership of partitions is checked -CREATE TABLE ptnowner(i int unique) PARTITION BY LIST (i); +CREATE TABLE ptnowner(i int unique not null) PARTITION BY LIST (i); CREATE INDEX ptnowner_i_idx ON ptnowner(i); CREATE TABLE ptnowner1 PARTITION OF ptnowner FOR VALUES IN (1); -CREATE ROLE regress_ptnowner; +CREATE ROLE regress_ptnowner LOGIN; CREATE TABLE ptnowner2 PARTITION OF ptnowner FOR VALUES IN (2); ALTER TABLE ptnowner1 OWNER TO regress_ptnowner; SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; ERROR: permission denied for table ptnowner +ALTER TABLE ptnowner1 REPLICA IDENTITY USING INDEX ptnowner1_i_key; +REPACK (CONCURRENTLY) ptnowner1; RESET SESSION AUTHORIZATION; ALTER TABLE ptnowner OWNER TO regress_ptnowner; CREATE TEMP TABLE ptnowner_oldnodes AS @@ -560,6 +562,11 @@ CREATE TEMP TABLE ptnowner_oldnodes AS SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; WARNING: permission denied to execute CLUSTER on "ptnowner2", skipping it +-- still can't repack without a replica identity +ALTER TABLE ptnowner1 REPLICA IDENTITY DEFAULT; +REPACK (CONCURRENTLY) ptnowner1; +ERROR: cannot process relation "ptnowner1" +HINT: Relation "ptnowner1" has no identity index. RESET SESSION AUTHORIZATION; SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; @@ -570,6 +577,15 @@ SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a ptnowner2 | t (3 rows) +SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a + JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; + relname | ?column? +-----------+---------- + ptnowner | t + ptnowner1 | f + ptnowner2 | t +(3 rows) + DROP TABLE ptnowner; DROP ROLE regress_ptnowner; -- Test CLUSTER with external tuplesorting diff --git a/src/test/regress/sql/cluster.sql b/src/test/regress/sql/cluster.sql index d14063a9683..1f471a8821a 100644 --- a/src/test/regress/sql/cluster.sql +++ b/src/test/regress/sql/cluster.sql @@ -254,14 +254,16 @@ REPACK (CONCURRENTLY) clstrpart; DROP TABLE clstrpart; -- Ownership of partitions is checked -CREATE TABLE ptnowner(i int unique) PARTITION BY LIST (i); +CREATE TABLE ptnowner(i int unique not null) PARTITION BY LIST (i); CREATE INDEX ptnowner_i_idx ON ptnowner(i); CREATE TABLE ptnowner1 PARTITION OF ptnowner FOR VALUES IN (1); -CREATE ROLE regress_ptnowner; +CREATE ROLE regress_ptnowner LOGIN; CREATE TABLE ptnowner2 PARTITION OF ptnowner FOR VALUES IN (2); ALTER TABLE ptnowner1 OWNER TO regress_ptnowner; SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; +ALTER TABLE ptnowner1 REPLICA IDENTITY USING INDEX ptnowner1_i_key; +REPACK (CONCURRENTLY) ptnowner1; RESET SESSION AUTHORIZATION; ALTER TABLE ptnowner OWNER TO regress_ptnowner; CREATE TEMP TABLE ptnowner_oldnodes AS @@ -269,7 +271,12 @@ CREATE TEMP TABLE ptnowner_oldnodes AS JOIN pg_class AS c ON c.oid=tree.relid; SET SESSION AUTHORIZATION regress_ptnowner; CLUSTER ptnowner USING ptnowner_i_idx; +-- still can't repack without a replica identity +ALTER TABLE ptnowner1 REPLICA IDENTITY DEFAULT; +REPACK (CONCURRENTLY) ptnowner1; RESET SESSION AUTHORIZATION; +SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a + JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a JOIN ptnowner_oldnodes b USING (oid) ORDER BY a.relname COLLATE "C"; DROP TABLE ptnowner; -- 2.47.3 --m6zy3l65ushq557m Content-Type: text/x-diff; charset=utf-8 Content-Disposition: attachment; filename="0002-REPACK-do-not-require-LOGIN-privileges.patch" ^ permalink raw reply [nested|flat] 41+ messages in thread
end of thread, other threads:[~2026-04-20 09:38 UTC | newest] Thread overview: 41+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2023-08-09 07:56 [PATCH v4 3/7] Row pattern recognition patch (planner). Tatsuo Ishii <[email protected]> 2026-04-20 09:38 [PATCH 1/2] REPACK: do not require the user to have REPLICATION Álvaro Herrera <[email protected]> 2026-04-20 09:38 [PATCH 1/2] REPACK: do not require the user to have REPLICATION Álvaro Herrera <[email protected]> 2026-04-20 09:38 [PATCH 1/2] REPACK: do not require the user to have REPLICATION Álvaro Herrera <[email protected]> 2026-04-20 09:38 [PATCH 1/2] REPACK: do not require the user to have REPLICATION Álvaro Herrera <[email protected]> 2026-04-20 09:38 [PATCH 1/2] REPACK: do not require the user to have REPLICATION Álvaro Herrera <[email protected]> 2026-04-20 09:38 [PATCH 1/2] REPACK: do not require the user to have REPLICATION Álvaro Herrera <[email protected]> 2026-04-20 09:38 [PATCH 1/2] REPACK: do not require the user to have REPLICATION Álvaro Herrera <[email protected]> 2026-04-20 09:38 [PATCH 1/2] REPACK: do not require the user to have REPLICATION Álvaro Herrera <[email protected]> 2026-04-20 09:38 [PATCH 1/2] REPACK: do not require the user to have REPLICATION Álvaro Herrera <[email protected]> 2026-04-20 09:38 [PATCH 1/2] REPACK: do not require the user to have REPLICATION Álvaro Herrera <[email protected]> 2026-04-20 09:38 [PATCH 1/2] REPACK: do not require the user to have REPLICATION Álvaro Herrera <[email protected]> 2026-04-20 09:38 [PATCH 1/2] REPACK: do not require the user to have REPLICATION Álvaro Herrera <[email protected]> 2026-04-20 09:38 [PATCH 1/2] REPACK: do not require the user to have REPLICATION Álvaro Herrera <[email protected]> 2026-04-20 09:38 [PATCH 1/2] REPACK: do not require the user to have REPLICATION Álvaro Herrera <[email protected]> 2026-04-20 09:38 [PATCH 1/2] REPACK: do not require the user to have REPLICATION Álvaro Herrera <[email protected]> 2026-04-20 09:38 [PATCH 1/2] REPACK: do not require the user to have REPLICATION Álvaro Herrera <[email protected]> 2026-04-20 09:38 [PATCH 1/2] REPACK: do not require the user to have REPLICATION Álvaro Herrera <[email protected]> 2026-04-20 09:38 [PATCH 1/2] REPACK: do not require the user to have REPLICATION Álvaro Herrera <[email protected]> 2026-04-20 09:38 [PATCH 1/2] REPACK: do not require the user to have REPLICATION Álvaro Herrera <[email protected]> 2026-04-20 09:38 [PATCH 1/2] REPACK: do not require the user to have REPLICATION Álvaro Herrera <[email protected]> 2026-04-20 09:38 [PATCH 1/2] REPACK: do not require the user to have REPLICATION Álvaro Herrera <[email protected]> 2026-04-20 09:38 [PATCH 1/2] REPACK: do not require the user to have REPLICATION Álvaro Herrera <[email protected]> 2026-04-20 09:38 [PATCH 1/2] REPACK: do not require the user to have REPLICATION Álvaro Herrera <[email protected]> 2026-04-20 09:38 [PATCH 1/2] REPACK: do not require the user to have REPLICATION Álvaro Herrera <[email protected]> 2026-04-20 09:38 [PATCH 1/2] REPACK: do not require the user to have REPLICATION Álvaro Herrera <[email protected]> 2026-04-20 09:38 [PATCH 1/2] REPACK: do not require the user to have REPLICATION Álvaro Herrera <[email protected]> 2026-04-20 09:38 [PATCH 1/2] REPACK: do not require the user to have REPLICATION Álvaro Herrera <[email protected]> 2026-04-20 09:38 [PATCH 1/2] REPACK: do not require the user to have REPLICATION Álvaro Herrera <[email protected]> 2026-04-20 09:38 [PATCH 1/2] REPACK: do not require the user to have REPLICATION Álvaro Herrera <[email protected]> 2026-04-20 09:38 [PATCH 1/2] REPACK: do not require the user to have REPLICATION Álvaro Herrera <[email protected]> 2026-04-20 09:38 [PATCH 1/2] REPACK: do not require the user to have REPLICATION Álvaro Herrera <[email protected]> 2026-04-20 09:38 [PATCH 1/2] REPACK: do not require the user to have REPLICATION Álvaro Herrera <[email protected]> 2026-04-20 09:38 [PATCH 1/2] REPACK: do not require the user to have REPLICATION Álvaro Herrera <[email protected]> 2026-04-20 09:38 [PATCH 1/2] REPACK: do not require the user to have REPLICATION Álvaro Herrera <[email protected]> 2026-04-20 09:38 [PATCH 1/2] REPACK: do not require the user to have REPLICATION Álvaro Herrera <[email protected]> 2026-04-20 09:38 [PATCH 1/2] REPACK: do not require the user to have REPLICATION Álvaro Herrera <[email protected]> 2026-04-20 09:38 [PATCH 1/2] REPACK: do not require the user to have REPLICATION Álvaro Herrera <[email protected]> 2026-04-20 09:38 [PATCH 1/2] REPACK: do not require the user to have REPLICATION Álvaro Herrera <[email protected]> 2026-04-20 09:38 [PATCH 1/2] REPACK: do not require the user to have REPLICATION Álvaro Herrera <[email protected]> 2026-04-20 09:38 [PATCH 1/2] REPACK: do not require the user to have REPLICATION Álvaro Herrera <[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