Repository navigation
Conversation
### What problem does this PR solve? Problem Summary: SELECT * FROM `t`$snapshots treats $snapshots as an implicit alias and silently reads the base table. Reject a dollar-prefixed unquoted suffix immediately adjacent to a quoted table name, with guidance to quote the complete name or separate an intended alias. Preserve valid system-table identifiers and explicit, whitespace-separated, or comment-separated aliases. ### Release note Queries that attach an unquoted dollar suffix directly to a quoted table name now fail explicitly instead of silently scanning the base table. ### Check List (For Author) - Test: All 255 standalone parser unit tests and FE Checkstyle passed; verified the six added regression SQL statements with the parser CLI. Cluster execution of the new regression suite is pending. - Behavior changed: Yes, reject ambiguous adjacent suffixes. - Does this need documentation: No.
Gabriel39
requested review from
924060929,
englefly,
morrySnow and
starocean999
as code owners
October 23, 2026 11:55
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
Contributor
Author
|
run buildall |
Contributor
Author
|
/review |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Reviewed PR #69541 at a3ef8d4012ab64f358413c56fb359c946f744f27. No new actionable findings. All three changed files and their parser-to-FE control flow were reviewed; no existing inline threads or additional user focus points were supplied.
| Critical checkpoint | Conclusion |
|---|---|
| Goal and proof | The grammar rejects an adjacent unquoted dollar suffix after a quoted query table before a base-table plan is built. Added negative and positive parser tests and six diagnostic-specific regression checks address the intended behavior. |
| Scope and reuse | One shared grammar helper/action and focused tests; no unrelated changes or duplicate FE implementation. |
| Concurrency and locking | No shared state, threads, or locks added; tokens belong to each parse invocation. |
| Lifecycle and initialization | No new ownership, resource lifetime, or static initialization dependency. |
| Configuration | No settings added. |
| Compatibility | The intentional SQL rejection is documented in the release note. Full system-table names and explicit, quoted, or separated aliases remain valid. No storage or function-symbol change. |
| Thrift/protobuf | No protocol definitions changed. |
| Parallel paths | FE and standalone parsing share the guard. Nested queries, joins, CTEs, view/create/insert queries, and DML source relations reach it. Separate DML-target, WARM UP, and administrative alias rules retain their pre-existing behavior, outside this focused query-read fix. |
| Special conditions and errors | Original token types and source offsets distinguish adjacency from intentional aliases, including comments and lean tokenization. Empty aliases and parsing without a tree remain safe; FE preserves the parse diagnostic. |
| Test coverage | Qualified and escaped names, non-ASCII names, suffix variants, no-tree parsing, valid identifiers, and whitespace/comment/AS aliases are covered. Regression checks require the new error text, excluding unrelated lookup failures. |
| Expected results | Identifier expectations agree with grammar normalization. Exception-only regression checks need no ordering or .out file; no result files changed. |
| Observability | The exception identifies the suffix location, SQL, and corrective syntax; no added metrics are needed for this local parse rejection. |
| Transactions and persistence | No journal, version-visibility, or failover behavior changed. |
| Data writes and atomicity | No write implementation changed; the new rejection occurs during parsing before execution. |
| FE/BE propagation | No cross-process variables or serialization added. |
| Performance and memory | A small token predicate per table relation adds no repeated SQL scan or retained state on successful parsing. Full diagnostic text is extracted only on failure. |
| Other issues | The additional recheck found no further substantiated defect or unresolved code question. |
Risk checks confirmed that identifier normalization replaces tree children without changing the raw tokens used by the guard. ANTLR records rule start/stop tokens independently of tree construction (ANTLR 4.13.1 parser source). Optional relation modifiers cannot consume an attached dollar alias or create false adjacency.
Coverage limits: static review only, as required. No builds, parser generation, unit tests, regression cluster tests, or runtime validation were performed; the author's reported test results were not independently reproduced.
Review converged after 2 round(s).
Contributor
TPC-H: Total hot run time: 28631 ms |
Contributor
TPC-DS: Total hot run time: 150333 ms |
Contributor
ClickBench: Total hot run time: 24.36 s |
Contributor
FE Regression 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
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?
SELECT * FROM `t`$snapshotscurrently parses$snapshotsas an implicit table alias and silently returns rows fromt. The same issue affects qualified names such as`catalog`.`db`.`t`$snapshots.Reject an unquoted dollar-prefixed suffix immediately adjacent to a quoted table name during parsing. The error explains how to quote the complete table name or separate an intended alias. Supported forms such as
t$snapshots,`t$snapshots`, and`t` AS $snapshotsremain valid, as do aliases separated by whitespace or comments.Add parser coverage for qualified names, escaped backticks, non-ASCII names, explicit and separated aliases, and parsing without building a tree. Add a p0 regression suite with six syntax-error checks that require no external catalog.
Release note
Queries that attach an unquoted dollar suffix directly to a quoted table name now fail explicitly instead of silently scanning the base table. Quote the entire system table name, for example
`t$snapshots`.Check List (For Author)
fe-sql-parsertests pass after the fix.test_quoted_table_suffixpassed on a locally built FE/BE cluster; all six syntax-error checks passed.Check List (For Reviewer who merge this PR)