Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
# Issue: https://github.com/opensearch-project/sql/issues/4896
# ArrayIndexOutOfBoundsException when querying index with dot-only field name in disabled object
#
# Root cause: When a document has a field name that is just "." (a single dot),
# the JsonPath constructor's split("\\.") returns an empty array, causing
# paths.get(0) to crash with ArrayIndexOutOfBoundsException.
#
# This can happen when an index has a disabled object field (enabled: false),
# which allows storing documents without validating inner field names.
#
# Fix: The query engine now tolerates corrupt/invalid field names by returning null
# for those fields (with a warning log), allowing the rest of the document to be
# processed normally.

setup:
- do:
query.settings:
body:
transient:
plugins.calcite.enabled: true
# Create index with disabled object field
- do:
indices.create:
index: test_disabled_object_4896
body:
mappings:
properties:
log:
type: object
enabled: false
"@timestamp":
type: date
message:
type: text

# Index document with "." field name inside disabled object
# OpenSearch allows this because the object is disabled (no validation of inner fields)
- do:
index:
index: test_disabled_object_4896
id: 1
refresh: true
body:
"@timestamp": "2025-11-26T17:10:00.000Z"
message: "test message"
log:
".": "problematic value"

---
teardown:
- do:
query.settings:
body:
transient:
plugins.calcite.enabled: false
- do:
indices.delete:
index: test_disabled_object_4896

---
"Query index with dot-only field name in disabled object should succeed with valid fields":
- skip:
features:
- headers
# Before the fix: ArrayIndexOutOfBoundsException: Index 0 out of bounds for length 0
# After the fix: Query succeeds, invalid field returns null with a warning log,
# and valid fields (@timestamp, message) are returned normally
# Query valid fields - should succeed
- do:
headers:
Content-Type: 'application/json'
ppl:
body:
query: source=test_disabled_object_4896 | fields @timestamp, message
- match: { "total": 1 }
- match: { "datarows.0.0": "2025-11-26 17:10:00" }
- match: { "datarows.0.1": "test message" }

# Query the log field specifically - should succeed without crash
# The log field with invalid "." field name will have null for that invalid subfield
- do:
headers:
Content-Type: 'application/json'
ppl:
body:
query: source=test_disabled_object_4896 | fields log
- match: { "total": 1 }
# The log field returns an empty object because the invalid "." field resolves to null
# and null values are omitted from JSON serialization
- match: { "datarows.0.0": {} }
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,16 @@
import java.time.format.DateTimeParseException;
import java.time.temporal.TemporalAccessor;
import java.util.ArrayList;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.Set;
import java.util.function.BiFunction;
import lombok.Getter;
import lombok.Setter;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.opensearch.OpenSearchParseException;
import org.opensearch.common.time.DateFormatter;
import org.opensearch.common.time.DateFormatters;
Expand Down Expand Up @@ -72,6 +76,15 @@

/** Construct ExprValue from OpenSearch response. */
public class OpenSearchExprValueFactory {
private static final Logger LOG = LogManager.getLogger(OpenSearchExprValueFactory.class);

/**
* Collects invalid field names encountered during parsing. Uses ThreadLocal to avoid log spam by
* logging all invalid fields once per construct() call instead of per field.
*/
private static final ThreadLocal<Set<String>> INVALID_FIELDS =
ThreadLocal.withInitial(HashSet::new);

/** The Mapping of Field and ExprType. */
private final Map<String, OpenSearchDataType> typeMapping;

Expand Down Expand Up @@ -164,14 +177,31 @@ public OpenSearchExprValueFactory(
* </pre>
*/
public ExprValue construct(String jsonString, boolean supportArrays) {
INVALID_FIELDS.get().clear();
try {
return parse(
new OpenSearchJsonContent(OBJECT_MAPPER.readTree(jsonString)),
TOP_PATH,
Optional.of(STRUCT),
fieldTypeTolerance || supportArrays);
ExprValue result =
parse(
new OpenSearchJsonContent(OBJECT_MAPPER.readTree(jsonString)),
TOP_PATH,
Optional.of(STRUCT),
fieldTypeTolerance || supportArrays);
logInvalidFields();
return result;
} catch (JsonProcessingException e) {
throw new IllegalStateException(String.format("invalid json: %s.", jsonString), e);
} finally {
INVALID_FIELDS.get().clear();
}
}

/** Log all invalid fields encountered during parsing (once per construct call). */
private void logInvalidFields() {
Set<String> invalidFields = INVALID_FIELDS.get();
if (!invalidFields.isEmpty()) {
LOG.warn(
"The following field(s) have invalid names and will return null: {}. "
+ "This can happen with disabled object fields that bypass field name validation.",
invalidFields);
}
}

Expand Down Expand Up @@ -373,18 +403,38 @@ private ExprValue parseStruct(Content content, String prefix, boolean supportArr
content
.map()
.forEachRemaining(
entry ->
entry -> {
String fieldKey = entry.getKey();
String fullFieldPath = makeField(prefix, fieldKey);
// Check for invalid field names (e.g., fields consisting only of dots like "." or
// "..")
// before creating JsonPath to avoid masking other IllegalArgumentExceptions
if (isInvalidFieldName(fieldKey)) {
// Collect invalid fields to log once per construct() call to avoid log spam
INVALID_FIELDS.get().add(fullFieldPath);
result.tupleValue().put(fieldKey, ExprNullValue.of());
} else {
populateValueRecursive(
result,
new JsonPath(entry.getKey()),
parse(
entry.getValue(),
makeField(prefix, entry.getKey()),
type(makeField(prefix, entry.getKey())),
supportArrays)));
new JsonPath(fieldKey),
parse(entry.getValue(), fullFieldPath, type(fullFieldPath), supportArrays));
}
});
return result;
}

/**
* Check if a field name is invalid. A field name is invalid if it consists only of dots (e.g.,
* ".", "..", "..."). Such field names cause issues because String.split("\\.") returns an empty
* array for them.
*
* @param fieldName The field name to check.
* @return true if the field name is invalid, false otherwise.
*/
private static boolean isInvalidFieldName(String fieldName) {
return fieldName.split("\\.").length == 0;
}

/**
* Populate the current ExprTupleValue recursively.
*
Expand Down Expand Up @@ -412,7 +462,14 @@ static class JsonPath {
private final List<String> paths;

public JsonPath(String rawPath) {
this.paths = List.of(rawPath.split("\\."));
String[] parts = rawPath.split("\\.");
// Handle edge case where split returns empty array (e.g., rawPath = "." or "..")
if (parts.length == 0) {
throw new IllegalArgumentException(
String.format(
"Invalid field name '%s': field names cannot consist only of dots", rawPath));
}
this.paths = List.of(parts);
}

public JsonPath(List<String> paths) {
Expand Down
Loading