clintropolis commented on code in PR #14976:
URL: https://github.com/apache/druid/pull/14976#discussion_r1325524916


##########
processing/src/main/java/org/apache/druid/math/expr/Function.java:
##########
@@ -2224,6 +2224,89 @@ public <T> ExprVectorProcessor<T> 
asVectorProcessor(Expr.VectorInputBindingInspe
     }
   }
 
+  /**
+   * SQL function "x IS NOT DISTINCT FROM y". Very similar to "x = y", i.e. 
{@link BinEqExpr}, except this function
+   * never returns null, and this function considers NULL itself to be 
not-distinct-from NULL.
+   */
+  class IsNotDistinctFromFunc implements Function
+  {
+    @Override
+    public String name()
+    {
+      return "notdistinctfrom";
+    }
+
+    @Override
+    public ExprEval apply(List<Expr> args, Expr.ObjectBinding bindings)
+    {
+      final ExprEval leftVal = args.get(0).eval(bindings);
+      final ExprEval rightVal = args.get(1).eval(bindings);
+
+      if (leftVal.value() == null || rightVal.value() == null) {
+        return ExprEval.ofLongBoolean(leftVal.value() == null && 
rightVal.value() == null);
+      }
+
+      final ExpressionType comparisonType = 
ExpressionTypeConversion.autoDetect(leftVal, rightVal);
+      switch (comparisonType.getType()) {
+        case STRING:
+          return ExprEval.ofLongBoolean(Objects.equals(leftVal.asString(), 
rightVal.asString()));
+        case LONG:
+          return ExprEval.ofLongBoolean(leftVal.asLong() == rightVal.asLong());
+        case DOUBLE:
+        default:

Review Comment:
   hmm, the comparison expressions all handle ARRAY types correctly, should we 
add it here too? See `evalArray` methods on the various 
`BinaryBooleanOpExprBase` implementations.



##########
processing/src/main/java/org/apache/druid/math/expr/Function.java:
##########
@@ -2224,6 +2224,89 @@ public <T> ExprVectorProcessor<T> 
asVectorProcessor(Expr.VectorInputBindingInspe
     }
   }
 
+  /**
+   * SQL function "x IS NOT DISTINCT FROM y". Very similar to "x = y", i.e. 
{@link BinEqExpr}, except this function
+   * never returns null, and this function considers NULL itself to be 
not-distinct-from NULL.
+   */
+  class IsNotDistinctFromFunc implements Function
+  {
+    @Override
+    public String name()
+    {
+      return "notdistinctfrom";

Review Comment:
   eh, it is consistent with `isnull`/`notnull`



##########
processing/src/main/java/org/apache/druid/math/expr/Function.java:
##########
@@ -2224,6 +2224,89 @@ public <T> ExprVectorProcessor<T> 
asVectorProcessor(Expr.VectorInputBindingInspe
     }
   }
 
+  /**
+   * SQL function "x IS NOT DISTINCT FROM y". Very similar to "x = y", i.e. 
{@link BinEqExpr}, except this function
+   * never returns null, and this function considers NULL itself to be 
not-distinct-from NULL.
+   */
+  class IsNotDistinctFromFunc implements Function
+  {
+    @Override
+    public String name()
+    {
+      return "notdistinctfrom";
+    }
+
+    @Override
+    public ExprEval apply(List<Expr> args, Expr.ObjectBinding bindings)
+    {
+      final ExprEval leftVal = args.get(0).eval(bindings);
+      final ExprEval rightVal = args.get(1).eval(bindings);
+
+      if (leftVal.value() == null || rightVal.value() == null) {
+        return ExprEval.ofLongBoolean(leftVal.value() == null && 
rightVal.value() == null);
+      }
+
+      final ExpressionType comparisonType = 
ExpressionTypeConversion.autoDetect(leftVal, rightVal);
+      switch (comparisonType.getType()) {
+        case STRING:
+          return ExprEval.ofLongBoolean(Objects.equals(leftVal.asString(), 
rightVal.asString()));
+        case LONG:
+          return ExprEval.ofLongBoolean(leftVal.asLong() == rightVal.asLong());
+        case DOUBLE:
+        default:
+          if (leftVal.isNumericNull() || rightVal.isNumericNull()) {
+            return ExprEval.ofLongBoolean(leftVal.isNumericNull() && 
rightVal.isNumericNull());

Review Comment:
   i think it doesn't really matter because its covered by the leftVal.value() 
== null || rightVal.value == null check that happens before we get here.
   
   This default case is also handling array and complex types, which .. is 
probably not super cool since the former always claims to be numeric null 
unless it contains only a single element which can be converted to or is a 
number, and the latter always claims to be numeric null, but maybe ok if we 
document the behavior doesn't work with these types.



-- 
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]

Reply via email to