Add support for long value return for CEIL, CEILING and FLOOR math fu…

…nctions (#1205) * Added long fix for CEIL, CEILING and FLOOR functions using LONG instead of INT for RETURN. Signed-off-by: MitchellGale-BitQuill <[email protected]> * Revert changes to JDBCConnection.java. Signed-off-by: MitchellGale-BitQuill <[email protected]> * Addressed PR comments. Signed-off-by: MitchellGale-BitQuill <[email protected]> * Made fixes to rst files. Signed-off-by: MitchellGale-BitQuill <[email protected]> * Simplify docs according to PR feedback. Signed-off-by: Yury-Fridlyand <[email protected]> Signed-off-by: MitchellGale-BitQuill <[email protected]> Signed-off-by: Yury-Fridlyand <[email protected]> Co-authored-by: Yury-Fridlyand <[email protected]> (cherry picked from commit dc5578a)
opensearch-project · Jan 9, 2023 · 6bd5de7 · 6bd5de7
1 parent 7fa65e7
commit 6bd5de7
Show file tree

Hide file tree

Showing 8 changed files with 195 additions and 67 deletions.
diff --git a/.../src/main/java/org/opensearch/sql/expression/operator/arthmetic/MathematicalFunction.java b/.../src/main/java/org/opensearch/sql/expression/operator/arthmetic/MathematicalFunction.java
@@ -119,16 +119,16 @@ private static DefaultFunctionResolver abs() {
   private static DefaultFunctionResolver ceil() {
     return FunctionDSL.define(BuiltinFunctionName.CEIL.getName(),
         FunctionDSL.impl(
-            FunctionDSL.nullMissingHandling(v -> new ExprIntegerValue(Math.ceil(v.doubleValue()))),
-            INTEGER, DOUBLE)
+            FunctionDSL.nullMissingHandling(v -> new ExprLongValue(Math.ceil(v.doubleValue()))),
+            LONG, DOUBLE)
     );
   }
 
   private static DefaultFunctionResolver ceiling() {
     return FunctionDSL.define(BuiltinFunctionName.CEILING.getName(),
         FunctionDSL.impl(
-            FunctionDSL.nullMissingHandling(v -> new ExprIntegerValue(Math.ceil(v.doubleValue()))),
-            INTEGER, DOUBLE)
+            FunctionDSL.nullMissingHandling(v -> new ExprLongValue(Math.ceil(v.doubleValue()))),
+            LONG, DOUBLE)
     );
   }
 
@@ -204,8 +204,8 @@ private static DefaultFunctionResolver exp() {
   private static DefaultFunctionResolver floor() {
     return FunctionDSL.define(BuiltinFunctionName.FLOOR.getName(),
         FunctionDSL.impl(
-            FunctionDSL.nullMissingHandling(v -> new ExprIntegerValue(Math.floor(v.doubleValue()))),
-            INTEGER, DOUBLE)
+            FunctionDSL.nullMissingHandling(v -> new ExprLongValue(Math.floor(v.doubleValue()))),
+            LONG, DOUBLE)
     );
   }
 

diff --git a/.../test/java/org/opensearch/sql/expression/operator/arthmetic/MathematicalFunctionTest.java b/.../test/java/org/opensearch/sql/expression/operator/arthmetic/MathematicalFunctionTest.java
@@ -191,13 +191,13 @@ public void ceil_int_value(Integer value) {
     FunctionExpression ceil = DSL.ceil(DSL.literal(value));
     assertThat(
         ceil.valueOf(valueEnv()),
-        allOf(hasType(INTEGER), hasValue((int) Math.ceil(value))));
-    assertEquals(String.format("ceil(%s)", value), ceil.toString());
+        allOf(hasType(LONG), hasValue((long) Math.ceil(value))));
+    assertEquals(String.format("ceil(%s)", value.toString()), ceil.toString());
 
     FunctionExpression ceiling = DSL.ceiling(DSL.literal(value));
     assertThat(
-        ceiling.valueOf(valueEnv()), allOf(hasType(INTEGER), hasValue((int) Math.ceil(value))));
-    assertEquals(String.format("ceiling(%s)", value), ceiling.toString());
+        ceiling.valueOf(valueEnv()), allOf(hasType(LONG), hasValue((long) Math.ceil(value))));
+    assertEquals(String.format("ceiling(%s)", value.toString()), ceiling.toString());
   }
 
   /**
@@ -208,13 +208,30 @@ public void ceil_int_value(Integer value) {
   public void ceil_long_value(Long value) {
     FunctionExpression ceil = DSL.ceil(DSL.literal(value));
     assertThat(
-        ceil.valueOf(valueEnv()), allOf(hasType(INTEGER), hasValue((int) Math.ceil(value))));
-    assertEquals(String.format("ceil(%s)", value), ceil.toString());
+        ceil.valueOf(valueEnv()), allOf(hasType(LONG), hasValue((long) Math.ceil(value))));
+    assertEquals(String.format("ceil(%s)", value.toString()), ceil.toString());
 
     FunctionExpression ceiling = DSL.ceiling(DSL.literal(value));
     assertThat(
-        ceiling.valueOf(valueEnv()), allOf(hasType(INTEGER), hasValue((int) Math.ceil(value))));
-    assertEquals(String.format("ceiling(%s)", value), ceiling.toString());
+        ceiling.valueOf(valueEnv()), allOf(hasType(LONG), hasValue((long) Math.ceil(value))));
+    assertEquals(String.format("ceiling(%s)", value.toString()), ceiling.toString());
+  }
+
+  /**
+   * Test ceil/ceiling with long value.
+   */
+  @ParameterizedTest(name = "ceil({0})")
+  @ValueSource(longs = {9223372036854775805L, -9223372036854775805L})
+  public void ceil_long_value_long(Long value) {
+    FunctionExpression ceil = DSL.ceil(DSL.literal(value));
+    assertThat(
+        ceil.valueOf(valueEnv()), allOf(hasType(LONG), hasValue((long) Math.ceil(value))));
+    assertEquals(String.format("ceil(%s)", value.toString()), ceil.toString());
+
+    FunctionExpression ceiling = DSL.ceiling(DSL.literal(value));
+    assertThat(
+        ceiling.valueOf(valueEnv()), allOf(hasType(LONG), hasValue((long) Math.ceil(value))));
+    assertEquals(String.format("ceiling(%s)", value.toString()), ceiling.toString());
   }
 
   /**
@@ -225,13 +242,13 @@ public void ceil_long_value(Long value) {
   public void ceil_float_value(Float value) {
     FunctionExpression ceil = DSL.ceil(DSL.literal(value));
     assertThat(
-        ceil.valueOf(valueEnv()), allOf(hasType(INTEGER), hasValue((int) Math.ceil(value))));
-    assertEquals(String.format("ceil(%s)", value), ceil.toString());
+        ceil.valueOf(valueEnv()), allOf(hasType(LONG), hasValue((long) Math.ceil(value))));
+    assertEquals(String.format("ceil(%s)", value.toString()), ceil.toString());
 
     FunctionExpression ceiling = DSL.ceiling(DSL.literal(value));
     assertThat(
-        ceiling.valueOf(valueEnv()), allOf(hasType(INTEGER), hasValue((int) Math.ceil(value))));
-    assertEquals(String.format("ceiling(%s)", value), ceiling.toString());
+        ceiling.valueOf(valueEnv()), allOf(hasType(LONG), hasValue((long) Math.ceil(value))));
+    assertEquals(String.format("ceiling(%s)", value.toString()), ceiling.toString());
   }
 
   /**
@@ -242,13 +259,13 @@ public void ceil_float_value(Float value) {
   public void ceil_double_value(Double value) {
     FunctionExpression ceil = DSL.ceil(DSL.literal(value));
     assertThat(
-        ceil.valueOf(valueEnv()), allOf(hasType(INTEGER), hasValue((int) Math.ceil(value))));
-    assertEquals(String.format("ceil(%s)", value), ceil.toString());
+        ceil.valueOf(valueEnv()), allOf(hasType(LONG), hasValue((long) Math.ceil(value))));
+    assertEquals(String.format("ceil(%s)", value.toString()), ceil.toString());
 
     FunctionExpression ceiling = DSL.ceiling(DSL.literal(value));
     assertThat(
-        ceiling.valueOf(valueEnv()), allOf(hasType(INTEGER), hasValue((int) Math.ceil(value))));
-    assertEquals(String.format("ceiling(%s)", value), ceiling.toString());
+        ceiling.valueOf(valueEnv()), allOf(hasType(LONG), hasValue((long) Math.ceil(value))));
+    assertEquals(String.format("ceiling(%s)", value.toString()), ceiling.toString());
   }
 
   /**
@@ -257,11 +274,11 @@ public void ceil_double_value(Double value) {
   @Test
   public void ceil_null_value() {
     FunctionExpression ceil = DSL.ceil(DSL.ref(DOUBLE_TYPE_NULL_VALUE_FIELD, DOUBLE));
-    assertEquals(INTEGER, ceil.type());
+    assertEquals(LONG, ceil.type());
     assertTrue(ceil.valueOf(valueEnv()).isNull());
 
     FunctionExpression ceiling = DSL.ceiling(DSL.ref(DOUBLE_TYPE_NULL_VALUE_FIELD, DOUBLE));
-    assertEquals(INTEGER, ceiling.type());
+    assertEquals(LONG, ceiling.type());
     assertTrue(ceiling.valueOf(valueEnv()).isNull());
   }
 
@@ -271,11 +288,11 @@ public void ceil_null_value() {
   @Test
   public void ceil_missing_value() {
     FunctionExpression ceil = DSL.ceil(DSL.ref(DOUBLE_TYPE_MISSING_VALUE_FIELD, DOUBLE));
-    assertEquals(INTEGER, ceil.type());
+    assertEquals(LONG, ceil.type());
     assertTrue(ceil.valueOf(valueEnv()).isMissing());
 
     FunctionExpression ceiling = DSL.ceiling(DSL.ref(DOUBLE_TYPE_MISSING_VALUE_FIELD, DOUBLE));
-    assertEquals(INTEGER, ceiling.type());
+    assertEquals(LONG, ceiling.type());
     assertTrue(ceiling.valueOf(valueEnv()).isMissing());
   }
 
@@ -557,7 +574,7 @@ public void floor_int_value(Integer value) {
     FunctionExpression floor = DSL.floor(DSL.literal(value));
     assertThat(
         floor.valueOf(valueEnv()),
-        allOf(hasType(INTEGER), hasValue((int) Math.floor(value))));
+        allOf(hasType(LONG), hasValue((long) Math.floor(value))));
     assertEquals(String.format("floor(%s)", value.toString()), floor.toString());
   }
 
@@ -570,7 +587,7 @@ public void floor_long_value(Long value) {
     FunctionExpression floor = DSL.floor(DSL.literal(value));
     assertThat(
         floor.valueOf(valueEnv()),
-        allOf(hasType(INTEGER), hasValue((int) Math.floor(value))));
+        allOf(hasType(LONG), hasValue((long) Math.floor(value))));
     assertEquals(String.format("floor(%s)", value.toString()), floor.toString());
   }
 
@@ -583,7 +600,7 @@ public void floor_float_value(Float value) {
     FunctionExpression floor = DSL.floor(DSL.literal(value));
     assertThat(
         floor.valueOf(valueEnv()),
-        allOf(hasType(INTEGER), hasValue((int) Math.floor(value))));
+        allOf(hasType(LONG), hasValue((long) Math.floor(value))));
     assertEquals(String.format("floor(%s)", value.toString()), floor.toString());
   }
 
@@ -596,7 +613,7 @@ public void floor_double_value(Double value) {
     FunctionExpression floor = DSL.floor(DSL.literal(value));
     assertThat(
         floor.valueOf(valueEnv()),
-        allOf(hasType(INTEGER), hasValue((int) Math.floor(value))));
+        allOf(hasType(LONG), hasValue((long) Math.floor(value))));
     assertEquals(String.format("floor(%s)", value.toString()), floor.toString());
   }
 
@@ -606,7 +623,7 @@ public void floor_double_value(Double value) {
   @Test
   public void floor_null_value() {
     FunctionExpression floor = DSL.floor(DSL.ref(DOUBLE_TYPE_NULL_VALUE_FIELD, DOUBLE));
-    assertEquals(INTEGER, floor.type());
+    assertEquals(LONG, floor.type());
     assertTrue(floor.valueOf(valueEnv()).isNull());
   }
 
@@ -616,7 +633,7 @@ public void floor_null_value() {
   @Test
   public void floor_missing_value() {
     FunctionExpression floor = DSL.floor(DSL.ref(DOUBLE_TYPE_MISSING_VALUE_FIELD, DOUBLE));
-    assertEquals(INTEGER, floor.type());
+    assertEquals(LONG, floor.type());
     assertTrue(floor.valueOf(valueEnv()).isMissing());
   }
 

diff --git a/docs/user/dql/functions.rst b/docs/user/dql/functions.rst
@@ -231,12 +231,42 @@ Example::
 CEIL
 ----
 
+An alias for `CEILING`_ function.
+
+
+CEILING
+-------
+
 Description
 >>>>>>>>>>>
 
-Specifications:
+Usage: CEILING(T) takes the ceiling of value T.
+
+Note: `CEIL`_ and CEILING functions have the same implementation & functionality
+
+Limitation: CEILING only works as expected when IEEE 754 double type displays decimal when stored.
+
+Argument type: INTEGER/LONG/FLOAT/DOUBLE
+
+Return type: LONG
+
+Example::
+
+   os> SELECT CEILING(0), CEILING(50.00005), CEILING(-50.00005);
+    fetched rows / total rows = 1/1
+    +--------------+---------------------+----------------------+
+    | CEILING(0)   | CEILING(50.00005)   | CEILING(-50.00005)   |
+    |--------------+---------------------+----------------------|
+    | 0            | 51                  | -50                  |
+    +--------------+---------------------+----------------------+
 
-1. CEIL(NUMBER T) -> T
+   os> SELECT CEILING(3147483647.12345), CEILING(113147483647.12345), CEILING(3147483647.00001);
+    fetched rows / total rows = 1/1
+    +-----------------------------+-------------------------------+-----------------------------+
+    | CEILING(3147483647.12345)   | CEILING(113147483647.12345)   | CEILING(3147483647.00001)   |
+    |-----------------------------+-------------------------------+-----------------------------|
+    | 3147483648                  | 113147483648                  | 3147483648                  |
+    +-----------------------------+-------------------------------+-----------------------------+
 
 
 CONV
@@ -424,10 +454,39 @@ FLOOR
 Description
 >>>>>>>>>>>
 
-Specifications:
+Usage: FLOOR(T) takes the floor of value T.
+
+Limitation: FLOOR only works as expected when IEEE 754 double type displays decimal when stored.
+
+Argument type: INTEGER/LONG/FLOAT/DOUBLE
+
+Return type: LONG
+
+Example::
 
-1. FLOOR(NUMBER T) -> T
+   os> SELECT FLOOR(0), FLOOR(50.00005), FLOOR(-50.00005);
+    fetched rows / total rows = 1/1
+    +------------+-------------------+--------------------+
+    | FLOOR(0)   | FLOOR(50.00005)   | FLOOR(-50.00005)   |
+    |------------+-------------------+--------------------|
+    | 0          | 50                | -51                |
+    +------------+-------------------+--------------------+
 
+   os> SELECT FLOOR(3147483647.12345), FLOOR(113147483647.12345), FLOOR(3147483647.00001);
+    fetched rows / total rows = 1/1
+    +---------------------------+-----------------------------+---------------------------+
+    | FLOOR(3147483647.12345)   | FLOOR(113147483647.12345)   | FLOOR(3147483647.00001)   |
+    |---------------------------+-----------------------------+---------------------------|
+    | 3147483647                | 113147483647                | 3147483647                |
+    +---------------------------+-----------------------------+---------------------------+
+
+    os> SELECT FLOOR(282474973688888.022), FLOOR(9223372036854775807.022), FLOOR(9223372036854775807.0000001);
+    fetched rows / total rows = 1/1
+    +------------------------------+----------------------------------+--------------------------------------+
+    | FLOOR(282474973688888.022)   | FLOOR(9223372036854775807.022)   | FLOOR(9223372036854775807.0000001)   |
+    |------------------------------+----------------------------------+--------------------------------------|
+    | 282474973688888              | 9223372036854775807              | 9223372036854775807                  |
+    +------------------------------+----------------------------------+--------------------------------------+
 
 LN
 --

diff --git a/docs/user/ppl/functions/math.rst b/docs/user/ppl/functions/math.rst
@@ -127,24 +127,42 @@ Example::
 CEIL
 ----
 
+An alias for `CEILING`_ function.
+
+
+CEILING
+-------
+
 Description
 >>>>>>>>>>>
 
-Usage: ceil(x) return the smallest integer value this is greater or equal to x.
+Usage: CEILING(T) takes the ceiling of value T.
+
+Note: `CEIL`_ and CEILING functions have the same implementation & functionality
+
+Limitation: CEILING only works as expected when IEEE 754 double type displays decimal when stored.
 
 Argument type: INTEGER/LONG/FLOAT/DOUBLE
 
-Return type: INTEGER
+Return type: LONG
 
 Example::
 
-    os> source=people | eval `CEIL(2.75)` = CEIL(2.75) | fields `CEIL(2.75)`
+    os> source=people | eval `CEILING(0)` = CEILING(0), `CEILING(50.00005)` = CEILING(50.00005), `CEILING(-50.00005)` = CEILING(-50.00005) | fields `CEILING(0)`, `CEILING(50.00005)`, `CEILING(-50.00005)`
     fetched rows / total rows = 1/1
-    +--------------+
-    | CEIL(2.75)   |
-    |--------------|
-    | 3            |
-    +--------------+
+    +--------------+---------------------+----------------------+
+    | CEILING(0)   | CEILING(50.00005)   | CEILING(-50.00005)   |
+    |--------------+---------------------+----------------------|
+    | 0            | 51                  | -50                  |
+    +--------------+---------------------+----------------------+
+
+    os> source=people | eval `CEILING(3147483647.12345)` = CEILING(3147483647.12345), `CEILING(113147483647.12345)` = CEILING(113147483647.12345), `CEILING(3147483647.00001)` = CEILING(3147483647.00001) | fields `CEILING(3147483647.12345)`, `CEILING(113147483647.12345)`, `CEILING(3147483647.00001)`
+    fetched rows / total rows = 1/1
+    +-----------------------------+-------------------------------+-----------------------------+
+    | CEILING(3147483647.12345)   | CEILING(113147483647.12345)   | CEILING(3147483647.00001)   |
+    |-----------------------------+-------------------------------+-----------------------------|
+    | 3147483648                  | 113147483648                  | 3147483648                  |
+    +-----------------------------+-------------------------------+-----------------------------+
 
 
 CONV
@@ -310,22 +328,39 @@ FLOOR
 Description
 >>>>>>>>>>>
 
-Usage: floor(x) return the largest integer value this is smaller or equal to x.
+Usage: FLOOR(T) takes the floor of value T.
 
-Argument type: INTEGER/LONG/FLOAT/DOUBLE
+Limitation: FLOOR only works as expected when IEEE 754 double type displays decimal when stored.
 
-Return type: INTEGER
+Argument type: a: INTEGER/LONG/FLOAT/DOUBLE
+
+Return type: LONG
 
 Example::
 
-    os> source=people | eval `FLOOR(2.75)` = FLOOR(2.75) | fields `FLOOR(2.75)`
+    os> source=people | eval `FLOOR(0)` = FLOOR(0), `FLOOR(50.00005)` = FLOOR(50.00005), `FLOOR(-50.00005)` = FLOOR(-50.00005) | fields `FLOOR(0)`, `FLOOR(50.00005)`, `FLOOR(-50.00005)`
     fetched rows / total rows = 1/1
-    +---------------+
-    | FLOOR(2.75)   |
-    |---------------|
-    | 2             |
-    +---------------+
+    +------------+-------------------+--------------------+
+    | FLOOR(0)   | FLOOR(50.00005)   | FLOOR(-50.00005)   |
+    |------------+-------------------+--------------------|
+    | 0          | 50                | -51                |
+    +------------+-------------------+--------------------+
 
+    os> source=people | eval `FLOOR(3147483647.12345)` = FLOOR(3147483647.12345), `FLOOR(113147483647.12345)` = FLOOR(113147483647.12345), `FLOOR(3147483647.00001)` = FLOOR(3147483647.00001) | fields `FLOOR(3147483647.12345)`, `FLOOR(113147483647.12345)`, `FLOOR(3147483647.00001)`
+    fetched rows / total rows = 1/1
+    +---------------------------+-----------------------------+---------------------------+
+    | FLOOR(3147483647.12345)   | FLOOR(113147483647.12345)   | FLOOR(3147483647.00001)   |
+    |---------------------------+-----------------------------+---------------------------|
+    | 3147483647                | 113147483647                | 3147483647                |
+    +---------------------------+-----------------------------+---------------------------+
+
+    os> source=people | eval `FLOOR(282474973688888.022)` = FLOOR(282474973688888.022), `FLOOR(9223372036854775807.022)` = FLOOR(9223372036854775807.022), `FLOOR(9223372036854775807.0000001)` = FLOOR(9223372036854775807.0000001) | fields `FLOOR(282474973688888.022)`, `FLOOR(9223372036854775807.022)`, `FLOOR(9223372036854775807.0000001)`
+    fetched rows / total rows = 1/1
+    +------------------------------+----------------------------------+--------------------------------------+
+    | FLOOR(282474973688888.022)   | FLOOR(9223372036854775807.022)   | FLOOR(9223372036854775807.0000001)   |
+    |------------------------------+----------------------------------+--------------------------------------|
+    | 282474973688888              | 9223372036854775807              | 9223372036854775807                  |
+    +------------------------------+----------------------------------+--------------------------------------+
 
 LN
 --

diff --git a/integ-test/src/test/java/org/opensearch/sql/legacy/TypeInformationIT.java b/integ-test/src/test/java/org/opensearch/sql/legacy/TypeInformationIT.java
@@ -40,7 +40,7 @@ public void testCeilWithLongFieldReturnsLong() {
         executeJdbcRequest("SELECT CEIL(balance) FROM " + TestsConstants.TEST_INDEX_ACCOUNT +
             " ORDER BY balance LIMIT 5");
 
-    verifySchema(response, schema("CEIL(balance)", null, "integer"));
+    verifySchema(response, schema("CEIL(balance)", null, "long"));
   }
 
   /*