Skip to content

Commit 76b3ec2

Browse files
authored
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>
1 parent fed92a3 commit 76b3ec2

6 files changed

Lines changed: 101 additions & 2 deletions

File tree

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
@@ -32,6 +32,9 @@ public interface Content {
3232
/** Is double value. */
3333
boolean isDouble();
3434

35+
/** Is int value. */
36+
boolean isInt();
37+
3538
/** Is long value. */
3639
boolean isLong();
3740

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: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -187,10 +187,16 @@ public ExprValue construct(String field, Object value, boolean supportArrays) {
187187

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

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

196202
if (type.equals(OpenSearchDataType.of(OpenSearchDataType.MappingType.GeoPoint))) {
@@ -210,6 +216,31 @@ private ExprValue parse(
210216
}
211217
}
212218

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

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -952,7 +952,7 @@ public void constructFromInvalidJsonThrowException() {
952952
public void noTypeFoundForMapping() {
953953
assertEquals(nullValue(), tupleValue("{\"not_exist\":[]}").get("not_exist"));
954954
// Only for test coverage, It is impossible in OpenSearch.
955-
assertEquals(nullValue(), tupleValue("{\"not_exist\":1}").get("not_exist"));
955+
assertEquals(ExprValueUtils.integerValue(1), tupleValue("{\"not_exist\":1}").get("not_exist"));
956956
}
957957

958958
@Test

0 commit comments

Comments
 (0)