fix(studio): don't scan dollar-quoted bodies for DDL in SQL editor#45050
Conversation
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.
|
The latest updates on your projects. Learn more about Vercel for GitHub. 8 Skipped Deployments
|
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI (base), Organization UI (inherited) Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds 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
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
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
📒 Files selected for processing (3)
apps/studio/components/interfaces/SQLEditor/SQLEditor.utils.test.tsapps/studio/lib/sql-event-parser.test.tsapps/studio/lib/sql-event-parser.ts
🎭 Playwright Test ResultsDetails
Flaky testsFeatures › sql-editor.spec.ts › SQL Editor › warns on CREATE TABLE without RLS and "Run and enable RLS" enables it Skipped testsFeatures › auth-users.spec.ts › should show web3 users as enabled when the matching web3 provider is enabled |
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))) |
There was a problem hiding this comment.
This file contains the actual changes; the rest are just tests.
Braintrust eval report
|
Fixes a false positive in the CREATE-TABLE-without-RLS warning modal added in #45008. The warning was firing on
CREATE FUNCTIONstatements because theSELECT..INTOdetector was matching plpgsql variable assignments inside$$…$$function bodies.Reported example that triggered the modal with no table actually being created:
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.CREATE TABLE fakeinside a$$…$$string literal to be detected —$$…$$is a string literal in Postgres, not DDL).Added:
SQLEditor.utils.test.tscovering: the exact reported function, DO blocks withselect into,create tabletext inside a function body, mixed top-levelCREATE TABLE+ function withINTOassignments, and custom$body$…$body$tags.sql-event-parser.test.ts.To test
create table foo (id int8 primary key);on its own — modal still appears as before.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 onlyfoo, notv.drop table x) — unaffected, modal still works.Summary by CodeRabbit
Bug Fixes
Tests