NobiGo commented on code in PR #3764:
URL: https://github.com/apache/calcite/pull/3764#discussion_r1678830535
##########
core/src/main/java/org/apache/calcite/sql/SqlNumericLiteral.java:
##########
@@ -99,15 +99,22 @@ 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();
+ SqlTypeName result;
+ // Will throw if the number cannot
+ // be represented as a long.
+ long l = bd.longValue();
+ if ((l >= Integer.MIN_VALUE) && (l <= Integer.MAX_VALUE)) {
+ result = SqlTypeName.INTEGER;
+ } else {
+ result = SqlTypeName.BIGINT;
+ }
+ return typeFactory.createSqlType(result);
+ } catch (ArithmeticException ex) {
+ // This indicates that the value does
+ // not fit in any integer type. Fallback to DECIMAL.
Review Comment:
Same as above we don't need break line and before `Fallback` there are
extra spaces.
##########
core/src/main/java/org/apache/calcite/sql/SqlNumericLiteral.java:
##########
@@ -99,15 +99,22 @@ 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();
+ SqlTypeName result;
+ // Will throw if the number cannot
Review Comment:
Maybe here does not need a break.
##########
core/src/main/java/org/apache/calcite/adapter/enumerable/EnumUtils.java:
##########
@@ -481,10 +482,23 @@ 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) {
+ 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);
Review Comment:
Keep consistent with other comment, Maybe should start with
```
// Generate
```
##########
core/src/main/java/org/apache/calcite/adapter/enumerable/EnumUtils.java:
##########
@@ -481,10 +482,23 @@ 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) {
Review Comment:
Maybe need to add code comment about Why boolean and other type is different?
--
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]
