sbroeder commented on code in PR #5297:
URL: https://github.com/apache/calcite/pull/5297#discussion_r4243927681
##########
core/src/main/java/org/apache/calcite/sql/validate/implicit/TypeCoercion.java:
##########
@@ -94,6 +94,10 @@ public interface TypeCoercion {
/**
* Determines common type for a comparison operator.
+ *
+ * <p>{@code NATURAL JOIN} and {@code JOIN USING} call this method directly
+ * (via {@link org.apache.calcite.sql2rel.SqlToRelConverter}); every
+ * implementation must handle those correctly.
Review Comment:
Ack
##########
core/src/test/java/org/apache/calcite/test/JdbcTest.java:
##########
@@ -1836,6 +1840,105 @@ private void checkResultSetMetaData(Connection
connection, String sql)
.returns("C0=5.5; C1=5; C2=1.4; C3=1; C4=0; C5=0\n");
}
+ /** Installs {@link TypeCoercionTest.WideningTypeCoercion} via
+ * {@link Hook#STRING_TO_QUERY} for use in boundary tests for
+ * <a
href="https://issues.apache.org/jira/browse/CALCITE-7827";>[CALCITE-7827]</a>. */
+ private static Consumer<Pair<FrameworkConfig, Holder<CalcitePrepare.Query>>>
+ wideningHook(String sql) {
+ return pair -> {
+ final FrameworkConfig config =
+ Frameworks.newConfigBuilder(pair.left)
+ .sqlValidatorConfig(pair.left.getSqlValidatorConfig()
+
.withTypeCoercionFactory(TypeCoercionTest.WideningTypeCoercion::new))
+ .build();
+ final Planner planner = Frameworks.getPlanner(config);
+ try {
+ final RelRoot root = planner.rel(planner.validate(planner.parse(sql)));
+ pair.right.set(CalcitePrepare.Query.of(root.project()));
+ } catch (Exception e) {
+ throw TestUtil.rethrow(e);
+ }
+ };
+ }
+
+ /** Test case for
+ * <a
href="https://issues.apache.org/jira/browse/CALCITE-7827";>[CALCITE-7827]
+ * Document precision loss in DECIMAL/REAL comparisons and how to widen them
to DOUBLE</a>.
+ *
+ * <p>Default behavior: DECIMAL(18, 3) compared with REAL narrows to REAL
+ * (24-bit mantissa, about 7 decimal digits). These two values collide in
+ * REAL, so the join incorrectly returns 1 row. */
+ @Test void testDecimalRealComparisonDefaultNarrowsToReal() {
+ CalciteAssert.that()
+ .query("SELECT *\n"
+ + "FROM (VALUES (CAST(59999943.000 AS DECIMAL(18, 3)))) AS d(k)\n"
+ + "JOIN (VALUES (CAST(59999945 AS REAL))) AS f(k) ON d.k = f.k")
+ .returnsCount(1);
+ }
+
+ /** Test case for
+ * <a
href="https://issues.apache.org/jira/browse/CALCITE-7827";>[CALCITE-7827]
+ * Document precision loss in DECIMAL/REAL comparisons and how to widen them
to DOUBLE</a>.
+ *
+ * <p>With {@link TypeCoercionTest.WideningTypeCoercion}, DECIMAL(18, 3)
+ * compared with REAL widens to DOUBLE. {@code CAST(140737488355328 AS REAL)}
+ * is exactly 2^47 (a power of two, exactly representable as REAL). DOUBLE
+ * has 53-bit mantissa; the unit in the last place (ULP) at 2^47 is
+ * 2^(47-52) = 0.03125. The DECIMAL value 140737488355328.016 exceeds the
+ * half-ULP (0.015625), so it rounds to the next DOUBLE above 2^47. The
+ * widened join correctly returns 0 rows. See
+ * {@link #testDecimalRealComparisonWidenedStillCollides} for the pair that
+ * still collides after widening. */
+ @Test void testDecimalRealComparisonWidenedToDoubleHelps() {
Review Comment:
Done
--
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]
