This is an automated email from the ASF dual-hosted git repository. mbudiu pushed a commit to branch issue2067 in repository https://gitbox.apache.org/repos/asf/calcite.git
commit a1e22fb788728f1bd450569eb37e5ab47e1b1f2a Author: Mihai Budiu <[email protected]> AuthorDate: Wed Jan 31 19:20:02 2024 -0800 Address review comments Signed-off-by: Mihai Budiu <[email protected]> --- .../java/org/apache/calcite/rex/RexBuilder.java | 18 +----- .../java/org/apache/calcite/rex/RexLiteral.java | 8 +-- .../java/org/apache/calcite/tools/RelBuilder.java | 2 +- .../main/java/org/apache/calcite/util/Util.java | 4 ++ .../calcite/rel/rel2sql/RelToSqlConverterTest.java | 6 +- .../java/org/apache/calcite/test/JdbcTest.java | 2 +- .../org/apache/calcite/test/RelOptRulesTest.java | 6 +- .../java/org/apache/calcite/util/UtilTest.java | 71 ++++++++++++++++++++++ .../org/apache/calcite/test/RelOptRulesTest.xml | 4 +- .../calcite/test/TypeCoercionConverterTest.xml | 12 ++-- 10 files changed, 99 insertions(+), 34 deletions(-) diff --git a/core/src/main/java/org/apache/calcite/rex/RexBuilder.java b/core/src/main/java/org/apache/calcite/rex/RexBuilder.java index e461b501d8..3ebabd164c 100644 --- a/core/src/main/java/org/apache/calcite/rex/RexBuilder.java +++ b/core/src/main/java/org/apache/calcite/rex/RexBuilder.java @@ -1427,14 +1427,6 @@ public class RexBuilder { return makeApproxLiteral(bd, typeFactory.createSqlType(SqlTypeName.DOUBLE)); } - /** - * Creates a double-precision literal from a double value. - */ - public RexLiteral makeApproxLiteral(Double d) { - return makeApproxLiteral(d, typeFactory.createSqlType(SqlTypeName.DOUBLE)); - } - - /** * Creates an approximate numeric literal (double or float). * @@ -1445,7 +1437,7 @@ public class RexBuilder { public RexLiteral makeApproxLiteral(@Nullable BigDecimal bd, RelDataType type) { assert SqlTypeFamily.APPROXIMATE_NUMERIC.getTypeNames().contains( type.getSqlTypeName()); - return makeLiteral(bd, type, SqlTypeName.DOUBLE); + return makeLiteral(bd != null ? bd.doubleValue() : null, type, SqlTypeName.DOUBLE); } /** @@ -2038,7 +2030,7 @@ public class RexBuilder { if (value instanceof Double) { return makeApproxLiteral((Double) value, type); } - return makeApproxLiteral((BigDecimal) value, type); + return makeApproxLiteral(((BigDecimal) value).doubleValue(), type); case BOOLEAN: return (Boolean) value ? booleanTrue : booleanFalse; case TIME: @@ -2198,14 +2190,10 @@ public class RexBuilder { .stripTrailingZeros(); case FLOAT: case DOUBLE: - if (o instanceof BigDecimal) { - return o; - } if (o instanceof Double) { return o; } - return new BigDecimal(((Number) o).doubleValue(), MathContext.DECIMAL64) - .stripTrailingZeros(); + return ((Number) o).doubleValue(); case CHAR: case VARCHAR: if (o instanceof NlsString) { diff --git a/core/src/main/java/org/apache/calcite/rex/RexLiteral.java b/core/src/main/java/org/apache/calcite/rex/RexLiteral.java index bcf00b41f4..5eb37687a0 100644 --- a/core/src/main/java/org/apache/calcite/rex/RexLiteral.java +++ b/core/src/main/java/org/apache/calcite/rex/RexLiteral.java @@ -116,7 +116,7 @@ import static java.util.Objects.requireNonNull; * {@link SqlTypeName#REAL}, * {@link SqlTypeName#FLOAT}</td> * <td>Approximate number, for example <code>6.023E-23</code>.</td> - * <td>{@link BigDecimal} or {@link Double}.</td> + * <td>{@link Double}.</td> * </tr> * <tr> * <td>{@link SqlTypeName#DATE}</td> @@ -332,7 +332,7 @@ public class RexLiteral extends RexNode { case DOUBLE: case FLOAT: case REAL: - return value instanceof BigDecimal || value instanceof Double; + return value instanceof Double; case DATE: return value instanceof DateString; case TIME: @@ -527,8 +527,8 @@ public class RexLiteral extends RexNode { } return litmus.succeed(); } else if (o instanceof Map) { - @SuppressWarnings("unchecked") final Map<Object, Object> map = (Map<Object, Object>) o; - for (Map.Entry<Object, Object> entry : map.entrySet()) { + @SuppressWarnings("unchecked") final Map<Object, Object> map = (Map) o; + for (Map.Entry entry : map.entrySet()) { if (!validConstant(entry.getKey(), litmus)) { return litmus.fail("not a constant: {}", entry.getKey()); } diff --git a/core/src/main/java/org/apache/calcite/tools/RelBuilder.java b/core/src/main/java/org/apache/calcite/tools/RelBuilder.java index e6927f22a1..4c7357da81 100644 --- a/core/src/main/java/org/apache/calcite/tools/RelBuilder.java +++ b/core/src/main/java/org/apache/calcite/tools/RelBuilder.java @@ -480,7 +480,7 @@ public class RelBuilder { return rexBuilder.makeExactLiteral((BigDecimal) value); } else if (value instanceof Float || value instanceof Double) { return rexBuilder.makeApproxLiteral( - ((Number) value).doubleValue()); + ((Number) value).doubleValue(), getTypeFactory().createSqlType(SqlTypeName.DOUBLE)); } else if (value instanceof Number) { return rexBuilder.makeExactLiteral( BigDecimal.valueOf(((Number) value).longValue())); diff --git a/core/src/main/java/org/apache/calcite/util/Util.java b/core/src/main/java/org/apache/calcite/util/Util.java index eda2b36e76..b9b8e2085b 100644 --- a/core/src/main/java/org/apache/calcite/util/Util.java +++ b/core/src/main/java/org/apache/calcite/util/Util.java @@ -573,6 +573,10 @@ public class Util { int len = unscaled.length(); int scale = bd.scale(); int e = len - scale - 1; + if (bd.stripTrailingZeros().equals(BigDecimal.ZERO)) { + // Without this adjustment 0.0 generates 0E-1 + e = 0; + } StringBuilder ret = new StringBuilder(); if (bd.signum() < 0) { diff --git a/core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java b/core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java index 74f4450df4..6d2612b329 100644 --- a/core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java +++ b/core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java @@ -407,7 +407,7 @@ class RelToSqlConverterTest { + "where \"net_weight\" <> 10 or \"net_weight\" is null"; final String expected = "SELECT \"product_id\", \"shelf_width\"\n" + "FROM \"foodmart\".\"product\"\n" - + "WHERE \"net_weight\" <> 10 OR \"net_weight\" IS NULL"; + + "WHERE \"net_weight\" <> CAST(10 AS DOUBLE) OR \"net_weight\" IS NULL"; sql(query).ok(expected); } @@ -4391,7 +4391,7 @@ class RelToSqlConverterTest { + " select \"product_id\", 0 as \"net_weight\"\n" + " from \"sales_fact_1997\") t0"; final String expected = "SELECT SUM(CASE WHEN \"product_id\" = 0" - + " THEN \"net_weight\" ELSE 0 END) AS \"NET_WEIGHT\"\n" + + " THEN \"net_weight\" ELSE 0E0 END) AS \"NET_WEIGHT\"\n" + "FROM (SELECT \"product_id\", \"net_weight\"\n" + "FROM \"foodmart\".\"product\"\n" + "UNION ALL\n" @@ -6507,7 +6507,7 @@ class RelToSqlConverterTest { + "PATTERN (\"STRT\" \"DOWN\" + \"UP\" +)\n" + "DEFINE " + "\"DOWN\" AS PREV(\"DOWN\".\"net_weight\", 0) = " - + "0 OR PREV(\"DOWN\".\"net_weight\", 0) = 1, " + + "CAST(0 AS DOUBLE) OR PREV(\"DOWN\".\"net_weight\", 0) = CAST(1 AS DOUBLE), " + "\"UP\" AS PREV(\"UP\".\"net_weight\", 0) > " + "PREV(\"UP\".\"net_weight\", 1))"; sql(sql).ok(expected); diff --git a/core/src/test/java/org/apache/calcite/test/JdbcTest.java b/core/src/test/java/org/apache/calcite/test/JdbcTest.java index 4eeb666d4f..4f4838d7ab 100644 --- a/core/src/test/java/org/apache/calcite/test/JdbcTest.java +++ b/core/src/test/java/org/apache/calcite/test/JdbcTest.java @@ -389,7 +389,7 @@ public class JdbcTest { + "expr#7=[null:JavaType(class java.lang.Integer)], " + "empid=[$t3], deptno=[$t4], name=[$t5], salary=[$t6], " + "commission=[$t7])\n" - + " EnumerableValues(tuples=[[{ 'Fred', 56, 123.4 }]])\n"; + + " EnumerableValues(tuples=[[{ 'Fred', 56, 123.4000015258789E0 }]])\n"; assertThat(resultSet.getString(1), isLinux(expected)); // With named columns diff --git a/core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java b/core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java index c578b92ebd..7c92606844 100644 --- a/core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java +++ b/core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java @@ -3342,7 +3342,8 @@ class RelOptRulesTest extends RelOptTestBase { } /** Test case for <a href="https://issues.apache.org/jira/browse/CALCITE-2067";> - * RexBuilder can't handle NaN,Infinity double constants</a>. */ + * [CALCITE-2067] RexLiteral cannot represent accurately floating point values, + * including NaN, Infinity</a>. */ @Test public void testDoubleReduction() { // Without the fix for CALCITE-2067 the result returned below is // 1008618.49. Ironically, that result is more accurate; however @@ -3355,7 +3356,8 @@ class RelOptRulesTest extends RelOptTestBase { } /** Test case for <a href="https://issues.apache.org/jira/browse/CALCITE-2067";> - * RexBuilder can't handle NaN,Infinity double constants</a>. */ + * [CALCITE-2067] RexLiteral cannot represent accurately floating point values, + * including NaN, Infinity</a>. */ @Test public void testDoubleReduction2() { // Without the fix for CALCITE-2067 the following expression is not // reduced to NaN, since NaN cannot be represented diff --git a/core/src/test/java/org/apache/calcite/util/UtilTest.java b/core/src/test/java/org/apache/calcite/util/UtilTest.java index fbca0845a4..8ef4c1c7d2 100644 --- a/core/src/test/java/org/apache/calcite/util/UtilTest.java +++ b/core/src/test/java/org/apache/calcite/util/UtilTest.java @@ -161,6 +161,10 @@ class UtilTest { @Test void testScientificNotation() { BigDecimal bd; + bd = new BigDecimal("0.0"); + TestUtil.assertEqualsVerbose( + "0E0", + Util.toScientificNotation(bd)); bd = new BigDecimal("0.001234"); TestUtil.assertEqualsVerbose( "1.234E-3", @@ -209,6 +213,73 @@ class UtilTest { Util.toScientificNotation(bd)); } + @Test void testDoubleScientificNotation() { + double d = Double.parseDouble("0.001234"); + TestUtil.assertEqualsVerbose( + "0.001234E0", + Util.toScientificNotation(d)); + d = Double.parseDouble("0.001"); + TestUtil.assertEqualsVerbose( + "0.001E0", + Util.toScientificNotation(d)); + d = Double.parseDouble("-0.001"); + TestUtil.assertEqualsVerbose( + "-0.001E0", + Util.toScientificNotation(d)); + d = Double.parseDouble("1"); + TestUtil.assertEqualsVerbose( + "1.0E0", + Util.toScientificNotation(d)); + d = Double.parseDouble("-1"); + TestUtil.assertEqualsVerbose( + "-1.0E0", + Util.toScientificNotation(d)); + d = Double.parseDouble("1.0"); + TestUtil.assertEqualsVerbose( + "1.0E0", + Util.toScientificNotation(d)); + d = Double.parseDouble("12345"); + TestUtil.assertEqualsVerbose( + "12345.0E0", + Util.toScientificNotation(d)); + d = Double.parseDouble("12345.00"); + TestUtil.assertEqualsVerbose( + "12345.0E0", + Util.toScientificNotation(d)); + d = Double.parseDouble("12345.001"); + TestUtil.assertEqualsVerbose( + "12345.001E0", + Util.toScientificNotation(d)); + + // test truncate + d = Double.parseDouble("1.23456789012345678901"); + TestUtil.assertEqualsVerbose( + "1.2345678901234567E0", + Util.toScientificNotation(d)); + d = Double.parseDouble("-1.23456789012345678901"); + TestUtil.assertEqualsVerbose( + "-1.2345678901234567E0", + Util.toScientificNotation(d)); + + // special values + d = Double.parseDouble("Infinity"); + TestUtil.assertEqualsVerbose( + "Infinity", + Util.toScientificNotation(d)); + d = Double.parseDouble("-Infinity"); + TestUtil.assertEqualsVerbose( + "-Infinity", + Util.toScientificNotation(d)); + d = Double.parseDouble("NaN"); + TestUtil.assertEqualsVerbose( + "NaN", + Util.toScientificNotation(d)); + d = Double.parseDouble("-0.0"); + TestUtil.assertEqualsVerbose( + "-0.0E0", + Util.toScientificNotation(d)); + } + @Test void testToJavaId() throws UnsupportedEncodingException { assertEquals( "ID$0$foo", diff --git a/core/src/test/resources/org/apache/calcite/test/RelOptRulesTest.xml b/core/src/test/resources/org/apache/calcite/test/RelOptRulesTest.xml index 2fa1dd5cb6..efc1f8bc57 100644 --- a/core/src/test/resources/org/apache/calcite/test/RelOptRulesTest.xml +++ b/core/src/test/resources/org/apache/calcite/test/RelOptRulesTest.xml @@ -1566,7 +1566,7 @@ case when cast(ename as double) < 5 then 0.0 </Resource> <Resource name="planBefore"> <![CDATA[ -LogicalProject(T=[CASE(<(CAST(CASE(>($1, 'abc'), $1, null:VARCHAR(20))):DOUBLE, 5), 0.0:DOUBLE, CASE(IS NOT NULL(CAST(CASE(>($1, 'abc'), $1, null:VARCHAR(20))):DOUBLE), CAST(CAST(CASE(>($1, 'abc'), $1, null:VARCHAR(20))):DOUBLE):DOUBLE NOT NULL, 1.0:DOUBLE))]) +LogicalProject(T=[CASE(<(CAST(CASE(>($1, 'abc'), $1, null:VARCHAR(20))):DOUBLE, 5), 0.0E0:DOUBLE, CASE(IS NOT NULL(CAST(CASE(>($1, 'abc'), $1, null:VARCHAR(20))):DOUBLE), CAST(CAST(CASE(>($1, 'abc'), $1, null:VARCHAR(20))):DOUBLE):DOUBLE NOT NULL, 1.0E0:DOUBLE))]) LogicalTableScan(table=[[CATALOG, SALES, EMP]]) ]]> </Resource> @@ -12291,7 +12291,7 @@ from emp]]> </Resource> <Resource name="planBefore"> <![CDATA[ -LogicalProject(NEWCOL=[CASE(false, 2.1:FLOAT, 1:FLOAT)]) +LogicalProject(NEWCOL=[CASE(false, CAST(2.1:DECIMAL(2, 1)):FLOAT NOT NULL, CAST(1):FLOAT NOT NULL)]) LogicalTableScan(table=[[CATALOG, SALES, EMP]]) ]]> </Resource> diff --git a/core/src/test/resources/org/apache/calcite/test/TypeCoercionConverterTest.xml b/core/src/test/resources/org/apache/calcite/test/TypeCoercionConverterTest.xml index c88d38118c..c5476150a7 100644 --- a/core/src/test/resources/org/apache/calcite/test/TypeCoercionConverterTest.xml +++ b/core/src/test/resources/org/apache/calcite/test/TypeCoercionConverterTest.xml @@ -158,11 +158,11 @@ LogicalTableModify(table=[[CATALOG, SALES, T1]], operation=[INSERT], flattened=[ LogicalUnion(all=[false]) LogicalUnion(all=[false]) LogicalUnion(all=[false]) - LogicalValues(tuples=[[{ 'a', 1, 1, 0, 0, 0, 0, 2021-11-28 00:00:00, 2021-11-28, X'0a', false }]]) - LogicalValues(tuples=[[{ 'b', 2, 2, 0, 0, 0, 0, 2021-11-28 00:00:00, 2021-11-28, X'0a', false }]]) - LogicalValues(tuples=[[{ 'c', 3, 3, 0, 0, 0, 0, 2021-11-28 00:00:00, 2021-11-28, X'0a', false }]]) - LogicalValues(tuples=[[{ 'd', 4, 4, 0, 0, 0, 0, 2021-11-28 00:00:00, 2021-11-28, X'0a', false }]]) - LogicalValues(tuples=[[{ 'e', 5, 5, 0, 0, 0, 0, 2021-11-28 00:00:00, 2021-11-28, X'0a', false }]]) + LogicalValues(tuples=[[{ 'a', 1, 1, 0, 0.0E0, 0.0E0, 0, 2021-11-28 00:00:00, 2021-11-28, X'0a', false }]]) + LogicalValues(tuples=[[{ 'b', 2, 2, 0, 0.0E0, 0.0E0, 0, 2021-11-28 00:00:00, 2021-11-28, X'0a', false }]]) + LogicalValues(tuples=[[{ 'c', 3, 3, 0, 0.0E0, 0.0E0, 0, 2021-11-28 00:00:00, 2021-11-28, X'0a', false }]]) + LogicalValues(tuples=[[{ 'd', 4, 4, 0, 0.0E0, 0.0E0, 0, 2021-11-28 00:00:00, 2021-11-28, X'0a', false }]]) + LogicalValues(tuples=[[{ 'e', 5, 5, 0, 0.0E0, 0.0E0, 0, 2021-11-28 00:00:00, 2021-11-28, X'0a', false }]]) ]]> </Resource> </TestCase> @@ -173,7 +173,7 @@ LogicalTableModify(table=[[CATALOG, SALES, T1]], operation=[INSERT], flattened=[ <Resource name="plan"> <![CDATA[ LogicalTableModify(table=[[CATALOG, SALES, T1]], operation=[INSERT], flattened=[false]) - LogicalValues(tuples=[[{ 'a', 1, 1, 0, 0, 0, 0, 2021-11-28 00:00:00, 2021-11-28, X'0a', false }, { 'b', 2, 2, 0, 0, 0, 0, 2021-11-28 00:00:00, 2021-11-28, X'0a', false }, { 'c', 3, 3, 0, 0, 0, 0, 2021-11-28 00:00:00, 2021-11-28, X'0a', false }, { 'd', 4, 4, 0, 0, 0, 0, 2021-11-28 00:00:00, 2021-11-28, X'0a', false }, { 'e', 5, 5, 0, 0, 0, 0, 2021-11-28 00:00:00, 2021-11-28, X'0a', false }]]) + LogicalValues(tuples=[[{ 'a', 1, 1, 0, 0.0E0, 0.0E0, 0, 2021-11-28 00:00:00, 2021-11-28, X'0a', false }, { 'b', 2, 2, 0, 0.0E0, 0.0E0, 0, 2021-11-28 00:00:00, 2021-11-28, X'0a', false }, { 'c', 3, 3, 0, 0.0E0, 0.0E0, 0, 2021-11-28 00:00:00, 2021-11-28, X'0a', false }, { 'd', 4, 4, 0, 0.0E0, 0.0E0, 0, 2021-11-28 00:00:00, 2021-11-28, X'0a', false }, { 'e', 5, 5, 0, 0.0E0, 0.0E0, 0, 2021-11-28 00:00:00, 2021-11-28, X'0a', false }]]) ]]> </Resource> </TestCase>
