julianhyde commented on code in PR #3917:
URL: https://github.com/apache/calcite/pull/3917#discussion_r1720089643
##########
linq4j/src/main/java/org/apache/calcite/linq4j/tree/Primitive.java:
##########
@@ -477,28 +483,41 @@ static BigDecimal checkOverflow(BigDecimal value, int
precision, int scale) {
return checkOverflow(decimal, precision, scale);
}
+ public @Nullable Object numberValue(Number value) {
+ return numberValue(value, RoundingMode.DOWN);
+ }
/**
Review Comment:
need space between methods
##########
linq4j/src/main/java/org/apache/calcite/linq4j/tree/Primitive.java:
##########
@@ -384,9 +384,15 @@ static void checkRoundedRange(Number value, double min,
double max) {
}
}
+ /** Called from BuiltInMethod.INTEGER_CAST */
Review Comment:
javadoc is same as next method
if methods have the same description, are both required?
##########
core/src/main/java/org/apache/calcite/rex/RexExecutorImpl.java:
##########
@@ -56,11 +56,19 @@
public class RexExecutorImpl implements RexExecutor {
private final DataContext dataContext;
+ private final SqlConformance sqlConformance;
public RexExecutorImpl(DataContext dataContext) {
this.dataContext = dataContext;
+ this.sqlConformance = SqlConformanceEnum.DEFAULT;
Review Comment:
I don't think conformance belongs here. Conformance governs validation
behavior, not runtime behavior.
##########
core/src/test/java/org/apache/calcite/test/CoreQuidemTest.java:
##########
@@ -88,6 +88,33 @@ public static void main(String[] args) throws Exception {
SqlConformanceEnum.MYSQL_5)
.with(CalciteAssert.Config.SCOTT)
.connect();
+ case "scott-default":
+ // Same as "scott", but uses Calcite's default conformance.
+ return CalciteAssert.that()
+ .with(CalciteConnectionProperty.PARSER_FACTORY,
+ ExtensionDdlExecutor.class.getName() + "#PARSER_FACTORY")
+ .with(CalciteConnectionProperty.CONFORMANCE,
+ SqlConformanceEnum.DEFAULT)
+ .with(CalciteAssert.Config.SCOTT)
+ .connect();
+ case "scott-sqlserver2008":
+ // Same as "scott", but uses Calcite's default conformance.
Review Comment:
comment is copy-pasted and identical to other comments
##########
core/src/main/java/org/apache/calcite/sql/validate/SqlConformance.java:
##########
@@ -583,4 +585,20 @@ public interface SqlConformance {
*/
@Experimental
boolean allowLenientCoercion();
+
+ /**
+ * Specifies a rounding behavior for numerical operations capable of
discarding precision.
+ *
+ * <p>Among the built-in conformance levels,
+ * {@link RoundingMode#HALF_UP} in
+ * {@link SqlConformanceEnum#MYSQL_5},
+ * {@link SqlConformanceEnum#ORACLE_10},
+ * {@link SqlConformanceEnum#ORACLE_12};
+ * {@link RoundingMode#DOWN} otherwise.
+ *
+ * <p> {@link org.apache.calcite.sql.SqlDialect.DatabaseProduct#CALCITE}'s
+ * default is {@link RoundingMode#DOWN} and {@link
org.apache.calcite.sql.SqlDialect.DatabaseProduct#POSTGRESQL},
+ * {@link org.apache.calcite.sql.SqlDialect.DatabaseProduct#MYSQL} and
{@link org.apache.calcite.sql.SqlDialect.DatabaseProduct#ORACLE} is {@link
RoundingMode#HALF_UP}.
Review Comment:
line is too long and can be wrapped
##########
core/src/test/resources/sql/misc.iq:
##########
@@ -2658,4 +2658,151 @@ FROM "hr"."emps";
!ok
+# [CALCITE-4838] Add RoundingMode in SqlConformance to document the rounding
mode when cast an approximate numeric to int
+!use scott-mysql
+
+select cast(5.5 as int),
+ cast(3.5 as int),
+ cast(1.6 as int),
+ cast(1.1 as int),
+ cast(1.0 as int),
+ cast(-1.0 as int),
+ cast(-1.1 as int),
+ cast(-1.6 as int),
+ cast(-2.5 as int),
+ cast(15.5 as int)
+from (values 0) as t(x);
++--------+--------+--------+--------+--------+--------+--------+--------+--------+--------+
+| EXPR$0 | EXPR$1 | EXPR$2 | EXPR$3 | EXPR$4 | EXPR$5 | EXPR$6 | EXPR$7 |
EXPR$8 | EXPR$9 |
++--------+--------+--------+--------+--------+--------+--------+--------+--------+--------+
+| 6 | 4 | 2 | 1 | 1 | -1 | -1 | -2 |
-3 | 16 |
++--------+--------+--------+--------+--------+--------+--------+--------+--------+--------+
+(1 row)
+
+!ok
+
+select cast(5.5 as tinyint),
+ cast(3.5 as tinyint),
+ cast(1.6 as tinyint),
+ cast(1.1 as tinyint),
+ cast(1.0 as tinyint),
+ cast(-1.0 as tinyint),
+ cast(-1.1 as tinyint),
+ cast(-1.6 as tinyint),
+ cast(-2.5 as tinyint),
+ cast(15.5 as tinyint)
+from (values 0) as t(x);
++--------+--------+--------+--------+--------+--------+--------+--------+--------+--------+
+| EXPR$0 | EXPR$1 | EXPR$2 | EXPR$3 | EXPR$4 | EXPR$5 | EXPR$6 | EXPR$7 |
EXPR$8 | EXPR$9 |
++--------+--------+--------+--------+--------+--------+--------+--------+--------+--------+
+| 6 | 4 | 2 | 1 | 1 | -1 | -1 | -2 |
-3 | 16 |
++--------+--------+--------+--------+--------+--------+--------+--------+--------+--------+
+(1 row)
+
+!ok
+
+
+select cast(5.5 as smallint),
Review Comment:
for each statement, add comments explaining what rounding mode is being used
##########
core/src/main/java/org/apache/calcite/adapter/enumerable/RexImpTable.java:
##########
@@ -1435,6 +1435,14 @@ public Expression handle(Expression x) {
}
}
+ static Expression getDefaultValue(Type type, RoundingMode roundingMode) {
+ Primitive p = Primitive.of(type);
+ if (p != null) {
+ return Expressions.constant(p.defaultValue, type, roundingMode);
+ }
+ return Expressions.constant(null, type);
+ }
+
static Expression getDefaultValue(Type type) {
Review Comment:
should the method without `roundingMode` be deprecated?
##########
core/src/main/java/org/apache/calcite/rex/RexExecutorImpl.java:
##########
@@ -133,7 +152,7 @@ public static RexExecutable getExecutable(RexBuilder
rexBuilder, List<RexNode> e
try {
code = compile(rexBuilder, constExps, (list, index, storageType) -> {
throw new UnsupportedOperationException();
- });
+ }, sqlConformance);
Review Comment:
code is more readable if the lambda is the last parameter
--
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]
