herunkang2018 commented on code in PR #3481:
URL: https://github.com/apache/calcite/pull/3481#discussion_r1375219714
##########
linq4j/src/main/java/org/apache/calcite/linq4j/tree/Primitive.java:
##########
@@ -367,28 +369,67 @@ public static List<Double> asList(double[] elements) {
}
/**
- * Converts a number into a value of the type
- * specified by this primitive.
+ * Check if a value after rounding falls within a specified range.
+ *
+ * @param value Value to compare.
+ * @param min Minimum value allowed.
+ * @param max Maximum value allowed.
+ */
+ static void checkRoundedRange(Number value, double min, double max) {
+ double dbl = value.doubleValue();
+ // The equivalent of DOWN rounding for BigDecimal
+ dbl = dbl > 0 ? Math.floor(dbl) : Math.ceil(dbl);
+ if (dbl < min || dbl > max) {
+ throw new ArithmeticException("Value " + value + " out of range");
+ }
+ }
+
+ /**
+ * Converts a number into a value of the type specified by this primitive
+ * using the SQL CAST rules. If the value conversion causes loss of
significant digits,
+ * an exception is thrown.
*
* @param value Value to convert.
- * @return The converted value, or null if the
- * conversion cannot be performed.
+ * @return The converted value, or null if the type of the result is
not a number.
*/
public @Nullable Object numberValue(Number value) {
switch (this) {
case BYTE:
+ checkRoundedRange(value, Byte.MIN_VALUE, Byte.MAX_VALUE);
return value.byteValue();
case CHAR:
+ // No overflow checks for char values
Review Comment:
Do we need to check overflow for `CHAR`?
##########
linq4j/src/main/java/org/apache/calcite/linq4j/tree/Primitive.java:
##########
@@ -367,28 +369,67 @@ public static List<Double> asList(double[] elements) {
}
/**
- * Converts a number into a value of the type
- * specified by this primitive.
+ * Check if a value after rounding falls within a specified range.
+ *
+ * @param value Value to compare.
+ * @param min Minimum value allowed.
+ * @param max Maximum value allowed.
+ */
+ static void checkRoundedRange(Number value, double min, double max) {
+ double dbl = value.doubleValue();
+ // The equivalent of DOWN rounding for BigDecimal
+ dbl = dbl > 0 ? Math.floor(dbl) : Math.ceil(dbl);
+ if (dbl < min || dbl > max) {
+ throw new ArithmeticException("Value " + value + " out of range");
+ }
+ }
+
+ /**
+ * Converts a number into a value of the type specified by this primitive
+ * using the SQL CAST rules. If the value conversion causes loss of
significant digits,
+ * an exception is thrown.
*
* @param value Value to convert.
- * @return The converted value, or null if the
- * conversion cannot be performed.
+ * @return The converted value, or null if the type of the result is
not a number.
*/
public @Nullable Object numberValue(Number value) {
switch (this) {
case BYTE:
+ checkRoundedRange(value, Byte.MIN_VALUE, Byte.MAX_VALUE);
return value.byteValue();
case CHAR:
+ // No overflow checks for char values
return (char) value.intValue();
case SHORT:
+ checkRoundedRange(value, Short.MIN_VALUE, Short.MAX_VALUE);
return value.shortValue();
case INT:
+ checkRoundedRange(value, Integer.MIN_VALUE, Integer.MAX_VALUE);
return value.intValue();
case LONG:
- return value.longValue();
+ // The value Long.MAX_VALUE cannot be represented exactly as a double,
+ // so we cannot use checkRoundedRange.
+ if (value instanceof Integer
Review Comment:
minor comment, it would be better to use Byte/Short/Integer/Long order for
more code readability.
##########
linq4j/src/main/java/org/apache/calcite/linq4j/tree/Primitive.java:
##########
@@ -367,28 +369,67 @@ public static List<Double> asList(double[] elements) {
}
/**
- * Converts a number into a value of the type
- * specified by this primitive.
+ * Check if a value after rounding falls within a specified range.
+ *
+ * @param value Value to compare.
+ * @param min Minimum value allowed.
+ * @param max Maximum value allowed.
+ */
+ static void checkRoundedRange(Number value, double min, double max) {
+ double dbl = value.doubleValue();
+ // The equivalent of DOWN rounding for BigDecimal
+ dbl = dbl > 0 ? Math.floor(dbl) : Math.ceil(dbl);
+ if (dbl < min || dbl > max) {
+ throw new ArithmeticException("Value " + value + " out of range");
+ }
+ }
+
+ /**
+ * Converts a number into a value of the type specified by this primitive
+ * using the SQL CAST rules. If the value conversion causes loss of
significant digits,
+ * an exception is thrown.
*
* @param value Value to convert.
- * @return The converted value, or null if the
- * conversion cannot be performed.
+ * @return The converted value, or null if the type of the result is
not a number.
*/
public @Nullable Object numberValue(Number value) {
switch (this) {
case BYTE:
+ checkRoundedRange(value, Byte.MIN_VALUE, Byte.MAX_VALUE);
return value.byteValue();
case CHAR:
+ // No overflow checks for char values
return (char) value.intValue();
case SHORT:
+ checkRoundedRange(value, Short.MIN_VALUE, Short.MAX_VALUE);
return value.shortValue();
case INT:
+ checkRoundedRange(value, Integer.MIN_VALUE, Integer.MAX_VALUE);
return value.intValue();
case LONG:
- return value.longValue();
+ // The value Long.MAX_VALUE cannot be represented exactly as a double,
Review Comment:
I would put the comment when checking the from type is float or double. I
think that will be easier to understand.
##########
site/_docs/reference.md:
##########
@@ -1469,7 +1469,7 @@ Algorithms for implicit conversion are subject to change
across Calcite releases
| Operator syntax | Description
|:----------------------------------------| :----------
-| CAST(value AS type) | Converts a value to a given type
+| CAST(value AS type) | Converts a value to a given type.
Casts between integer types truncate towards 0
Review Comment:
It would be better to remove the extra space.
##########
linq4j/src/main/java/org/apache/calcite/linq4j/tree/Primitive.java:
##########
@@ -367,28 +369,67 @@ public static List<Double> asList(double[] elements) {
}
/**
- * Converts a number into a value of the type
- * specified by this primitive.
+ * Check if a value after rounding falls within a specified range.
+ *
+ * @param value Value to compare.
+ * @param min Minimum value allowed.
+ * @param max Maximum value allowed.
+ */
+ static void checkRoundedRange(Number value, double min, double max) {
+ double dbl = value.doubleValue();
+ // The equivalent of DOWN rounding for BigDecimal
+ dbl = dbl > 0 ? Math.floor(dbl) : Math.ceil(dbl);
+ if (dbl < min || dbl > max) {
+ throw new ArithmeticException("Value " + value + " out of range");
+ }
+ }
+
+ /**
+ * Converts a number into a value of the type specified by this primitive
+ * using the SQL CAST rules. If the value conversion causes loss of
significant digits,
+ * an exception is thrown.
*
* @param value Value to convert.
- * @return The converted value, or null if the
- * conversion cannot be performed.
+ * @return The converted value, or null if the type of the result is
not a number.
*/
public @Nullable Object numberValue(Number value) {
switch (this) {
case BYTE:
+ checkRoundedRange(value, Byte.MIN_VALUE, Byte.MAX_VALUE);
return value.byteValue();
case CHAR:
+ // No overflow checks for char values
return (char) value.intValue();
case SHORT:
+ checkRoundedRange(value, Short.MIN_VALUE, Short.MAX_VALUE);
return value.shortValue();
case INT:
+ checkRoundedRange(value, Integer.MIN_VALUE, Integer.MAX_VALUE);
return value.intValue();
case LONG:
- return value.longValue();
+ // The value Long.MAX_VALUE cannot be represented exactly as a double,
+ // so we cannot use checkRoundedRange.
+ if (value instanceof Integer
+ || value instanceof Short
+ || value instanceof Byte
+ || value instanceof Long) {
+ return value.longValue();
+ }
+ if (value instanceof Float
+ || value instanceof Double) {
+ BigDecimal decimal = BigDecimal.valueOf(value.doubleValue())
+ // Round to an integer
+ .setScale(0, RoundingMode.DOWN);
+ // longValueExact will throw if out of range
Review Comment:
minor comment, longValueExact will throw `ArithmeticException` if out of
range
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
