-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Fix OOM in SQL Parser ErrorHandler due to infinite loop in ATN traversal #17023
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
- Fix infinite recursion in by tracking visited states during ATN traversal. - Add to verify the fix and prevent regression. - This resolves the OOM issue caused by invalid SQL queries like .
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.
Pull request overview
This PR fixes an Out-of-Memory (OOM) issue in the SQL parser's error handling caused by infinite recursion when traversing ANTLR's ATN (Augmented Transition Network) for error reporting. The bug occurred when parsing certain malformed SQL queries with syntax errors in LIMIT/OFFSET clauses.
Changes:
- Added visited state tracking in
ErrorHandler.isReachable()to prevent infinite loops in epsilon transition cycles - Added unit test to verify the parser correctly throws
ParsingExceptioninstead of hanging - Updated
.gitignoreto exclude ANTLR-generated files
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/relational/sql/parser/ErrorHandler.java | Fixed infinite loop in isReachable BFS traversal by adding visited set to track processed states; reformatted long comments |
| iotdb-core/datanode/src/test/java/org/apache/iotdb/db/queryengine/plan/relational/sql/ast/SqlParserErrorHandlerTest.java | Added unit test to verify parser throws exception instead of hanging on malformed LIMIT/OFFSET syntax |
| .gitignore | Added entry to ignore ANTLR-generated files in relational-grammar module |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
.../java/org/apache/iotdb/db/queryengine/plan/relational/sql/ast/SqlParserErrorHandlerTest.java
Show resolved
Hide resolved
.../java/org/apache/iotdb/db/queryengine/plan/relational/sql/ast/SqlParserErrorHandlerTest.java
Outdated
Show resolved
Hide resolved
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #17023 +/- ##
============================================
+ Coverage 39.22% 39.35% +0.12%
Complexity 212 212
============================================
Files 5075 5075
Lines 340393 340397 +4
Branches 43443 43444 +1
============================================
+ Hits 133536 133974 +438
+ Misses 206857 206423 -434 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
…ine/plan/relational/sql/ast/SqlParserErrorHandlerTest.java Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…ine/plan/relational/sql/ast/SqlParserErrorHandlerTest.java Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|



Description
Fixes an OOM issue caused by infinite recursion in
ErrorHandler.isReachablewhen parsing certain invalid SQL queries (e.g.,offset X limitYwith syntax errors). TheisReachablemethod was traversing the ANTLR ATN without tracking visited states, leading to infinite loops in the presence of epsilon transition cycles.Changes
ErrorHandler.java: Added avisitedset to theisReachableBFS traversal to prevent processing the same state multiple times.SqlParserErrorHandlerTest.java: A new unit test that reproduces the issue and verifies that the parser now correctly throws aParsingExceptioninstead of hanging.Verification
SqlParserErrorHandlerTestpasses.select * from t1 offset 40 limit1no longer causes OOM.