alex-plekhanov commented on code in PR #11635:
URL: https://github.com/apache/ignite/pull/11635#discussion_r1826571867
##########
modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/integration/DataTypesTest.java:
##########
@@ -43,6 +44,74 @@
* Test SQL data types.
*/
public class DataTypesTest extends AbstractBasicIntegrationTransactionalTest {
+ /** */
+ @Test
+ public void testRoundingOfNumerics() {
+ doTestCoercionOfNumerics(numericsToRound(), false, false);
+ }
+
+ /** */
+ @Test
+ public void testRoundingOfNumericsPrecasted() {
+ doTestCoercionOfNumerics(numericsToRound(), false, true);
+ }
+
+ /** */
+ @Test
+ public void testRoundingOfDynamicNumerics() {
+ doTestCoercionOfNumerics(numericsToRound(), true, false);
+ }
+
+ /** */
+ @Test
+ public void testRoundingOfDynamicNumericsPrecasted() {
+ doTestCoercionOfNumerics(numericsToRound(), true, true);
+ }
+
+ /** @return input type, input value, target type, expected result. */
+ private static List<List<Object>> numericsToRound() {
+ List<List<Object>> lst = new ArrayList<>(50);
+
+ lst.add(F.asList("DECIMAL(5,4)", BigDecimal.valueOf(1.4999d),
"DECIMAL(1)", new BigDecimal(1)));
+ lst.add(F.asList("DECIMAL(5,4)", BigDecimal.valueOf(-1.4999d),
"DECIMAL(1)", new BigDecimal(-1)));
+ lst.add(F.asList("DECIMAL(2,1)", BigDecimal.valueOf(1.5d),
"DECIMAL(1)", new BigDecimal(2)));
+ lst.add(F.asList("DECIMAL(2,1)", BigDecimal.valueOf(-1.5d),
"DECIMAL(1)", new BigDecimal(-2)));
Review Comment:
Lets also add a couple of cases for rounding to not integer value
(`DECIMAL(2, 1)` for example):
```
lst.add(F.asList("DECIMAL(5,4)", BigDecimal.valueOf(1.4999d),
"DECIMAL(2,1)", BigDecimal.valueOf(1.5)));
lst.add(F.asList("DECIMAL(5,4)", BigDecimal.valueOf(-1.4999d),
"DECIMAL(2,1)", BigDecimal.valueOf(-1.5)));
```
##########
modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/util/IgniteMath.java:
##########
@@ -308,17 +297,20 @@ public static short convertToShortExact(long x) {
/** Cast value to {@code short}, throwing an exception if the result
overflows an {@code short}. */
public static short convertToShortExact(double x) {
- if (x > Short.MAX_VALUE || x < Short.MIN_VALUE)
Review Comment:
```
x = round(x);
```
And left the old code?
In this case we don't need arrays SHORT_BOUNDS and other.
##########
modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/integration/DataTypesTest.java:
##########
@@ -43,6 +44,74 @@
* Test SQL data types.
*/
public class DataTypesTest extends AbstractBasicIntegrationTransactionalTest {
+ /** */
+ @Test
+ public void testRoundingOfNumerics() {
+ doTestCoercionOfNumerics(numericsToRound(), false, false);
+ }
+
+ /** */
+ @Test
+ public void testRoundingOfNumericsPrecasted() {
+ doTestCoercionOfNumerics(numericsToRound(), false, true);
+ }
+
+ /** */
+ @Test
+ public void testRoundingOfDynamicNumerics() {
+ doTestCoercionOfNumerics(numericsToRound(), true, false);
+ }
+
+ /** */
+ @Test
+ public void testRoundingOfDynamicNumericsPrecasted() {
+ doTestCoercionOfNumerics(numericsToRound(), true, true);
+ }
+
+ /** @return input type, input value, target type, expected result. */
+ private static List<List<Object>> numericsToRound() {
+ List<List<Object>> lst = new ArrayList<>(50);
+
+ lst.add(F.asList("DECIMAL(5,4)", BigDecimal.valueOf(1.4999d),
"DECIMAL(1)", new BigDecimal(1)));
+ lst.add(F.asList("DECIMAL(5,4)", BigDecimal.valueOf(-1.4999d),
"DECIMAL(1)", new BigDecimal(-1)));
+ lst.add(F.asList("DECIMAL(2,1)", BigDecimal.valueOf(1.5d),
"DECIMAL(1)", new BigDecimal(2)));
+ lst.add(F.asList("DECIMAL(2,1)", BigDecimal.valueOf(-1.5d),
"DECIMAL(1)", new BigDecimal(-2)));
+
+ for (String numTypeName : F.asList("DOUBLE", "FLOAT")) {
+ lst.add(F.asList(numTypeName, floatingVal(1.4999f, numTypeName),
"DECIMAL(1)", new BigDecimal(1)));
+ lst.add(F.asList(numTypeName, floatingVal(-1.4999f, numTypeName),
"DECIMAL(1)", new BigDecimal(-1)));
+ lst.add(F.asList(numTypeName, floatingVal(1.5f, numTypeName),
"DECIMAL(1)", new BigDecimal(2)));
+ lst.add(F.asList(numTypeName, floatingVal(-1.5f, numTypeName),
"DECIMAL(1)", new BigDecimal(-2)));
+
+ lst.add(F.asList(numTypeName, floatingVal(1.4999f, numTypeName),
"BIGINT", 1L));
+ lst.add(F.asList(numTypeName, floatingVal(-1.4999f, numTypeName),
"BIGINT", -1L));
+ lst.add(F.asList(numTypeName, floatingVal(1.5f, numTypeName),
"BIGINT", 2L));
+ lst.add(F.asList(numTypeName, floatingVal(-1.5f, numTypeName),
"BIGINT", -2L));
+
+ lst.add(F.asList(numTypeName, floatingVal(1.4999f, numTypeName),
"INT", 1));
+ lst.add(F.asList(numTypeName, floatingVal(-1.4999f, numTypeName),
"INT", -1));
+ lst.add(F.asList(numTypeName, floatingVal(1.5f, numTypeName),
"INT", 2));
+ lst.add(F.asList(numTypeName, floatingVal(-1.5f, numTypeName),
"INT", -2));
+
+ lst.add(F.asList(numTypeName, floatingVal(1.4999f, numTypeName),
"SMALLINT", (short)1));
+ lst.add(F.asList(numTypeName, floatingVal(-1.4999f, numTypeName),
"SMALLINT", (short)-1));
+ lst.add(F.asList(numTypeName, floatingVal(1.5f, numTypeName),
"SMALLINT", (short)2));
+ lst.add(F.asList(numTypeName, floatingVal(-1.5f, numTypeName),
"SMALLINT", (short)-2));
+
+ lst.add(F.asList(numTypeName, floatingVal(1.4999f, numTypeName),
"TINYINT", (byte)1));
+ lst.add(F.asList(numTypeName, floatingVal(-1.4999f, numTypeName),
"TINYINT", (byte)-1));
+ lst.add(F.asList(numTypeName, floatingVal(1.5f, numTypeName),
"TINYINT", (byte)2));
+ lst.add(F.asList(numTypeName, floatingVal(-1.5f, numTypeName),
"TINYINT", (byte)-2));
Review Comment:
This case is passed now:
```
lst.add(F.asList(numTypeName, floatingVal(127.5f, numTypeName),
"TINYINT", (byte)-128));
```
But should throw an overflow error. The same, I think, for other types.
##########
modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/integration/DataTypesTest.java:
##########
@@ -43,6 +44,74 @@
* Test SQL data types.
*/
public class DataTypesTest extends AbstractBasicIntegrationTransactionalTest {
+ /** */
+ @Test
+ public void testRoundingOfNumerics() {
+ doTestCoercionOfNumerics(numericsToRound(), false, false);
+ }
+
+ /** */
+ @Test
+ public void testRoundingOfNumericsPrecasted() {
+ doTestCoercionOfNumerics(numericsToRound(), false, true);
+ }
+
+ /** */
+ @Test
+ public void testRoundingOfDynamicNumerics() {
+ doTestCoercionOfNumerics(numericsToRound(), true, false);
+ }
+
+ /** */
+ @Test
+ public void testRoundingOfDynamicNumericsPrecasted() {
+ doTestCoercionOfNumerics(numericsToRound(), true, true);
+ }
+
+ /** @return input type, input value, target type, expected result. */
+ private static List<List<Object>> numericsToRound() {
+ List<List<Object>> lst = new ArrayList<>(50);
+
+ lst.add(F.asList("DECIMAL(5,4)", BigDecimal.valueOf(1.4999d),
"DECIMAL(1)", new BigDecimal(1)));
+ lst.add(F.asList("DECIMAL(5,4)", BigDecimal.valueOf(-1.4999d),
"DECIMAL(1)", new BigDecimal(-1)));
+ lst.add(F.asList("DECIMAL(2,1)", BigDecimal.valueOf(1.5d),
"DECIMAL(1)", new BigDecimal(2)));
+ lst.add(F.asList("DECIMAL(2,1)", BigDecimal.valueOf(-1.5d),
"DECIMAL(1)", new BigDecimal(-2)));
+
+ for (String numTypeName : F.asList("DOUBLE", "FLOAT")) {
+ lst.add(F.asList(numTypeName, floatingVal(1.4999f, numTypeName),
"DECIMAL(1)", new BigDecimal(1)));
+ lst.add(F.asList(numTypeName, floatingVal(-1.4999f, numTypeName),
"DECIMAL(1)", new BigDecimal(-1)));
+ lst.add(F.asList(numTypeName, floatingVal(1.5f, numTypeName),
"DECIMAL(1)", new BigDecimal(2)));
+ lst.add(F.asList(numTypeName, floatingVal(-1.5f, numTypeName),
"DECIMAL(1)", new BigDecimal(-2)));
+
+ lst.add(F.asList(numTypeName, floatingVal(1.4999f, numTypeName),
"BIGINT", 1L));
+ lst.add(F.asList(numTypeName, floatingVal(-1.4999f, numTypeName),
"BIGINT", -1L));
+ lst.add(F.asList(numTypeName, floatingVal(1.5f, numTypeName),
"BIGINT", 2L));
+ lst.add(F.asList(numTypeName, floatingVal(-1.5f, numTypeName),
"BIGINT", -2L));
Review Comment:
Lets add something like:
```
lst.add(F.asList(numTypeName,
floatingVal(9223372036854775807.4f, numTypeName), "BIGINT",
9223372036854775807L));
lst.add(F.asList(numTypeName,
floatingVal(-9223372036854775808.4f, numTypeName), "BIGINT",
-9223372036854775808L));
```
##########
modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/util/IgniteMath.java:
##########
@@ -308,17 +297,20 @@ public static short convertToShortExact(long x) {
/** Cast value to {@code short}, throwing an exception if the result
overflows an {@code short}. */
public static short convertToShortExact(double x) {
- if (x > Short.MAX_VALUE || x < Short.MIN_VALUE)
+ if (x < (Double)SHORT_BOUNDS[2] || x > (Double)SHORT_BOUNDS[3])
throw new ArithmeticException(SMALLINT.getName() + " overflow");
- return (short)x;
+ return (short)round(x);
}
/** Cast value to {@code short}, throwing an exception if the result
overflows an {@code short}. */
public static short convertToShortExact(Number x) {
- checkNumberLongBounds(SMALLINT, x);
Review Comment:
Perhaps we can round x, check bounds for long value, convert it to long and
leave convertToShortExact? In this case we don't need checkShortBounds and
other boilerplate methods.
Something like this:
```
x = round(x);
checkNumberLongBounds(SMALLINT, x);
return convertToShortExact(x.longValue());
```
Where `round`:
```
private static Number round(Number x) {
return convertToBigDecimal(x).setScale(0, NUMERIC_ROUNDING_MODE);
}
```
##########
modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/util/IgniteMath.java:
##########
@@ -333,16 +325,142 @@ public static byte convertToByteExact(long x) {
/** Cast value to {@code byte}, throwing an exception if the result
overflows an {@code byte}. */
public static byte convertToByteExact(double x) {
- if (x > Byte.MAX_VALUE || x < Byte.MIN_VALUE)
+ if (x < (Double)BYTE_BOUNDS[2] || x > (Double)BYTE_BOUNDS[3])
throw new ArithmeticException(TINYINT.getName() + " overflow");
- return (byte)x;
+ return (byte)round(x);
}
/** Cast value to {@code byte}, throwing an exception if the result
overflows an {@code byte}. */
public static byte convertToByteExact(Number x) {
- checkNumberLongBounds(TINYINT, x);
+ BigDecimal rounded = checkByteBounds(x);
+
+ if (rounded != null)
+ return rounded.byteValue();
+
+ return convertToBigDecimal(x).setScale(0,
NUMERIC_ROUNDING_MODE).byteValue();
+ }
+
+ /** */
+ public static BigDecimal convertToBigDecimal(Number val) {
+ BigDecimal dec;
+
+ if (val instanceof Float)
+ dec = BigDecimal.valueOf(val.floatValue());
+ else if (val instanceof Double)
+ dec = BigDecimal.valueOf(val.doubleValue());
+ else if (val instanceof BigDecimal)
+ dec = (BigDecimal)val;
+ else if (val instanceof BigInteger)
+ dec = new BigDecimal((BigInteger)val);
+ else
+ dec = BigDecimal.valueOf(val.longValue());
+
+ return dec;
+ }
+
+ /** */
+ @Nullable private static BigDecimal checkLongBounds(Number x) {
+ if (x instanceof BigDecimal) {
+ BigDecimal rounded = ((BigDecimal)x).setScale(0,
NUMERIC_ROUNDING_MODE);
+
+ if (rounded.compareTo((BigDecimal)LONG_BOUNDS[4]) >= 0 &&
rounded.compareTo((BigDecimal)LONG_BOUNDS[5]) <= 0)
+ return rounded;
+ }
+ else if (x instanceof Double) {
+ if (((Double)x).compareTo((Double)LONG_BOUNDS[2]) < 0 &&
((Double)x).compareTo((Double)LONG_BOUNDS[3]) > 0)
+ return null;
+ }
+ else if (x instanceof Float) {
+ if (((Float)x).compareTo((Float)LONG_BOUNDS[0]) < 0 &&
((Float)x).compareTo((Float)LONG_BOUNDS[1]) > 0)
+ return null;
+ }
+ else
+ return null;
+
+ throw new ArithmeticException(BIGINT.getName() + " overflow");
+ }
- return convertToByteExact(x.longValue());
+ /** */
+ @Nullable private static BigDecimal checkIntegerBounds(Number x) {
+ if (x instanceof BigDecimal) {
+ BigDecimal rounded = ((BigDecimal)x).setScale(0,
NUMERIC_ROUNDING_MODE);
+
+ if (rounded.compareTo((BigDecimal)INT_BOUNDS[4]) >= 0 &&
rounded.compareTo((BigDecimal)INT_BOUNDS[5]) <= 0)
+ return rounded;
+ }
+ else if (x instanceof Double) {
+ if (((Double)x).compareTo((Double)INT_BOUNDS[2]) < 0 &&
((Double)x).compareTo((Double)INT_BOUNDS[3]) > 0)
+ return null;
+ }
+ else if (x instanceof Float) {
+ if (((Float)x).compareTo((Float)INT_BOUNDS[0]) < 0 &&
((Float)x).compareTo((Float)INT_BOUNDS[1]) > 0)
+ return null;
+ }
+ else {
+ long longVal = x.longValue();
+
+ if (longVal >= Integer.MIN_VALUE && longVal <= Integer.MAX_VALUE)
+ return null;
+ }
+
+ throw new ArithmeticException(INTEGER.getName() + " overflow");
+ }
+
+ /** */
+ @Nullable private static BigDecimal checkShortBounds(Number x) {
+ if (x instanceof BigDecimal) {
+ BigDecimal rounded = ((BigDecimal)x).setScale(0,
NUMERIC_ROUNDING_MODE);
+
+ if (rounded.compareTo((BigDecimal)SHORT_BOUNDS[4]) >= 0 &&
rounded.compareTo((BigDecimal)SHORT_BOUNDS[5]) <= 0)
+ return rounded;
+ }
+ else if (x instanceof Double) {
+ if (((Double)x).compareTo((Double)SHORT_BOUNDS[2]) < 0 &&
((Double)x).compareTo((Double)SHORT_BOUNDS[3]) > 0)
+ return null;
+ }
+ else if (x instanceof Float) {
+ if (((Float)x).compareTo((Float)SHORT_BOUNDS[0]) < 0 &&
((Float)x).compareTo((Float)SHORT_BOUNDS[1]) > 0)
+ return null;
+ }
+ else {
+ long longVal = x.longValue();
+
+ if (longVal >= Short.MIN_VALUE && longVal <= Short.MAX_VALUE)
+ return null;
+ }
+
+ throw new ArithmeticException(SMALLINT.getName() + " overflow");
+ }
+
+ /** */
+ @Nullable private static BigDecimal checkByteBounds(Number x) {
+ if (x instanceof BigDecimal) {
+ BigDecimal rounded = ((BigDecimal)x).setScale(0,
NUMERIC_ROUNDING_MODE);
+
+ if (rounded.compareTo((BigDecimal)BYTE_BOUNDS[4]) >= 0 &&
rounded.compareTo((BigDecimal)BYTE_BOUNDS[5]) <= 0)
+ return rounded;
+ }
+ else if (x instanceof Double) {
+ if (((Double)x).compareTo((Double)BYTE_BOUNDS[2]) < 0 &&
((Double)x).compareTo((Double)BYTE_BOUNDS[3]) > 0)
+ return null;
+ }
+ else if (x instanceof Float) {
+ if (((Float)x).compareTo((Float)BYTE_BOUNDS[0]) < 0 &&
((Float)x).compareTo((Float)BYTE_BOUNDS[1]) > 0)
+ return null;
+ }
+ else {
+ long longVal = x.longValue();
+
+ if (longVal >= Byte.MIN_VALUE && longVal <= Byte.MAX_VALUE)
+ return null;
+ }
+
+ throw new ArithmeticException(TINYINT.getName() + " overflow");
+ }
+
+ /** */
+ private static double round(double x) {
Review Comment:
Perhaps `round` is not a good name. Becouse it prepares to round, but not do
round itself.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscr...@ignite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
