Skip to content

Commit 78d35cd

Browse files
bugmakerrrrrrbugmakerrrrrr
committed
enhance OS value parsing
Signed-off-by: panguixin <[email protected]>
1 parent d260e0e commit 78d35cd

File tree

10 files changed

+161
-34
lines changed

10 files changed

+161
-34
lines changed

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -401,7 +401,7 @@ public void testCompare() throws IOException {
401401
var result =
402402
executeQuery(
403403
String.format(
404-
"source=%s | eval `%s` = %s | fields `%s`",
404+
"source=%s | head 1 | eval `%s` = %s | fields `%s`",
405405
TEST_INDEX_DATATYPE_NONNUMERIC, name, functionCall, name));
406406
verifySchema(result, schema(name, null, "boolean"));
407407
verifyDataRows(result, rows(expectedResult));

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

+8-8
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ public void typeof_sql_types() throws IOException {
2929
JSONObject response =
3030
executeQuery(
3131
String.format(
32-
"source=%s | eval `str` = typeof('pewpew'),"
32+
"source=%s | head 1 | eval `str` = typeof('pewpew'),"
3333
+ " `double` = typeof(1.0),"
3434
+ "`int` = typeof(12345),"
3535
+ " `long` = typeof(1234567891011),"
@@ -42,7 +42,7 @@ public void typeof_sql_types() throws IOException {
4242
response =
4343
executeQuery(
4444
String.format(
45-
"source=%s | eval "
45+
"source=%s | head 1 | eval "
4646
+ "`timestamp` = typeof(CAST('1961-04-12 09:07:00' AS TIMESTAMP)),"
4747
+ "`time` = typeof(CAST('09:07:00' AS TIME)),"
4848
+ "`date` = typeof(CAST('1961-04-12' AS DATE))"
@@ -56,7 +56,7 @@ public void typeof_opensearch_types() throws IOException {
5656
JSONObject response =
5757
executeQuery(
5858
String.format(
59-
"source=%s | eval `double` = typeof(double_number), `long` ="
59+
"source=%s | head 1 | eval `double` = typeof(double_number), `long` ="
6060
+ " typeof(long_number),`integer` = typeof(integer_number), `byte` ="
6161
+ " typeof(byte_number),`short` = typeof(short_number), `float` ="
6262
+ " typeof(float_number),`half_float` = typeof(half_float_number),"
@@ -69,11 +69,11 @@ public void typeof_opensearch_types() throws IOException {
6969
response =
7070
executeQuery(
7171
String.format(
72-
"source=%s | eval `text` = typeof(text_value), `date` = typeof(date_value),"
73-
+ " `date_nanos` = typeof(date_nanos_value),`boolean` = typeof(boolean_value),"
74-
+ " `object` = typeof(object_value),`keyword` = typeof(keyword_value), `ip` ="
75-
+ " typeof(ip_value),`binary` = typeof(binary_value), `geo_point` ="
76-
+ " typeof(geo_point_value)"
72+
"source=%s | head 1 | eval `text` = typeof(text_value), `date` ="
73+
+ " typeof(date_value), `date_nanos` = typeof(date_nanos_value),`boolean` ="
74+
+ " typeof(boolean_value), `object` = typeof(object_value),`keyword` ="
75+
+ " typeof(keyword_value), `ip` = typeof(ip_value),`binary` ="
76+
+ " typeof(binary_value), `geo_point` = typeof(geo_point_value)"
7777
// TODO activate this test once `ARRAY` type supported, see
7878
// ExpressionAnalyzer::isTypeNotSupported
7979
// + ", `nested` = typeof(nested_value)"

‎integ-test/src/test/java/org/opensearch/sql/sql/DataTypeIT.java

+60
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
/*
2+
* Copyright OpenSearch Contributors
3+
* SPDX-License-Identifier: Apache-2.0
4+
*/
5+
6+
package org.opensearch.sql.sql;
7+
8+
import static org.opensearch.sql.legacy.SQLIntegTestCase.Index.DATA_TYPE_NONNUMERIC;
9+
import static org.opensearch.sql.legacy.SQLIntegTestCase.Index.DATA_TYPE_NUMERIC;
10+
import static org.opensearch.sql.util.MatcherUtils.rows;
11+
import static org.opensearch.sql.util.MatcherUtils.schema;
12+
import static org.opensearch.sql.util.MatcherUtils.verifyDataRows;
13+
import static org.opensearch.sql.util.MatcherUtils.verifySchema;
14+
15+
import java.io.IOException;
16+
import org.json.JSONObject;
17+
import org.junit.Test;
18+
import org.opensearch.sql.legacy.SQLIntegTestCase;
19+
20+
public class DataTypeIT extends SQLIntegTestCase {
21+
@Override
22+
public void init() throws IOException {
23+
loadIndex(DATA_TYPE_NUMERIC);
24+
loadIndex(DATA_TYPE_NONNUMERIC);
25+
}
26+
27+
@Test
28+
public void testReadingData() throws IOException {
29+
String query =
30+
String.format(
31+
"SELECT"
32+
+ " long_number,integer_number,short_number,byte_number,double_number,float_number,half_float_number,scaled_float_number"
33+
+ " FROM %s LIMIT 5",
34+
Index.DATA_TYPE_NUMERIC.getName());
35+
JSONObject result = executeJdbcRequest(query);
36+
verifySchema(
37+
result,
38+
schema("long_number", "long"),
39+
schema("integer_number", "integer"),
40+
schema("short_number", "short"),
41+
schema("byte_number", "byte"),
42+
schema("float_number", "float"),
43+
schema("double_number", "double"),
44+
schema("half_float_number", "float"),
45+
schema("scaled_float_number", "double"));
46+
verifyDataRows(
47+
result,
48+
rows(1, 2, 3, 4, 5.1, 6.2, 7.3, 8.4),
49+
rows(1, 2, 3, 4, 5.1, 6.2, 7.3, 8.4),
50+
rows(1, 2, 3, 4, 5.1, 6.2, 7.3, 8.4));
51+
52+
query =
53+
String.format(
54+
"SELECT boolean_value,keyword_value FROM %s LIMIT 5",
55+
Index.DATA_TYPE_NONNUMERIC.getName());
56+
result = executeJdbcRequest(query);
57+
verifySchema(result, schema("boolean_value", "boolean"), schema("keyword_value", "keyword"));
58+
verifyDataRows(result, rows(true, "123"), rows(true, "123"), rows(true, "123"));
59+
}
60+
}

‎integ-test/src/test/java/org/opensearch/sql/sql/DateTimeFormatsIT.java

+2-1
Original file line numberDiff line numberDiff line change
@@ -223,7 +223,8 @@ public void testDateNanosGroupBy() {
223223
@SneakyThrows
224224
public void testDateNanosWithNanos() {
225225
String query =
226-
String.format("SELECT date_nanos_value" + " FROM %s", TEST_INDEX_DATATYPE_NONNUMERIC);
226+
String.format(
227+
"SELECT date_nanos_value" + " FROM %s limit 1", TEST_INDEX_DATATYPE_NONNUMERIC);
227228
JSONObject result = executeQuery(query);
228229
verifySchema(result, schema("date_nanos_value", null, "timestamp"));
229230
verifyDataRows(result, rows("2019-03-24 01:34:46.123456789"));

‎integ-test/src/test/java/org/opensearch/sql/sql/SystemFunctionIT.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ public void typeof_opensearch_types() {
4646
String.format(
4747
"SELECT typeof(double_number),typeof(long_number), typeof(integer_number),"
4848
+ " typeof(byte_number), typeof(short_number),typeof(float_number),"
49-
+ " typeof(half_float_number), typeof(scaled_float_number) from %s;",
49+
+ " typeof(half_float_number), typeof(scaled_float_number) from %s limit 1;",
5050
TEST_INDEX_DATATYPE_NUMERIC));
5151
verifyDataRows(
5252
response, rows("DOUBLE", "LONG", "INTEGER", "BYTE", "SHORT", "FLOAT", "FLOAT", "DOUBLE"));
@@ -61,7 +61,7 @@ public void typeof_opensearch_types() {
6161
// TODO activate this test once `ARRAY` type supported, see
6262
// ExpressionAnalyzer::isTypeNotSupported
6363
// + ", typeof(nested_value)"
64-
+ " from %s;",
64+
+ " from %s limit 1;",
6565
TEST_INDEX_DATATYPE_NONNUMERIC));
6666
verifyDataRows(
6767
response,

‎integ-test/src/test/resources/datatypes.json

+5-1
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,6 @@
11
{"index":{"_id":"1"}}
2-
{"boolean_value": true, "keyword_value": "keyword", "text_value": "text", "binary_value": "U29tZSBiaW5hcnkgYmxvYg==", "date_value": "2020-10-13 13:00:00", "date_nanos_value": "2019-03-23T21:34:46.123456789-04:00", "ip_value": "127.0.0.1", "object_value": {"first": "Dale", "last": "Dale"}, "nested_value": [{"first" : "John", "last" : "Smith"}, {"first" : "Alice", "last" : "White"}], "geo_point_value": { "lat": 40.71, "lon": 74.00 }}
2+
{"boolean_value": true, "keyword_value": "123", "text_value": "text", "binary_value": "U29tZSBiaW5hcnkgYmxvYg==", "date_value": "2020-10-13 13:00:00", "date_nanos_value": "2019-03-23T21:34:46.123456789-04:00", "ip_value": "127.0.0.1", "object_value": {"first": "Dale", "last": "Dale"}, "nested_value": [{"first" : "John", "last" : "Smith"}, {"first" : "Alice", "last" : "White"}], "geo_point_value": { "lat": 40.71, "lon": 74.00 }}
3+
{"index":{"_id":"2"}}
4+
{"boolean_value": "true", "keyword_value": 123, "text_value": "text", "binary_value": "U29tZSBiaW5hcnkgYmxvYg==", "date_value": "2020-10-13 13:00:00", "date_nanos_value": "2019-03-23T21:34:46.123456789-04:00", "ip_value": "127.0.0.1", "object_value": {"first": "Dale", "last": "Dale"}, "nested_value": [{"first" : "John", "last" : "Smith"}, {"first" : "Alice", "last" : "White"}], "geo_point_value": { "lat": 40.71, "lon": 74.00 }}
5+
{"index":{"_id":"3"}}
6+
{"boolean_value": ["true"], "keyword_value": [123], "text_value": "text", "binary_value": "U29tZSBiaW5hcnkgYmxvYg==", "date_value": "2020-10-13 13:00:00", "date_nanos_value": "2019-03-23T21:34:46.123456789-04:00", "ip_value": "127.0.0.1", "object_value": {"first": "Dale", "last": "Dale"}, "nested_value": [{"first" : "John", "last" : "Smith"}, {"first" : "Alice", "last" : "White"}], "geo_point_value": { "lat": 40.71, "lon": 74.00 }}

‎integ-test/src/test/resources/datatypes_numeric.json

+4
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,6 @@
11
{"index":{"_id":"1"}}
22
{"long_number": 1, "integer_number": 2, "short_number": 3, "byte_number": 4, "double_number": 5.1, "float_number": 6.2, "half_float_number": 7.3, "scaled_float_number": 8.4}
3+
{"index":{"_id":"2"}}
4+
{"long_number": "1", "integer_number": "2", "short_number": "3", "byte_number": "4", "double_number": "5.1", "float_number": "6.2", "half_float_number": "7.3", "scaled_float_number": "8.4"}
5+
{"index":{"_id":"3"}}
6+
{"long_number": ["1"], "integer_number": ["2"], "short_number": ["3"], "byte_number": ["4"], "double_number": ["5.1"], "float_number": ["6.2"], "half_float_number": ["7.3"], "scaled_float_number": ["8.4"]}

‎opensearch/src/main/java/org/opensearch/sql/opensearch/data/utils/OpenSearchJsonContent.java

+44-7
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
import lombok.RequiredArgsConstructor;
1515
import org.apache.commons.lang3.tuple.Pair;
1616
import org.opensearch.OpenSearchParseException;
17+
import org.opensearch.common.Numbers;
1718
import org.opensearch.common.geo.GeoPoint;
1819
import org.opensearch.common.geo.GeoUtils;
1920
import org.opensearch.common.xcontent.json.JsonXContentParser;
@@ -29,32 +30,32 @@ public class OpenSearchJsonContent implements Content {
2930

3031
@Override
3132
public Integer intValue() {
32-
return value().intValue();
33+
return (int) extractDoubleValue(value());
3334
}
3435

3536
@Override
3637
public Long longValue() {
37-
return value().longValue();
38+
return extractLongValue(value());
3839
}
3940

4041
@Override
4142
public Short shortValue() {
42-
return value().shortValue();
43+
return (short) extractDoubleValue(value());
4344
}
4445

4546
@Override
4647
public Byte byteValue() {
47-
return (byte) value().shortValue();
48+
return (byte) extractDoubleValue(value());
4849
}
4950

5051
@Override
5152
public Float floatValue() {
52-
return value().floatValue();
53+
return (float) extractDoubleValue(value());
5354
}
5455

5556
@Override
5657
public Double doubleValue() {
57-
return value().doubleValue();
58+
return extractDoubleValue(value());
5859
}
5960

6061
@Override
@@ -64,7 +65,7 @@ public String stringValue() {
6465

6566
@Override
6667
public Boolean booleanValue() {
67-
return value().booleanValue();
68+
return extractBooleanValue(value());
6869
}
6970

7071
@Override
@@ -148,4 +149,40 @@ public Pair<Double, Double> geoValue() {
148149
private JsonNode value() {
149150
return value;
150151
}
152+
153+
/** Get double value from JsonNode if possible. */
154+
private double extractDoubleValue(JsonNode node) {
155+
if (node.isTextual()) {
156+
return Double.parseDouble(node.textValue());
157+
}
158+
if (node.isNumber()) {
159+
return node.doubleValue();
160+
} else {
161+
throw new OpenSearchParseException("node must be a number");
162+
}
163+
}
164+
165+
/** Get long value from JsonNode if possible. */
166+
private long extractLongValue(JsonNode node) {
167+
if (node.isTextual()) {
168+
return Numbers.toLong(node.textValue(), true);
169+
}
170+
if (node.isNumber()) {
171+
return node.longValue();
172+
} else {
173+
throw new OpenSearchParseException("node must be a number");
174+
}
175+
}
176+
177+
/** Get boolean value from JsonNode if possible. */
178+
private boolean extractBooleanValue(JsonNode node) {
179+
if (node.isTextual()) {
180+
return Boolean.parseBoolean(node.textValue());
181+
}
182+
if (node.isBoolean()) {
183+
return node.booleanValue();
184+
} else {
185+
throw new OpenSearchParseException("node must be a boolean");
186+
}
187+
}
151188
}

‎opensearch/src/main/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprValueFactory.java

+6-9
Original file line numberDiff line numberDiff line change
@@ -60,10 +60,8 @@
6060
import org.opensearch.sql.data.model.ExprValue;
6161
import org.opensearch.sql.data.type.ExprCoreType;
6262
import org.opensearch.sql.data.type.ExprType;
63-
import org.opensearch.sql.opensearch.data.type.OpenSearchBinaryType;
6463
import org.opensearch.sql.opensearch.data.type.OpenSearchDataType;
6564
import org.opensearch.sql.opensearch.data.type.OpenSearchDateType;
66-
import org.opensearch.sql.opensearch.data.type.OpenSearchIpType;
6765
import org.opensearch.sql.opensearch.data.utils.Content;
6866
import org.opensearch.sql.opensearch.data.utils.ObjectContent;
6967
import org.opensearch.sql.opensearch.data.utils.OpenSearchJsonContent;
@@ -346,9 +344,8 @@ private ExprValue parseArray(
346344
if (content.objectValue() instanceof ObjectNode) {
347345
result.add(parseStruct(content, prefix, supportArrays));
348346
// non-object type arrays are only supported when parsing inner_hits of OS response.
349-
} else if (!(type instanceof OpenSearchDataType
350-
&& ((OpenSearchDataType) type).getExprType().equals(ARRAY))
351-
&& !supportArrays) {
347+
} else if (((OpenSearchDataType) type).getExprType().equals(ARRAY) == false
348+
&& supportArrays == false) {
352349
return parseInnerArrayValue(content.array().next(), prefix, type, supportArrays);
353350
} else {
354351
content
@@ -415,10 +412,10 @@ private ExprValue parseGeoPoint(Content content, boolean supportArrays) {
415412
*/
416413
private ExprValue parseInnerArrayValue(
417414
Content content, String prefix, ExprType type, boolean supportArrays) {
418-
if (type instanceof OpenSearchIpType
419-
|| type instanceof OpenSearchBinaryType
420-
|| type instanceof OpenSearchDateType) {
421-
return parse(content, prefix, Optional.of(type), supportArrays);
415+
if (content.isArray()) {
416+
return parseArray(content, prefix, type, supportArrays);
417+
} else if (typeActionMap.containsKey(type)) {
418+
return typeActionMap.get(type).apply(content, type);
422419
} else if (content.isString()) {
423420
return parse(content, prefix, Optional.of(OpenSearchDataType.of(STRING)), supportArrays);
424421
} else if (content.isLong()) {

‎opensearch/src/test/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprValueFactoryTest.java

+29-5
Original file line numberDiff line numberDiff line change
@@ -156,6 +156,7 @@ public void constructNullArrayValue() {
156156
public void constructByte() {
157157
assertAll(
158158
() -> assertEquals(byteValue((byte) 1), tupleValue("{\"byteV\":1}").get("byteV")),
159+
() -> assertEquals(byteValue((byte) 1), tupleValue("{\"byteV\":\"1\"}").get("byteV")),
159160
() -> assertEquals(byteValue((byte) 1), constructFromObject("byteV", 1)),
160161
() -> assertEquals(byteValue((byte) 1), constructFromObject("byteV", "1.0")));
161162
}
@@ -164,6 +165,7 @@ public void constructByte() {
164165
public void constructShort() {
165166
assertAll(
166167
() -> assertEquals(shortValue((short) 1), tupleValue("{\"shortV\":1}").get("shortV")),
168+
() -> assertEquals(shortValue((short) 1), tupleValue("{\"shortV\":\"1\"}").get("shortV")),
167169
() -> assertEquals(shortValue((short) 1), constructFromObject("shortV", 1)),
168170
() -> assertEquals(shortValue((short) 1), constructFromObject("shortV", "1.0")));
169171
}
@@ -172,19 +174,16 @@ public void constructShort() {
172174
public void constructInteger() {
173175
assertAll(
174176
() -> assertEquals(integerValue(1), tupleValue("{\"intV\":1}").get("intV")),
177+
() -> assertEquals(integerValue(1), tupleValue("{\"intV\":\"1\"}").get("intV")),
175178
() -> assertEquals(integerValue(1), constructFromObject("intV", 1)),
176179
() -> assertEquals(integerValue(1), constructFromObject("intV", "1.0")));
177180
}
178181

179-
@Test
180-
public void constructIntegerValueInStringValue() {
181-
assertEquals(integerValue(1), constructFromObject("intV", "1"));
182-
}
183-
184182
@Test
185183
public void constructLong() {
186184
assertAll(
187185
() -> assertEquals(longValue(1L), tupleValue("{\"longV\":1}").get("longV")),
186+
() -> assertEquals(longValue(1L), tupleValue("{\"longV\":\"1\"}").get("longV")),
188187
() -> assertEquals(longValue(1L), constructFromObject("longV", 1L)),
189188
() -> assertEquals(longValue(1L), constructFromObject("longV", "1.0")));
190189
}
@@ -193,13 +192,15 @@ public void constructLong() {
193192
public void constructFloat() {
194193
assertAll(
195194
() -> assertEquals(floatValue(1f), tupleValue("{\"floatV\":1.0}").get("floatV")),
195+
() -> assertEquals(floatValue(1f), tupleValue("{\"floatV\":\"1.0\"}").get("floatV")),
196196
() -> assertEquals(floatValue(1f), constructFromObject("floatV", 1f)));
197197
}
198198

199199
@Test
200200
public void constructDouble() {
201201
assertAll(
202202
() -> assertEquals(doubleValue(1d), tupleValue("{\"doubleV\":1.0}").get("doubleV")),
203+
() -> assertEquals(doubleValue(1d), tupleValue("{\"doubleV\":\"1.0\"}").get("doubleV")),
203204
() -> assertEquals(doubleValue(1d), constructFromObject("doubleV", 1d)));
204205
}
205206

@@ -215,6 +216,7 @@ public void constructString() {
215216
public void constructBoolean() {
216217
assertAll(
217218
() -> assertEquals(booleanValue(true), tupleValue("{\"boolV\":true}").get("boolV")),
219+
() -> assertEquals(booleanValue(true), tupleValue("{\"boolV\":\"true\"}").get("boolV")),
218220
() -> assertEquals(booleanValue(true), constructFromObject("boolV", true)),
219221
() -> assertEquals(booleanValue(true), constructFromObject("boolV", "true")),
220222
() -> assertEquals(booleanValue(true), constructFromObject("boolV", 1)),
@@ -755,6 +757,27 @@ public void constructGeoPointFromUnsupportedFormatShouldThrowException() {
755757
assertEquals("lat must be a number", exception.getMessage());
756758
}
757759

760+
@Test
761+
public void constructNumberFromUnsupportedFormatShouldThrowException() {
762+
OpenSearchParseException exception =
763+
assertThrows(
764+
OpenSearchParseException.class, () -> tupleValue("{\"intV\": false}").get("intV"));
765+
assertEquals("node must be a number", exception.getMessage());
766+
767+
exception =
768+
assertThrows(
769+
OpenSearchParseException.class, () -> tupleValue("{\"longV\": false}").get("intV"));
770+
assertEquals("node must be a number", exception.getMessage());
771+
}
772+
773+
@Test
774+
public void constructBooleanFromUnsupportedFormatShouldThrowException() {
775+
OpenSearchParseException exception =
776+
assertThrows(
777+
OpenSearchParseException.class, () -> tupleValue("{\"boolV\": 1}").get("boolV"));
778+
assertEquals("node must be a boolean", exception.getMessage());
779+
}
780+
758781
@Test
759782
public void constructBinary() {
760783
assertEquals(
@@ -769,6 +792,7 @@ public void constructBinary() {
769792
@Test
770793
public void constructFromOpenSearchArrayReturnFirstElement() {
771794
assertEquals(integerValue(1), tupleValue("{\"intV\":[1, 2, 3]}").get("intV"));
795+
assertEquals(integerValue(1), tupleValue("{\"intV\":[\"1\", 2, 3]}").get("intV"));
772796
assertEquals(
773797
new ExprTupleValue(
774798
new LinkedHashMap<String, ExprValue>() {

0 commit comments

Comments
 (0)