kgyrtkirk commented on code in PR #15766:
URL: https://github.com/apache/druid/pull/15766#discussion_r1472829531
##########
sql/src/main/java/org/apache/druid/sql/calcite/filtration/CombineAndSimplifyBounds.java:
##########
@@ -270,6 +269,18 @@ private static DimFilter doSimplify(final List<DimFilter>
children, boolean disj
childrenToAdd.add(Ranges.toFilter(rangeRefKey, range));
}
}
+ } else {
+ if (disjunction && rangeSet.asRanges().size() == 2) {
Review Comment:
I was making a comment - but didn't finished it:
for the conditions
```
dim1 NOT BETWEEN 'a' AND 'c' AND dim1 NOT BETWEEN 'e' AND 'g'
```
the equiv non-between form is like this:
```
(dim1 < 'a' or c < dim1 ) AND ( dim1 < 'e' OR 'g' < dim1 )
```
because it has 2 `OR` leafs which would be handled - its ok.
the more complicated case could be:
```
x < -10 or ( x between 0 and 10 )
```
for which there is no need to add `not between` stuff - it won't be much
better => if it doesn't span `all` there is no need for those things - as the
negated ranges will not be that much better
for something like
```
x < -10 or ( x between 0 and 10 ) or 100 < x
```
it could possibly be rewritten to
```
not ( x between -10 and 0 ) and not (x between 10 and 100)
not ( ( x between -10 and 0 ) or (x between 10 and 100) )
```
I've added some stuff - but since these were not covered by existing tests;
I had to add some new tests as well....
probably recognizing `not(rangeFilter())` could be also beneficial; but I've
left that out for now
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
