rubenada commented on code in PR #3481:
URL: https://github.com/apache/calcite/pull/3481#discussion_r1375882002
##########
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:
I agree with @mihaibudiu , in the absence of clear specs, following the lead
of Postgresql seems a good idea. I see the comment was improved to clarify this
fact 👍
##########
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:
I agree with @mihaibudiu , in the absence of clear specs, following the lead
of Postgresql seems a good idea. I see the comment was improved to clarify this
fact 👍
--
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]
