test: run upstream's unsafe tests in test-core by utkarash2991 · Pull Request #260 · supabase/supautils

utkarash2991 · 2026-10-24T21:07:33Z

Postgres keeps a few regression files out of installcheck because they change cluster-wide state, for example alter user all. Our test cluster is disposable, so we can run them. This stages src/test/modules/unsafe_tests next to the core regress files and adds the four tests to the schedule.

Those tests run as a superuser and the role handlers return for superusers before reading the statement. Under TEST_CORE the hook now reads the same role fields the handlers read, then returns as usual, so the core run exercises them too.

The files come from xpg's regress output as of supabase/xpg#17. xpg is pinned to v2.7.0, which ships those files and moves us to the current Postgres minors.

Stacked on #259.

AndrewJackson2020 · 2026-10-25T00:20:33Z

Question: if the new pg_regress files are generated from the script then do we really need to store the generated scripts in VCS at all? We currently have .SQL.in files that are used to generate .SQL files but we only track the .SQL.in files in VCS.

utkarash2991 · 2026-10-26T10:28:06Z

Question: if the new pg_regress files are generated from the script then do we really need to store the generated scripts in VCS at all? We currently have .SQL.in files that are used to generate .SQL files but we only track the .SQL.in files in VCS.

Good question: I added it bcoz the test matrix is bit different and provide ease to debug test failures . The .out files have to be tracked either way since they are the assertions, and when one of them changes in a review you want the .sql next to it to see which statement and role produced the diff.

If your worry is that tracked files can drift from the generator then we can add a check in the CI. WDYT?

AndrewJackson2020 · 2026-10-26T13:59:02Z

The .out files have to be tracked either way since they are the assertions.

Do we even need assertions here? This whole exercise is basically trying to go through the entire parse tree and finding segfaults right? It would probably be sufficient to just run every sql and wait for crashes.

Also another thing that surprises me the more I think about it: why was this not picked up in the core tests? I would image the alter role all set ... statement would be executed at some point during the suite, was that not the case?

steve-chavez · 2026-10-26T17:20:07Z

Each form runs as every role the policy treats differently, then again with every supautils setting reset to its default so it has to fall through to core.

@utkarash2991 Could you elaborate on the motivation here? Is it about running every feature (with its config turned on) against pg core?

We already run core regress tests (ref), so I'm wondering if there's some overlap here.

steve-chavez · 2026-10-26T17:30:09Z

Is it about running every feature (with its config turned on) against pg core?

If it's about that, another option would be making the supautils configs PGC_USER only for testing, that would hit all supautils code paths for core regress tests.

utkarash2991 · 2026-10-26T18:08:39Z

Do we even need assertions here? This whole exercise is basically trying to go through the entire parse tree and finding segfaults right? It would probably be sufficient to just run every sql and wait for crashes.

@AndrewJackson2020 I think crashes are only one part. The other part is the hook taking a wrong decision on some odd form, like blocking something it should allow or elevating when it should not. The .out file records what the hook decides for each form + role, so if any branch changes we see it as a diff.
Also pg_regress needs the expected file anyway as there is no crash only mode in it.

Also another thing that surprises me the more I think about it: why was this not picked up in the core tests? I would image the alter role all set ... statement would be executed at some point during the suite, was that not the case?

upstream does have alter role all in src/test/modules/unsafe_tests/sql/rolenames.sql, which is kept out of installcheck by design (see the README there, it has side effects on existing roles). We only run src/test/regress in test-core. Even if we ran it, it would not have caught this, those statements run as superuser and the hook returns early for superuser. The crash needs a non superuser issuing the form, which core never tests bcoz it just gets permission denied.

Could you elaborate on the motivation here? Is it about running every feature (with its config turned on) against pg core?

@steve-chavez Not exactly about features vs core. The gaps I see in core suite are:

it does not have these grammar forms at all,
its role ddl runs under core's own fixture roles so our reserved role / privileged role branches never fire,
nothing in the tests records what the hook should decide for each form + role, so a change in a branch goes unnoticed.

The matrix covers these and the reset pass is to make sure the hook stays out of the way when nothing is configured. Configs are already on during test-core so IMO PGC_USER wont close this gap as the real gap is forms and roles not config.

steve-chavez · 2026-10-26T19:07:08Z

upstream does have alter role all in src/test/modules/unsafe_tests/sql/rolenames.sql, which is kept out of installcheck by design (see the README there, it has side effects on existing roles).

For reference:

This directory doesn't actually contain any extension module.
Instead it is a home for regression tests that we don't want to run
during "make installcheck" because they could have side-effects that
seem undesirable for a production installation.
An example is that rolenames.sql tests ALTER USER ALL and so could
have effects on pre-existing roles.
https://github.com/postgres/postgres/blob/a8eaec1c3baf3c9a274b5068d19c8050abccb2ba/src/test/modules/unsafe_tests/README#L1

So the above should not be unsafe for us since our test infra produces a disposable/temporary pg. We could run those tests.

Even if we ran it, it would not have caught this, those statements run as superuser and the hook returns early for superuser.

We could use some instrumentation and run the hooks (even on superuser) just for testing.

Overall what I'm finding is that the new python script and the generated SQL files are not looking maintainable: it's a lot of LOCs that have to be kept in sync with upstream. I think we should find a way to reuse pg core regress tests which should already contain the relevant SQL combinations.

utkarash2991 · 2026-10-26T20:45:27Z

We could use some instrumentation and run the hooks (even on superuser) just for testing.
Overall what I'm finding is that the new python script and the generated SQL files are not looking maintainable: it's a lot of LOCs that have to be kept in sync with upstream. I think we should find a way to reuse pg core regress tests which should already contain the relevant SQL combinations.

@steve-chavez Fair call. Let me give it a try. Will update here soon.

imor · 2026-10-27T08:19:04Z

I second Steve's opinion about maintainability. It looks like one of those cases where AI output can be really verbose without producing understandable code. When I tried to read the script, it just went over my head.

AndrewJackson2020 · 2026-10-29T19:37:07Z

@utkarash2991 just a heads up, I notice that you are bumping xpg in this PR now. Supautils tests blow up with more recent postgres versions, specifically the hint functionality. See [0] for some discussion and [1] for my attempt to fix hints. I will say that deprecating hints may be on the table, either way some review/attention that will probably help move this PR along.

[0] #263
[1] #267

Postgres keeps src/test/modules/unsafe_tests out of installcheck because they change cluster-wide state. Our cluster is disposable, so stage the four files next to the core regress files when xpg provides them, and patch alter_system_table for the hint supautils adds.

The core tests run as a superuser, and the handlers return for superusers before reading the statement. Under TEST_CORE read the same role fields the handlers read, then return as usual.

xpg now installs src/test/modules/unsafe_tests into its regress output. Pinned to the commit until the next xpg tag.

The tag has the unsafe tests and the binary name fix.

steve-chavez · 2026-11-04T17:04:32Z

@utkarash2991 Just saw the request for review, I think we need to first merge #267, which only needs some cleanup. cc @AndrewJackson2020

AndrewJackson2020 · 2026-11-10T14:31:25Z

@utkarash2991 Just saw the request for review, I think we need to first merge #267, which only needs some cleanup. cc @AndrewJackson2020

I merged #267 @utkarash2991 please rebase this PR, I believe the tests should be fixed after that.

AndrewJackson2020 · 2026-11-10T18:12:50Z

@utkarash2991 I think we should also merge #263 before this one. Bumps xpg from 2.5.0 -> 2.6.0 and so bundles any changes needed to the .out.patch files with that. I think it should be good to merge if you want to review.

AndrewJackson2020 · 2026-11-10T21:32:47Z

Looking at the macOS tests it looks like the asan/ubsan output is leaking to when running on clang. Need to fix this.

AndrewJackson2020 · 2026-11-10T21:52:15Z

The macOS errors should be fixable via this PR to xpg. Apologies for the inconvenience.

supabase/xpg#26

AndrewJackson2020 · 2026-11-11T15:02:57Z

    nixpkgs.url = "github:NixOS/nixpkgs/91c9a64ce2a84e648d0cf9671274bb9c2fb9ba60";
    xpg = {
-      url = "github:supabase/xpg/v2.5.0";
+      url = "github:supabase/xpg/v2.7.0";


Please bump this to v2.7.1 to get macOS CI working again. See below.

https://github.com/supabase/vault/pull/85/changes

utkarash2991 requested a review from a team as a code owner October 24, 2026 21:07

utkarash2991 added this pull request to stack #261 October 24, 2026 21:08

utkarash2991 requested review from AndrewJackson2020 and imor October 24, 2026 21:12

utkarash2991 mentioned this pull request Oct 28, 2026

fix: guard role lookups against statements that name no role #259

Merged

utkarash2991 mentioned this pull request Oct 28, 2026

feat: ship unsafe_tests in the regress output supabase/xpg#17

Merged

utkarash2991 force-pushed the utkarashsingh/psqlext-1816-hook-forms branch from 15e8dc5 to 85eeb0c Compare October 28, 2026 19:54

utkarash2991 changed the title ~~test: run every form of every hooked statement as every role~~ test: run upstream's unsafe tests in test-core Oct 28, 2026

This was referenced Oct 29, 2026

fix: keep the plain xpg name for multi-version builds supabase/xpg#18

Merged

test: role spec forms and unset settings #269

Open

utkarash2991 force-pushed the utkarashsingh/psqlext-1816-hook-forms branch from f4fd383 to 13ca7c5 Compare October 29, 2026 12:41

utkarash2991 force-pushed the utkarashsingh/psqlext-1816-hook-forms branch from 13ca7c5 to 82b4758 Compare October 29, 2026 22:51

Base automatically changed from utkarashsingh/psqlext-1816-hook-handlers to master October 29, 2026 23:01

utkarash2991 force-pushed the utkarashsingh/psqlext-1816-hook-forms branch from 82b4758 to 588f199 Compare October 29, 2026 23:01

utkarash2991 added 3 commits November 1, 2026 13:13

test: read role specs for superusers in the core run

6ad9ffc

The core tests run as a superuser, and the handlers return for superusers before reading the statement. Under TEST_CORE read the same role fields the handlers read, then return as usual.

chore: bump xpg to ship the unsafe tests

1a6d090

xpg now installs src/test/modules/unsafe_tests into its regress output. Pinned to the commit until the next xpg tag.

utkarash2991 force-pushed the utkarashsingh/psqlext-1816-hook-forms branch from 588f199 to 1a6d090 Compare November 1, 2026 12:13

AndrewJackson2020 reviewed Nov 3, 2026

View reviewed changes

Comment thread flake.nix Resolved

chore: pin xpg v2.7.0

c62b1f1

The tag has the unsafe tests and the binary name fix.

utkarash2991 requested a review from steve-chavez November 4, 2026 15:42

utkarash2991 mentioned this pull request Nov 10, 2026

fix: add hints to planner hook #267

Merged

AndrewJackson2020 reviewed Nov 11, 2026

View reviewed changes

Conversation

utkarash2991 commented Oct 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AndrewJackson2020 commented Oct 25, 2026

Uh oh!

utkarash2991 commented Oct 26, 2026

Uh oh!

AndrewJackson2020 commented Oct 26, 2026

Uh oh!

steve-chavez commented Oct 26, 2026

Uh oh!

steve-chavez commented Oct 26, 2026

Uh oh!

utkarash2991 commented Oct 26, 2026

Uh oh!

steve-chavez commented Oct 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

utkarash2991 commented Oct 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

imor commented Oct 27, 2026

Uh oh!

AndrewJackson2020 commented Oct 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

steve-chavez commented Nov 4, 2026

Uh oh!

AndrewJackson2020 commented Nov 10, 2026

Uh oh!

AndrewJackson2020 commented Nov 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AndrewJackson2020 commented Nov 10, 2026

Uh oh!

AndrewJackson2020 commented Nov 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AndrewJackson2020 Nov 11, 2026

Choose a reason for hiding this comment

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

utkarash2991 commented Oct 24, 2026 •

edited

Loading

steve-chavez commented Oct 26, 2026 •

edited

Loading

utkarash2991 commented Oct 26, 2026 •

edited

Loading

AndrewJackson2020 commented Oct 29, 2026 •

edited

Loading

AndrewJackson2020 commented Nov 10, 2026 •

edited

Loading

AndrewJackson2020 commented Nov 10, 2026 •

edited

Loading