Skip to content

fix(studio): don't scan dollar-quoted bodies for DDL in SQL editor#45050

Merged
alaister merged 3 commits into
masterfrom
fix/sql-editor-rls-warning-dollar-quoted-false-positive
Apr 20, 2026
Merged

fix(studio): don't scan dollar-quoted bodies for DDL in SQL editor#45050
alaister merged 3 commits into
masterfrom
fix/sql-editor-rls-warning-dollar-quoted-false-positive

Conversation

@alaister

@alaister alaister commented Apr 20, 2026

Copy link
Copy Markdown
Member

Fixes a false positive in the CREATE-TABLE-without-RLS warning modal added in #45008. The warning was firing on CREATE FUNCTION statements because the SELECT..INTO detector was matching plpgsql variable assignments inside $$…$$ function bodies.

Reported example that triggered the modal with no table actually being created:

create or replace function schema_checks()
returns jsonb
language plpgsql
as $$
declare
  ret jsonb;
begin
  select jsonb_build_object('value', 'ok') into ret;
  return ret;
end;
$$;

Changed:

  • SQLEventParser.match() now strips the body of $tag$…$tag$ blocks before running detectors. Tags are kept as markers; content is blanked out so function bodies, DO blocks, and dollar-quoted string literals are never scanned as DDL.
  • Updated a pre-existing parser test that asserted the buggy behaviour (it expected CREATE TABLE fake inside a $$…$$ string literal to be detected — $$…$$ is a string literal in Postgres, not DDL).

Added:

  • Regression tests in SQLEditor.utils.test.ts covering: the exact reported function, DO blocks with select into, create table text inside a function body, mixed top-level CREATE TABLE + function with INTO assignments, and custom $body$…$body$ tags.
  • Parser-level regression test in sql-event-parser.test.ts.

To test

  • In the SQL editor, paste the function from the Slack report and run it — the RLS warning modal should not appear.
  • Run create table foo (id int8 primary key); on its own — modal still appears as before.
  • Run create table foo (id int8); create or replace function bar() returns int language plpgsql as $$ declare v int; begin select 1 into v; return v; end; $$; — modal should flag only foo, not v.
  • Run an existing destructive query (drop table x) — unaffected, modal still works.

Summary by CodeRabbit

  • Bug Fixes

    • Parser no longer treats DDL/DML-like text inside PL/pgSQL functions, DO blocks, or dollar-quoted bodies (including nested/custom tags) as top-level CREATE TABLE/SELECT INTO, preventing false detections and UI warnings.
  • Tests

    • Added unit and e2e regression tests covering dollar-quoted blocks, nested dollar tags, DO blocks, SELECT INTO inside functions, and positive controls with a real top-level CREATE TABLE.

The RLS warning modal was firing on CREATE FUNCTION statements because
the SELECT..INTO detector was matching plpgsql variable assignments
inside $$...$$ function bodies. Strip dollar-quoted content before
running event detectors so function bodies, DO blocks, and string
literals aren't scanned as DDL.
@alaister alaister requested a review from a team as a code owner April 20, 2026 12:18
@vercel

vercel Bot commented Apr 20, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

8 Skipped Deployments
Project Deployment Actions Updated (UTC)
studio Ignored Ignored Apr 20, 2026 0:39am
design-system Skipped Skipped Apr 20, 2026 0:39am
docs Skipped Skipped Apr 20, 2026 0:39am
learn Skipped Skipped Apr 20, 2026 0:39am
studio-self-hosted Skipped Skipped Apr 20, 2026 0:39am
studio-staging Skipped Skipped Apr 20, 2026 0:39am
ui-library Skipped Skipped Apr 20, 2026 0:39am
zone-www-dot-com Skipped Skipped Apr 20, 2026 0:39am

Request Review

@supabase

supabase Bot commented Apr 20, 2026

Copy link
Copy Markdown

This pull request has been ignored for the connected project xguihxuzqibwxjnimxev because there are no changes detected in supabase directory. You can change this behaviour in Project Integrations Settings ↗︎.


Preview Branches by Supabase.
Learn more about Supabase Branching ↗︎.

@coderabbitai

coderabbitai Bot commented Apr 20, 2026

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI (base), Organization UI (inherited)

Review profile: CHILL

Plan: Pro

Run ID: 19512b7c-d386-4ffc-b07a-17f2d227aef4

📥 Commits

Reviewing files that changed from the base of the PR and between 3719e66 and f88dd40.

📒 Files selected for processing (1)
  • e2e/studio/features/sql-editor.spec.ts

📝 Walkthrough

Walkthrough

Adds a helper that strips contents of PostgreSQL dollar-quoted blocks before comment removal and statement splitting, and adds unit and e2e regression tests to ensure DDL-like text inside dollar-quoted/procedural bodies (and SELECT ... INTO) are not reported as top-level table-creation events.

Changes

Cohort / File(s) Summary
Parser Implementation
apps/studio/lib/sql-event-parser.ts
Adds stripDollarQuoteBodies(sql) and applies it in getTableEvents before comment removal and statement splitting so dollar-quoted bodies do not generate false DDL/DML events.
Parser Unit Tests
apps/studio/lib/sql-event-parser.test.ts
Rewrote/renamed dollar-quote tests and added regression cases: ensure $$...$$ bodies, nested/custom dollar-quoted dynamic SQL, and plpgsql SELECT ... INTO are ignored for table events; includes positive control for a real top-level CREATE TABLE.
Editor Utils Tests
apps/studio/components/interfaces/SQLEditor/SQLEditor.utils.test.ts
Added multiple regression tests for getCreateTablesMissingRLS covering SELECT ... INTO assignments, DO $$...$$ blocks, plpgsql function bodies, nested/custom dollar-quote tags, and a positive control with a real top-level CREATE TABLE.
End-to-end Test
e2e/studio/features/sql-editor.spec.ts
New Playwright e2e that verifies no RLS warning is shown for a CREATE FUNCTION with a plpgsql $$...$$ body containing SELECT ... INTO; includes robust setup/teardown and snippet cleanup.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 I nibbled dollar-quote hay all night,
Hid inner crumbs so parsers read right.
No phantom CREATEs in function burrows,
Only true tables the tests now follow.
Hop, patch, and munch — code carrot delight!

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely summarizes the main fix: preventing dollar-quoted function bodies from being scanned for DDL statements in the SQL editor.
Description check ✅ Passed The description is mostly complete with clear problem statement, solution explanation, test cases added, and testing instructions. However, the template requirement checklist (CONTRIBUTING.md acknowledgment and 'what kind of change' categorization) is not filled out.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/sql-editor-rls-warning-dollar-quoted-false-positive

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@apps/studio/lib/sql-event-parser.ts`:
- Line 61: The parser currently calls stripDollarQuoteBodies after splitting
statements, allowing nested dollar-quoted SQL to be split incorrectly; move the
call to stripDollarQuoteBodies so getTableEvents invokes it on the raw SQL
before using splitStatements, and update the dollar-quote regex to use a
backreference pattern like (\$[a-zA-Z0-9_]*\$)[\s\S]*?\1 to ensure
opening/closing tags must match when blanking bodies; also add a regression test
that parses a PL/pgSQL function containing nested dollar-quoted dynamic SQL with
different tags (e.g., $fn$ containing $sql$...$sql$) to ensure create table
inside the inner tag is not reported.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI (base), Organization UI (inherited)

Review profile: CHILL

Plan: Pro

Run ID: 99f0a274-1229-42fe-bc6b-b4e2d205ecc0

📥 Commits

Reviewing files that changed from the base of the PR and between 154f3eb and 440c525.

📒 Files selected for processing (3)
  • apps/studio/components/interfaces/SQLEditor/SQLEditor.utils.test.ts
  • apps/studio/lib/sql-event-parser.test.ts
  • apps/studio/lib/sql-event-parser.ts

Comment thread apps/studio/lib/sql-event-parser.ts Outdated
@github-actions

github-actions Bot commented Apr 20, 2026

Copy link
Copy Markdown
Contributor

🎭 Playwright Test Results

passed  200 passed
flaky  2 flaky
skipped  5 skipped

Details

stats  207 tests across 22 suites
duration  6 minutes, 45 seconds
commit  f88dd40

Flaky tests

Features › sql-editor.spec.ts › SQL Editor › warns on CREATE TABLE without RLS and "Run and enable RLS" enables it
Features › sql-editor.spec.ts › SQL Editor › does not warn on CREATE FUNCTION with plpgsql SELECT..INTO variable assignment

Skipped tests

Features › auth-users.spec.ts › should show web3 users as enabled when the matching web3 provider is enabled
Features › sql-editor.spec.ts › SQL Editor › snippet favourite works as expected
Features › sql-editor.spec.ts › SQL Editor › share with team works as expected
Features › sql-editor.spec.ts › SQL Editor › folders works as expected
Features › sql-editor.spec.ts › SQL Editor › other SQL snippets actions work as expected

Addresses PR review: nested dollar-quoted dynamic SQL with different
tags (e.g. $fn$ containing $sql$...$sql$) was previously leaking inner
semicolons through splitStatements, because the splitter's dollar-quote
pattern doesn't enforce matching tags. Moving stripDollarQuoteBodies to
run before splitting — and relying on the backreference in its regex to
require matching tags — keeps the whole outer body intact.
…sitive

Verifies the SQL editor does not fire the CREATE-TABLE-without-RLS
warning modal when a CREATE FUNCTION statement contains a plpgsql
SELECT..INTO variable assignment inside its $$...$$ body.
// a function body (which is just literal text in Postgres) isn't treated
// as a comment by removeComments, and so inner semicolons inside the body
// can't confuse splitStatements.
const statements = this.splitStatements(this.removeComments(this.stripDollarQuoteBodies(sql)))

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file contains the actual changes; the rest are just tests.

@alaister alaister merged commit 2555e81 into master Apr 20, 2026
31 checks passed
@alaister alaister deleted the fix/sql-editor-rls-warning-dollar-quoted-false-positive branch April 20, 2026 13:19
@github-actions

github-actions Bot commented Apr 20, 2026

Copy link
Copy Markdown
Contributor

Braintrust eval report

Assistant (master-1776691279)

Score Average Improvements Regressions
Completeness 100% (+0pp) - -
Conciseness 29.4% (+0pp) 4 🟢 6 🔴
Correctness 100% (+0pp) - -
Docs Faithfulness 76.9% (+3pp) 3 🟢 2 🔴
Goal Completion 94.1% (+1pp) 2 🟢 1 🔴
Tool Usage 73.8% (-5pp) - 1 🔴
Knowledge Usage 100% (+0pp) - -
SQL Identifier Quoting 100% (+0pp) - -
SQL Validity 100% (+0pp) - -
URL Validity 100% (+50pp) - -
Time_to_first_token 0.01tok (+0tok) 6 🟢 11 🔴
Llm_calls 12.71 (+0.29) 11 🟢 5 🔴
Tool_calls 3.24 (+0) 10 🟢 5 🔴
Errors 0 (+0) - -
Llm_errors 0 (+0) - -
Tool_errors 0 (+0) - -
Prompt_tokens 23507.59tok (-626.9tok) 9 🟢 8 🔴
Prompt_cached_tokens 0tok (+0tok) - -
Prompt_cache_creation_tokens 0tok (+0tok) - -
Completion_tokens 562.78tok (-56.08tok) 8 🟢 9 🔴
Completion_reasoning_tokens 0tok (+0tok) - -
Completion_accepted_prediction_tokens 0tok (+0tok) - -
Completion_rejected_prediction_tokens 0tok (+0tok) - -
Completion_audio_tokens 0tok (+0tok) - -
Total_tokens 24070.37tok (-682.98tok) 9 🟢 8 🔴
Estimated_cost 0.01$ (0$) 9 🟢 6 🔴
Duration 21.83s (-4.01s) 9 🟢 8 🔴
Llm_duration 56.68s (-15.29s) 10 🟢 7 🔴

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.

2 participants