Repository navigation
Generic: Parse IN lists that start with a parenthesized subquery - #2586
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2586 +/- ##
==========================================
+ Coverage 81.09% 81.13% +0.03%
==========================================
Files 42 42
Lines 33682 33729 +47
Branches 33682 33729 +47
==========================================
+ Hits 27316 27367 +51
- Misses 2796 2797 +1
+ Partials 3570 3565 -5 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
EthanVieira
left a comment
EthanVieira
left a comment
There was a problem hiding this comment.
Overall lgtm but made a note about improving diagnostics.
| if !self.failed_in_subquery_positions.contains(&start) { | ||
| if let Some(subquery) = self.maybe_parse(|p| { | ||
| let subquery = p.parse_query()?; | ||
| p.expect_token(&Token::RParen)?; |
There was a problem hiding this comment.
It looks like we will discard the error for an incorrect subquery and always report it as a failed list. I think the diagnostics could be improved here in certain scenarios.
For example:
SELECT * FROM t WHERE x IN (SELECT y FROM u would report a missing ) at y when it should report a missing ) at the end of the line.
We could store a missing ) error for the subquery and then if the list interpretation also fails, report the clearer error.
Maybe a heuristic like:
- If parsing the
SELECTquery itself fails, there’s no saved closing-parenthesis error. The list parser’s error is reported. - If the query parses but its required
)is missing, the parser saves that error. - If the list interpretation succeeds, the saved error is ignored.
- If the list also fails, the parser reports the saved
)error. One exception would be that a recursion-limit error takes priority.
|
Thank you @LucaCappelletti94 and @EthanVieira for the review |
4 participants
Since #1793,
x IN ((SELECT a FROM u), 1)fails withExpected: ), found: ,.parse_intakes any query it can read afterIN (as the whole subquery, so a list whose first item is a parenthesized scalar subquery never reaches the list branch, but they are valid Postgres and SQLite syntax.The speculative subquery now only counts when
)follows it, and otherwise the parser rewinds and reads a list, soIN ((SELECT a FROM u))and theUNIONcase from #1793 still produceInSubquery.Since this type of recursions sometimes scale horribly, I double checked to verify the scalability and seems rather clean, expecially when compared to the current main.
Case accepted by current main
mainCase rejected by current main
main