]> git.ipfire.org Git - thirdparty/postgresql.git/commitdiff
REPACK: do not require REPLICATION or LOGIN
authorÁlvaro Herrera <alvherre@kurilemu.de>
Mon, 20 Apr 2026 13:44:23 +0000 (15:44 +0200)
committerÁlvaro Herrera <alvherre@kurilemu.de>
Mon, 20 Apr 2026 13:44:23 +0000 (15:44 +0200)
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.

Similarly, there's no reason to require that the table owner has the
LOGIN privilege.  Bypass the default behavior in the background worker
launch sequence.

Because there are now successful concurrent repack runs in the
regression tests, we're forced to run test_plan_advice under
wal_level=replica, so add that.  Also, move the cluster.sql test to a
different parallel group in parallel_schedule: apparently the use of the
repack worker causes it to exceed the maximum limit of processes in some
runs (the actual limit reached is the number of XIDs in a snapshot's xip
array).

Author: Antonin Houska <ah@cybertec.at>
Reported-by: Justin Pryzby <pryzby@telsasoft.com>
Reviewed-by: Chao Li <lic@highgo.com>
Discussion: https://postgr.es/m/aeJHPNmL4vVy3oPw@pryzbyj2023

src/backend/commands/repack_worker.c
src/test/modules/test_plan_advice/t/001_replan_regress.pl
src/test/regress/expected/cluster.out
src/test/regress/parallel_schedule
src/test/regress/sql/cluster.sql

index b17edd771e28e1927b935ac2cc93d0f9d99782fc..c40f8c98e06608ef7f7694080f483db1e7f93077 100644 (file)
@@ -106,8 +106,9 @@ RepackWorkerMain(Datum main_arg)
        pq_set_parallel_leader(shared->backend_pid,
                                                   shared->backend_proc_number);
 
-       /* Connect to the database. */
-       BackgroundWorkerInitializeConnectionByOid(shared->dbid, shared->roleid, 0);
+       /* Connect to the database. LOGIN is not required. */
+       BackgroundWorkerInitializeConnectionByOid(shared->dbid, shared->roleid,
+                                                                                         BGWORKER_BYPASS_ROLELOGINCHECK);
 
        /*
         * Transaction is needed to open relation, and it also provides us with a
@@ -214,7 +215,6 @@ repack_setup_logical_decoding(Oid relid)
        /*
         * Make sure we can use logical decoding.
         */
-       CheckSlotPermissions();
        CheckLogicalDecodingRequirements(true);
 
        /*
index 38ffa4d11aef3842b2459fea117595f889088e53..452b179a665f75c6a11b25ab3183e61d3283d391 100644 (file)
@@ -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
index 6127b215a868eb428068f5773867bd820ca2a555..712701349851c30f6188da367809a8c2a17609ca 100644 (file)
@@ -543,7 +543,7 @@ 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;
@@ -552,6 +552,8 @@ 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
index 288e94dc408e9f02b3b6fb384741a4de3a65d938..5d4f910155efa44350a19a73d884cff2774fb6e3 100644 (file)
@@ -102,7 +102,7 @@ test: publication subscription
 # Another group of parallel tests
 # select_views depends on create_view
 # ----------
-test: select_views portals_p2 foreign_key cluster dependency guc bitmapops combocid tsearch tsdicts foreign_data window xmlmap functional_deps advisory_lock indirect_toast equivclass stats_rewrite graph_table
+test: select_views portals_p2 foreign_key dependency guc bitmapops combocid tsearch tsdicts foreign_data window xmlmap functional_deps advisory_lock indirect_toast equivclass stats_rewrite graph_table
 
 # ----------
 # Another group of parallel tests (JSON related)
@@ -128,7 +128,7 @@ test: partition_merge partition_split partition_join partition_prune reloptions
 # ----------
 # Another group of parallel tests (compression)
 # ----------
-test: compression compression_lz4 compression_pglz
+test: compression compression_lz4 compression_pglz cluster
 
 # event_trigger depends on create_am and cannot run concurrently with
 # any test that runs DDL
index d14063a9683754fee486999fcf37ff435d795d03..6746236ffec3b67a164a1eadd10e71d1e73d879a 100644 (file)
@@ -254,7 +254,7 @@ 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;
@@ -262,6 +262,8 @@ 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;