SQL: rewrite ROUND and TRUNCATE functions with a different optional parameter handling method (#40242)

* Rewrite Round and Truncate functions to have a slightly different
approach to handling the optional parameter in the constructor. Until now
the optional parameter was considered 0 if the value was missing and the
constructor was filling in this value. The current solution is to have
the optional parameter as null right until the actual calculation is done.

(cherry picked from commit 3e314f8)

Author: astefan

x-pack/plugin/sql/qa/src/main/resources/agg.csv-spec

@@ -192,6 +192,23 @@ ROUND(SQRT(CAST(EXP(languages) AS SMALLINT)),2):d|  COUNT(1):l
 null                                             |10
 ;
 
+groupByRoundWithTwoParams
+SELECT ROUND(YEAR("birth_date"), -2) FROM test_emp GROUP BY ROUND(YEAR("birth_date"), -2);
+
+ROUND(YEAR(birth_date [Z]),-2)
+-----------------------------
+null
+2000
+;
+
+groupByTruncateWithTwoParams
+SELECT TRUNCATE(YEAR("birth_date"), -2) FROM test_emp GROUP BY TRUNCATE(YEAR("birth_date"), -2);
+
+TRUNCATE(YEAR(birth_date [Z]),-2)
+--------------------------------
+null
+1900
+;
 
 //
 // Grouping functions

x-pack/plugin/sql/qa/src/main/resources/agg.sql-spec

@@ -50,6 +50,31 @@ SELECT emp_no * 2 AS e FROM test_emp GROUP BY e ORDER BY e;
 groupByModScalar
 SELECT (emp_no % 3) + 1 AS e FROM test_emp GROUP BY e ORDER BY e;
 
+// group by nested functions with no alias
+//https://github.com/elastic/elasticsearch/issues/40239
+groupByTruncate-Ignore
+SELECT CAST(TRUNCATE(EXTRACT(YEAR FROM "birth_date")) AS INTEGER) AS x FROM test_emp GROUP BY CAST(TRUNCATE(EXTRACT(YEAR FROM "birth_date")) AS INTEGER) ORDER BY CAST(TRUNCATE(EXTRACT(YEAR FROM "birth_date")) AS INTEGER);
+//https://github.com/elastic/elasticsearch/issues/40239
+groupByRound-Ignore
+SELECT CAST(ROUND(EXTRACT(YEAR FROM "birth_date")) AS INTEGER) AS x FROM test_emp GROUP BY CAST(ROUND(EXTRACT(YEAR FROM "birth_date")) AS INTEGER) ORDER BY CAST(ROUND(EXTRACT(YEAR FROM "birth_date")) AS INTEGER);
+groupByAtan2
+SELECT ATAN2(YEAR("birth_date"), 5) AS x FROM test_emp GROUP BY ATAN2(YEAR("birth_date"), 5) ORDER BY ATAN2(YEAR("birth_date"), 5);
+groupByPower
+SELECT POWER(YEAR("birth_date"), 2) AS x FROM test_emp GROUP BY POWER(YEAR("birth_date"), 2) ORDER BY POWER(YEAR("birth_date"), 2);
+//https://github.com/elastic/elasticsearch/issues/40239
+groupByPowerWithCast-Ignore
+SELECT CAST(POWER(YEAR("birth_date"), 2) AS DOUBLE) AS x FROM test_emp GROUP BY CAST(POWER(YEAR("birth_date"), 2) AS DOUBLE) ORDER BY CAST(POWER(YEAR("birth_date"), 2) AS DOUBLE);
+groupByConcat
+SELECT LEFT(CONCAT("first_name", "last_name"), 3) AS x FROM test_emp GROUP BY LEFT(CONCAT("first_name", "last_name"), 3) ORDER BY LEFT(CONCAT("first_name", "last_name"), 3) LIMIT 15;
+groupByLocateWithTwoParams
+SELECT LOCATE('a', CONCAT("first_name", "last_name")) AS x FROM test_emp GROUP BY LOCATE('a', CONCAT("first_name", "last_name")) ORDER BY LOCATE('a', CONCAT("first_name", "last_name"));
+groupByLocateWithThreeParams
+SELECT LOCATE('a', CONCAT("first_name", "last_name"), 3) AS x FROM test_emp GROUP BY LOCATE('a', CONCAT("first_name", "last_name"), 3) ORDER BY LOCATE('a', CONCAT("first_name", "last_name"), 3);
+groupByRoundAndTruncateWithTwoParams
+SELECT ROUND(SIN(TRUNCATE("salary", 2)), 2) AS x FROM "test_emp" GROUP BY ROUND(SIN(TRUNCATE("salary", 2)), 2) ORDER BY ROUND(SIN(TRUNCATE("salary", 2)), 2) LIMIT 5;
+groupByRoundAndTruncateWithOneParam
+SELECT ROUND(SIN(TRUNCATE(languages))) AS x FROM "test_emp" GROUP BY ROUND(SIN(TRUNCATE(languages))) ORDER BY ROUND(SIN(TRUNCATE(languages))) LIMIT 5;
+
 // multiple group by
 groupByMultiOnText
 SELECT gender g, languages l FROM "test_emp" GROUP BY gender, languages ORDER BY gender ASC, languages ASC;

x-pack/plugin/sql/qa/src/main/resources/math.csv-spec

@@ -15,7 +15,7 @@ null                                   |10
 truncateWithNoSecondParameterWithAsciiHavingAndOrderBy
 SELECT TRUNCATE(ASCII(LEFT(first_name, 1))), COUNT(*) count FROM test_emp GROUP BY ASCII(LEFT(first_name, 1)) HAVING COUNT(*) > 5 ORDER BY TRUNCATE(ASCII(LEFT(first_name, 1))) DESC;
 
-TRUNCATE(ASCII(LEFT(first_name,1)),0):i|     count:l
+TRUNCATE(ASCII(LEFT(first_name,1))):i|     count:l
 ---------------------------------------+---------------
 null                                   |10             
 66                                     |7              
@@ -163,7 +163,7 @@ ROUND(AVG(salary),2):d| AVG(salary):d |   COUNT(1):l
 groupByAndOrderByRoundWithNoSecondParameter
 SELECT ROUND(AVG(salary)), ROUND(salary) rounded, AVG(salary), COUNT(*) FROM test_emp GROUP BY rounded ORDER BY rounded DESC LIMIT 10;
 
-ROUND(AVG(salary),0):d|   rounded:i   | AVG(salary):d |   COUNT(1):l    
+ROUND(AVG(salary)):d|   rounded:i   | AVG(salary):d |   COUNT(1):l    
 ----------------------+---------------+---------------+---------------
 74999.0               |74999          |74999.0        |1              
 74970.0               |74970          |74970.0        |1              

x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/Processors.java

@@ -12,6 +12,7 @@
 import org.elasticsearch.xpack.sql.expression.function.scalar.datetime.NonIsoDateTimeProcessor;
 import org.elasticsearch.xpack.sql.expression.function.scalar.datetime.QuarterProcessor;
 import org.elasticsearch.xpack.sql.expression.function.scalar.math.BinaryMathProcessor;
+import org.elasticsearch.xpack.sql.expression.function.scalar.math.BinaryOptionalMathProcessor;
 import org.elasticsearch.xpack.sql.expression.function.scalar.math.MathProcessor;
 import org.elasticsearch.xpack.sql.expression.function.scalar.string.BinaryStringNumericProcessor;
 import org.elasticsearch.xpack.sql.expression.function.scalar.string.BinaryStringStringProcessor;
@@ -68,7 +69,6 @@ public static List<NamedWriteableRegistry.Entry> getNamedWriteables() {
         // arithmetic
         entries.add(new Entry(Processor.class, BinaryArithmeticProcessor.NAME, BinaryArithmeticProcessor::new));
         entries.add(new Entry(Processor.class, UnaryArithmeticProcessor.NAME, UnaryArithmeticProcessor::new));
-        entries.add(new Entry(Processor.class, BinaryMathProcessor.NAME, BinaryMathProcessor::new));
         // comparators
         entries.add(new Entry(Processor.class, BinaryComparisonProcessor.NAME, BinaryComparisonProcessor::new));
         entries.add(new Entry(Processor.class, InProcessor.NAME, InProcessor::new));
@@ -82,6 +82,8 @@ public static List<NamedWriteableRegistry.Entry> getNamedWriteables() {
         entries.add(new Entry(Processor.class, NonIsoDateTimeProcessor.NAME, NonIsoDateTimeProcessor::new));
         entries.add(new Entry(Processor.class, QuarterProcessor.NAME, QuarterProcessor::new));
         // math
+        entries.add(new Entry(Processor.class, BinaryMathProcessor.NAME, BinaryMathProcessor::new));
+        entries.add(new Entry(Processor.class, BinaryOptionalMathProcessor.NAME, BinaryOptionalMathProcessor::new));
         entries.add(new Entry(Processor.class, MathProcessor.NAME, MathProcessor::new));
         // string
         entries.add(new Entry(Processor.class, StringProcessor.NAME, StringProcessor::new));

x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/math/BinaryMathProcessor.java

@@ -25,26 +25,7 @@ public enum BinaryMathOperation implements BiFunction<Number, Number, Number> {
 
         ATAN2((l, r) -> Math.atan2(l.doubleValue(), r.doubleValue())),
         MOD(Arithmetics::mod),
-        POWER((l, r) -> Math.pow(l.doubleValue(), r.doubleValue())),
-        ROUND((l, r) -> {
-            if (r instanceof Float || r instanceof Double) {
-                throw new SqlIllegalArgumentException("An integer number is required; received [{}] as second parameter", r);
-            }
-
-            double tenAtScale = Math.pow(10., r.longValue());
-            double middleResult = l.doubleValue() * tenAtScale;
-            int sign = middleResult > 0 ? 1 : -1;
-            return Math.round(Math.abs(middleResult)) / tenAtScale * sign;
-        }),
-        TRUNCATE((l, r) -> {
-            if (r instanceof Float || r instanceof Double) {
-                throw new SqlIllegalArgumentException("An integer number is required; received [{}] as second parameter", r);
-            }
-
-            double tenAtScale = Math.pow(10., r.longValue());
-            double g = l.doubleValue() * tenAtScale;
-            return (((l.doubleValue() < 0) ? Math.ceil(g) : Math.floor(g)) / tenAtScale);
-        });
+        POWER((l, r) -> Math.pow(l.doubleValue(), r.doubleValue()));
 
         private final BiFunction<Number, Number, Number> process;
 
@@ -79,7 +60,7 @@ public String getWriteableName() {
     @Override
     protected void checkParameter(Object param) {
         if (!(param instanceof Number)) {
-            throw new SqlIllegalArgumentException("A number is required; received {}", param);
+            throw new SqlIllegalArgumentException("A number is required; received [{}]", param);
         }
     }
 }

x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/math/BinaryNumericFunction.java

@@ -69,6 +69,6 @@ public boolean equals(Object obj) {
         BinaryNumericFunction other = (BinaryNumericFunction) obj;
         return Objects.equals(other.left(), left())
             && Objects.equals(other.right(), right())
-                && Objects.equals(other.operation, operation);
+            && Objects.equals(other.operation, operation);
     }
 }

x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/math/BinaryOptionalMathPipe.java

@@ -0,0 +1,115 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License;
+ * you may not use this file except in compliance with the Elastic License.
+ */
+
+package org.elasticsearch.xpack.sql.expression.function.scalar.math;
+
+import org.elasticsearch.xpack.sql.execution.search.SqlSourceBuilder;
+import org.elasticsearch.xpack.sql.expression.Expression;
+import org.elasticsearch.xpack.sql.expression.function.scalar.math.BinaryOptionalMathProcessor.BinaryOptionalMathOperation;
+import org.elasticsearch.xpack.sql.expression.gen.pipeline.Pipe;
+import org.elasticsearch.xpack.sql.tree.NodeInfo;
+import org.elasticsearch.xpack.sql.tree.Location;
+
+import java.util.Arrays;
+import java.util.List;
+import java.util.Objects;
+
+public class BinaryOptionalMathPipe extends Pipe {
+
+    private final Pipe left, right;
+    private final BinaryOptionalMathOperation operation;
+
+    public BinaryOptionalMathPipe(Location location, Expression expression, Pipe left, Pipe right, BinaryOptionalMathOperation operation) {
+        super(location, expression, right == null ? Arrays.asList(left) : Arrays.asList(left, right));
+        this.left = left;
+        this.right = right;
+        this.operation = operation;
+    }
+
+    @Override
+    public final Pipe replaceChildren(List<Pipe> newChildren) {
+        int childrenSize = newChildren.size();
+        if (childrenSize > 2 || childrenSize < 1) {
+            throw new IllegalArgumentException("expected [1 or 2] children but received [" + newChildren.size() + "]");
+        }
+        return replaceChildren(newChildren.get(0), childrenSize == 1 ? null : newChildren.get(1));
+    }
+
+    @Override
+    public final Pipe resolveAttributes(AttributeResolver resolver) {
+        Pipe newLeft = left.resolveAttributes(resolver);
+        Pipe newRight = right == null ? right : right.resolveAttributes(resolver);
+        if (newLeft == left && newRight == right) {
+            return this;
+        }
+        return replaceChildren(newLeft, newRight);
+    }
+
+    @Override
+    public boolean supportedByAggsOnlyQuery() {
+        return right == null ? left.supportedByAggsOnlyQuery() : left.supportedByAggsOnlyQuery() || right.supportedByAggsOnlyQuery();
+    }
+
+    @Override
+    public boolean resolved() {
+        return left.resolved() && (right == null || right.resolved());
+    }
+
+    protected Pipe replaceChildren(Pipe newLeft, Pipe newRight) {
+        return new BinaryOptionalMathPipe(location(), expression(), newLeft, newRight, operation);
+    }
+
+    @Override
+    public final void collectFields(SqlSourceBuilder sourceBuilder) {
+        left.collectFields(sourceBuilder);
+        if (right != null) {
+            right.collectFields(sourceBuilder);
+        }
+    }
+
+    @Override
+    protected NodeInfo<BinaryOptionalMathPipe> info() {
+        return NodeInfo.create(this, BinaryOptionalMathPipe::new, expression(), left, right, operation);
+    }
+
+    @Override
+    public BinaryOptionalMathProcessor asProcessor() {
+        return new BinaryOptionalMathProcessor(left.asProcessor(), right == null ? null : right.asProcessor(), operation);
+    }
+    
+    public Pipe right() {
+        return right;
+    }
+    
+    public Pipe left() {
+        return left;
+    }
+    
+    public BinaryOptionalMathOperation operation() {
+        return operation;
+    }
+
+    @Override
+    public int hashCode() {
+        return Objects.hash(left, right, operation);
+    }
+
+    @Override
+    public boolean equals(Object obj) {
+        if (this == obj) {
+            return true;
+        }
+
+        if (obj == null || getClass() != obj.getClass()) {
+            return false;
+        }
+
+        BinaryOptionalMathPipe other = (BinaryOptionalMathPipe) obj;
+        return Objects.equals(left, other.left) 
+                && Objects.equals(right, other.right)
+                && Objects.equals(operation, other.operation);
+    }
+}