mihaibudiu commented on code in PR #3589:
URL: https://github.com/apache/calcite/pull/3589#discussion_r1506378183
##########
core/src/main/java/org/apache/calcite/adapter/enumerable/EnumUtils.java:
##########
@@ -406,11 +407,24 @@ public static Expression convert(Expression operand, Type
fromType,
if (toPrimitive != null) {
if (fromPrimitive != null) {
// E.g. from "float" to "double"
- return Expressions.convert_(
+ if (toPrimitive == Primitive.BOOLEAN) {
+ return Expressions.convert_(
+ operand, toPrimitive.getPrimitiveClass());
+ }
+ return Expressions.convertChecked(
operand, toPrimitive.getPrimitiveClass());
}
- if (fromNumber || fromBox == Primitive.CHAR) {
- // Generate "x.shortValue()".
+ if (fromType == BigDecimal.class && toPrimitive.isFixedNumeric()) {
+ // Conversion from decimal to an exact type
+ ConstantExpression zero = Expressions.constant(0);
+ // Elsewhere Calcite uses this rounding mode implicitly, so we have to
be consistent.
+ // E.g., this is the rounding mode used by BigDecimal.longValue().
+ Expression rounding = Expressions.constant(RoundingMode.DOWN);
+ // rounded = operand.setScale(0, RoundingMode.DOWN);
+ Expression rounded = Expressions.call(operand, "setScale", zero,
rounding);
+ // return rounded.to*ValueExact()
+ return Expressions.unboxExact(rounded, toPrimitive);
Review Comment:
conversion from decimal to some integer type needs rounding, here we use the
DOWN rounding, which is implicitly used in Calcite wherever
BigDecimal.longValue is used. If a different rounding mode is desired we should
file an issue.
##########
core/src/main/java/org/apache/calcite/rex/RexBuilder.java:
##########
@@ -699,22 +699,23 @@ boolean canRemoveCastFromLiteral(RelDataType toType,
@Nullable Comparable value,
if (SqlTypeName.INT_TYPES.contains(sqlType)) {
final BigDecimal decimalValue = (BigDecimal) value;
- final int s = decimalValue.scale();
- if (s != 0) {
+ try {
+ // Will throw if the value does not fit in a long
+ long l = decimalValue.longValueExact();
Review Comment:
check if the decimal value will fit in the target type
##########
core/src/main/java/org/apache/calcite/adapter/enumerable/EnumUtils.java:
##########
@@ -406,11 +407,24 @@ public static Expression convert(Expression operand, Type
fromType,
if (toPrimitive != null) {
if (fromPrimitive != null) {
// E.g. from "float" to "double"
- return Expressions.convert_(
+ if (toPrimitive == Primitive.BOOLEAN) {
+ return Expressions.convert_(
+ operand, toPrimitive.getPrimitiveClass());
+ }
+ return Expressions.convertChecked(
Review Comment:
convertChecked is implemented in this PR.
##########
core/src/main/java/org/apache/calcite/sql/SqlNumericLiteral.java:
##########
@@ -99,17 +99,23 @@ public boolean isExact() {
if (isExact) {
int scaleValue = requireNonNull(scale, "scale");
if (0 == scaleValue) {
- BigDecimal bd = getValueNonNull();
- SqlTypeName result;
- long l = bd.longValue();
- if ((l >= Integer.MIN_VALUE) && (l <= Integer.MAX_VALUE)) {
- result = SqlTypeName.INTEGER;
- } else {
- result = SqlTypeName.BIGINT;
+ try {
+ BigDecimal bd = getValueNonNull();
Review Comment:
numeric literals are always represented as Decimal by the parser.
Here we check whether the value can be represented by a narrower type
without information loss.
##########
core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorImpl.java:
##########
@@ -3334,20 +3333,14 @@ private void registerOperandSubQueries(
@Override public void validateLiteral(SqlLiteral literal) {
switch (literal.getTypeName()) {
case DECIMAL:
- // Decimal and long have the same precision (as 64-bit integers), so
- // the unscaled value of a decimal must fit into a long.
-
- // REVIEW jvs 4-Aug-2004: This should probably be calling over to
- // the available calculator implementations to see what they
- // support. For now use ESP instead.
- //
- // jhyde 2006/12/21: I think the limits should be baked into the
- // type system, not dependent on the calculator implementation.
- BigDecimal bd = literal.getValueAs(BigDecimal.class);
- BigInteger unscaled = bd.unscaledValue();
- long longValue = unscaled.longValue();
- if (!BigInteger.valueOf(longValue).equals(unscaled)) {
- // overflow
+ // Accept any decimal value that does not exceed the max
+ // precision of the type system.
+ final RelDataTypeSystem typeSystem = getTypeFactory().getTypeSystem();
+ final int maxPrecision = typeSystem.getMaxNumericPrecision();
+ final BigDecimal bd = literal.getValueAs(BigDecimal.class);
+ final BigDecimal noTrailingZeros = bd.stripTrailingZeros();
+ // If we don't strip trailing zeros we may reject values such as
1.000....0.
+ if (noTrailingZeros.precision() > maxPrecision) {
Review Comment:
this code will accept any literal which fits in the specified precision.
I am not sure whether we have to check the scale too.
--
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: commits-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
