zinking commented on a change in pull request #2434:
URL: https://github.com/apache/calcite/pull/2434#discussion_r649712956
##########
File path:
elasticsearch/src/main/java/org/apache/calcite/adapter/elasticsearch/PredicateAnalyzer.java
##########
@@ -200,22 +201,6 @@ private static boolean supportedRexCall(RexCall call) {
}
}
- /**
Review comment:
I'd suggest keep the comments but modify them slightly
so that I can still grab the context regarding `sarg`
##########
File path:
elasticsearch/src/main/java/org/apache/calcite/adapter/elasticsearch/PredicateAnalyzer.java
##########
@@ -741,6 +730,10 @@ private CompoundQueryExpression(boolean partial,
BoolQueryBuilder builder) {
@Override public QueryExpression notIn(LiteralExpression literal) {
throw new PredicateAnalyzerException("notIn cannot be applied to a
compound expression");
}
+
+ @Override public QueryExpression range(LiteralExpression literal) {
Review comment:
I thought all `sargs` are supported now, aren't they?
##########
File path:
elasticsearch/src/main/java/org/apache/calcite/adapter/elasticsearch/PredicateAnalyzer.java
##########
@@ -873,8 +866,37 @@ private SimpleQueryExpression(NamedFieldExpression rel) {
builder = boolQuery().mustNot(termsQuery(getFieldReference(), iterable));
return this;
}
- }
+ @Override public QueryExpression range(LiteralExpression literal) {
+ Iterator<?> iterator = ((Iterable<?>) literal.value()).iterator();
+ BoolQueryBuilder boolQuery = boolQuery();
+ while (iterator.hasNext()) {
+ Range range = (Range) iterator.next();
Review comment:
I don't have much context, but usually I would assume it should be some
RangeExpression, or at least some utils to extract the Range expressions,
instead of expose the iterator directly here?
would that be better ?
--
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.
For queries about this service, please contact Infrastructure at:
[email protected]
