[improvement](parser) Assign query organization to a single owner - #67429
Open
morrySnow wants to merge 2 commits into
Open
[improvement](parser) Assign query organization to a single owner#67429morrySnow wants to merge 2 commits into
morrySnow wants to merge 2 commits into
Conversation
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
16 tasks
morrySnow
force-pushed
the
codex/antlr4-query-organization-owner
branch
from
September 2, 2026 09:01
773c4ee to
1293889
Compare
morrySnow
changed the title
morrySnow
changed the title
morrySnow
force-pushed
the
codex/antlr4-query-organization-owner
branch
from
September 2, 2026 16:32
1293889 to
0bf9111
Compare
Contributor
Author
|
run buildall |
morrySnow
marked this pull request as ready for review
September 2, 2026 16:36
morrySnow
requested review from
924060929,
englefly and
starocean999
as code owners
September 2, 2026 16:36
Contributor
TPC-H: Total hot run time: 16716 ms |
Contributor
TPC-DS: Total hot run time: 82716 ms |
Contributor
ClickBench: Total hot run time: 14.62 s |
Contributor
FE Regression Coverage ReportIncrement line coverage |
1 similar comment
Contributor
FE Regression Coverage ReportIncrement line coverage |
morrySnow
added a commit
to morrySnow/incubator-doris
that referenced
this pull request
Sep 3, 2026
### What problem does this PR solve? Issue Number: None Related PR: apache#67429 Problem Summary: P2 added parser state and a notifyErrorListeners override only to retain historical first-error positions after queryOrganization gained a single owner. Those remapped positions are no longer required and the compatibility code adds unnecessary state and grammar actions. Remove the override and all supporting state while retaining the ownership predicates. Update the invalid-query tests to assert ANTLR default positions. ### Release note Malformed ORDER BY and LIMIT clauses now use ANTLR default error positions. ### Check List (For Author) - Test: Unit Test - ./run-fe-ut.sh --run org.apache.doris.sqlparser.QueryOrganizationBehaviorTest (48 passed) - mvn checkstyle:check -pl fe-sql-parser (0 violations) - Behavior changed: Yes. Three incomplete ORDER BY or LIMIT forms now report the natural end-of-input position. - Does this need documentation: No
Contributor
Author
|
run buildall |
### What problem does this PR solve? Issue Number: None Related PR: apache#67427 Problem Summary: The grammar allowed both querySpecification and its outer query to consume ORDER BY and LIMIT clauses, which introduced full-context ambiguity. Assign each clause group to exactly one owner according to ANSI mode and query level, preserve baseline first-error positions, and adapt the FE visitor to the new labeled contexts. LL_EXACT fallback and ambiguity for the target decisions drop to zero. Ordered SELECT end-to-end latency improves by 7.2% in legacy mode and 6.2% in ANSI mode; parser-only latency improves by 6.5% and 2.8% respectively. The plain SELECT control has no regression above the 3% threshold. ### Release note None ### Check List (For Author) - Test: Unit Test and manual performance/semantic validation - Maven parser and benchmark package: 238 tests passed - FE QueryOrganizationPlanTest and NereidsParserTest: 90 tests passed - Tracked SQL corpus: 4,610 files with identical parse/error signatures in both modes - Reused the original LL_EXACT profile, invalid-SQL matrix, and JMH comparison; the P2 functional patch has the same stable patch ID - Behavior changed: No - Does this need documentation: No
### What problem does this PR solve? Issue Number: None Related PR: apache#67429 Problem Summary: P2 added parser state and a notifyErrorListeners override only to retain historical first-error positions after queryOrganization gained a single owner. Those remapped positions are no longer required and the compatibility code adds unnecessary state and grammar actions. Remove the override and all supporting state while retaining the ownership predicates. Update the invalid-query tests to assert ANTLR default positions. ### Release note Malformed ORDER BY and LIMIT clauses now use ANTLR default error positions. ### Check List (For Author) - Test: Unit Test - ./run-fe-ut.sh --run org.apache.doris.sqlparser.QueryOrganizationBehaviorTest (48 passed) - mvn checkstyle:check -pl fe-sql-parser (0 violations) - Behavior changed: Yes. Three incomplete ORDER BY or LIMIT forms now report the natural end-of-input position. - Does this need documentation: No
morrySnow
force-pushed
the
codex/antlr4-query-organization-owner
branch
from
September 3, 2026 08:11
b6c7537 to
535e7ae
Compare
Contributor
Author
|
run buildall |
Contributor
FE UT Coverage ReportIncrement line coverage |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What problem does this PR solve?
Issue Number: None
Related PR: #67427
Problem Summary: The grammar allowed both
querySpecificationand its outerqueryto consumeORDER BYandLIMIT, so ANTLR repeatedly enteredqueryOrganizationand reported full-context ambiguities. This change gives every clause group one owner according to ANSI mode and query level, makes the rule structurally non-empty, and preserves the existing accepted/rejected SQL matrix. It intentionally uses ANTLR's default error reporting; three incompleteORDER BY/LIMITforms now report the end-of-input position instead of re-anchoring the error to the clause keyword.P1 (#67427) has been merged into
master. This PR is rebased onto the latestmaster(b4c6d132185) and contains only P2 changes, so it remains independently reviewable and mergeable.Benchmark
The original P2 benchmark is reused without rerunning it. After measurement, the follow-up cleanup only removed error-position compatibility state and grammar actions; it added no parsing decisions and removed work from the valid-SQL path. Lower latency is better. The target input is
SELECT a, b, c FROM t WHERE a > 1 ORDER BY a, b DESC LIMIT 20 OFFSET 10; the control input has the same SELECT without query-organization clauses.5e0eadb13e9; parser jar SHA-256530f0ed45c4bac3a096373a1e932aae0025c603a6f905c3d975a2ad53bd0e7ca; benchmark jar SHA-2567543595b2a87f55b9b53538336cfedf55c13484302ffb487efcfb4fdd66b1c0b773c4ee1027; parser jar SHA-25665730a22a5262370a832bda7a04ae4b8e0de5c107be65c5ae80f95c711842f59; benchmark jar SHA-25647284ffd6779e9d534a19a682de5a4d468f83a62a98e048982807497f1f665b6fe/fe-sql-parser-benchmark/src/main/java/org/apache/doris/sqlparser/benchmark/QueryOrganizationBenchmark.javaOrdered SELECT target
The candidate change is calculated from the mean of C1 and C2 against B1. Values are us/op with JMH's 99.9% error estimate.
Plain SELECT control
No control-path latency regression exceeds the 3% threshold.
Allocation
The same artifacts were measured separately with
-prof gc. Timing under the profiler was noisy, so this table uses only normalized allocation.All allocation changes are below the 3% threshold.
Profile attribution
queryOrganization13,196 times and examined 49,902 lookahead tokens; SSB and Trino profiling reported 40 and 20 related ambiguities.querySpecificationandqueryOrganizationtarget decisions.Semantic differential
*.sqlfiles; manifest SHA-256567e209d57e5eaf6546ff03bf887437b8d647ed5f7ecb85bc657b987dd04be10SELECT 1 ORDER BY: pos 9 -> 17SELECT 1 LIMIT: pos 9 -> 14SELECT 1 LIMIT 1 ORDER BY: pos 17 -> 25Release note
Malformed
ORDER BYandLIMITclauses now use ANTLR's default error positions.Check List (For Author)
Test
QueryOrganizationPlanTestandNereidsParserTest, 90 passedfe-sql-parserandfe-coreCheckstyle, 0 violationsPostProcessorreference currently in master'sLimitClausePrefixTestwas removed locally for test compilation and restored afterward; it is not part of this PRBehavior changed:
Does this need documentation?
Check List (For Reviewer who merge this PR)