lowka commented on code in PR #2786:
URL: https://github.com/apache/ignite-3/pull/2786#discussion_r1390659922
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/prepare/IgniteSqlToRelConvertor.java:
##########
@@ -83,6 +87,28 @@ public class IgniteSqlToRelConvertor extends
SqlToRelConverter {
}
}
+ @Override
+ protected RexNode convertExtendedExpression(
Review Comment:
This won't fix problem with BIGINT literals and this code should be removed.
See comments below.
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/prepare/IgniteTypeCoercion.java:
##########
@@ -281,6 +287,10 @@ protected boolean needToCast(SqlValidatorScope scope,
SqlNode node, RelDataType
return TypeUtils.customDataTypeNeedCast(typeFactory, fromType,
toType);
}
+ if (fromType.getSqlTypeName() == SqlTypeName.ROW) {
Review Comment:
It seems there is a bug in handling of Scalar subqueries in DML operations
and this is not the way to fix it because this makes a cast from
`RecordType(some_type)` to `some_type` legal, when it is obviously not the
case.
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/prepare/IgniteSqlValidator.java:
##########
@@ -479,6 +522,64 @@ private void checkTypesInteroperability(SqlValidatorScope
scope, SqlNode expr) {
throw
SqlUtil.newContextException(expr.getParserPosition(), ex);
}
}
+
+ if (castOp) {
+ literalCanFitType(expr, returnType);
+ }
+ }
+ }
+
+ /** Check literal can fit to declared exact numeric type, work only for
single literal. */
+ private void literalCanFitType(SqlNode expr, RelDataType toType) {
+ if (INT_TYPES.contains(toType.getSqlTypeName())) {
+ SqlLiteral literal = Objects.requireNonNullElseGet(litExtractor,
LiteralExtractor::new).getLiteral(expr);
+
+ if (literal == null || literal.toValue() == null) {
+ return;
+ }
+
+ long max = Commons.getMaxValue(toType);
Review Comment:
I think it is better to use calcite's `SqlTypeName::getLimit` here see
`RexUtils::toSaturatedValue`.
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/prepare/IgniteTypeCoercion.java:
##########
@@ -271,6 +271,12 @@ protected boolean needToCast(SqlValidatorScope scope,
SqlNode node, RelDataType
if (fromType == null) {
return false;
}
+
+ if (fromType.getSqlTypeName() == SqlTypeName.BIGINT &&
toType.getSqlTypeName() == SqlTypeName.BIGINT) {
Review Comment:
As mentioned earlier this is not necessary as well because it does not fix a
problem with BIGINT literals.
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/util/Commons.java:
##########
@@ -861,4 +864,46 @@ public static SqlQueryType getQueryType(SqlNode sqlNode) {
return null;
}
}
+
+ /** Returns the minimum unscaled value of a numeric type.
+ *
+ * @param type a numeric type
+ */
+ public static long getMinValue(RelDataType type) {
+ SqlTypeName typeName = type.getSqlTypeName();
+ switch (typeName) {
+ case TINYINT:
+ return Byte.MIN_VALUE;
+ case SMALLINT:
+ return Short.MIN_VALUE;
+ case INTEGER:
+ return Integer.MIN_VALUE;
+ case BIGINT:
+ case DECIMAL:
+ return
NumberUtil.getMinUnscaled(type.getPrecision()).longValue();
+ default:
+ throw new AssertionError("getMinValue(" + typeName + ")");
+ }
+ }
+
+ /** Returns the maximum unscaled value of a numeric type.
+ *
+ * @param type a numeric type
+ */
+ public static long getMaxValue(RelDataType type) {
Review Comment:
Both this and `getMinValue` can be removed in favour of calcite's
`SqlTypeName::getLimit`
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/prepare/IgniteSqlValidator.java:
##########
@@ -178,6 +194,33 @@ public void validateUpdate(SqlUpdate call) {
syncSelectList(select, call);
}
+ /** {@inheritDoc} */
+ @Override
+ protected void checkTypeAssignment(
+ SqlValidatorScope sourceScope,
+ SqlValidatorTable table,
+ RelDataType sourceRowType,
+ RelDataType targetRowType,
+ SqlNode query
+ ) {
+ boolean coerced = false;
+
+ if (config().typeCoercionEnabled()) {
+ if (SqlTypeUtil.equalAsStructSansNullability(typeFactory,
Review Comment:
Special casing for BIGINT does not fix the problem with BIGINT literals in
apache calcite (there is a bug in calcite). Add this code should be removed.
--
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]
