Skip to content

Commit 644b6a4

Browse files
opensearch-trigger-bot[bot]github-actions[bot]qianheng-aws
authored
[Backport 2.x] Support struct field with dynamic disabled (opensearch-project#3861)
* Support struct field with dynamic disabled (opensearch-project#3829) * Support struct field with dynamic disabled Signed-off-by: Heng Qian <qianheng@amazon.com> * Fix UT Signed-off-by: Heng Qian <qianheng@amazon.com> * Address comments Signed-off-by: Heng Qian <qianheng@amazon.com> * Revert change in GSON Signed-off-by: Heng Qian <qianheng@amazon.com> * Revert change in GSON Signed-off-by: Heng Qian <qianheng@amazon.com> --------- Signed-off-by: Heng Qian <qianheng@amazon.com> (cherry picked from commit 76b3ec2) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> * Fix compiling error Signed-off-by: Heng Qian <qianheng@amazon.com> * Add UT Signed-off-by: Heng Qian <qianheng@amazon.com> --------- Signed-off-by: Heng Qian <qianheng@amazon.com> Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: Heng Qian <qianheng@amazon.com>
1 parent 433bb9e commit 644b6a4

6 files changed

Lines changed: 130 additions & 3 deletions

File tree

‎integ-test/src/yamlRestTest/resources/rest-api-spec/test/issues/3343.yml‎

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
setup:
2+
- do:
3+
indices.create:
4+
index: test
5+
body:
6+
settings:
7+
number_of_shards: 1
8+
number_of_replicas: 0
9+
mappings:
10+
properties:
11+
profile:
12+
type: object
13+
dynamic: false
14+
- do:
15+
query.settings:
16+
body:
17+
transient:
18+
plugins.calcite.enabled : true
19+
plugins.calcite.fallback.allowed : false
20+
21+
---
22+
teardown:
23+
- do:
24+
query.settings:
25+
body:
26+
transient:
27+
plugins.calcite.enabled : false
28+
plugins.calcite.fallback.allowed : true
29+
30+
---
31+
"Handle struct field with dynamic mapping disabled":
32+
- skip:
33+
features:
34+
- headers
35+
- allowed_warnings
36+
- do:
37+
bulk:
38+
index: test
39+
refresh: true
40+
body:
41+
- '{ "index": { "_index": "test" } }'
42+
- '{ "profile": { "age": 1 } }'
43+
- '{ "index": { "_index": "test" } }'
44+
- '{ "profile": { "address": "a" } }'
45+
- do:
46+
allowed_warnings:
47+
- 'Loading the fielddata on the _id field is deprecated and will be removed in future versions. If you require sorting or aggregating on this field you should also include the id in the body of your documents, and map this field as a keyword field that has [doc_values] enabled'
48+
headers:
49+
Content-Type: 'application/json'
50+
ppl:
51+
body:
52+
query: 'source=test'
53+
- match: {"total": 2}
54+
- match: {"schema": [{"name": "profile", "type": "struct"}]}
55+
- match: {"datarows": [[{"age": 1}], [{"address": "a"}]]}

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,9 @@ public interface Content {
2929
/** Is double value. */
3030
boolean isDouble();
3131

32+
/** Is int value. */
33+
boolean isInt();
34+
3235
/** Is long value. */
3336
boolean isLong();
3437

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,11 @@ public boolean isNumber() {
104104
return value instanceof Number;
105105
}
106106

107+
@Override
108+
public boolean isInt() {
109+
return value instanceof Integer;
110+
}
111+
107112
@Override
108113
public boolean isFloat() {
109114
return value instanceof Float;

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,11 @@ public boolean isNumber() {
9292
return value().isNumber();
9393
}
9494

95+
@Override
96+
public boolean isInt() {
97+
return value.isInt();
98+
}
99+
95100
@Override
96101
public boolean isLong() {
97102
return value().isLong();

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

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -186,10 +186,16 @@ public ExprValue construct(String field, Object value, boolean supportArrays) {
186186

187187
private ExprValue parse(
188188
Content content, String field, Optional<ExprType> fieldType, boolean supportArrays) {
189-
if (content.isNull() || !fieldType.isPresent()) {
189+
if (content.isNull()) {
190190
return ExprNullValue.of();
191191
}
192192

193+
// Field type may be not defined in mapping if users have disabled dynamic mapping.
194+
// Then try to parse content directly based on the value itself
195+
if (fieldType.isEmpty()) {
196+
return parseContent(content);
197+
}
198+
193199
final ExprType type = fieldType.get();
194200

195201
if (type.equals(OpenSearchDataType.of(OpenSearchDataType.MappingType.GeoPoint))) {
@@ -211,6 +217,29 @@ private ExprValue parse(
211217
}
212218
}
213219

220+
static ExprValue parseContent(Content content) {
221+
if (content.isNumber()) {
222+
if (content.isInt()) {
223+
return new ExprIntegerValue(content.intValue());
224+
} else if (content.isLong()) {
225+
return new ExprLongValue(content.longValue());
226+
} else if (content.isFloat()) {
227+
return new ExprFloatValue(content.floatValue());
228+
} else if (content.isDouble()) {
229+
return new ExprDoubleValue(content.doubleValue());
230+
} else {
231+
// Default case for number, treat as double
232+
return new ExprDoubleValue(content.doubleValue());
233+
}
234+
} else if (content.isString()) {
235+
return new ExprStringValue(content.stringValue());
236+
} else if (content.isBoolean()) {
237+
return ExprBooleanValue.of(content.booleanValue());
238+
}
239+
// Default case, treat as a string value
240+
return new ExprStringValue(content.objectValue().toString());
241+
}
242+
214243
/**
215244
* In OpenSearch, it is possible field doesn't have type definition in mapping. but has empty
216245
* value. For example, {"empty_field": []}.

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

Lines changed: 32 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,8 +37,14 @@
3737
import static org.opensearch.sql.utils.DateTimeUtils.UTC_ZONE_ID;
3838

3939
import com.fasterxml.jackson.core.JsonProcessingException;
40+
import com.fasterxml.jackson.databind.JsonNode;
4041
import com.fasterxml.jackson.databind.ObjectMapper;
42+
import com.fasterxml.jackson.databind.node.DecimalNode;
43+
import com.fasterxml.jackson.databind.node.FloatNode;
44+
import com.fasterxml.jackson.databind.node.JsonNodeFactory;
45+
import com.fasterxml.jackson.databind.node.ObjectNode;
4146
import com.google.common.collect.ImmutableMap;
47+
import java.math.BigDecimal;
4248
import java.time.Instant;
4349
import java.time.LocalDate;
4450
import java.time.LocalTime;
@@ -179,6 +185,7 @@ public void constructInteger() {
179185
assertAll(
180186
() -> assertEquals(integerValue(1), tupleValue("{\"intV\":1}").get("intV")),
181187
() -> assertEquals(integerValue(1), constructFromObject("intV", 1)),
188+
() -> assertEquals(integerValue(1), constructFromObject("noType", 1)),
182189
() -> assertEquals(integerValue(1), constructFromObject("intV", "1.0")));
183190
}
184191

@@ -961,8 +968,31 @@ public void constructFromInvalidJsonThrowException() {
961968
@Test
962969
public void noTypeFoundForMapping() {
963970
assertEquals(nullValue(), tupleValue("{\"not_exist\":[]}").get("not_exist"));
964-
// Only for test coverage, It is impossible in OpenSearch.
965-
assertEquals(nullValue(), tupleValue("{\"not_exist\":1}").get("not_exist"));
971+
assertEquals(integerValue(1), tupleValue("{\"not_exist\":1}").get("not_exist"));
972+
assertEquals(
973+
longValue(1234567890123456789L),
974+
tupleValue("{\"not_exist\":1234567890123456789}").get("not_exist"));
975+
assertEquals(doubleValue(1.1), tupleValue("{\"not_exist\":1.1}").get("not_exist"));
976+
assertEquals(stringValue("1"), tupleValue("{\"not_exist\":\"1\"}").get("not_exist"));
977+
assertEquals(booleanValue(true), tupleValue("{\"not_exist\":true}").get("not_exist"));
978+
979+
// construct from JsonNode directly
980+
JsonNode jsonNode = new FloatNode(1.0f);
981+
assertEquals(
982+
floatValue(1.0f),
983+
OpenSearchExprValueFactory.parseContent(new OpenSearchJsonContent(jsonNode)));
984+
jsonNode = new DecimalNode(new BigDecimal("1.0"));
985+
assertEquals(
986+
doubleValue(1.0),
987+
OpenSearchExprValueFactory.parseContent(new OpenSearchJsonContent(jsonNode)));
988+
jsonNode = new DecimalNode(new BigDecimal("1.0"));
989+
assertEquals(
990+
doubleValue(1.0),
991+
OpenSearchExprValueFactory.parseContent(new OpenSearchJsonContent(jsonNode)));
992+
jsonNode = new ObjectNode(new JsonNodeFactory(false), Map.of("key", jsonNode));
993+
assertEquals(
994+
stringValue("{\"key\":1.0}"),
995+
OpenSearchExprValueFactory.parseContent(new OpenSearchJsonContent(jsonNode)));
966996
}
967997

968998
@Test

0 commit comments

Comments
 (0)