Skip to content

Commit 1977083

Browse files
authored
Fix DOUBLE to STRING cast rendering zero values in scientific notation (#3982)
* Fix casting double 0.0 to string Signed-off-by: Yuanchun Shen <yuanchu@amazon.com> * Fix float to string casting precision lost with custom FormatNumberFunction This commit fixes float to string casting by replacing the use of SqlLibraryOperators.FORMAT_NUMBER with a custom FormatNumberFunction implementation. The new implementation converts the number to a BigDecimal before formatting to preserve precision and avoid issues like 6.2 becoming 6.199999809265137. Signed-off-by: Yuanchun Shen <yuanchu@amazon.com> * Simplify the implementation of fp number to string cast Signed-off-by: Yuanchun Shen <yuanchu@amazon.com> * Update implementation of NumberToStringFunction Signed-off-by: Yuanchun Shen <yuanchu@amazon.com> * Cast decimal with NUMBER_TO_STRING function Signed-off-by: Yuanchun Shen <yuanchu@amazon.com> * Test cast decimal Signed-off-by: Yuanchun Shen <yuanchu@amazon.com> --------- Signed-off-by: Yuanchun Shen <yuanchu@amazon.com>
1 parent 37d2d64 commit 1977083

File tree

4 files changed

+98
-4
lines changed

4 files changed

+98
-4
lines changed

core/src/main/java/org/opensearch/sql/calcite/ExtendedRexBuilder.java

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -105,28 +105,29 @@ public RexNode makeCast(
105105
boolean safe,
106106
RexLiteral format) {
107107
final SqlTypeName sqlType = type.getSqlTypeName();
108+
RelDataType sourceType = exp.getType();
108109
// Calcite bug which doesn't consider to cast literal to boolean
109110
if (exp instanceof RexLiteral && sqlType == SqlTypeName.BOOLEAN) {
110111
if (exp.equals(makeLiteral("1", typeFactory.createSqlType(SqlTypeName.CHAR, 1)))) {
111112
return makeLiteral(true, type);
112113
} else if (exp.equals(makeLiteral("0", typeFactory.createSqlType(SqlTypeName.CHAR, 1)))) {
113114
return makeLiteral(false, type);
114-
} else if (SqlTypeUtil.isExactNumeric(exp.getType())) {
115+
} else if (SqlTypeUtil.isExactNumeric(sourceType)) {
115116
return makeCall(
116117
type,
117118
SqlStdOperatorTable.NOT_EQUALS,
118-
ImmutableList.of(exp, makeZeroLiteral(exp.getType())));
119+
ImmutableList.of(exp, makeZeroLiteral(sourceType)));
119120
// TODO https://github.com/opensearch-project/sql/issues/3443
120121
// Current, we align the behaviour of Spark and Postgres, to align with OpenSearch V2,
121122
// enable following commented codes.
122123
// } else {
123124
// return makeCall(type,
124125
// SqlStdOperatorTable.NOT_EQUALS,
125-
// ImmutableList.of(exp, makeZeroLiteral(exp.getType())));
126+
// ImmutableList.of(exp, makeZeroLiteral(sourceType)));
126127
}
127128
} else if (OpenSearchTypeFactory.isUserDefinedType(type)) {
128129
var udt = ((AbstractExprRelDataType<?>) type).getUdt();
129-
var argExprType = OpenSearchTypeFactory.convertRelDataTypeToExprType(exp.getType());
130+
var argExprType = OpenSearchTypeFactory.convertRelDataTypeToExprType(sourceType);
130131
return switch (udt) {
131132
case EXPR_DATE -> makeCall(type, PPLBuiltinOperators.DATE, List.of(exp));
132133
case EXPR_TIME -> makeCall(type, PPLBuiltinOperators.TIME, List.of(exp));
@@ -150,6 +151,13 @@ public RexNode makeCast(
150151
String.format(Locale.ROOT, "Cannot cast from %s to %s", argExprType, udt.name()));
151152
};
152153
}
154+
// Use a custom operator when casting floating point or decimal number to a character type.
155+
// This patch is necessary because in Calcite, 0.0F is cast to 0E0, decimal 0.x to x
156+
else if ((SqlTypeUtil.isApproximateNumeric(sourceType) || SqlTypeUtil.isDecimal(sourceType))
157+
&& SqlTypeUtil.isCharacter(type)) {
158+
// NUMBER_TO_STRING uses java's built-in method to get the string representation of a number
159+
return makeCall(type, PPLBuiltinOperators.NUMBER_TO_STRING, List.of(exp));
160+
}
153161
return super.makeCast(pos, type, exp, matchNullability, safe, format);
154162
}
155163
}

core/src/main/java/org/opensearch/sql/expression/function/PPLBuiltinOperators.java

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,7 @@
7979
import org.opensearch.sql.expression.function.udf.math.DivideFunction;
8080
import org.opensearch.sql.expression.function.udf.math.EulerFunction;
8181
import org.opensearch.sql.expression.function.udf.math.ModFunction;
82+
import org.opensearch.sql.expression.function.udf.math.NumberToStringFunction;
8283

8384
/** Defines functions and operators that are implemented only by PPL */
8485
public class PPLBuiltinOperators extends ReflectiveSqlOperatorTable {
@@ -376,6 +377,8 @@ public class PPLBuiltinOperators extends ReflectiveSqlOperatorTable {
376377
RELEVANCE_QUERY_FUNCTION_INSTANCE.toUDF("query_string", false);
377378
public static final SqlOperator MULTI_MATCH =
378379
RELEVANCE_QUERY_FUNCTION_INSTANCE.toUDF("multi_match", false);
380+
public static final SqlOperator NUMBER_TO_STRING =
381+
new NumberToStringFunction().toUDF("NUMBER_TO_STRING");
379382

380383
/**
381384
* Returns the PPL specific operator table, creating it if necessary.
Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
/*
2+
* Copyright OpenSearch Contributors
3+
* SPDX-License-Identifier: Apache-2.0
4+
*/
5+
6+
package org.opensearch.sql.expression.function.udf.math;
7+
8+
import java.util.List;
9+
import org.apache.calcite.adapter.enumerable.NotNullImplementor;
10+
import org.apache.calcite.adapter.enumerable.NullPolicy;
11+
import org.apache.calcite.adapter.enumerable.RexToLixTranslator;
12+
import org.apache.calcite.linq4j.tree.Expression;
13+
import org.apache.calcite.linq4j.tree.Expressions;
14+
import org.apache.calcite.rex.RexCall;
15+
import org.apache.calcite.sql.type.SqlReturnTypeInference;
16+
import org.opensearch.sql.calcite.utils.PPLOperandTypes;
17+
import org.opensearch.sql.calcite.utils.PPLReturnTypes;
18+
import org.opensearch.sql.expression.function.ImplementorUDF;
19+
import org.opensearch.sql.expression.function.UDFOperandMetadata;
20+
21+
/**
22+
* A custom implementation of number to string cast.
23+
*
24+
* <p>This operator is necessary because Calcite's built-in CAST converts floating point 0.0 to 0E0,
25+
* and converts decimal 0.123 to .123 when casting them to string.
26+
*/
27+
public class NumberToStringFunction extends ImplementorUDF {
28+
public NumberToStringFunction() {
29+
super(new NumberToStringImplementor(), NullPolicy.ANY);
30+
}
31+
32+
@Override
33+
public SqlReturnTypeInference getReturnTypeInference() {
34+
return PPLReturnTypes.STRING_FORCE_NULLABLE;
35+
}
36+
37+
@Override
38+
public UDFOperandMetadata getOperandMetadata() {
39+
return PPLOperandTypes.NUMERIC;
40+
}
41+
42+
public static class NumberToStringImplementor implements NotNullImplementor {
43+
44+
@Override
45+
public Expression implement(
46+
RexToLixTranslator translator, RexCall call, List<Expression> translatedOperands) {
47+
Expression operand = translatedOperands.get(0);
48+
return Expressions.call(Expressions.box(operand), "toString");
49+
}
50+
}
51+
}

integ-test/src/test/java/org/opensearch/sql/ppl/CastFunctionIT.java

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
import static org.opensearch.sql.util.MatcherUtils.verifySchema;
1919

2020
import java.io.IOException;
21+
import java.util.Locale;
2122
import org.json.JSONObject;
2223
import org.junit.Test;
2324
import org.opensearch.sql.common.antlr.SyntaxCheckException;
@@ -273,6 +274,24 @@ public void testCastNumericSTRING() throws IOException {
273274
verifyDataRows(actual, rows(true));
274275
}
275276

277+
@Test
278+
public void testCastDecimal() throws IOException {
279+
JSONObject actual =
280+
executeQuery(
281+
String.format(
282+
"source=%s | eval s = cast(0.99 as string), f = cast(12.9 as float), "
283+
+ "d = cast(100.00 as double), i = cast(2.9 as int) "
284+
+ "| fields s, f, d, i",
285+
TEST_INDEX_DATATYPE_NONNUMERIC));
286+
verifySchema(
287+
actual,
288+
schema("s", "string"),
289+
schema("f", "float"),
290+
schema("d", "double"),
291+
schema("i", "int"));
292+
verifyDataRows(actual, rows("0.99", 12.9f, 100.0d, 2));
293+
}
294+
276295
@Test
277296
public void testCastDate() throws IOException {
278297
JSONObject actual =
@@ -432,4 +451,17 @@ public void testCastToIP() throws IOException {
432451
"IP address string 'invalid_ip' is not valid. Error details: invalid_ip IP Address error:"
433452
+ " validation options do not allow you to specify a non-segmented single value");
434453
}
454+
455+
@Test
456+
public void testCastDoubleAsString() throws IOException {
457+
JSONObject actual =
458+
executeQuery(
459+
String.format(
460+
Locale.ROOT,
461+
"source=%s | head 1 | eval d = cast(0 as double) | eval s = cast(d as string) |"
462+
+ " fields s",
463+
TEST_INDEX_STATE_COUNTRY));
464+
verifySchema(actual, schema("s", "string"));
465+
verifyDataRows(actual, rows("0.0"));
466+
}
435467
}

0 commit comments

Comments
 (0)