NobiGo commented on code in PR #4066:
URL: https://github.com/apache/calcite/pull/4066#discussion_r1859778639
##########
core/src/main/java/org/apache/calcite/sql2rel/ConvertToChecked.java:
##########
@@ -50,13 +51,18 @@ public ConvertToChecked(RexBuilder builder) {
* Visitor which rewrites an expression tree such that all
* arithmetic operations that produce numeric values use checked arithmetic.
*/
- static class ConvertRexToChecked extends RexShuttle {
+ class ConvertRexToChecked extends RexShuttle {
private final RexBuilder builder;
ConvertRexToChecked(RexBuilder builder) {
this.builder = builder;
}
+ @Override public RexNode visitSubQuery(RexSubQuery subQuery) {
+ RelNode result = subQuery.rel.accept(ConvertToChecked.this);
Review Comment:
SubQuery has operands that need to handle? maybe SQL like:
```
select count(*) as c
from "scott".emp as e
where -CAST(-32768 AS SMALLINT) not in (
select deptno
from dept
where dname = e.ename)
```
`-CAST(-32768 AS SMALLINT)` will be the Subquery's operands.
##########
core/src/main/java/org/apache/calcite/sql2rel/ConvertToChecked.java:
##########
@@ -92,8 +98,7 @@ static class ConvertRexToChecked extends RexShuttle {
result = call;
}
return builder.makeCast(call.getType(), result);
- } else if (!SqlTypeName.EXACT_TYPES.contains(resultType)
- || (resultType == SqlTypeName.DECIMAL)) {
Review Comment:
Is this a related change?
##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -3297,6 +3297,27 @@ static void checkOverlaps(OverlapChecker c) {
DECIMAL_OVERFLOW, true);
}
+ /** Test case for <a
href="https://issues.apache.org/jira/browse/CALCITE-6706";>[CALCITE-6706]
+ * Checked arithmetic does not take effect in subqueries</a>. */
+ @Test void testCastOverflow() {
+ final SqlOperatorFixture f =
fixture().withConformance(SqlConformanceEnum.BIG_QUERY);
+ f.checkFails("SELECT -CAST(-32768 AS SMALLINT)",
+ ".*Value 32768 does not fit in a SMALLINT", true);
+ f.checkFails("SELECT CAST(32768 AS SMALLINT)",
+ "Value 32768 out of range", true);
+ f.checkFails("SELECT -CAST(32768 AS SMALLINT)",
+ "Value 32768 out of range", true);
+ final SqlOperatorFixture f0 = fixture();
+ // This query does not fail if checked arithmetic is not used
+ f0.checkScalar("SELECT -CAST(-32768 AS SMALLINT)",
+ "-32768", "SMALLINT");
+ // The last two queries should fail in any conformance level
Review Comment:
Because `32768 AS SMALLINT` is an illegal expression? Can we add a statement
to explain that the out-of-bounds are caused by the parameter type, not the
final result type?
--
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]
