IGNITE-28896 SQL Calcite: Adjust fractional value handling in FETCH, OFFSET, and LIMIT

modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/LogicalRelImplementor.java

@@ -17,6 +17,8 @@
 
 package org.apache.ignite.internal.processors.query.calcite.exec;
 
+import java.math.BigDecimal;
+import java.math.RoundingMode;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Comparator;
@@ -571,7 +573,7 @@ private boolean hasExchange(RelNode rel) {
                 ctx,
                 rowType,
                 idxBndRel.first() ? cmp : cmp.reversed(),
-                0,
+                SortNode.OFFSET_DEFAULT,
                 1
             );
 
@@ -1076,21 +1078,17 @@ private long validateAndGetFetchOffsetParams(RexNode node, String op) {
                 IgniteQueryErrorCode.UNEXPECTED_ELEMENT_TYPE);
         }
 
-        long paramAsLong;
-
         try {
-            paramAsLong = IgniteMath.convertToLongExact((Number)param);
+            BigDecimal paramAsDecimal = IgniteMath.convertToBigDecimal((Number)param);
+
+            if (paramAsDecimal.signum() < 0)
+                throw new IllegalArgumentException("Negative value for " + op);
+
+            return IgniteMath.convertToLongExact(paramAsDecimal, RoundingMode.DOWN);
         }
         catch (RuntimeException ex) {
             throw new IgniteSQLException(IgniteResource.INSTANCE.illegalFetchLimit(op).str(),
                 IgniteQueryErrorCode.UNEXPECTED_ELEMENT_TYPE, ex);
         }
-
-        if (paramAsLong < 0) {
-            throw new IgniteSQLException(IgniteResource.INSTANCE.illegalFetchLimit(op).str(),
-                IgniteQueryErrorCode.UNEXPECTED_ELEMENT_TYPE);
-        }
-
-        return paramAsLong;
     }
 }

modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/prepare/IgniteSqlValidator.java

@@ -18,6 +18,7 @@
 package org.apache.ignite.internal.processors.query.calcite.prepare;
 
 import java.math.BigDecimal;
+import java.math.RoundingMode;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.EnumSet;
@@ -300,12 +301,14 @@ private boolean definedDynParam(SqlDynamicParam param) {
     /** */
     private void checkLimitOffset(Number offsetFetchLimit, SqlNode n, String nodeName) {
         try {
-            long res = IgniteMath.convertToLongExact(offsetFetchLimit);
+            BigDecimal val = IgniteMath.convertToBigDecimal(offsetFetchLimit);
 
-            if (res < 0)
-                throw newValidationError(n, IgniteResource.INSTANCE.illegalFetchLimit(nodeName));
+            if (val.signum() < 0)
+                throw new IllegalArgumentException("Negative value for " + nodeName);
+
+            IgniteMath.convertToLongExact(val, RoundingMode.DOWN);
         }
-        catch (ArithmeticException e) {
+        catch (RuntimeException e) {
             throw newValidationError(n, IgniteResource.INSTANCE.illegalFetchLimit(nodeName));
         }
     }

modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/type/IgniteTypeSystem.java

@@ -19,6 +19,7 @@
 
 import java.io.Serializable;
 import java.math.BigDecimal;
+import java.math.RoundingMode;
 import org.apache.calcite.rel.type.RelDataType;
 import org.apache.calcite.rel.type.RelDataTypeFactory;
 import org.apache.calcite.rel.type.RelDataTypeSystem;
@@ -126,4 +127,9 @@ public class IgniteTypeSystem extends RelDataTypeSystemImpl implements Serializa
     @Override public boolean shouldConvertRaggedUnionTypesToVarying() {
         return true;
     }
+
+    /** {@inheritDoc} */
+    @Override public RoundingMode roundingMode() {
+        return RoundingMode.HALF_UP;
+    }
 }

modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/util/IgniteMath.java

@@ -21,6 +21,7 @@
 import java.math.BigInteger;
 import java.math.RoundingMode;
 import org.apache.calcite.sql.type.SqlTypeName;
+import org.apache.ignite.internal.processors.query.calcite.type.IgniteTypeSystem;
 
 import static org.apache.calcite.sql.type.SqlTypeName.BIGINT;
 import static org.apache.calcite.sql.type.SqlTypeName.INTEGER;
@@ -72,7 +73,7 @@ public class IgniteMath {
     private static final double BYTE_MIN_EXT = Byte.MIN_VALUE - 1d;
 
     /** */
-    public static final RoundingMode NUMERIC_ROUNDING_MODE = RoundingMode.HALF_UP;
+    public static final RoundingMode NUMERIC_ROUNDING_MODE = IgniteTypeSystem.INSTANCE.roundingMode();
 
     /** Returns the sum of its arguments, throwing an exception if the result overflows an {@code long}. */
     public static long addExact(long x, long y) {
@@ -267,7 +268,12 @@ public static byte divideExact(byte x, byte y) {
 
     /** Cast value to {@code long}, throwing an exception if the result overflows an {@code long}. */
     public static long convertToLongExact(Number x) {
-        x = round(x);
+        return convertToLongExact(x, NUMERIC_ROUNDING_MODE);
+    }
+
+    /** Cast value to {@code long}, throwing an exception if the result overflows an {@code long}. */
+    public static long convertToLongExact(Number x, RoundingMode roundingMode) {
+        x = round(x, roundingMode);
 
         checkNumberLongBounds(BIGINT, x);
 
@@ -411,11 +417,16 @@ else if (x instanceof Float) {
 
     /** */
     private static double extendToRound(double x) {
-        return x < 0.0d ? x - 0.5d : x + 0.5d;
+        return round(x).doubleValue();
+    }
+
+    /** */
+    private static BigDecimal round(Number x, RoundingMode roundingMode) {
+        return convertToBigDecimal(x).setScale(0, roundingMode);
     }
 
     /** */
     private static BigDecimal round(Number x) {
-        return convertToBigDecimal(x).setScale(0, NUMERIC_ROUNDING_MODE);
+        return round(x, NUMERIC_ROUNDING_MODE);
     }
 }

modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/integration/DynamicParametersIntegrationTest.java

@@ -154,10 +154,6 @@ public void testDynamicParameters() {
         assertQuery("SELECT id FROM person ORDER BY id LIMIT ?").withParams(1D).returns(0).check();
         assertQuery("SELECT id FROM person ORDER BY id LIMIT ?").withParams(1F).returns(0).check();
         assertQuery("SELECT id FROM person ORDER BY id LIMIT ?").withParams(1L).returns(0).check();
-        assertQuery("SELECT id FROM person ORDER BY id LIMIT ?").withParams(1.4).returns(0).check();
-        assertQuery("SELECT id FROM person ORDER BY id LIMIT ?").withParams(1.5).returns(0).returns(1).check();
-        assertQuery("SELECT id FROM person ORDER BY id LIMIT ?").withParams(1.6).returns(0).returns(1).check();
-
         assertQuery("SELECT id FROM person ORDER BY id LIMIT ?").withParams(new BigDecimal(1)).returns(0).check();
 
         assertQuery("SELECT id FROM person WHERE name LIKE ? ORDER BY id LIMIT ?").withParams("I%", 1)
@@ -170,6 +166,56 @@ public void testDynamicParameters() {
             .returns(3).returns(4).check();
     }
 
+    /** */
+    @Test
+    public void testFractionalLimitOffset() {
+        createAndPopulateTable();
+
+        assertQuery("SELECT id FROM person ORDER BY id LIMIT ?").withParams(0.5).resultSize(0).check();
+        assertQuery("SELECT id FROM person ORDER BY id LIMIT ?").withParams(1.4).returns(0).check();
+        assertQuery("SELECT id FROM person ORDER BY id LIMIT ?").withParams(1.6).returns(0).check();
+        assertThrowsSqlException("SELECT id FROM person ORDER BY id LIMIT ?", null, BigDecimal.valueOf(-1.5));
+        assertThrowsSqlException("SELECT id FROM person ORDER BY id LIMIT ?", null, BigDecimal.valueOf(-0.5));
+
+        assertQuery("SELECT id FROM person ORDER BY id FETCH FIRST ? ROWS ONLY")
+            .withParams(BigDecimal.valueOf(0.5))
+            .resultSize(0)
+            .check();
+        assertQuery("SELECT id FROM person ORDER BY id FETCH FIRST ? ROWS ONLY")
+            .withParams(BigDecimal.valueOf(1.3))
+            .returns(0)
+            .check();
+        assertQuery("SELECT id FROM person ORDER BY id FETCH FIRST ? ROWS ONLY")
+            .withParams(BigDecimal.valueOf(1.6))
+            .returns(0)
+            .check();
+        assertThrowsSqlException("SELECT id FROM person ORDER BY id FETCH FIRST ? ROWS ONLY", null, BigDecimal.valueOf(-1.5));
+        assertThrowsSqlException("SELECT id FROM person ORDER BY id FETCH FIRST ? ROWS ONLY", null, BigDecimal.valueOf(-0.5));
+
+        assertQuery("SELECT id FROM person ORDER BY id OFFSET ? ROWS")
+            .withParams(BigDecimal.valueOf(0.5))
+            .returns(0)
+            .returns(1)
+            .returns(2)
+            .returns(3)
+            .returns(4)
+            .check();
+        assertQuery("SELECT id FROM person ORDER BY id OFFSET ? ROWS")
+            .withParams(BigDecimal.valueOf(2.3))
+            .returns(2)
+            .returns(3)
+            .returns(4)
+            .check();
+        assertQuery("SELECT id FROM person ORDER BY id OFFSET ? ROWS")
+            .withParams(BigDecimal.valueOf(2.6))
+            .returns(2)
+            .returns(3)
+            .returns(4)
+            .check();
+        assertThrowsSqlException("SELECT id FROM person ORDER BY id OFFSET ? ROWS", null, BigDecimal.valueOf(-0.5));
+        assertThrowsSqlException("SELECT id FROM person ORDER BY id OFFSET ? ROWS", null, BigDecimal.valueOf(-1.5));
+    }
+
     /** Tests the same query with different type of parameters to cover case with check right plans cache work. **/
     @Test
     public void testWithDifferentParametersTypes() {

modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/integration/LimitOffsetIntegrationTest.java

@@ -93,12 +93,12 @@ public class LimitOffsetIntegrationTest extends AbstractBasicIntegrationTransact
 
     /** */
     @Test
-    public void testNestedLimitOffsetWithUnion() {
-        sql("INSERT into TEST_REPL VALUES (1, 'a'), (2, 'b'), (3, 'c'), (4, 'd')");
+    public void testNestedLimitOffsetWithUnion() throws Exception {
+        fillCache(cacheRepl, 4);
 
-        assertQuery("(SELECT id FROM TEST_REPL WHERE id = 2) UNION ALL " +
+        assertQuery("(SELECT id FROM TEST_REPL WHERE id = 1) UNION ALL " +
             "SELECT id FROM (select id from (SELECT id FROM TEST_REPL OFFSET 2) order by id OFFSET 1)"
-        ).returns(2).returns(4).check();
+        ).returns(1).returns(3).check();
     }
 
     /** Tests correctness of fetch / offset params. */
@@ -121,10 +121,33 @@ public void testInvalidLimitOffset() {
         assertThrows("SELECT * FROM TEST_REPL OFFSET -1 ROWS",
             IgniteSQLException.class, null);
 
+        assertThrowsSqlException("SELECT * FROM TEST_REPL OFFSET -1.5 ROWS", null);
+
+        assertThrowsSqlException("SELECT * FROM TEST_REPL OFFSET -0.5 ROWS", null);
+
         assertThrows("SELECT * FROM TEST_REPL OFFSET 2+1 ROWS",
             IgniteSQLException.class, null);
     }
 
+    /** */
+    @Test
+    public void testFractionalLimitOffset() throws Exception {
+        fillCache(cacheRepl, 4);
+
+        assertQuery("SELECT id FROM TEST_REPL ORDER BY id LIMIT 0.5").check();
+        assertQuery("SELECT id FROM TEST_REPL ORDER BY id LIMIT 1.2").returns(0).check();
+        assertQuery("SELECT id FROM TEST_REPL ORDER BY id LIMIT 1.5").returns(0).check();
+
+        assertQuery("SELECT id FROM TEST_REPL ORDER BY id FETCH FIRST 0.5 ROWS ONLY").check();
+        assertQuery("SELECT id FROM TEST_REPL ORDER BY id FETCH FIRST 1.3 ROWS ONLY").returns(0).check();
+        assertQuery("SELECT id FROM TEST_REPL ORDER BY id FETCH FIRST 1.6 ROWS ONLY").returns(0).check();
+
+        assertQuery("SELECT id FROM TEST_REPL ORDER BY id OFFSET 0.5 ROWS")
+            .returns(0).returns(1).returns(2).returns(3).check();
+        assertQuery("SELECT id FROM TEST_REPL ORDER BY id OFFSET 2.3 ROWS").returns(2).returns(3).check();
+        assertQuery("SELECT id FROM TEST_REPL ORDER BY id OFFSET 2.6 ROWS").returns(2).returns(3).check();
+    }
+
     /**
      *
      */

modules/calcite/src/test/sql/order/test_limit.test

@@ -28,14 +28,12 @@ query I
 SELECT a FROM test ORDER BY a LIMIT 1.5
 ----
 11
-12
 
 # decimal limit
 query I
 SELECT a FROM test ORDER BY a LIMIT 1.6
 ----
 11
-12
 
 # decimal limit
 query I
@@ -54,6 +52,18 @@ SELECT a FROM test ORDER BY a FETCH FIRST 1.2 ROWS ONLY
 ----
 11
 
+# decimal limit
+query I
+SELECT a FROM test ORDER BY a FETCH FIRST 1.5 ROWS ONLY
+----
+11
+
+# decimal limit
+query I
+SELECT a FROM test ORDER BY a FETCH FIRST 1.6 ROWS ONLY
+----
+11
+
 # decimal offset/limit
 query I
 SELECT a FROM test ORDER BY a OFFSET 1.1 ROWS FETCH FIRST 1.1 ROWS ONLY

modules/calcite/src/test/sql/types/decimal/test_decimal.test

@@ -15,7 +15,7 @@ DECIMAL(32767, 0)
 query II
 SELECT '0.1'::DECIMAL::VARCHAR, '922337203685478.758'::DECIMAL::VARCHAR;
 ----
-0	922337203685478
+0	922337203685479
 
 # test basic string conversions
 query II
@@ -27,7 +27,7 @@ SELECT '0.1'::DECIMAL(1,1)::VARCHAR, '922337203685478.758'::DECIMAL(18,3)::VARCH
 query II
 SELECT '-0.1'::DECIMAL::VARCHAR, '-922337203685478.758'::DECIMAL::VARCHAR;
 ----
-0	-922337203685478
+0	-922337203685479
 
 # negative values
 query II

modules/calcite/src/test/sql/types/decimal/test_decimal_ops.test

@@ -154,7 +154,7 @@ SELECT ROUND('100.3'::DECIMAL), ROUND('-127012.3'::DECIMAL)
 query II
 SELECT ROUND('10.5'::DECIMAL), ROUND('-10.5'::DECIMAL)
 ----
-10	-10
+11	-11
 
 query II
 SELECT ROUND('10.5'::DECIMAL(3,1)), ROUND('-10.5'::DECIMAL(3,1))
@@ -252,4 +252,4 @@ SELECT ROUND('1049578239572094512.32415'::DECIMAL(30,10), 0)::VARCHAR,
 query I
 SELECT (SELECT '1.0'::DECIMAL(2,1));
 ----
-1.0
+1.0