HIVE-27102: Upgrade Calcite to 1.33.0 and Avatica to 1.23.0 #5196
Conversation
asf-ci-hive
added
tests pending
tests failed
and removed
tests pending
tests failed
labels
asf-ci-hive
added
tests pending
tests failed
and removed
tests failed
tests pending
labels
asf-ci-hive
added
tests pending
tests failed
and removed
tests failed
tests pending
labels
This was failing because during logical planning key*1 is getting reduced to key in 1.33, whereas it was not reduced in 1.25. Changing it to key*2 will stop the reduction.
`NOT(SEARCH(...))` is simplified to `SEARCH[(..), (...)]` in RexSimplify# simplifyNot Removed testComputeRangePredicateSelectivityNotBetweenRightLowerThanLeft as it is not possible to create a RexNode with right < left
tries. This can be removed after upgrading to Calcite 1.38 https://issues.apache.org/jira/browse/CALCITE-6453
soumyakanti3578
force-pushed
the
calcite-upgrade-1.33
branch
from
298d298
to
47cf072
Compare
| private static RexNode simplify(RexSimplify simplifier, RexNode node) { | ||
| RexNode result = node; | ||
| int maxTries = 5; | ||
| for (int i = 0; i < maxTries; i++) { | ||
| RexNode simplified = simplifier.simplify(result); | ||
| if (simplified.equals(result)) { | ||
| break; | ||
| } | ||
| result = simplified; | ||
| } | ||
|
|
||
| return result; |
There was a problem hiding this comment.
This part indicates that there is some part of flakiness/nondeterminism in the mix. Moreover, the simplifier is rather costly so calling it multiple times can have an impact on performance. Why do we need to do this? What is the expression that requires multiple rounds of simplification? This probably deserves logging a dedicated CALCITE ticket. Unfortunately this cannot be committed as such we need to work out a solution.
There was a problem hiding this comment.
Yes, this is rather crude, and I will remove it. I just wanted to test that if the RexNode is completely simplified at this point, then we won't run into the infinite for loop issue - this is explained by Krisztian here.
There is already a ticket in Calcite to fix this: CALCITE-6453.
So the issue is really with certain CASTs not getting reduced, and we have to call simplify twice to reduce it.
… a max of 5" This reverts commit 9724d26.
|
What changes were proposed in this pull request?
Upgrade Calcite to 1.33, Avatica to 1.23
Why are the changes needed?
Does this PR introduce any user-facing change?
Logical explain plans can have SEARCH operators instead of BETWEENs and INs
Is the change a dependency upgrade?
Yes
How was this patch tested?
With Ptests