Skip to content

Fix parsing of dot-containing field names #4897

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
ahkcs wants to merge 5 commits into from
Closed

Fix parsing of dot-containing field names #4897

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
2c1eabb
Support dot-only field names in PPL queries
ahkcs Dec 3, 2025
7d0b0c3
Add comprehensive UT and yaml test
ahkcs Dec 3, 2025
8dd6f41
Update UT
ahkcs Dec 3, 2025
b4dd36f
fix IT
ahkcs Dec 3, 2025
0139bf1
Update yaml test
ahkcs Dec 4, 2025
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
215 changes: 215 additions & 0 deletions integ-test/src/yamlRestTest/resources/rest-api-spec/test/issues/4896.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,215 @@
# Issue: https://github.com/opensearch-project/sql/issues/4896
# ArrayIndexOutOfBoundsException when querying index with dot-containing field names
#
# Root cause: JSON field names containing dots (e.g., ".", "..", "a...", ".a")
# were incorrectly split as path separators, causing crashes or data corruption.
#
# This can happen when an index has a disabled object field (enabled: false),
# which allows storing documents without validating inner field names.
#
# Fix: Treat JSON field names as literal strings, not dot-separated paths.
# Dots in field names are literal characters, not path separators.

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

# Bulk index all test documents
- do:
Copy link
Collaborator

@penghuo penghuo Dec 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change L35-L112 single doc index to bulk index

  - do:
      bulk:
        index: test
        refresh: true
        body:
          - '{"index": {}}'
          - '{"id": 1}'
          - '{"index": {}}'
          - '{"id": 2}'
          - '{"index": {}}'
          - '{"id": 3}'

Copy link
Contributor Author

@ahkcs ahkcs Dec 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated

bulk:
index: test_disabled_object_4896
refresh: true
body:
# Document 1: Single dot field name "."
- '{"index": {"_id": "1"}}'
- '{"@timestamp": "2025-11-26T17:10:00.000Z", "message": "single dot", "log": {".": "single dot value"}}'
# Document 2: Multiple dots field name ".."
- '{"index": {"_id": "2"}}'
- '{"@timestamp": "2025-11-26T17:11:00.000Z", "message": "multiple dots", "log": {"..": "double dot value"}}'
# Document 3: Trailing dots field name "a..."
- '{"index": {"_id": "3"}}'
- '{"@timestamp": "2025-11-26T17:12:00.000Z", "message": "trailing dots", "log": {"a...": "trailing dots value"}}'
# Document 4: Leading dot field name ".a"
- '{"index": {"_id": "4"}}'
- '{"@timestamp": "2025-11-26T17:13:00.000Z", "message": "leading dot", "log": {".a": "leading dot value"}}'
# Document 5: Middle dots field name "a..b"
- '{"index": {"_id": "5"}}'
- '{"@timestamp": "2025-11-26T17:14:00.000Z", "message": "middle dots", "log": {"a..b": "middle dots value"}}'
# Document 6: Multiple unusual fields in same object
- '{"index": {"_id": "6"}}'
- '{"@timestamp": "2025-11-26T17:15:00.000Z", "message": "mixed fields", "log": {".": "dot1", "..": "dot2", "normal": "normal value"}}'
# Document 7: Malformed top-level field name "log." (trailing dot)
- '{"index": {"_id": "7"}}'
- '{"@timestamp": "2025-11-26T17:16:00.000Z", "message": "malformed top-level", "log.": {"nested": "value in log."}}'

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

---
"Single dot field name returns actual value":
- skip:
features:
- headers

- do:
headers:
Content-Type: 'application/json'
ppl:
body:
query: source=test_disabled_object_4896 | where message = 'single dot' | fields log
- match: { "total": 1 }
- match: { "datarows.0.0": { ".": "single dot value" } }

---
"Multiple dots field name returns actual value":
- skip:
features:
- headers

- do:
headers:
Content-Type: 'application/json'
ppl:
body:
query: source=test_disabled_object_4896 | where message = 'multiple dots' | fields log
- match: { "total": 1 }
- match: { "datarows.0.0": { "..": "double dot value" } }

---
"Trailing dots field name preserves original name":
- skip:
features:
- headers

# Field "a..." should NOT be truncated to "a"
- do:
headers:
Content-Type: 'application/json'
ppl:
body:
query: source=test_disabled_object_4896 | where message = 'trailing dots' | fields log
- match: { "total": 1 }
- match: { "datarows.0.0": { "a...": "trailing dots value" } }

---
"Leading dot field name preserves original name":
- skip:
features:
- headers

# Field ".a" should NOT create nested structure
- do:
headers:
Content-Type: 'application/json'
ppl:
body:
query: source=test_disabled_object_4896 | where message = 'leading dot' | fields log
Copy link
Collaborator

@penghuo penghuo Dec 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is results if using fields log.a? should be log.a not exist, right?, user should use fields log.'.a'

Copy link
Contributor Author

@ahkcs ahkcs Dec 4, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I have added test cases to verify this behavior
fields log.a → returns null
fields log.'.a'→ returns"leading dot value"

- match: { "total": 1 }
- match: { "datarows.0.0": { ".a": "leading dot value" } }

# Querying "log.a" should NOT return the ".a" field value
# Because "log.a" means nested field "a" inside "log", not literal ".a"
- do:
headers:
Content-Type: 'application/json'
ppl:
body:
query: source=test_disabled_object_4896 | where message = 'leading dot' | fields log.a
- match: { "total": 1 }
- match: { "datarows.0.0": null }

# To access literal field ".a" inside log, use backticks: log.`.a`
- do:
headers:
Content-Type: 'application/json'
ppl:
body:
query: source=test_disabled_object_4896 | where message = 'leading dot' | fields log.`.a`
- match: { "total": 1 }
- match: { "datarows.0.0": "leading dot value" }

---
"Middle dots field name preserves original name":
- skip:
features:
- headers

# Field "a..b" should NOT create nested structure a -> "" -> b
- do:
headers:
Content-Type: 'application/json'
ppl:
body:
query: source=test_disabled_object_4896 | where message = 'middle dots' | fields log
- match: { "total": 1 }
- match: { "datarows.0.0": { "a..b": "middle dots value" } }

---
"Multiple unusual fields coexist with normal fields":
- skip:
features:
- headers

- do:
headers:
Content-Type: 'application/json'
ppl:
body:
query: source=test_disabled_object_4896 | where message = 'mixed fields' | fields log
- match: { "total": 1 }
- match: { "datarows.0.0": { ".": "dot1", "..": "dot2", "normal": "normal value" } }

---
"Malformed top-level field name preserves original name":
- skip:
features:
- headers

# Top-level field "log." should be preserved, not truncated to "log"
- do:
headers:
Content-Type: 'application/json'
ppl:
body:
query: source=test_disabled_object_4896 | where message = 'malformed top-level'
- match: { "total": 1 }

---
"Query all documents with unusual field names succeeds":
- skip:
features:
- headers

# All 7 documents should be queryable without crash
- do:
headers:
Content-Type: 'application/json'
ppl:
body:
query: source=test_disabled_object_4896
- match: { "total": 7 }
53 changes: 49 additions & 4 deletions ...ch/src/main/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprValueFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -376,7 +376,10 @@ private ExprValue parseStruct(Content content, String prefix, boolean supportArr
entry ->
populateValueRecursive(
result,
new JsonPath(entry.getKey()),
// Use fromPath() to split dot-separated field names into nested paths
// (e.g., "log.json" -> ["log", "json"] for flattening support).
// fromPath() handles edge cases like "." or ".." that would otherwise crash.
JsonPath.fromPath(entry.getKey()),
parse(
entry.getValue(),
makeField(prefix, entry.getKey()),
Expand Down Expand Up @@ -411,11 +414,53 @@ static void populateValueRecursive(ExprTupleValue result, JsonPath path, ExprVal
static class JsonPath {
private final List<String> paths;

public JsonPath(String rawPath) {
this.paths = List.of(rawPath.split("\\."));
/**
* Create a JsonPath from a literal field name (no splitting by dots). Use this when the field
* name comes directly from JSON object keys, where dots are literal characters in the field
* name, not path separators.
*
* @param fieldName The literal field name (e.g., ".", "..", "a...", ".a")
* @return A JsonPath with a single element containing the literal field name
*/
public static JsonPath literal(String fieldName) {
return new JsonPath(List.of(fieldName));
}

public JsonPath(List<String> paths) {
/**
* Create a JsonPath by splitting a dot-separated path into components. Use this when the path
* represents a nested field structure (e.g., "log.json.time" → ["log", "json", "time"]).
*
* <p>Handles edge cases:
*
* <ul>
* <li>Dot-only field names like "." or ".." - split returns empty array, use literal
* <li>Leading/trailing/consecutive dots like ".a", "a.", "a..b" - split produces empty
* strings, use literal to avoid creating nested structures with empty keys
* </ul>
*
* @param path The dot-separated path
* @return A JsonPath with components split by dots, or literal if splitting would produce empty
* keys
*/
public static JsonPath fromPath(String path) {
// Use -1 limit to preserve trailing empty strings (e.g., "a..." -> ["a", "", "", ""])
String[] parts = path.split("\\.", -1);
Copy link
Collaborator

@Swiddis Swiddis Dec 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thought: Is this the right place for this logic?

As a wider design point (not just this PR): We've had bugs in different features that boil down to people rolling their own logic for field parsing. Spath was a victim too, and I've called out similar cases in past PRs.

Ideally we should be trying to move this logic into a central module so every part of the code is using the same logic with appropriate edge-case handling. I know QualifiedName defines DELIMITER = "." in a full constant and contains some logic like this, while other places still do these kinds of array operations directly.

Especially since there's more-or-less an implicit assumption in many code paths that . defines nesting, I'm expecting this isn't the last we'll see of this issue. Having the code be reusable for later seems like a good investment.

// Handle edge cases where splitting would produce problematic results:
// 1. Empty array (dot-only field names like "." or "..")
// 2. Array with empty strings (e.g., ".a" -> ["", "a"], "a..." -> ["a", "", "", ""])
// In these cases, treat the original string as a literal field name
if (parts.length == 0) {
return new JsonPath(List.of(path));
}
for (String part : parts) {
if (part.isEmpty()) {
return new JsonPath(List.of(path));
}
}
return new JsonPath(List.of(parts));
}

private JsonPath(List<String> paths) {
this.paths = paths;
}

Expand Down
Loading
Loading