[fix](view) Prevent invalid alias rewrite in view definitions#63353
Draft
starocean999 wants to merge 2 commits into
Draft
[fix](view) Prevent invalid alias rewrite in view definitions#63353starocean999 wants to merge 2 commits into
starocean999 wants to merge 2 commits into
Conversation
Contributor
Author
|
/review |
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Requesting changes for regression-test issues. The view alias-star fix itself is small and has both FE and regression coverage, but the new regression case does not follow Doris regression-test standards and the generated result file is not whitespace-clean.
Critical checkpoint conclusions:
- Goal/test: the stated CREATE VIEW alias-star aggregate issue is addressed by clearing inherited SQL indexes on subquery alias outputs, with a unit test and regression test.
- Scope/focus: focused in the canonical PR diff.
- Concurrency/lifecycle/config/compatibility: no new concurrency, lifecycle, persisted format, config, or FE-BE protocol issue found.
- Parallel paths: no additional parallel view-rewrite path requiring the same change was found.
- Tests: coverage was added, but the regression test should hardcode the single view name, drop before use, and preserve the view after completion; the .out also has a
git diff --checkwhitespace error. - Observability/performance/transactionality: not applicable to this view rewrite; no additional issue found.
- User focus: no additional user-provided review focus was supplied.
Contributor
Author
|
/review |
Contributor
There was a problem hiding this comment.
Review result: no new blocking issues found in the current PR diff.
Critical checkpoint conclusions:
- Goal/test coverage: The change targets invalid create-view SQL rewriting when alias-star expansion crosses an aggregate subquery. The FE unit test and regression test cover the reported shape.
- Scope/focus: The implementation is small and localized to
LogicalSubQueryAliasoutput slot metadata, with focused tests. - Concurrency/lifecycle: No new shared mutable state, locks, threads, or lifecycle-sensitive objects are introduced.
- Configuration/compatibility: No configs, persisted storage formats, protocol fields, or FE-BE compatibility surfaces are changed.
- Parallel paths: The create-view rewrite path was checked around star expansion, slot binding, and
PlanSlotFinder; clearing the inherited child SQL index on alias outputs is consistent with preventing alias-expanded synthetic slots from rewriting inner SQL positions while preserving explicit outer references through rebinding. - Conditional checks/error handling: No new error-handling paths or defensive conditionals are introduced.
- Test result review: The current regression output is deterministic and whitespace-clean; earlier existing comments covered the previous EOF whitespace and test cleanup/naming issues.
- Observability/performance: No new runtime path requiring logs/metrics; no meaningful performance risk identified.
- Data correctness/transactions: The change is analyzer metadata only and does not affect transaction, version visibility, delete bitmap, or storage semantics.
User focus points: none were provided, and no additional focus-specific issues were found.
Contributor
Author
|
run buildall |
Contributor
TPC-H: Total hot run time: 31019 ms |
Contributor
TPC-DS: Total hot run time: 168736 ms |
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?
Creating a view that expands a subquery alias star over an aggregate subquery could persist an invalid inline view definition. During view SQL rewrite, subquery alias output slots inherited the child slot SQL positions, so inner aggregate columns could be rewritten to self-references such as c.pack_factory. Querying the view then failed with Unknown column in AGGREGATE clause.
Related PR: #xxx
Problem Summary:
Release note
None
Check List (For Author)
Test
Behavior changed:
Does this need documentation?
Check List (For Reviewer who merge this PR)