From 45c0834daf75ddd5e3c13f0a1057e3718cdaddf9 Mon Sep 17 00:00:00 2001 From: Andrei Stefan Date: Wed, 27 Nov 2019 14:48:08 +0200 Subject: [PATCH] Fix NULL handling for FLOOR and CEIL math functions --- .../sql/qa/src/main/resources/math.sql-spec | 6 ++++++ .../expression/function/scalar/math/Ceil.java | 6 +++++- .../expression/function/scalar/math/Floor.java | 6 +++++- .../math/MathFunctionProcessorTests.java | 18 ++++++++++++++++++ 4 files changed, 34 insertions(+), 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 index 96ffa773aabaa..c6adaf9523ad0 100644 --- 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/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/math/Ceil.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/math/Ceil.java index 556f53918d892..5c9438c677221 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/math/Ceil.java +++ b/x-pack/plugin/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/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/math/Floor.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/math/Floor.java index 03d6606b0e9fd..a77a4e497d31a 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/math/Floor.java +++ b/x-pack/plugin/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/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/expression/function/scalar/math/MathFunctionProcessorTests.java b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/expression/function/scalar/math/MathFunctionProcessorTests.java index 9ff32c5a05741..579ca5f056d63 100644 --- a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/expression/function/scalar/math/MathFunctionProcessorTests.java +++ b/x-pack/plugin/sql/src/test/java/org/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)); + } }