Re: SQL Property Graph Queries (SQL/PGQ)

On Thu, Mar 19, 2026 at 2:46 AM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>
> Ashutosh Bapat <ashutosh(dot)bapat(dot)oss(at)gmail(dot)com> writes:
> > On Wed, Mar 18, 2026 at 12:59 PM Peter Eisentraut <peter(at)eisentraut(dot)org> wrote:
> >> 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 just came across a side effect of this same thing. After running
> "make installcheck", there are roles left behind:
>
> regression=# \du
> List of roles
> Role name | Attributes
> -------------------------------+------------------------------------------------------------
> postgres | Superuser, Create role, Create DB, Replication, Bypass RLS
> regress_graph_rls_alice | Cannot login
> regress_graph_rls_bob | Cannot login
> regress_graph_rls_carol | Cannot login
> regress_graph_rls_dave | Cannot login
> regress_graph_rls_exempt_user | Cannot login, Bypass RLS
> regress_graph_rls_group1 | Cannot login
> regress_graph_rls_group2 | Cannot login
>
> I don't think this is acceptable at all. The contract for
> installcheck testing, IMO, is that it doesn't leave behind any
> permanent global effects other than the existence of the regression
> database. Roles with elevated privileges seem like a particularly
> dangerous thing to leave behind.
>
> So I think we *must* drop all these roles at the end of the test,
> regardless of the fact that not doing so breaks regress_dump_restore.
> If you want to test dump/restore behavior with some non-default roles,
> you have to do it in a TAP test that is using a purely temporary
> installation. (But I hope we have adequate coverage for that
> already.)
>
> Your patch looks like it does that, but the test script's setup
> could also be simplified, since it no longer has to contend with
> the possibility that those roles are already there.

Thanks for the explanation. The test changes are on the right track then.

rowsecurity.sql has similar DROP statements in the setup and at the
end. The comment there mentions "-- Clean up in case a prior
regression run failed
". I guess the global objects could be left behind in case the "make
installcheck" aborts while after the global objects are created but
before the test finishes. Rerunning the test would fail. But I didn't
go deeper to investigate exactly when we need those DROP statements. I
just copied that pattern in graph_table_rls.sql. Do you think we can
simplify rowsecurity.sql setup the same way?

--
Best Wishes,
Ashutosh Bapat