fix: Allow merging OR-ed bigint single value ranges #15738
+334
−7
Conversation
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
✅ Deploy Preview for meta-velox canceled.
|
meta-cla
bot
added
the
CLA Signed
…r#15738) Summary: Since BIGINT ranges can sometimes be converted to values filters (i.e. if all the range filters are single value), sequential merging of several BIGINT ranges can cause a generic MultiRange filter to be created, which cannot be evaluated. e.g. `(id = 100 OR id = 200 OR id = 300)` is converted by HiveConnectorUtil::extractFiltersFromRemainingFilter in the following stages: ``` I1209 13:29:11.955765 1472505 [CPUThreadPool0] ExprToSubfieldFilter.cpp:584] tryMergeBigintRanges called with 2 disjuncts I1209 13:29:11.955819 1472505 [CPUThreadPool0] ExprToSubfieldFilter.cpp:586] Disjunct 0: BigintRange: [100, 100] no nulls I1209 13:29:11.955826 1472505 [CPUThreadPool0] ExprToSubfieldFilter.cpp:586] Disjunct 1: BigintRange: [200, 200] no nulls I1209 13:29:11.955830 1472505 [CPUThreadPool0] ExprToSubfieldFilter.cpp:602] Creating BigintValues with 2 values I1209 13:29:11.955852 1472505 [CPUThreadPool0] ExprToSubfieldFilter.cpp:584] tryMergeBigintRanges called with 2 disjuncts I1209 13:29:11.955862 1472505 [CPUThreadPool0] ExprToSubfieldFilter.cpp:586] Disjunct 0: Filter(BigintValuesUsingBitmask, deterministic, no nulls) I1209 13:29:11.955871 1472505 [CPUThreadPool0] ExprToSubfieldFilter.cpp:586] Disjunct 1: BigintRange: [300, 300] no nulls W1209 13:29:11.955875 1472505 [CPUThreadPool0] ExprToSubfieldFilter.cpp:609] tryMergeBigintRanges returning nullptr - not all disjuncts are BigintRange/BigintMultiRange ``` The filter is instead merged into a generic MultiRange filter with bigint ranges, which cannot be evaluated: ``` Filter(MultiRange, deterministic, no nulls): testInt64Range() is not supported. ``` Generic MultiRange does not support BIGINT type by design. Instead of converting to a MultiRange, merge logic should have explicit handling for BIGINT value filters so it can handle these gracefully Differential Revision: D88789308
hdikeman
force-pushed
the
export-D88789308
branch
from
b065022 to
1a77f08
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Summary:
Since BIGINT ranges can sometimes be converted to values filters (i.e. if all the range filters are single value), sequential merging of several BIGINT ranges can cause a generic MultiRange filter to be created, which cannot be evaluated.
e.g.
(id = 100 OR id = 200 OR id = 300)is converted by HiveConnectorUtil::extractFiltersFromRemainingFilter in the following stages:
The filter is instead merged into a generic MultiRange filter with bigint ranges, which cannot be evaluated:
It appeared intentional that the generic MultiRange cannot support BigintRange evaluation, since there is a BigintMultiRange for that purpose. But an alternate fix could be to add evaluation support for bigint to MultiRange
Instead of converting to a MultiRange, merge logic should have explicit handling for BIGINT value filters so it can handle these gracefully
Also adding support for the following negative filters:
Differential Revision: D88789308