xgbtxy commented on code in PR #5361:
URL: https://github.com/apache/calcite/pull/5361#discussion_r4372861210
##########
core/src/test/java/org/apache/calcite/test/JdbcTest.java:
##########
@@ -2955,6 +2955,62 @@ private void checkNullableTimestamp(CalciteAssert.Config
config) {
.returns("desc=SameName\n");
}
+ /** Regression test for
+ * <a
href="https://issues.apache.org/jira/browse/CALCITE-7880";>[CALCITE-7880]
+ * Arithmetic on an unqualified USING column can overflow its common
type</a>. */
+ @ParameterizedTest
Review Comment:
Added in
[9650c0bdb](https://github.com/apache/calcite/commit/9650c0bdb9c047011b98f9467a87146eb471d323).
The Java comments now derive the expected rows from the matched/unmatched keys
and explain the type controls. `sql/join.iq` contains the exact issue query
with result and type assertions, preceded by a qualified `f.k + 1` BIGINT
arithmetic control.
Against the original validator, the qualified control passed and only the
unqualified query differed: INTEGER/-2147483648 instead of BIGINT/2147483648.
With the fix, all 47 core Quidem files pass. The complete default `gradlew
build` also passed: 22,263 passing test records, zero failures/errors, with
existing skips retained.
The two upstream workflows currently require repository approval. Could you
approve this revision for CI?
##########
core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java:
##########
@@ -6266,6 +6266,20 @@ private ImmutableList<ImmutableBitSet>
cube(ImmutableBitSet... sets) {
.ok();
}
+ /** Tests that a common column has the same widened type with and without
Review Comment:
Added the standard CALCITE-7880 issue Javadoc to this validator regression
in
[9650c0bdb](https://github.com/apache/calcite/commit/9650c0bdb9c047011b98f9467a87146eb471d323).
##########
core/src/test/java/org/apache/calcite/test/JdbcTest.java:
##########
@@ -2955,6 +2955,62 @@ private void checkNullableTimestamp(CalciteAssert.Config
config) {
.returns("desc=SameName\n");
}
+ /** Regression test for
+ * <a
href="https://issues.apache.org/jira/browse/CALCITE-7880";>[CALCITE-7880]
+ * Arithmetic on an unqualified USING column can overflow its common
type</a>. */
+ @ParameterizedTest
+ @MethodSource("joinUsingMixedNumericTypes")
+ void testJoinUsingMixedNumericCommonColumn(String join, String leftType,
+ String rightType) {
+ final String from = " from (values (cast(2147483647 as " + leftType + ")))
as l(k) "
+ + join + " (values (cast(2147483647 as " + rightType + "))) as r(k)"
+ + (join.startsWith("natural") ? "" : " using (k)");
+ final String nullability = join.contains("full") ? "" : " NOT NULL";
+ CalciteAssert.that()
+ .query("select k + 1 as x" + from)
+ .returns("X=2147483648\n")
+ .typeIs("[X BIGINT" + nullability + "]");
+ CalciteAssert.that()
+ .query("select *" + from)
+ .typeIs("[K BIGINT" + nullability + "]");
+ // A qualified reference still denotes the original input column.
+ CalciteAssert.that()
+ .query("select l.k" + from)
+ .typeIs("[K " + leftType.toUpperCase(Locale.ROOT)
+ + (join.contains("right") || join.contains("full") ? "" : " NOT
NULL")
+ + "]");
+ }
+
+ private static Stream<Arguments> joinUsingMixedNumericTypes() {
+ return Stream.of("join", "left join", "right join", "full join",
+ "natural join", "natural left join", "natural right join", "natural
full join")
+ .flatMap(
+ join -> Stream.of(arguments(join, "integer", "bigint"),
+ arguments(join, "bigint", "integer")));
+ }
+
+ @ParameterizedTest
+ @MethodSource("joinUsingMixedNumericNulls")
+ void testJoinUsingMixedNumericCommonColumnWithNulls(String join, String
leftType,
Review Comment:
Added the standard CALCITE-7880 issue Javadoc here in
[9650c0bdb](https://github.com/apache/calcite/commit/9650c0bdb9c047011b98f9467a87146eb471d323),
along with the derivation of the LEFT/RIGHT expected rows.
--
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]
