diff --git a/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/LogicalRelImplementor.java b/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/LogicalRelImplementor.java index 64bcc6e6526c5..b5c1c06f1ce81 100644 --- a/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/LogicalRelImplementor.java +++ b/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; } } diff --git a/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/prepare/IgniteSqlValidator.java b/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/prepare/IgniteSqlValidator.java index cd044a629bd4b..22d9be1f083dd 100644 --- a/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/prepare/IgniteSqlValidator.java +++ b/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)); } } diff --git a/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/type/IgniteTypeSystem.java b/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/type/IgniteTypeSystem.java index a0500966cf6ec..8161f63ba111d 100644 --- a/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/type/IgniteTypeSystem.java +++ b/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; + } } diff --git a/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/util/IgniteMath.java b/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/util/IgniteMath.java index 020b747a45a5e..1aeaf2716fef9 100644 --- a/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/util/IgniteMath.java +++ b/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); } } diff --git a/modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/integration/DynamicParametersIntegrationTest.java b/modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/integration/DynamicParametersIntegrationTest.java index 0b68a9773acd4..45f6db66f08b4 100644 --- a/modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/integration/DynamicParametersIntegrationTest.java +++ b/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() { diff --git a/modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/integration/LimitOffsetIntegrationTest.java b/modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/integration/LimitOffsetIntegrationTest.java index 6beccead71248..04eeb5e543e70 100644 --- a/modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/integration/LimitOffsetIntegrationTest.java +++ b/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(); + } + /** * */ diff --git a/modules/calcite/src/test/sql/order/test_limit.test b/modules/calcite/src/test/sql/order/test_limit.test index 4cdfe041e35f6..00ce8f9822d5e 100644 --- a/modules/calcite/src/test/sql/order/test_limit.test +++ b/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 diff --git a/modules/calcite/src/test/sql/types/decimal/test_decimal.test b/modules/calcite/src/test/sql/types/decimal/test_decimal.test index d0733c8f1752a..c3cb24fcf17b0 100644 --- a/modules/calcite/src/test/sql/types/decimal/test_decimal.test +++ b/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 diff --git a/modules/calcite/src/test/sql/types/decimal/test_decimal_ops.test b/modules/calcite/src/test/sql/types/decimal/test_decimal_ops.test index 94ab2cd0ceb2f..86c529e269a8f 100644 --- a/modules/calcite/src/test/sql/types/decimal/test_decimal_ops.test +++ b/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 \ No newline at end of file +1.0