Skip to content

perf(rls): wrap RLS policy functions in (select ...) for per-statement evaluation#3

Merged
AmmarSaleh50 merged 2 commits into
OpenStudy-dev:mainfrom
dmitrymaranik:perf/wrap-rls-initplan
Jun 27, 2026
Merged

perf(rls): wrap RLS policy functions in (select ...) for per-statement evaluation#3
AmmarSaleh50 merged 2 commits into
OpenStudy-dev:mainfrom
dmitrymaranik:perf/wrap-rls-initplan

Conversation

@dmitrymaranik

Copy link
Copy Markdown
Contributor

Closes #2.

The per-user / per-tenant RLS policies call their session function (auth.uid() / current_setting(...)) directly in USING/WITH CHECK, which Postgres re-evaluates once per row. This wraps each call in a scalar subquery so the planner evaluates it once per statement (an InitPlan) and caches it — the optimization Supabase documents for RLS.

  • Predicate-equivalent(select f()) returns the same value, so row visibility is unchanged.
  • Verified with the static analyzer that flagged it (pgrls): the PERF001 findings on these policies clear to 0 after the change, with nothing else altered.
  • Added as a new migration (20260517000000_wrap_rls_perf_initplan.sql); the SQL is pgrls fix --rule PERF001 output.

Happy to adjust the migration filename/placement to your conventions.

@AmmarSaleh50

Copy link
Copy Markdown
Collaborator

Thanks for this — nicely researched, and the InitPlan pattern is exactly right. I checked it against the schema and the migration plumbing (placement, filename ordering, and the runner wrapping each file in its own transaction all check out), and it's predicate-equivalent so row visibility is unchanged.

One thing before I merge: every one of these policies is defined with both a USING and a WITH CHECK clause (see migrations/20260516050001_rls_policies.sql), and ALTER POLICY … USING (…) only rewrites USING. So WITH CHECK — which runs on every INSERT/UPDATE row — still calls current_setting() per row, meaning the write path doesn't actually get the optimization. That's probably also worth a second look at the "PERF001 → 0" claim; pgrls may only be scanning USING.

Could you extend each statement to set both clauses? e.g.

ALTER POLICY courses_user_isolation ON public.courses
    USING (user_id = (SELECT current_setting('app.user_id', true))::uuid)
    WITH CHECK (user_id = (SELECT current_setting('app.user_id', true))::uuid);

Filename and placement are fine as-is. Thanks again!

@dmitrymaranik

Copy link
Copy Markdown
Contributor Author

Done -- updated to set both clauses on all 16 policies. You nailed it: ALTER POLICY ... USING (...) only rewrites USING, so the write path (WITH CHECK, per INSERT/UPDATE row) wasn't getting the optimization.

And your hunch about the "PERF001 -> 0" claim was spot-on -- pgrls's PERF001 was only scanning USING. That was a real gap in the analyzer; I fixed it upstream (it now flags and fixes WITH CHECK too, released in 0.44.0), re-ran it here, and it found all 16 policies call current_setting() in WITH CHECK as well. The migration now sets both USING and WITH CHECK on every policy -- verified on Postgres 16 (PERF001 -> 0, predicate-equivalent). Thanks for the careful review; it directly improved the tool.

@AmmarSaleh50

Copy link
Copy Markdown
Collaborator

Perfect — both clauses wrapped on all 16 policies, still predicate-equivalent, and the migration ordering/runner are happy. And thanks for fixing PERF001 upstream to cover WITH CHECK too; glad the review fed back into the tool. Merging now — appreciate the contribution! 🙏

@AmmarSaleh50 AmmarSaleh50 merged commit 0770272 into OpenStudy-dev:main Jun 27, 2026
@dmitrymaranik

Copy link
Copy Markdown
Contributor Author

Thanks for merging, and for the thorough review — the kind that genuinely makes a contribution better. This came out of pgrls, an open-source Postgres RLS linter/verifier, and your WITH CHECK catch fed straight back into it (PERF001 now covers WITH CHECK, not just USING). If it looks useful for keeping OpenStudy's RLS solid, a star would help others find it. And happy to help wire it into CI if you'd ever want regression coverage on the policies.

@AmmarSaleh50

Copy link
Copy Markdown
Collaborator

Thanks again! I'll keep pgrls in mind — RLS here is currently inert (bypass role) so CI coverage isn't pressing yet, but I may take you up on that when we flip it on. Good luck with the project!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

RLS hardening: wrap current_setting() in (select …) for per-statement eval (+ optional FORCE RLS)

2 participants