Skip to content

Commit bca5a72

Browse files
committed
Update
Signed-off-by: Kai Huang <ahkcs@amazon.com>
1 parent a287523 commit bca5a72

3 files changed

Lines changed: 45 additions & 45 deletions

File tree

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

Lines changed: 24 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,10 @@
77
#
88
# This can happen when an index has a disabled object field (enabled: false),
99
# which allows storing documents without validating inner field names.
10+
#
11+
# Fix: The query engine now tolerates corrupt/invalid field names by returning null
12+
# for those fields (with a warning log), allowing the rest of the document to be
13+
# processed normally.
1014

1115
setup:
1216
- do:
@@ -26,6 +30,8 @@ setup:
2630
enabled: false
2731
"@timestamp":
2832
type: date
33+
message:
34+
type: text
2935

3036
# Index document with "." field name inside disabled object
3137
# OpenSearch allows this because the object is disabled (no validation of inner fields)
@@ -36,6 +42,7 @@ setup:
3642
refresh: true
3743
body:
3844
"@timestamp": "2025-11-26T17:10:00.000Z"
45+
message: "test message"
3946
log:
4047
".": "problematic value"
4148

@@ -51,20 +58,30 @@ teardown:
5158
index: test_disabled_object_4896
5259

5360
---
54-
"Query index with dot-only field name in disabled object should return meaningful error":
61+
"Query index with dot-only field name in disabled object should succeed with valid fields":
5562
- skip:
5663
features:
5764
- headers
58-
- regex
59-
# Query should return meaningful error instead of ArrayIndexOutOfBoundsException
60-
# The error is returned as 500 Internal Server Error with a meaningful message
6165
# Before the fix: ArrayIndexOutOfBoundsException: Index 0 out of bounds for length 0
62-
# After the fix: Invalid field name '.': field names cannot consist only of dots
66+
# After the fix: Query succeeds, invalid field returns null with a warning log,
67+
# and valid fields (@timestamp, message) are returned normally
68+
# Query valid fields - should succeed
69+
- do:
70+
headers:
71+
Content-Type: 'application/json'
72+
ppl:
73+
body:
74+
query: source=test_disabled_object_4896 | fields @timestamp, message
75+
- match: { "total": 1 }
76+
- match: { "datarows.0.0": "2025-11-26 17:10:00" }
77+
- match: { "datarows.0.1": "test message" }
78+
79+
# Query all fields including log - should succeed without crash
80+
# The log field with invalid "." field name will have null for that field
6381
- do:
64-
catch: request
6582
headers:
6683
Content-Type: 'application/json'
6784
ppl:
6885
body:
6986
query: source=test_disabled_object_4896
70-
- match: { "$body": "/Invalid field name.*cannot consist only of dots/" }
87+
- match: { "total": 1 }

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

Lines changed: 21 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,8 @@
3838
import java.util.function.BiFunction;
3939
import lombok.Getter;
4040
import lombok.Setter;
41+
import org.apache.logging.log4j.LogManager;
42+
import org.apache.logging.log4j.Logger;
4143
import org.opensearch.OpenSearchParseException;
4244
import org.opensearch.common.time.DateFormatter;
4345
import org.opensearch.common.time.DateFormatters;
@@ -72,6 +74,8 @@
7274

7375
/** Construct ExprValue from OpenSearch response. */
7476
public class OpenSearchExprValueFactory {
77+
private static final Logger LOG = LogManager.getLogger(OpenSearchExprValueFactory.class);
78+
7579
/** The Mapping of Field and ExprType. */
7680
private final Map<String, OpenSearchDataType> typeMapping;
7781

@@ -373,15 +377,25 @@ private ExprValue parseStruct(Content content, String prefix, boolean supportArr
373377
content
374378
.map()
375379
.forEachRemaining(
376-
entry ->
380+
entry -> {
381+
String fieldKey = entry.getKey();
382+
String fullFieldPath = makeField(prefix, fieldKey);
383+
try {
377384
populateValueRecursive(
378385
result,
379-
new JsonPath(entry.getKey()),
380-
parse(
381-
entry.getValue(),
382-
makeField(prefix, entry.getKey()),
383-
type(makeField(prefix, entry.getKey())),
384-
supportArrays)));
386+
new JsonPath(fieldKey),
387+
parse(entry.getValue(), fullFieldPath, type(fullFieldPath), supportArrays));
388+
} catch (IllegalArgumentException e) {
389+
// Return null for invalid field names (e.g., fields consisting only of dots)
390+
// This can happen with disabled object fields that bypass field name validation
391+
// Log warning to hint users about corrupt data
392+
LOG.warn(
393+
"Field '{}' has invalid name and will return null: {}",
394+
fullFieldPath,
395+
e.getMessage());
396+
result.tupleValue().put(fieldKey, ExprNullValue.of());
397+
}
398+
});
385399
return result;
386400
}
387401

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

Lines changed: 0 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -1041,37 +1041,6 @@ public void testPopulateValueRecursive() {
10411041
assertEquals(expectedValue, tupleValue);
10421042
}
10431043

1044-
@Test
1045-
public void testJsonPathWithDotOnlyFieldName() {
1046-
IllegalArgumentException ex =
1047-
assertThrows(IllegalArgumentException.class, () -> new JsonPath("."));
1048-
assertTrue(ex.getMessage().contains("Invalid field name"));
1049-
assertTrue(ex.getMessage().contains("cannot consist only of dots"));
1050-
}
1051-
1052-
@Test
1053-
public void testJsonPathWithMultipleDotsFieldName() {
1054-
IllegalArgumentException ex =
1055-
assertThrows(IllegalArgumentException.class, () -> new JsonPath(".."));
1056-
assertTrue(ex.getMessage().contains("Invalid field name"));
1057-
assertTrue(ex.getMessage().contains("cannot consist only of dots"));
1058-
}
1059-
1060-
@Test
1061-
public void testJsonPathWithTripleDotsFieldName() {
1062-
IllegalArgumentException ex =
1063-
assertThrows(IllegalArgumentException.class, () -> new JsonPath("..."));
1064-
assertTrue(ex.getMessage().contains("Invalid field name"));
1065-
}
1066-
1067-
@Test
1068-
public void testJsonPathWithValidFieldName() {
1069-
// Regular field names should work
1070-
JsonPath path = new JsonPath("log.json.time");
1071-
assertEquals("log", path.getRootPath());
1072-
assertEquals(3, path.getPaths().size());
1073-
}
1074-
10751044
public Map<String, ExprValue> tupleValue(String jsonString) {
10761045
final ExprValue construct = exprValueFactory.construct(jsonString, false);
10771046
return construct.tupleValue();

0 commit comments

Comments
 (0)