[GitHub] [druid] suneet-s commented on a change in pull request #9488: Match GREATEST/LEAST function behavior to other DBs

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

suneet-s commented on a change in pull request #9488: Match GREATEST/LEAST function behavior to other DBs
URL: https://github.com/apache/druid/pull/9488#discussion_r391112693
 
 

 ##########
 File path: core/src/main/java/org/apache/druid/math/expr/Function.java
 ##########
 @@ -976,6 +981,163 @@ protected ExprEval eval(double x, double y)
     }
   }
 
+  class GreatestFunc extends ReduceFunc
+  {
+    public static final String NAME = "greatest";
+
+    public GreatestFunc()
+    {
+      super(
+          Math::max,
+          Math::max,
+          BinaryOperator.maxBy(Comparator.naturalOrder())
+      );
+    }
+
+    @Override
+    public String name()
+    {
+      return NAME;
+    }
+  }
+
+  class LeastFunc extends ReduceFunc
+  {
+    public static final String NAME = "least";
+
+    public LeastFunc()
+    {
+      super(
+          Math::min,
+          Math::min,
+          BinaryOperator.minBy(Comparator.naturalOrder())
+      );
+    }
+
+    @Override
+    public String name()
+    {
+      return NAME;
+    }
+  }
+
+  abstract class ReduceFunc implements Function
+  {
+    private final DoubleBinaryOperator doubleReducer;
+    private final LongBinaryOperator longReducer;
+    private final BinaryOperator<String> stringReducer;
+
+    ReduceFunc(
+        DoubleBinaryOperator doubleReducer,
+        LongBinaryOperator longReducer,
+        BinaryOperator<String> stringReducer
+    )
+    {
+      this.doubleReducer = doubleReducer;
+      this.longReducer = longReducer;
+      this.stringReducer = stringReducer;
+    }
+
+    @Override
+    public void validateArguments(List<Expr> args)
+    {
+      // anything goes
+    }
+
+    @Override
+    public ExprEval apply(List<Expr> args, Expr.ObjectBinding bindings)
+    {
+      if (args.isEmpty()) {
+        return ExprEval.of(null);
+      }
+
+      ExprAnalysis exprAnalysis = analyzeExprs(args, bindings);
+      if (exprAnalysis == null) {
+        // The GREATEST/LEAST functions are not in the SQL standard, but most (e.g., MySQL, Oracle) return NULL if any
+        // are NULL. Others (e.g., Postgres) only return NULL if all are NULL, otherwise the NULLs are ignored.
+        return ExprEval.of(null);
 
 Review comment:
   Sorry if I'm mis-reading this.
   
   It looks like this patch introduces expressions to calculate the greatest / least across multiple expressions. Native druid queries have a post aggregator that should do this. When I was thinking of implementing this I thought there would be a translation from sql to the native druid query with the correct post aggregator (maybe somewhere in here - OperatorConversions#toPostAggregator)
   
   Otherwise we'd want to make sure this behavior stays in sync with the *PostAggregators

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

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