On Thu Aug 27, 2026 at 3:33 PM -03, Bharath Rupireddy wrote: > On Thu, Aug 27, 2026 at 11:16 AM Matheus Alcantara >> I'm attaching a patch to use BGWORKER_BYPASS_ALLOWCONN on >> BackgroundWorkerInitializeConnectionByOid as parallel.c. > > +1. If a user is able to connect and run REPACK, there's no reason to > block REPACK (CONCURRENTLY). > > One general concern I have: is it okay for a role that has neither the > login privilege nor CONNECT on the database to perform logical > decoding for concurrent repack? It may well be okay, because the > repack worker already bypasses the login check for the role it uses to > connect to the database. >
I think so. IIUC the privilege decision is not made in the worker. The leader checks ACL_MAINTAIN against the invoking user in repack_is_permitted_for_relation(), and only then starts the worker. The role the worker connects as is not an authorization decision, it's just the identity the work runs under. >> I also added a test case for it. > > I quickly checked and there's no test case for bypassing the login > check. Can we add one while here? > Added this new test case on attached. -- Matheus Alcantara EDB: https://www.enterprisedb.com
From 4d1cd5c49f0c4cfce919995413bea056f6e50117 Mon Sep 17 00:00:00 2001 From: Matheus Alcantara <[email protected]> Date: Thu, 27 Aug 2026 15:07:39 -0300 Subject: [PATCH v2] Let the REPACK decoding worker bypass connection privilege checks REPACK (CONCURRENTLY) launches a background worker to decode the changes made while the table is being rewritten. Since cluster_rel() switches the current user to the owner of the table being repacked, that is the role the worker connects as, and the worker asked for a bypass of the LOGIN check only. The CONNECT privilege on the database was therefore still checked against the table owner, who has no reason to have it, so the command could fail with ERROR: permission denied for database "d" DETAIL: User does not have CONNECT privilege. CONTEXT: REPACK decoding worker Pass BGWORKER_BYPASS_ALLOWCONN as well, as parallel.c already does for parallel workers: the backend that launched the worker is connected to the database already, and the worker only decodes changes on its behalf. Reported-by: Nathan Bossart <[email protected]> Author: Matheus Alcantara <[email protected]> Discussion: https://www.postgresql.org/message-id/apBbzFd_EYAfHV45@nathan --- contrib/test_decoding/expected/repack.out | 19 +++++++++++++++++++ contrib/test_decoding/sql/repack.sql | 21 +++++++++++++++++++++ src/backend/commands/repack_worker.c | 9 ++++++++- 3 files changed, 48 insertions(+), 1 deletion(-) diff --git a/contrib/test_decoding/expected/repack.out b/contrib/test_decoding/expected/repack.out index 5ddc63238c5..a65ef660b06 100644 --- a/contrib/test_decoding/expected/repack.out +++ b/contrib/test_decoding/expected/repack.out @@ -29,6 +29,25 @@ SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a DROP TABLE ptnowner; DROP ROLE regress_ptnowner; +-- The decoding worker runs as the owner of the table being repacked, but it +-- should not need the LOGIN attribute. +CREATE ROLE regress_repack_nologin NOLOGIN; +CREATE TABLE repack_nologin (i int PRIMARY KEY); +ALTER TABLE repack_nologin OWNER TO regress_repack_nologin; +REPACK (CONCURRENTLY) repack_nologin; +DROP TABLE repack_nologin; +DROP ROLE regress_repack_nologin; +-- The decoding worker runs as the owner of the table being repacked, but it +-- should not need the CONNECT privilege on the database. +CREATE ROLE regress_repack_noconn NOLOGIN; +CREATE TABLE repack_noconn (i int PRIMARY KEY); +ALTER TABLE repack_noconn OWNER TO regress_repack_noconn; +SELECT current_database() AS datname \gset +REVOKE CONNECT ON DATABASE :"datname" FROM PUBLIC; +REPACK (CONCURRENTLY) repack_noconn; +GRANT CONNECT ON DATABASE :"datname" TO PUBLIC; +DROP TABLE repack_noconn; +DROP ROLE regress_repack_noconn; -- Verify that REPACK (CONCURRENTLY) doesn't lose "attmissingval" columns CREATE TABLE rpk_missing (id int PRIMARY KEY); INSERT INTO rpk_missing SELECT generate_series(1, 3); diff --git a/contrib/test_decoding/sql/repack.sql b/contrib/test_decoding/sql/repack.sql index f461f5479f4..4068b707f30 100644 --- a/contrib/test_decoding/sql/repack.sql +++ b/contrib/test_decoding/sql/repack.sql @@ -24,6 +24,27 @@ SELECT a.relname, a.relfilenode=b.relfilenode FROM pg_class a DROP TABLE ptnowner; DROP ROLE regress_ptnowner; +-- The decoding worker runs as the owner of the table being repacked, but it +-- should not need the LOGIN attribute. +CREATE ROLE regress_repack_nologin NOLOGIN; +CREATE TABLE repack_nologin (i int PRIMARY KEY); +ALTER TABLE repack_nologin OWNER TO regress_repack_nologin; +REPACK (CONCURRENTLY) repack_nologin; +DROP TABLE repack_nologin; +DROP ROLE regress_repack_nologin; + +-- The decoding worker runs as the owner of the table being repacked, but it +-- should not need the CONNECT privilege on the database. +CREATE ROLE regress_repack_noconn NOLOGIN; +CREATE TABLE repack_noconn (i int PRIMARY KEY); +ALTER TABLE repack_noconn OWNER TO regress_repack_noconn; +SELECT current_database() AS datname \gset +REVOKE CONNECT ON DATABASE :"datname" FROM PUBLIC; +REPACK (CONCURRENTLY) repack_noconn; +GRANT CONNECT ON DATABASE :"datname" TO PUBLIC; +DROP TABLE repack_noconn; +DROP ROLE regress_repack_noconn; + -- Verify that REPACK (CONCURRENTLY) doesn't lose "attmissingval" columns CREATE TABLE rpk_missing (id int PRIMARY KEY); INSERT INTO rpk_missing SELECT generate_series(1, 3); diff --git a/src/backend/commands/repack_worker.c b/src/backend/commands/repack_worker.c index af7e2a94764..a49b681059f 100644 --- a/src/backend/commands/repack_worker.c +++ b/src/backend/commands/repack_worker.c @@ -101,8 +101,15 @@ RepackWorkerMain(Datum main_arg) pq_set_parallel_leader(shared->backend_pid, shared->backend_proc_number); - /* Connect to the database. LOGIN is not required. */ + /* + * Connect to the database. Like parallel workers do, we skip the + * connection authorization checks. The backend that launched us is + * already connected to this database, and the role we run as is the owner + * of the table being repacked, which needs neither the LOGIN attribute + * nor the CONNECT privilege for our purposes. + */ BackgroundWorkerInitializeConnectionByOid(shared->dbid, shared->roleid, + BGWORKER_BYPASS_ALLOWCONN | BGWORKER_BYPASS_ROLELOGINCHECK); /* -- 2.50.1 (Apple Git-155)
