Da 1945 handle within any exists in parser - #202
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the SQL++ parser dependency to a newer lark-sqlpp version and adds unit-level regression tests to ensure collection-expression forms (e.g., ANY ... WITHIN ..., EXISTS path) parse successfully and remain classified as read-only so the read-only guard does not incorrectly block legitimate SELECT statements.
Changes:
- Bump
lark-sqlppdependency to>=0.2.1. - Add unit regression tests covering parsing + read-only classification for collection-expression query forms.
- Add a unit test asserting these queries are not blocked by the read-only guard and still reach the mocked cluster query call.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| tests/unit/test_query_tools_unit.py | Adds regression coverage for collection-expression parsing/classification and read-only guard passthrough. |
| pyproject.toml | Raises the minimum lark-sqlpp version to pick up grammar fixes required by the new tests. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # Collection-expression query forms — ANY / SOME / EVERY / WITHIN / EXISTS used | ||
| # as bare paths or expressions (not subqueries). lark-sqlpp < 0.2 could not | ||
| # parse these: `parse_sqlpp` raised UnexpectedCharacters / UnexpectedEOF, so the | ||
| # read-only write guard rejected legitimate read-only SELECTs. lark-sqlpp 0.2 | ||
| # fixes the grammar (see DA-1945 and |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 3 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (2)
tests/unit/test_query_tools_unit.py:224
- The comment says the regression suite covers EVERY, but no EVERY query form is included in COLLECTION_EXPR_QUERIES. This makes the test documentation inaccurate (and could mislead future changes). Either add an EVERY case or remove EVERY from the description.
# Collection-expression query forms — ANY / SOME / EVERY / WITHIN / EXISTS used
tests/unit/test_query_tools_unit.py:259
- This docstring mentions EVERY, but the parametrized query list below does not include an EVERY example. Update the docstring to match the actual coverage (or add an EVERY case).
"""lark-sqlpp 0.2 must parse ANY/SOME/EVERY/WITHIN/EXISTS collection
SanjanaBoggaramJ
left a comment
SanjanaBoggaramJ
left a comment
There was a problem hiding this comment.
changes approved
No description provided.