ravipesala commented on a change in pull request #3145: [CARBONDATA-3315] Fix for Range Filter failing with two between clauses as children of OR expression
URL:
https://github.com/apache/carbondata/pull/3145#discussion_r265120131
##########
File path: core/src/main/java/org/apache/carbondata/core/scan/expression/RangeExpressionEvaluator.java
##########
@@ -223,12 +223,27 @@ private void fillExpressionMap(Map<String, List<FilterModificationNode>> filterE
addFilterExpressionMap(filterExpressionMap, currentNode, parentNode);
}
- for (Expression exp : currentNode.getChildren()) {
- if (null != exp) {
- fillExpressionMap(filterExpressionMap, exp, currentNode);
- if (exp instanceof OrExpression) {
- replaceWithRangeExpression(filterExpressionMap);
- filterExpressionMap.clear();
+
+ // In case of Or Exp we have to evaluate both the subtrees of expression separately
+ // else it will combine the results of both the subtrees into one expression
+ // which wont give us correct result
+ if (currentNode instanceof OrExpression) {
+ Map<String, List<FilterModificationNode>> filterExpressionMapNew =
+ new HashMap<>(CarbonCommonConstants.DEFAULT_COLLECTION_SIZE);
+ if (null != ((OrExpression) currentNode).left) {
+ fillExpressionMap(filterExpressionMapNew, ((OrExpression) currentNode).left, currentNode);
+ replaceWithRangeExpression(filterExpressionMapNew);
+ filterExpressionMapNew.clear();
Review comment:
This piece of code is dupicated, please extract to separate method
----------------------------------------------------------------
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:
[hidden email]
With regards,
Apache Git Services