[GitHub] [druid] clintropolis commented on a change in pull request #11184: vectorize logical operators and boolean functions

[GitBox <gi...@apache.org>]

clintropolis commented on a change in pull request #11184:
URL: https://github.com/apache/druid/pull/11184#discussion_r759804640



##########
File path: core/src/main/java/org/apache/druid/math/expr/BinaryOperatorExpr.java
##########
@@ -157,3 +158,67 @@ protected ExprEval evalString(@Nullable String left, @Nullable String right)
 
   protected abstract double evalDouble(double left, double right);
 }
+
+@SuppressWarnings("ClassName")
+abstract class BinaryBooleanOpExprBase extends BinaryOpExprBase
+{
+  BinaryBooleanOpExprBase(String op, Expr left, Expr right)
+  {
+    super(op, left, right);
+  }
+
+  @Override
+  public ExprEval eval(ObjectBinding bindings)
+  {
+    ExprEval leftVal = left.eval(bindings);
+    ExprEval rightVal = right.eval(bindings);
+
+    // Result of any Binary expressions is null if any of the argument is null.
+    // e.g "select null * 2 as c;" or "select null + 1 as c;" will return null as per Standard SQL spec.
+    if (NullHandling.sqlCompatible() && (leftVal.value() == null || rightVal.value() == null)) {
+      return ExprEval.of(null);
+    }
+
+    ExpressionType type = ExpressionTypeConversion.autoDetect(leftVal, rightVal);
+    boolean result;
+    switch (type.getType()) {

Review comment:
       What you describe is the way vector expression processing works, but the non-vectorized implementations of eval are filled with all sorts of branches (in part due to not always being able to know the input types at setup time), so this implementation is just being consistent with other non-vectorized eval implementations. Using vector processors with a vector size of 1 for the non-vectorized engine does seem to offer a light performance increase, since the majority of branches can be eliminated at setup time as well as being stronger typed so numeric primitives can avoid boxing/unboxing, (but there is still a lot of overhead wrapper objects that show their weight when there is one per row instead of one per batch).
   
   This whole area is ripe for code generation of some sort, i've been trying to get the base vectorized implementation in place so we have a baseline to compare different strategies against, so maybe in that world we can have better implementations for non-vectorized expression processing as well, but we're not quite there yet I think.




-- 
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-unsubscribe@druid.apache.org

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



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org