-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
HIVE-24167: Compilation fails due to equivalence mapping violation when CTE materialization is enabled #5452
Conversation
d226c96
to
d35216b
Compare
|
ql/src/java/org/apache/hadoop/hive/ql/plan/mapper/PlanMapper.java
Outdated
Show resolved
Hide resolved
ql/src/java/org/apache/hadoop/hive/ql/plan/mapper/PlanMapper.java
Outdated
Show resolved
Hide resolved
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.
Thanks for reviewing the PR. I am updating the branch based on the discussion.
d35216b
to
43fe86c
Compare
43fe86c
to
1388b2d
Compare
1388b2d
to
1053a5c
Compare
@zabetak Updated. Thanks for reviewing this PR and giving many suggestions. I believe the change is much simpler |
1053a5c
to
2ca6f97
Compare
I rebased it again because the previous base revision was affected by HIVE-28611 |
|
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.
LGTM!
@okumin Now that you are a committer you can merge this yourself.
The PR/JIRA summary are a bit narrow given the nature of the problem and the fix that we are about to merge. Consider updating the commit message and JIRA with something more general/descriptive.
Examples
HIVE-24167: Compilation fails due to equivalence mapping violation when CTE materialization is enabled
HIVE-24167: Equivalence mapping violation in PlanMapper when CTE materialization is enabled
Thanks. Let me pick up the first option.
Based on this rule, I will wait for 1 day and return here. |
What changes were proposed in this pull request?
With this change, Hive skips using or persisting runtime stats when PlanMapper throws equivalence mapping violation.
In some more contexts, we've tried to resolve the violation in #5037 and #5077. However, we couldn't find an obvious way to ideally avoid the issue, which means a modification will likely cause another problem. That's why we are currently biased toward the current approach.
https://issues.apache.org/jira/browse/HIVE-24167
Why are the changes needed?
When this issue occurs, the query always fails. We think the disadvantage of failure is not well balanced with the advantage of fast failure. A query might run with a bit poorer performance without runtime stats, but it is not critical in most cases. That's why we would try disabling runtime stats in this case.
Does this PR introduce any user-facing change?
In the new behavior, affected queries will not fail but run without runtime statistics.
Is the change a dependency upgrade?
No.
How was this patch tested?
qtests.