fix(tests): split prime.sql so non-superuser harnesses can source it#2175
Merged
Conversation
nix/tests/prime.sql is loaded by both superuser-context harnesses (pg_regress in nix/checks.nix, the docker image test, the local migrate-tool) and supadev's test-postgres-engines-with-smoke.sh, which runs as the non-superuser postgres role against a hosted project. Seven extensions are intentionally excluded from supautils' privileged_extensions list (labeled "may be unsafe" in ansible/files/postgresql_config/supautils.conf.j2): amcheck, file_fdw, lo, pageinspect, pg_freespacemap, pg_surgery, pg_visibility Hosted non-superuser sessions cannot create these, so prime.sql halts with "permission denied to create extension amcheck" (alphabetically first) under psql -v ON_ERROR_STOP=1 — which became supadev's default in supabase/supadev@c6be2cb. Split into: - prime.sql: the safe set, sourced by all 4 consumers - prime-superuser.sql (new): the 7 gated extensions, sourced only by the 3 superuser-context harnesses
PostgreSQL Extension Dependency Analysis: PR #2175
SummaryNo extensions had dependencies with MAJOR version updates. Full Analysis ResultsPostgreSQL 15 Extension DependenciesPostgreSQL 17 Extension DependenciesOrioleDB 17 Extension Dependencies |
PostgreSQL Package Dependency Analysis: PR #2175
SummaryNo packages had MAJOR version updates. Full Analysis ResultsPostgreSQL 15 Dependency ChangesExtracting PostgreSQL 15 dependencies...
Runtime Closure Size
Raw Dependency ClosurePostgreSQL 17 Dependency ChangesExtracting PostgreSQL 17 dependencies...
Runtime Closure Size
Raw Dependency Closure |
mmlb
requested changes
May 28, 2026
| -- the docker-image-test, and the local migrate-tool. supadev's hosted | ||
| -- engines-with-smoke test sources `prime.sql` only. | ||
| -- | ||
| -- Keep this list in sync with the "may be unsafe" list in supautils.conf.j2. |
Contributor
There was a problem hiding this comment.
absent an automated way to ensure this, I usually like to mirror this sort of comment in the other file so that hopefully when it gets updated someone sees it and will do the right thing here (🤞)
Contributor
Author
There was a problem hiding this comment.
@mmlb Thanks. Taken care in latest revision.
Contributor
There was a problem hiding this comment.
well thats pretty terrible now that I look at it. Was hoping for an exclude list or something :D. Guess we'll just deal with it for now.
…f.j2 Mirrors the "keep in sync" comment that lives in prime-superuser.sql so that anyone editing the supautils privileged_extensions list gets a pointer back to the test-side counterpart. Addresses review feedback on PR #2175.
Contributor
Author
|
@mmlb thought about this — none of the directions I tried felt clean enough for this PR:
Punting for now. Your "exclude list" instinct lines up most closely with the second option — happy to spin up a follow-up ticket if you'd like to pursue that direction. |
Contributor
This isn't the worse option imo, but not necessary for this PR. I think a small refactor to the .j2 file would make sense too. Create some vars for the various lists then actually use jinja to create the final privileged_extensions. Not quite the next bullet point, simpler version of it. Then the check could be pretty simple too. Again not needed for this PR anyway.
|
mmlb
reviewed
May 28, 2026
| -- Keep this list in sync with the "may be unsafe" list in supautils.conf.j2. | ||
|
|
||
| set client_min_messages = warning; | ||
|
|
Contributor
There was a problem hiding this comment.
looking at j2 file it has:
# omitted because may be unsafe: adminpack, amcheck, file_fdw, lo, old_snapshot, pageinspect, pg_freespacemap, pg_surgery, pg_visibility
You're missing adminpack and old_snapshot here, was that on purpose or didn't notice or something else?
Contributor
Author
There was a problem hiding this comment.
Both adminpack and old_snapshot are not part of PG17/contrib, hence the sql doesn't have them. I have updated the comment in prime-superuser.sql to document the asymmetry rather than introduce a DO ... EXCEPTION workaround. Let me know if you'd prefer the silently-skip version.
Contributor
Author
There was a problem hiding this comment.
I think a small refactor to the .j2 file would make sense too. Create some vars for the various lists then actually use jinja to create the final privileged_extensions. Not quite the next bullet point, simpler version of it. Then the check could be pretty simple too.
Yeah thats cool to do. I have created a Linear task to take that up in next PR: https://linear.app/supabase/issue/PSQL-1265/
…y-be-unsafe list adminpack and old_snapshot are also in supautils.conf.j2's "may be unsafe" list but were removed from contrib in PG 17. They're loaded by nix/tests/sql/z_15_ext_interface.sql for the PG 15 pg_regress path; adding them unconditionally to prime-superuser.sql would break PG 17 with "extension is not available". Header now documents the asymmetry to match the actual file contents.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Split
nix/tests/prime.sqlinto:prime.sql— extensions any privileged-extensions-gated session can create. Sourced by all 4 consumers.prime-superuser.sql(new) — the 7 extensions excluded from supautils'privileged_extensionslist. Sourced only by superuser-context harnesses.Why
prime.sqlis a dual-use file:nix/checks.nix)supabase_adminsupabase_admintest-postgres-engines-with-smoke.shpostgres(hosted)The 7 supautils-gated extensions (
amcheck, file_fdw, lo, pageinspect, pg_freespacemap, pg_surgery, pg_visibility) cannot be created by a non-superuser. Pre-supadev@c6be2cb (2026-05-14, fail-on-error by default) the error was silently swallowed; post that change, psql halts at the first failing extension (amcheck, alphabetically first) and engines-with-smoke fails.First surfaced in the PG 15.18 / 17.10 release smoke run (against supabase/postgres#2155) — the engines-with-smoke test failed on all 3 engines (PG15, PG17, PG17-oriole) with
permission denied to create extension "amcheck":