Jackie-Jiang commented on code in PR #14385:
URL: https://github.com/apache/pinot/pull/14385#discussion_r1984393385


##########
pinot-core/src/main/java/org/apache/pinot/core/query/optimizer/filter/TimePredicateFilterOptimizer.java:
##########
@@ -411,6 +427,108 @@ && isStringLiteral(dateTimeConvertOperands.get(3)),
     }
   }
 
+  private void optimizeDateTrunc(Function filterFunction, FilterKind 
filterKind) {
+    List<Expression> filterOperands = filterFunction.getOperands();
+    List<Expression> dateTruncOperands = 
filterOperands.get(0).getFunctionCall().getOperands();
+
+    if (dateTruncOperands.get(1).isSetLiteral()) {

Review Comment:
   We might want to apply some simply operand checks to prevent 
`IndexOutOfBoundException` which can be quite confusing.



##########
pinot-core/src/main/java/org/apache/pinot/core/query/optimizer/filter/TimePredicateFilterOptimizer.java:
##########
@@ -411,6 +427,108 @@ && isStringLiteral(dateTimeConvertOperands.get(3)),
     }
   }
 
+  private void optimizeDateTrunc(Function filterFunction, FilterKind 
filterKind) {
+    List<Expression> filterOperands = filterFunction.getOperands();
+    List<Expression> dateTruncOperands = 
filterOperands.get(0).getFunctionCall().getOperands();
+
+    if (dateTruncOperands.get(1).isSetLiteral()) {

Review Comment:
   Literal shouldn't reach here. Do you see a scenario where literal is passed 
here?



##########
pinot-core/src/main/java/org/apache/pinot/core/query/optimizer/filter/TimePredicateFilterOptimizer.java:
##########
@@ -411,6 +427,108 @@ && isStringLiteral(dateTimeConvertOperands.get(3)),
     }
   }
 
+  private void optimizeDateTrunc(Function filterFunction, FilterKind 
filterKind) {
+    List<Expression> filterOperands = filterFunction.getOperands();
+    List<Expression> dateTruncOperands = 
filterOperands.get(0).getFunctionCall().getOperands();
+
+    if (dateTruncOperands.get(1).isSetLiteral()) {
+      return;
+    }
+
+    Long lowerMillis = null;
+    Long upperMillis = null;

Review Comment:
   They can be primitive `long`



##########
pinot-core/src/main/java/org/apache/pinot/core/query/optimizer/filter/TimePredicateFilterOptimizer.java:
##########
@@ -411,6 +427,108 @@ && isStringLiteral(dateTimeConvertOperands.get(3)),
     }
   }
 
+  private void optimizeDateTrunc(Function filterFunction, FilterKind 
filterKind) {
+    List<Expression> filterOperands = filterFunction.getOperands();
+    List<Expression> dateTruncOperands = 
filterOperands.get(0).getFunctionCall().getOperands();
+
+    if (dateTruncOperands.get(1).isSetLiteral()) {
+      return;
+    }
+
+    Long lowerMillis = null;
+    Long upperMillis = null;
+    boolean lowerInclusive = true;
+    boolean upperInclusive = true;
+    List<Expression> operands = new ArrayList<>(dateTruncOperands);

Review Comment:
   Do we need to make a copy here?



-- 
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: commits-unsubscr...@pinot.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org

Reply via email to