Fix NULL handling for FLOOR and CEIL math functions (#49644)

(cherry picked from commit 034f4cf)
elastic · Dec 3, 2019 · 311867e · 311867e
1 parent e14d00c
commit 311867e
Show file tree

Hide file tree

Showing 4 changed files with 34 additions and 2 deletions.
diff --git a/x-pack/plugin/sql/qa/src/main/resources/math.sql-spec b/x-pack/plugin/sql/qa/src/main/resources/math.sql-spec
@@ -17,6 +17,8 @@ SELECT ATAN(emp_no) m, first_name FROM "test_emp" WHERE emp_no < 10010 ORDER BY
 mathCeil
 // H2 returns CEIL as a double despite the value being an integer; we return a long as the other DBs
 SELECT CAST(CEIL(emp_no) AS INT) m, first_name FROM "test_emp" WHERE emp_no < 10010 ORDER BY emp_no;
+mathCeilWithNulls
+SELECT CAST(CEIL(languages) AS INT) m FROM "test_emp" ORDER BY emp_no;
 mathCos
 SELECT COS(emp_no) m, first_name FROM "test_emp" WHERE emp_no < 10010 ORDER BY emp_no;
 mathCosh
@@ -31,6 +33,8 @@ mathExpm1
 SELECT EXP(emp_no) m, first_name FROM "test_emp" WHERE emp_no < 10010 ORDER BY emp_no;
 mathFloor
 SELECT CAST(FLOOR(emp_no) AS INT) m, first_name FROM "test_emp" WHERE emp_no < 10010 ORDER BY emp_no;
+mathFloorWithNulls
+SELECT CAST(FLOOR(languages) AS INT) m FROM "test_emp" ORDER BY emp_no;
 mathLog
 SELECT LOG(emp_no) m, first_name FROM "test_emp" WHERE emp_no < 10010 ORDER BY emp_no;
 mathLog10
@@ -49,6 +53,8 @@ mathSqrt
 SELECT SQRT(emp_no) m, first_name FROM "test_emp" WHERE emp_no < 10010 ORDER BY emp_no;
 mathTan
 SELECT TAN(emp_no) m, first_name FROM "test_emp" WHERE emp_no < 10010 ORDER BY emp_no;
+mathFloorAndCeilWithNullLiteral
+SELECT CAST(FLOOR(CAST(NULL AS DOUBLE)) AS INT) fnull, CAST(CEIL(CAST(NULL AS LONG)) AS INT) cnull, gender FROM "test_emp" ORDER BY emp_no;
 
 //
 // Combined methods

diff --git a/...n/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/math/Ceil.java b/...n/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/math/Ceil.java
@@ -33,7 +33,11 @@ protected Ceil replaceChild(Expression newChild) {
 
     @Override
     public Number fold() {
-        return DataTypeConversion.toInteger((double) super.fold(), dataType());
+        Object result = super.fold();
+        if (result == null) {
+            return null;
+        }
+        return DataTypeConversion.toInteger((double) result, dataType());
     }
 
     @Override

diff --git a/.../sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/math/Floor.java b/.../sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/math/Floor.java
@@ -33,7 +33,11 @@ protected Floor replaceChild(Expression newChild) {
 
     @Override
     public Object fold() {
-        return DataTypeConversion.toInteger((double) super.fold(), dataType());
+        Object result = super.fold();
+        if (result == null) {
+            return null;
+        }
+        return DataTypeConversion.toInteger((double) result, dataType());
     }
 
     @Override

diff --git a/...g/elasticsearch/xpack/sql/expression/function/scalar/math/MathFunctionProcessorTests.java b/...g/elasticsearch/xpack/sql/expression/function/scalar/math/MathFunctionProcessorTests.java
@@ -55,4 +55,22 @@ public void testRandom() {
         assertNotNull(proc.process(null));
         assertNotNull(proc.process(randomLong()));
     }
+
+    public void testFloor() {
+        MathProcessor proc = new MathProcessor(MathOperation.FLOOR);
+        assertNull(proc.process(null));
+        assertNotNull(proc.process(randomLong()));
+        assertEquals(3.0, proc.process(3.3));
+        assertEquals(3.0, proc.process(3.9));
+        assertEquals(-13.0, proc.process(-12.1));
+    }
+
+    public void testCeil() {
+        MathProcessor proc = new MathProcessor(MathOperation.CEIL);
+        assertNull(proc.process(null));
+        assertNotNull(proc.process(randomLong()));
+        assertEquals(4.0, proc.process(3.3));
+        assertEquals(4.0, proc.process(3.9));
+        assertEquals(-12.0, proc.process(-12.1));
+    }
 }