lowka commented on code in PR #2786:
URL: https://github.com/apache/ignite-3/pull/2786#discussion_r1381298908
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/prepare/IgniteSqlValidator.java:
##########
@@ -479,6 +487,61 @@ private void checkTypesInteroperability(SqlValidatorScope
scope, SqlNode expr) {
throw
SqlUtil.newContextException(expr.getParserPosition(), ex);
}
}
+
+ literalCanFitType(expr, returnType);
+ }
+ }
+
+ /** Check literal can fit to declared exact numeric type, work only for
single literal. */
+ private static void literalCanFitType(SqlNode expr, RelDataType toType) {
+ if (INT_TYPES.contains(toType.getSqlTypeName())) {
+ LiteralExtractor litExtractor = new LiteralExtractor();
+
+ SqlLiteral literal = litExtractor.getLiteral(expr);
+
+ if (literal == null || literal.toValue() == null) {
+ return;
+ }
+
+ long max = Commons.getMaxValue(toType);
+ long min = Commons.getMinValue(toType);
+
+ String litValue = Objects.requireNonNull(literal.toValue());
+
+ try {
+ if (Long.valueOf(litValue).compareTo(max) > 0 ||
Long.valueOf(litValue).compareTo(min) < 0) {
+ throw new SqlException(STMT_PARSE_ERR,
NUMERIC_FIELD_OVERFLOW_ERROR);
+ }
+ } catch (NumberFormatException e) {
+ throw new SqlException(STMT_PARSE_ERR,
NUMERIC_FIELD_OVERFLOW_ERROR, e);
+ }
+ }
+ }
+
+ private static class LiteralExtractor extends SqlBasicVisitor<SqlNode> {
+ private @Nullable SqlLiteral extracted = null;
+
+ private @Nullable SqlLiteral getLiteral(SqlNode expr) {
+ try {
+ expr.accept(this);
+ } catch (Util.FoundOne e) {
+ Util.swallow(e, null);
+ }
+ return extracted;
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public SqlNode visit(SqlLiteral literal) {
+ extracted = extracted != null ? null : literal;
+ return literal;
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public SqlNode visit(SqlDynamicParam param) {
+ extracted = null;
+ throw new Util.FoundOne(param);
Review Comment:
We do not care about the param here. Perhaps we can throw
`Util.FoundOne.NULL` instead?
##########
modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/planner/ImplicitCastsTest.java:
##########
@@ -346,18 +341,14 @@ public Stream<DynamicTest> testUpdate() {
checkStatement()
.table("t1", "c1", NativeTypes.INT32)
- .sql("UPDATE t1 SET c1 = 'abc'")
- .project("$t0", "CAST(_UTF-8'abc'):INTEGER NOT NULL"),
Review Comment:
I think this test case is either no longer necessary or should be updated to
reflect that fact that numeric literals are checked during the validation
phase.
##########
modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/planner/ImplicitCastsTest.java:
##########
@@ -346,18 +341,14 @@ public Stream<DynamicTest> testUpdate() {
checkStatement()
.table("t1", "c1", NativeTypes.INT32)
- .sql("UPDATE t1 SET c1 = 'abc'")
- .project("$t0", "CAST(_UTF-8'abc'):INTEGER NOT NULL"),
Review Comment:
The same could be applied to the several test cases for `SELECT IN`
--
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]
