joooger commented on code in PR #5343:
URL: https://github.com/apache/calcite/pull/5343#discussion_r4316985606
##########
core/src/main/java/org/apache/calcite/rex/RexSimplify.java:
##########
@@ -2330,20 +2333,20 @@ private static <C extends Comparable<C>> RangeSet<C>
residue(RexNode ref,
final RangeSet<C> notEqualsRangeSet =
ImmutableRangeSet.of(pointRange).complement();
if (result.enclosesAll(notEqualsRangeSet)) {
- result = RangeSets.rangeSetAll();
+ result.add(Range.all());
continue;
}
- result = RangeSets.minus(result, pointRange);
+ result.remove(pointRange);
break;
default:
final Range<C> r1 = range(comparison.kind, c1);
if (result.encloses(r1)) {
// Given these predicates, term is always satisfied.
// e.g. r0 is "$0 < 10", r1 is "$0 < 5"
- result = RangeSets.rangeSetAll();
+ result.add(Range.all());
continue;
}
- result = result.subRangeSet(r1);
+ result.removeAll(ImmutableRangeSet.of(r1).complement());
}
if (result.isEmpty()) {
break; // short-cut
Review Comment:
Good catch, the `break` only left the switch. Done in e2da6d205: it now
returns an empty range set, which no later predicate can change.
##########
core/src/main/java/org/apache/calcite/rex/RexSimplify.java:
##########
@@ -2330,20 +2333,20 @@ private static <C extends Comparable<C>> RangeSet<C>
residue(RexNode ref,
final RangeSet<C> notEqualsRangeSet =
ImmutableRangeSet.of(pointRange).complement();
if (result.enclosesAll(notEqualsRangeSet)) {
- result = RangeSets.rangeSetAll();
+ result.add(Range.all());
Review Comment:
Done in e2da6d205. Once the result is all values, every later predicate is
enclosed by it, so returning immediately gives the same result.
##########
core/src/main/java/org/apache/calcite/rex/RexSimplify.java:
##########
@@ -2330,20 +2333,20 @@ private static <C extends Comparable<C>> RangeSet<C>
residue(RexNode ref,
final RangeSet<C> notEqualsRangeSet =
ImmutableRangeSet.of(pointRange).complement();
if (result.enclosesAll(notEqualsRangeSet)) {
- result = RangeSets.rangeSetAll();
+ result.add(Range.all());
continue;
}
- result = RangeSets.minus(result, pointRange);
+ result.remove(pointRange);
break;
default:
final Range<C> r1 = range(comparison.kind, c1);
if (result.encloses(r1)) {
// Given these predicates, term is always satisfied.
// e.g. r0 is "$0 < 10", r1 is "$0 < 5"
- result = RangeSets.rangeSetAll();
+ result.add(Range.all());
Review Comment:
Done in e2da6d205.
##########
core/src/main/java/org/apache/calcite/rex/RexSimplify.java:
##########
@@ -2305,7 +2305,10 @@ private <C extends Comparable<C>> RexNode
simplifyUsingPredicates(RexNode e,
*/
private static <C extends Comparable<C>> RangeSet<C> residue(RexNode ref,
RangeSet<C> r0, List<RexNode> predicates, Class<C> clazz) {
- RangeSet<C> result = r0;
+ // Narrow a single mutable range set in place. Copying the range set for
Review Comment:
Agreed, reworded in e2da6d205. The comment now also mentions
simplifyAndTerms, since NOT IN in a filter goes through the AND path.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
