HIVE-29559: materializeCTE to query Analyzer from DDLSemanticAnalyzerFactory#6423
Open
konstantinb wants to merge 4 commits intoapache:masterfrom
Open
HIVE-29559: materializeCTE to query Analyzer from DDLSemanticAnalyzerFactory#6423konstantinb wants to merge 4 commits intoapache:masterfrom
konstantinb wants to merge 4 commits intoapache:masterfrom
Conversation
…r instead of SemanticAnalyzer
….out file generated with proper test driver this time
asf-ci-hive
added
tests pending
tests passed
and removed
tests unstable
tests pending
labels
…iginally intended by HIVE-28724
asf-ci-hive
added
tests pending
tests passed
and removed
tests passed
tests pending
labels
konstantinb
changed the title
|
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 changes were proposed in this pull request?
HIVE-29559: SemanticAnalyzer.materializeCTE to query SemanticAnalyzer instance from DDLSemanticAnalyzerFactory
Why are the changes needed?
As originally intended by the changes for HIVE-28724, the PR #5973 has introduced a new CreateTableAnalyzer class and registered it with DDLSemanticAnalyzerFactory
Unfortunately, the PR has also introduced a new CalcitePlanner.The materializeCTE() method is almost an exact copy of SemanticAnalyzer.materializeCTE(), with only one difference: it explicitly uses CreateTableAnalyzer instead of SemanticAnalyzer.
Because some code was removed from SemanticAnalyzer, its own implementation of materializeCTE() broke during the merge of this PR. The new query file of this PR reproduces the NPE happening with the current master branch code.
The first, "easy" and working choice was to copy the whole materializeCTE() from CalcitePlanner to SemanticAnalyzer and remove materializeCTE() from CalcitePlanner completely. I decided to take it a bit further due to the following reasons:
Does this PR introduce any user-facing change?
No
How was this patch tested?
Unit tests, new query file, all existing tests pass