Skip to content

Commit 882b488

Browse files
asifabasharasifabashar
committed
for all cases, return double
Signed-off-by: Asif Bashar <asif.bashar@gmail.com>
1 parent 15817ae commit 882b488

4 files changed

Lines changed: 31 additions & 88 deletions

File tree

‎core/src/main/java/org/opensearch/sql/expression/function/udf/ToNumberFunction.java‎

Lines changed: 4 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,8 @@
1414
import org.apache.calcite.linq4j.tree.Expression;
1515
import org.apache.calcite.linq4j.tree.Expressions;
1616
import org.apache.calcite.rex.RexCall;
17+
import org.apache.calcite.sql.type.ReturnTypes;
1718
import org.apache.calcite.sql.type.SqlReturnTypeInference;
18-
import org.apache.calcite.sql.type.SqlTypeName;
1919
import org.opensearch.sql.calcite.utils.PPLOperandTypes;
2020
import org.opensearch.sql.expression.function.ImplementorUDF;
2121
import org.opensearch.sql.expression.function.UDFOperandMetadata;
@@ -47,66 +47,8 @@ public ToNumberFunction() {
4747

4848
@Override
4949
public SqlReturnTypeInference getReturnTypeInference() {
50-
return (opBinding) -> {
51-
// Try to determine if the result will be Long or Double based on the input
52-
int base = 10;
53-
if (opBinding.getOperandCount() > 1) {
54-
55-
base = opBinding.getOperandLiteralValue(1, Integer.class);
56-
}
5750

58-
if (opBinding.getOperandCount() > 0 && opBinding.isOperandLiteral(0, false)) {
59-
String literal = opBinding.getOperandLiteralValue(0, String.class);
60-
if (literal != null) {
61-
try {
62-
// Check if it's a decimal number
63-
if (base != 10) {
64-
return opBinding
65-
.getTypeFactory()
66-
.createTypeWithNullability(
67-
opBinding.getTypeFactory().createSqlType(SqlTypeName.BIGINT), true);
68-
}
69-
if (literal.contains(".")) {
70-
return opBinding
71-
.getTypeFactory()
72-
.createTypeWithNullability(
73-
opBinding
74-
.getTypeFactory()
75-
.createSqlType(org.apache.calcite.sql.type.SqlTypeName.DOUBLE),
76-
true);
77-
} else {
78-
// Check if it's an integer that fits in Long
79-
Long.parseLong(literal);
80-
return opBinding
81-
.getTypeFactory()
82-
.createTypeWithNullability(
83-
opBinding
84-
.getTypeFactory()
85-
.createSqlType(org.apache.calcite.sql.type.SqlTypeName.BIGINT),
86-
true);
87-
}
88-
} catch (NumberFormatException e) {
89-
// If parsing fails, default to Double (matches the runtime behavior)
90-
return opBinding
91-
.getTypeFactory()
92-
.createTypeWithNullability(
93-
opBinding
94-
.getTypeFactory()
95-
.createSqlType(org.apache.calcite.sql.type.SqlTypeName.DOUBLE),
96-
true);
97-
}
98-
}
99-
}
100-
// Default to Double when we can't determine the type at compile time or bigint type is
101-
// confirmed
102-
return opBinding
103-
.getTypeFactory()
104-
.createTypeWithNullability(
105-
opBinding
106-
.getTypeFactory()
107-
.createSqlType(org.apache.calcite.sql.type.SqlTypeName.DOUBLE),
108-
true);
109-
};
51+
return ReturnTypes.DOUBLE_FORCE_NULLABLE;
11052
}
11153

11254
@Override
@@ -120,6 +62,7 @@ public static class ToNumberImplementor implements NotNullImplementor {
12062
public Expression implement(
12163
RexToLixTranslator translator, RexCall call, List<Expression> translatedOperands) {
12264
Expression fieldValue = translatedOperands.get(0);
65+
int base = 10;
12366
if (translatedOperands.size() > 1) {
12467
Expression baseExpr = translatedOperands.get(1);
12568
return Expressions.call(ToNumberFunction.class, "toNumber", fieldValue, baseExpr);
@@ -152,7 +95,7 @@ public static Number toNumber(String numStr, int base) {
15295
result = bigInteger.longValue();
15396
}
15497
} catch (Exception e) {
155-
// Return null when parsing fails, matches function behavior
98+
15699
}
157100
return result;
158101
}

‎docs/user/ppl/functions/conversion.rst‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,7 @@ Following example converts a string in binary to the number representation::
145145
+-----------+
146146
| int_value |
147147
|-----------|
148-
| 21 |
148+
| 21.0 |
149149
+-----------+
150150

151151

@@ -156,7 +156,7 @@ Following example converts a string in hex to the number representation::
156156
+-----------+
157157
| int_value |
158158
|-----------|
159-
| 64052 |
159+
| 64052.0 |
160160
+-----------+
161161

162162
Following example converts a string in decimal to the number representation::
@@ -166,7 +166,7 @@ Following example converts a string in decimal to the number representation::
166166
+-----------+
167167
| int_value |
168168
|-----------|
169-
| 4598 |
169+
| 4598.0 |
170170
+-----------+
171171

172172
Following example converts a string in decimal with fraction to the number representation::

‎integ-test/src/test/java/org/opensearch/sql/ppl/ConversionFunctionIT.java‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ public void testHex() throws IOException {
3939
"source=%s |head 1| eval a = tonumber('FF12CA',16) | fields a",
4040
TEST_INDEX_ACCOUNT));
4141

42-
verifySchema(actual, schema("a", "bigint"));
42+
verifySchema(actual, schema("a", "double"));
4343

4444
verifyDataRows(actual, rows(16716490));
4545
}
@@ -52,7 +52,7 @@ public void testBinary() throws IOException {
5252
"source=%s |head 1| eval a = tonumber('0110111',2) | fields a",
5353
TEST_INDEX_ACCOUNT));
5454

55-
verifySchema(actual, schema("a", "bigint"));
55+
verifySchema(actual, schema("a", "double"));
5656

5757
verifyDataRows(actual, rows(55));
5858
}
@@ -65,7 +65,7 @@ public void testOctal() throws IOException {
6565
"source=%s |head 1| eval a = tonumber('20415442',8) | fields a",
6666
TEST_INDEX_ACCOUNT));
6767

68-
verifySchema(actual, schema("a", "bigint"));
68+
verifySchema(actual, schema("a", "double"));
6969

7070
verifyDataRows(actual, rows(4332322));
7171
}
@@ -78,7 +78,7 @@ public void testOctalWithUnsupportedValue() throws IOException {
7878
"source=%s |head 1| eval a = tonumber('20415.442',8) | fields a",
7979
TEST_INDEX_ACCOUNT));
8080

81-
verifySchema(actual, schema("a", "bigint"));
81+
verifySchema(actual, schema("a", "double"));
8282

8383
assertEquals(actual.getJSONArray("datarows").getJSONArray(0).get(0), null);
8484
}
@@ -91,7 +91,7 @@ public void testBinaryWithUnsupportedValue() throws IOException {
9191
"source=%s |head 1| eval a = tonumber('1010.11',2) | fields a",
9292
TEST_INDEX_ACCOUNT));
9393

94-
verifySchema(actual, schema("a", "bigint"));
94+
verifySchema(actual, schema("a", "double"));
9595

9696
assertEquals(actual.getJSONArray("datarows").getJSONArray(0).get(0), null);
9797
}
@@ -103,7 +103,7 @@ public void testHexWithUnsupportedValue() throws IOException {
103103
String.format(
104104
"source=%s |head 1| eval a = tonumber('A.B',16) | fields a", TEST_INDEX_ACCOUNT));
105105

106-
verifySchema(actual, schema("a", "bigint"));
106+
verifySchema(actual, schema("a", "double"));
107107

108108
assertEquals(actual.getJSONArray("datarows").getJSONArray(0).get(0), null);
109109
}

‎ppl/src/test/java/org/opensearch/sql/ppl/calcite/CalcitePPLToNumberFunctionTest.java‎

Lines changed: 18 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -24,11 +24,11 @@ public void testNumberBinary() {
2424
+ " LogicalProject(int_value=[TONUMBER('010101':VARCHAR, 2)])\n"
2525
+ " LogicalTableScan(table=[[scott, EMP]])\n";
2626
verifyLogical(root, expectedLogical);
27-
String expectedResult = "int_value=21\n";
27+
String expectedResult = "int_value=21.0\n";
2828
verifyResult(root, expectedResult);
2929

3030
String expectedSparkSql =
31-
"SELECT TONUMBER('010101', 2) `int_value`\nFROM `scott`.`EMP`\nLIMIT 1";
31+
"SELECT `TONUMBER`('010101', 2) `int_value`\nFROM `scott`.`EMP`\nLIMIT 1";
3232
verifyPPLToSparkSQL(root, expectedSparkSql);
3333
}
3434

@@ -45,7 +45,7 @@ public void testNumberBinaryUnsupportedResultNull() {
4545
verifyResult(root, expectedResult);
4646

4747
String expectedSparkSql =
48-
"SELECT TONUMBER('010.101', 2) `int_value`\nFROM `scott`.`EMP`\nLIMIT 1";
48+
"SELECT `TONUMBER`('010.101', 2) `int_value`\nFROM `scott`.`EMP`\nLIMIT 1";
4949
verifyPPLToSparkSQL(root, expectedSparkSql);
5050
}
5151

@@ -58,11 +58,11 @@ public void testNumberHex() {
5858
+ " LogicalProject(int_value=[TONUMBER('FA34':VARCHAR, 16)])\n"
5959
+ " LogicalTableScan(table=[[scott, EMP]])\n";
6060
verifyLogical(root, expectedLogical);
61-
String expectedResult = "int_value=64052\n";
61+
String expectedResult = "int_value=64052.0\n";
6262
verifyResult(root, expectedResult);
6363

6464
String expectedSparkSql =
65-
"SELECT TONUMBER('FA34', 16) `int_value`\nFROM `scott`.`EMP`\nLIMIT 1";
65+
"SELECT `TONUMBER`('FA34', 16) `int_value`\nFROM `scott`.`EMP`\nLIMIT 1";
6666
verifyPPLToSparkSQL(root, expectedSparkSql);
6767
}
6868

@@ -80,7 +80,7 @@ public void testNumberHexUnsupportedValuesResultNull() {
8080
verifyResult(root, expectedResult);
8181

8282
String expectedSparkSql =
83-
"SELECT TONUMBER('FA.34', 16) `double_value`\nFROM `scott`.`EMP`\nLIMIT 1";
83+
"SELECT `TONUMBER`('FA.34', 16) `double_value`\nFROM `scott`.`EMP`\nLIMIT 1";
8484
verifyPPLToSparkSQL(root, expectedSparkSql);
8585
}
8686

@@ -95,11 +95,11 @@ public void testNumberHexMinLimit() {
9595
+ " LogicalProject(long_value=[TONUMBER('-7FFFFFFFFFFFFFFF':VARCHAR, 16)])\n"
9696
+ " LogicalTableScan(table=[[scott, EMP]])\n";
9797
verifyLogical(root, expectedLogical);
98-
String expectedResult = "long_value=-9223372036854775807\n";
98+
String expectedResult = "long_value=-9.223372036854776E18\n";
9999
verifyResult(root, expectedResult);
100100

101101
String expectedSparkSql =
102-
"SELECT TONUMBER('-7FFFFFFFFFFFFFFF', 16) `long_value`\nFROM `scott`.`EMP`\nLIMIT 1";
102+
"SELECT `TONUMBER`('-7FFFFFFFFFFFFFFF', 16) `long_value`\nFROM `scott`.`EMP`\nLIMIT 1";
103103

104104
verifyPPLToSparkSQL(root, expectedSparkSql);
105105
}
@@ -115,11 +115,11 @@ public void testNumberHexMaxLimit() {
115115
+ " LogicalProject(long_value=[TONUMBER('7FFFFFFFFFFFFFFF':VARCHAR, 16)])\n"
116116
+ " LogicalTableScan(table=[[scott, EMP]])\n";
117117
verifyLogical(root, expectedLogical);
118-
String expectedResult = "long_value=9223372036854775807\n";
118+
String expectedResult = "long_value=9.223372036854776E18\n";
119119
verifyResult(root, expectedResult);
120120

121121
String expectedSparkSql =
122-
"SELECT TONUMBER('7FFFFFFFFFFFFFFF', 16) `long_value`\nFROM `scott`.`EMP`\nLIMIT 1";
122+
"SELECT `TONUMBER`('7FFFFFFFFFFFFFFF', 16) `long_value`\nFROM `scott`.`EMP`\nLIMIT 1";
123123

124124
verifyPPLToSparkSQL(root, expectedSparkSql);
125125
}
@@ -135,11 +135,11 @@ public void testNumberHexOverNegativeMaxLimit() {
135135
+ " LogicalProject(long_value=[TONUMBER('-FFFFFFFFFFFFFFFF':VARCHAR, 16)])\n"
136136
+ " LogicalTableScan(table=[[scott, EMP]])\n";
137137
verifyLogical(root, expectedLogical);
138-
String expectedResult = "long_value=1\n";
138+
String expectedResult = "long_value=1.0\n";
139139
verifyResult(root, expectedResult);
140140

141141
String expectedSparkSql =
142-
"SELECT TONUMBER('-FFFFFFFFFFFFFFFF', 16) `long_value`\nFROM `scott`.`EMP`\nLIMIT 1";
142+
"SELECT `TONUMBER`('-FFFFFFFFFFFFFFFF', 16) `long_value`\nFROM `scott`.`EMP`\nLIMIT 1";
143143

144144
verifyPPLToSparkSQL(root, expectedSparkSql);
145145
}
@@ -154,11 +154,11 @@ public void testNumberHexOverPositiveMaxLimit() {
154154
+ " LogicalProject(long_value=[TONUMBER('FFFFFFFFFFFFFFFF':VARCHAR, 16)])\n"
155155
+ " LogicalTableScan(table=[[scott, EMP]])\n";
156156
verifyLogical(root, expectedLogical);
157-
String expectedResult = "long_value=-1\n";
157+
String expectedResult = "long_value=-1.0\n";
158158
verifyResult(root, expectedResult);
159159

160160
String expectedSparkSql =
161-
"SELECT TONUMBER('FFFFFFFFFFFFFFFF', 16) `long_value`\nFROM `scott`.`EMP`\nLIMIT 1";
161+
"SELECT `TONUMBER`('FFFFFFFFFFFFFFFF', 16) `long_value`\nFROM `scott`.`EMP`\nLIMIT 1";
162162

163163
verifyPPLToSparkSQL(root, expectedSparkSql);
164164
}
@@ -172,10 +172,10 @@ public void testNumber() {
172172
+ " LogicalProject(int_value=[TONUMBER('4598':VARCHAR)])\n"
173173
+ " LogicalTableScan(table=[[scott, EMP]])\n";
174174
verifyLogical(root, expectedLogical);
175-
String expectedResult = "int_value=4598\n";
175+
String expectedResult = "int_value=4598.0\n";
176176
verifyResult(root, expectedResult);
177177

178-
String expectedSparkSql = "SELECT TONUMBER('4598') `int_value`\nFROM `scott`.`EMP`\nLIMIT 1";
178+
String expectedSparkSql = "SELECT `TONUMBER`('4598') `int_value`\nFROM `scott`.`EMP`\nLIMIT 1";
179179
verifyPPLToSparkSQL(root, expectedSparkSql);
180180
}
181181

@@ -192,7 +192,7 @@ public void testNumberDecimal() {
192192
verifyResult(root, expectedResult);
193193

194194
String expectedSparkSql =
195-
"SELECT TONUMBER('4598.54922') `int_value`\nFROM `scott`.`EMP`\nLIMIT 1";
195+
"SELECT `TONUMBER`('4598.54922') `int_value`\nFROM `scott`.`EMP`\nLIMIT 1";
196196
verifyPPLToSparkSQL(root, expectedSparkSql);
197197
}
198198

@@ -209,7 +209,7 @@ public void testNumberUnsupportedResultNull() {
209209
verifyResult(root, expectedResult);
210210

211211
String expectedSparkSql =
212-
"SELECT TONUMBER('4A598.54922') `int_value`\nFROM `scott`.`EMP`\nLIMIT 1";
212+
"SELECT `TONUMBER`('4A598.54922') `int_value`\nFROM `scott`.`EMP`\nLIMIT 1";
213213
verifyPPLToSparkSQL(root, expectedSparkSql);
214214
}
215215
}

0 commit comments

Comments
 (0)