On Wed, Mar 18, 2026 at 12:59 PM Peter Eisentraut <[email protected]> wrote: > > On 17.03.26 14:57, Peter Eisentraut wrote: > > On 16.03.26 16:54, Ashutosh Bapat wrote: > >> The patch looks fine to me. While reviewing it, I noticed that the > >> function has an extra loop to count the number of variables. I don't > >> think it's needed. The count can be obtained from the list length. In > >> the attached patch, I have removed that loop. Am I missing something? > >> > >> 0001 is your patch > >> 0002 removes the loop + some cosmetic changes > > > > committed > > There are still some pg_upgrade-related failures on the buildfarm, but > AFAICT these are not specifically from this feature but more from the > test design of the PG_TEST_EXTRA="regress_dump_restore" test. It looks > like we need to drop all users at the end of > src/test/regress/sql/graph_table_rls.sql to make this work. >
PFA the patch. I tried using --no-owner with pg_restore and pg_dump but that doesn't work since it doesn't exclude policies. I had to add "reassign owner to" so that we could retain as many objects as possible. Also had to drop the policies which are applicable to users being dropped. Reassign doesn't work on policies, ofc. I remember thinking about using pg_dumpall instead of pg_dump in 002_pg_upgrade, but IIRC, we faced some issues doing so. Didn't try that this time. If that works, we may not need to drop users. But I am not sure if the test coverage we will get for dump/restore is worth it. -- Best Wishes, Ashutosh Bapat
From 628a819f3c44c91b8a2880eb18fd7a3bd3abf806 Mon Sep 17 00:00:00 2001 From: Ashutosh Bapat <[email protected]> Date: Wed, 18 Mar 2026 17:48:42 +0530 Subject: [PATCH v20260318 1/2] Cleanup users and roles in graph_table_rls test This test leaves behind the roles and users it creates. 002_pg_upgrade test dumps and restore the regression when PG_TEST_EXTRA contains regress_dump_restore. The global objects such as users and roles are not dumped by pg_dump. But it still dumps the policies associated with users, and commands to set the ownership. Restoring these policies and the ownerships fails since the users and roles do not exist. To fix this failure we could use --no-owner, but it does not exclude the policy objects associated with users. Hence drop the users, roles and policies that depend upon them at the end of the test. Author: Ashutosh Bapat <[email protected]> Reported by: Peter Eisentraut <[email protected]> --- src/test/regress/expected/graph_table_rls.out | 17 ++++++++++++++++- src/test/regress/sql/graph_table_rls.sql | 17 ++++++++++++++++- 2 files changed, 32 insertions(+), 2 deletions(-) diff --git a/src/test/regress/expected/graph_table_rls.out b/src/test/regress/expected/graph_table_rls.out index 5cbd69dab7b..0e719c7ebd7 100644 --- a/src/test/regress/expected/graph_table_rls.out +++ b/src/test/regress/expected/graph_table_rls.out @@ -771,4 +771,19 @@ ERROR: query would be affected by row-level security policy for table "document HINT: To disable the policy for the table's owner, use ALTER TABLE NO FORCE ROW LEVEL SECURITY. -- Clean up DEALLOCATE graph_rls_query; --- leave objects behind for pg_upgrade/pg_dump tests +-- leave as many objects behind for pg_upgrade/pg_dump tests as possible. The +-- pg_dump test only dumps the regression database, not the global objects like +-- users and roles. Reassign ownership of all objects to superuser and drop +-- users and roles created in this test. Policies can not be reassigned, so drop +-- them explicitly. +RESET SESSION AUTHORIZATION; +REASSIGN OWNED BY regress_graph_rls_alice TO current_user; +DROP USER regress_graph_rls_alice; +DROP USER regress_graph_rls_bob; +DROP USER regress_graph_rls_carol; +DROP USER regress_graph_rls_dave; +DROP USER regress_graph_rls_exempt_user; +DROP POLICY p3 ON document_people; +DROP POLICY p4 ON document_people; +DROP ROLE regress_graph_rls_group1; +DROP ROLE regress_graph_rls_group2; diff --git a/src/test/regress/sql/graph_table_rls.sql b/src/test/regress/sql/graph_table_rls.sql index 044bc27ce9f..5837eac402e 100644 --- a/src/test/regress/sql/graph_table_rls.sql +++ b/src/test/regress/sql/graph_table_rls.sql @@ -360,4 +360,19 @@ EXECUTE graph_rls_query; -- error -- Clean up DEALLOCATE graph_rls_query; --- leave objects behind for pg_upgrade/pg_dump tests +-- leave as many objects behind for pg_upgrade/pg_dump tests as possible. The +-- pg_dump test only dumps the regression database, not the global objects like +-- users and roles. Reassign ownership of all objects to superuser and drop +-- users and roles created in this test. Policies can not be reassigned, so drop +-- them explicitly. +RESET SESSION AUTHORIZATION; +REASSIGN OWNED BY regress_graph_rls_alice TO current_user; +DROP USER regress_graph_rls_alice; +DROP USER regress_graph_rls_bob; +DROP USER regress_graph_rls_carol; +DROP USER regress_graph_rls_dave; +DROP USER regress_graph_rls_exempt_user; +DROP POLICY p3 ON document_people; +DROP POLICY p4 ON document_people; +DROP ROLE regress_graph_rls_group1; +DROP ROLE regress_graph_rls_group2; base-commit: 29bf4ee7496ca594495deb2599e8a44c6338525e -- 2.34.1
