Skip to content

Commit

Permalink
[CALCITE-2067] RexBuilder can't handle NaN, Infinity double constants
Browse files Browse the repository at this point in the history
Signed-off-by: Mihai Budiu <mbudiu@feldera.com>
  • Loading branch information
mihaibudiu authored and julianhyde committed Sep 25, 2024
1 parent 1beb5ff commit a15930f
Show file tree
Hide file tree
Showing 23 changed files with 280 additions and 73 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,8 @@ private static String getLiteralType(Object literal) {
}
} else if (String.class.equals(literal.getClass())) {
return "string";
} else if (literal instanceof Double) {
return "float";
}
throw new UnsupportedOperationException("Unsupported literal " + literal);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -373,7 +373,7 @@ static void initializeArrowState(@TempDir Path sharedTempDir)
String sql = "select * from arrowdata\n"
+ " where \"floatField\"=15.0";
String plan = "PLAN=ArrowToEnumerableConverter\n"
+ " ArrowFilter(condition=[=(CAST($2):DOUBLE, 15.0)])\n"
+ " ArrowFilter(condition=[=(CAST($2):DOUBLE, 15.0E0)])\n"
+ " ArrowTableScan(table=[[ARROW, ARROWDATA]], fields=[[0, 1, 2, 3]])\n\n";
String result = "intField=15; stringField=15; floatField=15.0; longField=15\n";

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1498,7 +1498,7 @@ public static SqlNode toSql(RexLiteral literal) {
case EXACT_NUMERIC: {
if (SqlTypeName.APPROX_TYPES.contains(typeName)) {
return SqlLiteral.createApproxNumeric(
castNonNull(literal.getValueAs(BigDecimal.class)).toPlainString(), POS);
castNonNull(literal.getValueAs(Double.class)).toString(), POS);
} else {
return SqlLiteral.createExactNumeric(
castNonNull(literal.getValueAs(BigDecimal.class)).toPlainString(), POS);
Expand Down
38 changes: 33 additions & 5 deletions core/src/main/java/org/apache/calcite/rex/RexBuilder.java
Original file line number Diff line number Diff line change
Expand Up @@ -1437,7 +1437,21 @@ public RexLiteral makeApproxLiteral(BigDecimal bd) {
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);
}

/**
* Creates an approximate numeric literal (double or float)
* from a Double value.
*
* @param val literal value
* @param type approximate numeric type
* @return new literal
*/
public RexLiteral makeApproxLiteral(Double val, RelDataType type) {
assert SqlTypeFamily.APPROXIMATE_NUMERIC.getTypeNames().contains(
type.getSqlTypeName());
return makeLiteral(val, type, SqlTypeName.DOUBLE);
}

/**
Expand Down Expand Up @@ -2013,7 +2027,10 @@ public RexNode makeLiteral(@Nullable Object value, RelDataType type,
case FLOAT:
case REAL:
case DOUBLE:
return makeApproxLiteral((BigDecimal) value, type);
if (value instanceof Double) {
return makeApproxLiteral((Double) value, type);
}
return makeApproxLiteral(((BigDecimal) value).doubleValue(), type);
case BOOLEAN:
return (Boolean) value ? booleanTrue : booleanFalse;
case TIME:
Expand Down Expand Up @@ -2159,13 +2176,24 @@ public RexNode makeLambdaCall(RexNode expr, List<RexLambdaRef> parameters) {
type.getSqlTypeName());
return new BigDecimal(((Number) o).longValue());
case REAL:
case FLOAT:
case DOUBLE:
if (o instanceof BigDecimal) {
return o;
}
return new BigDecimal(((Number) o).doubleValue(), MathContext.DECIMAL64)
// Float values are stored as Doubles
if (o instanceof Float) {
return ((Float) o).doubleValue();
}
if (o instanceof Double) {
return o;
}
return new BigDecimal(((Number) o).doubleValue(), MathContext.DECIMAL32)
.stripTrailingZeros();
case FLOAT:
case DOUBLE:
if (o instanceof Double) {
return o;
}
return ((Number) o).doubleValue();
case CHAR:
case VARCHAR:
if (o instanceof NlsString) {
Expand Down
79 changes: 56 additions & 23 deletions core/src/main/java/org/apache/calcite/rex/RexLiteral.java
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@

import java.io.PrintWriter;
import java.math.BigDecimal;
import java.math.MathContext;
import java.nio.ByteBuffer;
import java.nio.charset.Charset;
import java.text.SimpleDateFormat;
Expand Down Expand Up @@ -112,9 +113,11 @@
* <td>{@link BigDecimal}</td>
* </tr>
* <tr>
* <td>{@link SqlTypeName#DOUBLE}</td>
* <td>{@link SqlTypeName#DOUBLE},
* {@link SqlTypeName#REAL},
* {@link SqlTypeName#FLOAT}</td>
* <td>Approximate number, for example <code>6.023E-23</code>.</td>
* <td>{@link BigDecimal}</td>
* <td>{@link Double}.</td>
* </tr>
* <tr>
* <td>{@link SqlTypeName#DATE}</td>
Expand Down Expand Up @@ -193,7 +196,7 @@ public class RexLiteral extends RexNode {
/**
* The value of this literal. Must be consistent with its type, as per
* {@link #valueMatchesType}. For example, you can't store an
* {@link Integer} value here just because you feel like it -- all numbers are
* {@link Integer} value here just because you feel like it -- all exact numbers are
* represented by a {@link BigDecimal}. But since this field is private, it
* doesn't really matter how the values are stored.
*/
Expand All @@ -204,12 +207,9 @@ public class RexLiteral extends RexNode {
*/
private final RelDataType type;

// TODO jvs 26-May-2006: Use SqlTypeFamily instead; it exists
// for exactly this purpose (to avoid the confusion which results
// from overloading SqlTypeName).
/**
* An indication of the broad type of this literal -- even if its type isn't
* a SQL type. Sometimes this will be different than the SQL type; for
* a SQL type. Sometimes this will be different from the SQL type; for
* example, all exact numbers, including integers have typeName
* {@link SqlTypeName#DECIMAL}. See {@link #valueMatchesType} for the
* definitive story.
Expand Down Expand Up @@ -294,10 +294,10 @@ public final String computeDigest(
}

/**
* Returns true if {@link RexDigestIncludeType#OPTIONAL} digest would include data type.
* Returns whether {@link RexDigestIncludeType} digest would include data type.
*
* @see RexCall#computeDigest(boolean)
* @return true if {@link RexDigestIncludeType#OPTIONAL} digest would include data type
* @return whether {@link RexDigestIncludeType} digest would include data type
*/
@RequiresNonNull("type")
RexDigestIncludeType digestIncludesType(
Expand Down Expand Up @@ -328,11 +328,12 @@ public static boolean valueMatchesType(
}
// fall through
case DECIMAL:
case BIGINT:
return value instanceof BigDecimal;
case DOUBLE:
case FLOAT:
case REAL:
case BIGINT:
return value instanceof BigDecimal;
return value instanceof Double;
case DATE:
return value instanceof DateString;
case TIME:
Expand Down Expand Up @@ -660,8 +661,14 @@ private static void appendAsJava(@Nullable Comparable value, StringBuilder sb,
break;
case DOUBLE:
case FLOAT:
assert value instanceof BigDecimal;
sb.append(Util.toScientificNotation((BigDecimal) value));
if (value instanceof BigDecimal) {
sb.append(Util.toScientificNotation((BigDecimal) value));
} else {
assert value instanceof Double;
Double d = (Double) value;
String repr = Util.toScientificNotation(d);
sb.append(repr);
}
break;
case BIGINT:
assert value instanceof BigDecimal;
Expand Down Expand Up @@ -1075,22 +1082,48 @@ public boolean isNull() {
case BIGINT:
case INTEGER:
case SMALLINT:
case TINYINT:
case DOUBLE:
case REAL:
case FLOAT:
case TINYINT: {
BigDecimal bd = (BigDecimal) value;
if (clazz == Long.class) {
return clazz.cast(((BigDecimal) value).longValue());
return clazz.cast(bd.longValue());
} else if (clazz == Integer.class) {
return clazz.cast(((BigDecimal) value).intValue());
return clazz.cast(bd.intValue());
} else if (clazz == Short.class) {
return clazz.cast(((BigDecimal) value).shortValue());
return clazz.cast(bd.shortValue());
} else if (clazz == Byte.class) {
return clazz.cast(((BigDecimal) value).byteValue());
return clazz.cast(bd.byteValue());
} else if (clazz == Double.class) {
return clazz.cast(((BigDecimal) value).doubleValue());
return clazz.cast(bd.doubleValue());
} else if (clazz == Float.class) {
return clazz.cast(((BigDecimal) value).floatValue());
return clazz.cast(bd.floatValue());
}
break;
}
case DOUBLE:
case REAL:
case FLOAT:
if (value instanceof Double) {
Double d = (Double) value;
if (clazz == Long.class) {
return clazz.cast(d.longValue());
} else if (clazz == Integer.class) {
return clazz.cast(d.intValue());
} else if (clazz == Short.class) {
return clazz.cast(d.shortValue());
} else if (clazz == Byte.class) {
return clazz.cast(d.byteValue());
} else if (clazz == Double.class) {
// Cast still needed, since the Java compiler does not understand
// that T is double.
return clazz.cast(d);
} else if (clazz == Float.class) {
return clazz.cast(d.floatValue());
} else if (clazz == BigDecimal.class) {
// This particular conversion is lossy, since in general BigDecimal cannot
// represent accurately FP values. However, this is the best we can do.
// This conversion used to be in RexBuilder, used when creating a RexLiteral.
return clazz.cast(new BigDecimal(d, MathContext.DECIMAL64).stripTrailingZeros());
}
}
break;
case DATE:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -480,7 +480,7 @@ public RexLiteral literal(@Nullable Object value) {
return rexBuilder.makeExactLiteral((BigDecimal) value);
} else if (value instanceof Float || value instanceof Double) {
return rexBuilder.makeApproxLiteral(
BigDecimal.valueOf(((Number) value).doubleValue()));
((Number) value).doubleValue(), getTypeFactory().createSqlType(SqlTypeName.DOUBLE));
} else if (value instanceof Number) {
return rexBuilder.makeExactLiteral(
BigDecimal.valueOf(((Number) value).longValue()));
Expand Down
19 changes: 19 additions & 0 deletions core/src/main/java/org/apache/calcite/util/Util.java
Original file line number Diff line number Diff line change
Expand Up @@ -534,6 +534,21 @@ public static void println(
pw.println();
}

/**
* Formats a double value to a String ensuring that the output
* is in scientific notation if the value is not "special".
* (Special values include infinities and NaN.)
*/
public static String toScientificNotation(Double d) {
String repr = Double.toString(d);
if (!repr.toLowerCase(Locale.ENGLISH).contains("e")
&& !d.isInfinite()
&& !d.isNaN()) {
repr += "E0";
}
return repr;
}

/**
* Formats a {@link BigDecimal} value to a string in scientific notation For
* example<br>
Expand All @@ -558,6 +573,10 @@ public static String toScientificNotation(BigDecimal bd) {
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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -407,7 +407,7 @@ private static String toSql(RelNode root, SqlDialect dialect,
+ "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);
}

Expand Down Expand Up @@ -4391,7 +4391,7 @@ private SqlDialect nonOrdinalDialect() {
+ " 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"
Expand Down Expand Up @@ -6507,7 +6507,7 @@ private void checkLiteral2(String expression, String expected) {
+ "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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -389,9 +389,11 @@ interface Action {
final RexCall first =
(RexCall) rexBuilder.makeCall(SqlStdOperatorTable.LN,
rexBuilder.makeLiteral(3, integer, true));
// Division by zero causes an exception during evaluation
final RexCall second =
(RexCall) rexBuilder.makeCall(SqlStdOperatorTable.LN,
rexBuilder.makeLiteral(-2, integer, true));
(RexCall) rexBuilder.makeCall(SqlStdOperatorTable.DIVIDE_INTEGER,
rexBuilder.makeLiteral(-2, integer, true),
rexBuilder.makeLiteral(0, integer, true));
executor.reduce(rexBuilder, ImmutableList.of(first, second),
reducedValues);
assertThat(reducedValues, hasSize(2));
Expand Down
2 changes: 1 addition & 1 deletion core/src/test/java/org/apache/calcite/test/JdbcTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -389,7 +389,7 @@ static void forEachExpand(Runnable r) {
+ "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
Expand Down
27 changes: 27 additions & 0 deletions core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -3341,6 +3341,33 @@ private void checkPushJoinThroughUnionOnRightDoesNotMatchSemiOrAntiJoin(JoinRelT
.check();
}

/** Test case for <a href="https://issues.apache.org/jira/browse/CALCITE-2067">
* [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
// it is not the result returned by the pow() function, which is
// 1008618.4899999999
final String sql = "SELECT power(1004.3, 2)";
sql(sql)
.withRule(CoreRules.PROJECT_REDUCE_EXPRESSIONS)
.check();
}

/** Test case for <a href="https://issues.apache.org/jira/browse/CALCITE-2067">
* [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 Infinity, since Infinity cannot be represented
// as a BigDecimal value.
final String sql2 = "SELECT 1.0 / 0.0e0";
sql(sql2)
.withRule(CoreRules.PROJECT_REDUCE_EXPRESSIONS)
.check();
}

/** Tests that {@link UnionMergeRule} does nothing if its arguments have
* are different set operators, {@link Union} and {@link Intersect}. */
@Test void testMergeSetOpMixed() {
Expand Down
Loading

0 comments on commit a15930f

Please sign in to comment.