-
Notifications
You must be signed in to change notification settings - Fork 83
feat(webui): Modify ANTLR grammar to support parsing individual SQL fields. #1521
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughFive new parser rules were added to the SQL grammar file to support standalone parsing of SQL fragments, including select item lists, sort item lists, and boolean expressions with explicit end-of-file markers. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 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 |
hoophalab
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
junhaoliao
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
deferring to @hoophalab 's review
|
File was missing a new line, please re approve |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
components/webui/client/src/sql-parser/Sql.g4(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: package-image
- GitHub Check: lint-check (macos-15)
- GitHub Check: lint-check (ubuntu-24.04)
🔇 Additional comments (2)
components/webui/client/src/sql-parser/Sql.g4 (2)
13-15: LGTM: Appropriate for parsing WHERE clause fragments.The rule correctly uses EOF to parse boolean expressions in isolation, suitable for WHERE clause syntax highlighting. The structure is consistent with the other standalone rules.
1-3: All referenced parser rules are correctly defined in SqlBase.Verification confirmed that
selectItem,booleanExpression, andsortItemare all properly defined in the imported SqlBase.g4 grammar. The new parser rules in Sql.g4 correctly reference these existing rules through the import statement.
| selectItemList | ||
| : selectItem (',' selectItem)* | ||
| ; | ||
|
|
||
| standaloneSelectItemList | ||
| : selectItemList EOF | ||
| ; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick | 🔵 Trivial
Consider adding documentation comments.
The grammar rules are well-structured for parsing SELECT clause fragments. However, adding comments would improve maintainability and clarify the intended usage for future syntax highlighting.
Consider adding documentation:
+// Parse a comma-separated list of SELECT items (e.g., "col1, col2, func(col3)")
selectItemList
: selectItem (',' selectItem)*
;
+// Standalone entry point for parsing SELECT items in isolation
standaloneSelectItemList
: selectItemList EOF
;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| selectItemList | |
| : selectItem (',' selectItem)* | |
| ; | |
| standaloneSelectItemList | |
| : selectItemList EOF | |
| ; | |
| // Parse a comma-separated list of SELECT items (e.g., "col1, col2, func(col3)") | |
| selectItemList | |
| : selectItem (',' selectItem)* | |
| ; | |
| // Standalone entry point for parsing SELECT items in isolation | |
| standaloneSelectItemList | |
| : selectItemList EOF | |
| ; |
🤖 Prompt for AI Agents
In components/webui/client/src/sql-parser/Sql.g4 around lines 5 to 11, the
grammar rules selectItemList and standaloneSelectItemList lack documentation;
add brief ANTLR comments immediately above each rule (using // or /* */)
describing the rule's purpose, intended input and output shape, examples of
valid snippets (e.g., "a, b AS alias"), and any edge-cases (e.g., trailing
commas disallowed), so future maintainers can quickly understand usage and
constraints.
| sortItemList | ||
| : sortItem (',' sortItem)* | ||
| ; | ||
|
|
||
| standaloneSortItemList | ||
| : sortItemList EOF | ||
| ; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick | 🔵 Trivial
LGTM: Consistent pattern for ORDER BY clause parsing.
The rules follow the same pattern as the SELECT item rules, providing a clean, consistent approach for parsing ORDER BY clause fragments with proper EOF handling.
Consider adding documentation similar to the SELECT rules:
+// Parse a comma-separated list of ORDER BY sort items (e.g., "col1 ASC, col2 DESC")
sortItemList
: sortItem (',' sortItem)*
;
+// Standalone entry point for parsing ORDER BY items in isolation
standaloneSortItemList
: sortItemList EOF
;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| sortItemList | |
| : sortItem (',' sortItem)* | |
| ; | |
| standaloneSortItemList | |
| : sortItemList EOF | |
| ; | |
| // Parse a comma-separated list of ORDER BY sort items (e.g., "col1 ASC, col2 DESC") | |
| sortItemList | |
| : sortItem (',' sortItem)* | |
| ; | |
| // Standalone entry point for parsing ORDER BY items in isolation | |
| standaloneSortItemList | |
| : sortItemList EOF | |
| ; |
🤖 Prompt for AI Agents
In components/webui/client/src/sql-parser/Sql.g4 around lines 17 to 23, add a
short grammar comment/documentation for sortItemList and standaloneSortItemList
mirroring the SELECT item rules: describe that sortItemList parses one or more
sortItem separated by commas and is used inside ORDER BY clauses, and that
standaloneSortItemList enforces end-of-input (EOF) for parsing isolated ORDER BY
fragments; include a brief example of usage and note why the EOF rule exists for
standalone parsing to aid future maintainers.
junhaoliao
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
deferring to @hoophalab 's review
Description
Modifies grammar to support parsing SELECT, WHERE and ORDER BY. Will be used for syntax highlighting in future PR.
Checklist
breaking change.
Validation performed
Not yet since unused
Summary by CodeRabbit