fix(psql): strip trailing semicolon before appending auto-LIMIT - #58
Conversation
The psql console's auto-LIMIT helper appended ` LIMIT 1000` verbatim, so `SELECT 1;` produced `SELECT 1; LIMIT 1000` — two statements, the second invalid. Strip any trailing `;` and surrounding whitespace first. Adds Vitest coverage so it can't regress. Closes #38
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughExtracts an inline SQL LIMIT helper into ChangesQuery Limit Utility Extraction and Enhancement
🎯 3 (Moderate) | ⏱️ ~20 minutes
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add 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: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/utils/applyQueryLimit.test.ts`:
- Around line 10-28: The test suite is missing a regression test for queries
that end with a SQL line comment after a semicolon (e.g. "SELECT 1; --
comment"), which currently allows the LIMIT to be commented out; add a new spec
in applyQueryLimit.test.ts under the "trailing semicolon stripping" describe
block that calls applyQueryLimit with an input like "SELECT 1; -- comment" (and
one with trailing whitespace after the comment) and expects "SELECT 1 LIMIT
1000" to ensure applyQueryLimit correctly strips the semicolon and comment
before appending the LIMIT.
In `@src/utils/applyQueryLimit.ts`:
- Line 49: The current trimmed = query.trim().replace(/;\s*$/, "") fails when
the query ends with a trailing SQL comment; update this logic to first strip
trailing comments (both single-line -- and block /* */) from the trimmed query
(reuse the same comment-stripping approach used earlier in the function for
query detection), then remove a trailing semicolon from that comment-stripped
string, and finally use that cleaned string for appending the LIMIT (or reattach
preserved comments after adding LIMIT if desired). Target the trimmed variable
and the semicolon-removal step in the applyQueryLimit function so trailing
comments cannot cause the LIMIT to be commented out.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8e0f2399-f989-4e7f-a25c-4b82520d92d3
📒 Files selected for processing (3)
src/components/layout/MainContent.tsxsrc/utils/applyQueryLimit.test.tssrc/utils/applyQueryLimit.ts
Follow-up to the original #38 fix: when a query ends with a SQL comment after the semicolon (e.g. `SELECT 1; -- foo`), the regex `/;\s*$/` doesn't match (the string ends with `"foo"`, not `;`), so LIMIT was appended after the comment. Since `--` extends to end-of-line, the LIMIT clause was silently commented out — a worse outcome than the original syntax error, because the safety LIMIT was completely bypassed without warning. Strip trailing whitespace, semicolons, line comments (`--`), and block comments (`/* */`) iteratively so any combination at the tail is handled. Surgical to the tail only — inline comments mid-query are preserved. Tests added for: trailing line comment, trailing block comment, multi-line block comment, line comment with no trailing semicolon, and a regression guard that an inline mid-query comment is preserved. Flagged by CodeRabbit on PR #58.
Summary
LIMIT 1000verbatim, soSELECT 1;producedSELECT 1; LIMIT 1000— two statements, the second invalid (syntax error at or near "LIMIT").MainContent.tsxinto a puresrc/utils/applyQueryLimit.tsso it can be unit-tested without jsdom, then strip any trailing;+ whitespace before appending.Testing
src/utils/applyQueryLimit.test.tscovering the regression (SELECT 1;,SELECT 1,SELECT 1;,SELECT 1 WHERE x = 1;all →... LIMIT 1000with no trailing;) plus skip-cases (non-SELECT, already-has-LIMIT, CTE, UNION).npx tsc --noEmitclean.Closes #38
Summary by CodeRabbit
New Features
Tests