-
Notifications
You must be signed in to change notification settings - Fork 181
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.
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
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,89 @@ | ||
| # Issue: https://github.com/opensearch-project/sql/issues/4896 | ||
| # ArrayIndexOutOfBoundsException when querying index with dot-only field name | ||
| # | ||
| # 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: When split returns empty array, treat the original string as a literal | ||
| # field name, allowing the data to be returned properly. | ||
|
|
||
| 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: | ||
| ".": "dot field 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 should return actual value": | ||
| - skip: | ||
| features: | ||
| - headers | ||
|
|
||
| # Test 1: Query valid fields - should succeed | ||
| # Before fix: This would crash because the entire document is parsed | ||
| # After fix: Query succeeds, all fields returned normally | ||
| - do: | ||
| headers: | ||
| Content-Type: 'application/json' | ||
| ppl: | ||
| body: | ||
| query: source=test_disabled_object_4896 | fields @timestamp, message | ||
dai-chen marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| - match: { "total": 1 } | ||
| - match: { "datarows.0.0": "2025-11-26 17:10:00" } | ||
| - match: { "datarows.0.1": "test message" } | ||
|
|
||
| # Test 2: Query the log field with "." subfield | ||
| # Before fix: ArrayIndexOutOfBoundsException crash | ||
| # After fix: Returns the log object with the "." field containing actual value | ||
| - do: | ||
| headers: | ||
| Content-Type: 'application/json' | ||
| ppl: | ||
| body: | ||
| query: source=test_disabled_object_4896 | fields log | ||
| - match: { "total": 1 } | ||
| # The log field should contain the "." subfield with its value | ||
| - match: { "datarows.0.0": { ".": "dot field value" } } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1041,6 +1041,41 @@ public void testPopulateValueRecursive() { | |
| assertEquals(expectedValue, tupleValue); | ||
| } | ||
|
|
||
| @Test | ||
| public void constructWithDotOnlyFieldNameReturnsValue() { | ||
| // Field name "." (single dot) should return the actual value, not crash or return null | ||
| // This can happen with disabled object fields in OpenSearch | ||
| Map<String, ExprValue> result = tupleValue("{\"structV\":{\".\":\"value\"}}"); | ||
| ExprValue structValue = result.get("structV"); | ||
| // The "." field should contain the actual value | ||
| assertEquals(stringValue("value"), structValue.tupleValue().get(".")); | ||
| } | ||
|
|
||
| @Test | ||
| public void constructWithMultipleDotsFieldNameReturnsValue() { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. np: Could you verify what if the malformed field is not on top level? E.g.,
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Updated to have more comprehensive test cases for both UT and yaml test |
||
| // Field name ".." (multiple dots) should also return the actual value | ||
| Map<String, ExprValue> result = tupleValue("{\"structV\":{\"..\":\"value\"}}"); | ||
| ExprValue structValue = result.get("structV"); | ||
| assertEquals(stringValue("value"), structValue.tupleValue().get("..")); | ||
| } | ||
|
|
||
| @Test | ||
| public void constructWithDotFieldAlongsideValidFieldsReturnsAll() { | ||
| // Both dot field and valid fields should be returned | ||
| Map<String, ExprValue> result = | ||
| tupleValue( | ||
| "{\"structV\":{\".\":\"dotValue\",\"id\":1,\"state\":\"WA\"},\"stringV\":\"test\"}"); | ||
|
|
||
| // stringV field should be returned normally | ||
| assertEquals(stringValue("test"), result.get("stringV")); | ||
|
|
||
| // All fields inside structV should be returned | ||
| ExprValue structValue = result.get("structV"); | ||
| assertEquals(stringValue("dotValue"), structValue.tupleValue().get(".")); | ||
| assertEquals(integerValue(1), structValue.tupleValue().get("id")); | ||
| assertEquals(stringValue("WA"), structValue.tupleValue().get("state")); | ||
| } | ||
|
|
||
| public Map<String, ExprValue> tupleValue(String jsonString) { | ||
| final ExprValue construct = exprValueFactory.construct(jsonString, false); | ||
| return construct.tupleValue(); | ||
|
|
||
There was a problem hiding this comment.
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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated