mihaibudiu commented on code in PR #5361:
URL: https://github.com/apache/calcite/pull/5361#discussion_r4371901327
##########
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:
This one needs the same comment pointing to the JIRA issue
##########
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:
Here you need to use the same standard format for the javadoc describing the
issue.
##########
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:
These tests are hard to read for a human, but let's keep them since they are
parameterized.
But can you add a comment explaining how you have validated that their
results are correct?
Can you also add at least one quidem test containing the example of the
issue?
--
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]
