Skip to content

Fix ClassCastException for value-storing aggregates on nested PPL fields #4360

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

Merged
LantaoJin merged 4 commits into from
Sep 26, 2025
Merged

Fix ClassCastException for value-storing aggregates on nested PPL fields #4360

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d160c1a
fix first/last
ahkcs Sep 23, 2025
ebfff45
fix
ahkcs Sep 23, 2025
6d001f7
add tests
ahkcs Sep 23, 2025
daa85d6
update approach
ahkcs Sep 25, 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
110 changes: 110 additions & 0 deletions integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalcitePPLAggregationIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import static org.opensearch.sql.legacy.TestsConstants.TEST_INDEX_DATATYPE_NUMERIC;
import static org.opensearch.sql.legacy.TestsConstants.TEST_INDEX_DATE_FORMATS;
import static org.opensearch.sql.legacy.TestsConstants.TEST_INDEX_LOGS;
import static org.opensearch.sql.legacy.TestsConstants.TEST_INDEX_TELEMETRY;
import static org.opensearch.sql.util.MatcherUtils.assertJsonEquals;
import static org.opensearch.sql.util.MatcherUtils.rows;
import static org.opensearch.sql.util.MatcherUtils.schema;
Expand Down Expand Up @@ -41,6 +42,7 @@ public void init() throws Exception {
loadIndex(Index.DATE_FORMATS);
loadIndex(Index.DATA_TYPE_NUMERIC);
loadIndex(Index.LOGS);
loadIndex(Index.TELEMETRY);
loadIndex(Index.TIME_TEST_DATA);
}

Expand Down Expand Up @@ -1246,4 +1248,112 @@ public void testLimitAfterAggregation() throws IOException {
verifySchema(response, schema("count()", "bigint"), schema("age", "int"));
verifyDataRows(response, rows(1, 39), rows(2, 36), rows(1, 34));
}

@Test
public void testFirstLastWithSimpleField() throws IOException {
// This should work - testing simple field first
JSONObject actual =
executeQuery(
String.format("source=%s | stats first(severityNumber)", TEST_INDEX_TELEMETRY));
verifySchema(actual, schema("first(severityNumber)", "int"));
verifyDataRows(actual, rows(9));
}

@Test
public void testFirstLastWithDeepNestedField() throws IOException {
// This test should now work with the fix for ClassCastException
JSONObject actual =
executeQuery(
String.format(
"source=%s | stats first(`resource.attributes.telemetry.sdk.language`)",
TEST_INDEX_TELEMETRY));
verifySchema(actual, schema("first(`resource.attributes.telemetry.sdk.language`)", "string"));
verifyDataRows(actual, rows("java"));
}

@Test
public void testLastWithDeepNestedField() throws IOException {
// This test should now work with the fix for ClassCastException
JSONObject actual =
executeQuery(
String.format(
"source=%s | stats last(`resource.attributes.telemetry.sdk.language`)",
TEST_INDEX_TELEMETRY));
verifySchema(actual, schema("last(`resource.attributes.telemetry.sdk.language`)", "string"));
verifyDataRows(actual, rows("rust"));
}

@Test
public void testFirstLastWithDeepNestedFieldByGroup() throws IOException {
// This test should now work with the fix for ClassCastException
JSONObject actual =
executeQuery(
String.format(
"source=%s | stats first(`resource.attributes.telemetry.sdk.language`) by"
+ " severityNumber",
TEST_INDEX_TELEMETRY));
verifySchema(
actual,
schema("first(`resource.attributes.telemetry.sdk.language`)", "string"),
schema("severityNumber", "int"));
verifyDataRows(actual, rows("java", 9), rows("python", 12), rows("go", 16));
}

@Test
public void testMinWithDeepNestedField() throws IOException {
// Test that min() works with deeply nested fields after the ClassCastException fix
JSONObject actual =
executeQuery(
String.format(
"source=%s | stats min(`resource.attributes.telemetry.sdk.language`)",
TEST_INDEX_TELEMETRY));
verifySchema(actual, schema("min(`resource.attributes.telemetry.sdk.language`)", "string"));
verifyDataRows(
actual, rows("go")); // Alphabetically first: go < java < javascript < python < rust
}

@Test
public void testMaxWithDeepNestedField() throws IOException {
// Test that max() works with deeply nested fields after the ClassCastException fix
JSONObject actual =
executeQuery(
String.format(
"source=%s | stats max(`resource.attributes.telemetry.sdk.language`)",
TEST_INDEX_TELEMETRY));
verifySchema(actual, schema("max(`resource.attributes.telemetry.sdk.language`)", "string"));
verifyDataRows(
actual, rows("rust")); // Alphabetically last: go < java < javascript < python < rust
}

@Test
public void testMinMaxWithDeepNestedFieldByGroup() throws IOException {
// Test that min() and max() work with deeply nested fields and grouping
JSONObject actual =
executeQuery(
String.format(
"source=%s | stats min(`resource.attributes.telemetry.sdk.language`) by"
+ " severityNumber | sort severityNumber",
TEST_INDEX_TELEMETRY));
verifySchema(
actual,
schema("min(`resource.attributes.telemetry.sdk.language`)", "string"),
schema("severityNumber", "int"));
// severityNumber 9: java, javascript -> min = java
// severityNumber 12: python, rust -> min = python
// severityNumber 16: go -> min = go
verifyDataRows(actual, rows("java", 9), rows("python", 12), rows("go", 16));
}

@Test
public void testMinMaxMultipleNestedFields() throws IOException {
// Test min/max with multiple nested field aggregations in one query
JSONObject actual =
executeQuery(
String.format(
"source=%s | stats min(`resource.attributes.telemetry.sdk.language`) as min_lang,"
+ " max(`resource.attributes.telemetry.sdk.language`) as max_lang",
TEST_INDEX_TELEMETRY));
verifySchema(actual, schema("min_lang", "string"), schema("max_lang", "string"));
Copy link
Member

@LantaoJin LantaoJin Sep 25, 2025

Choose a reason for hiding this comment

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

What if the type of nested language is an integer, can the ppl work?

Copy link
Contributor Author

@ahkcs ahkcs Sep 25, 2025

Choose a reason for hiding this comment

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

Yes, in ObjectContent.java, first the extractFinalPrimitiveValue method will extract the value as Object and then we'll add toString() to it.

Copy link
Member

@LantaoJin LantaoJin Sep 25, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

No I mean what if the mapping contains

                    "properties": {
                      "version": {
                        "type": "integer"
                      }

and query with | stats min(`resource.attributes.telemetry.sdk.version`).
Can you update the mapping and data, and test above query?

Copy link
Contributor Author

@ahkcs ahkcs Sep 25, 2025

Choose a reason for hiding this comment

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

Changed the approach - moved the fix from ObjectContent.java to OpenSearchExprValueFactory.construct() for better universal handling. Updated the telemetry mapping to test for types like integer and boolean

verifyDataRows(actual, rows("go", "rust"));
}
}
5 changes: 5 additions & 0 deletions integ-test/src/test/java/org/opensearch/sql/legacy/SQLIntegTestCase.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -660,6 +660,11 @@ public enum Index {
"_doc",
getDeepNestedIndexMapping(),
"src/test/resources/deep_nested_index_data.json"),
TELEMETRY(
TestsConstants.TEST_INDEX_TELEMETRY,
"_doc",
getMappingFile("telemetry_test_mapping.json"),
"src/test/resources/telemetry_test_data.json"),
DATA_TYPE_NUMERIC(
TestsConstants.TEST_INDEX_DATATYPE_NUMERIC,
"_doc",
Expand Down
1 change: 1 addition & 0 deletions integ-test/src/test/java/org/opensearch/sql/legacy/TestsConstants.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ public class TestsConstants {
public static final String TEST_INDEX_DATE = TEST_INDEX + "_date";
public static final String TEST_INDEX_DATE_TIME = TEST_INDEX + "_datetime";
public static final String TEST_INDEX_DEEP_NESTED = TEST_INDEX + "_deep_nested";
public static final String TEST_INDEX_TELEMETRY = TEST_INDEX + "_telemetry";
public static final String TEST_INDEX_STRINGS = TEST_INDEX + "_strings";
public static final String TEST_INDEX_DATATYPE_NUMERIC = TEST_INDEX + "_datatypes_numeric";
public static final String TEST_INDEX_DATATYPE_NONNUMERIC = TEST_INDEX + "_datatypes_nonnumeric";
Expand Down
37 changes: 37 additions & 0 deletions integ-test/src/test/resources/indexDefinitions/telemetry_test_mapping.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
{
"mappings": {
"properties": {
"resource": {
"properties": {
"attributes": {
"properties": {
"telemetry": {
"properties": {
"sdk": {
"properties": {
"language": {
"type": "keyword",
"ignore_above": 256
},
"name": {
"type": "keyword",
"ignore_above": 256
},
"version": {
"type": "keyword",
"ignore_above": 256
}
}
}
}
}
}
}
}
},
"severityNumber": {
"type": "integer"
}
}
}
}
10 changes: 10 additions & 0 deletions integ-test/src/test/resources/telemetry_test_data.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{"index":{"_id":"1"}}
{"resource": {"attributes": {"telemetry": {"sdk": {"language": "java", "name": "opentelemetry", "version": "1.0.0"}}}}, "severityNumber": 9}
{"index":{"_id":"2"}}
{"resource": {"attributes": {"telemetry": {"sdk": {"language": "python", "name": "opentelemetry", "version": "1.1.0"}}}}, "severityNumber": 12}
{"index":{"_id":"3"}}
{"resource": {"attributes": {"telemetry": {"sdk": {"language": "javascript", "name": "opentelemetry", "version": "1.2.0"}}}}, "severityNumber": 9}
{"index":{"_id":"4"}}
{"resource": {"attributes": {"telemetry": {"sdk": {"language": "go", "name": "opentelemetry", "version": "1.3.0"}}}}, "severityNumber": 16}
{"index":{"_id":"5"}}
{"resource": {"attributes": {"telemetry": {"sdk": {"language": "rust", "name": "opentelemetry", "version": "1.4.0"}}}}, "severityNumber": 12}
30 changes: 30 additions & 0 deletions opensearch/src/main/java/org/opensearch/sql/opensearch/data/utils/ObjectContent.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,9 +56,39 @@ public Double doubleValue() {

@Override
public String stringValue() {
if (value instanceof Map) {
// Handle nested field Maps by recursively extracting the final primitive value
@SuppressWarnings("unchecked")
Map<String, Object> map = (Map<String, Object>) value;
Object finalValue = extractFinalPrimitiveValue(map);
if (finalValue != null && !(finalValue instanceof Map)) {
return finalValue.toString();
}
// Fallback to map string representation if we can't extract a primitive
return map.toString();
}
return (String) value;
}

/**
* Recursively extracts the final primitive value from nested Map structures. For example:
* {attributes={telemetry={sdk={language=java}}}} -> "java"
*/
@SuppressWarnings("unchecked")
private Object extractFinalPrimitiveValue(Object value) {
if (value == null || !(value instanceof Map)) {
return value;
}

Map<String, Object> map = (Map<String, Object>) value;
if (map.size() == 1) {
Object singleValue = map.values().iterator().next();
return extractFinalPrimitiveValue(singleValue);
}

return value;
}

@Override
public Boolean booleanValue() {
if (value instanceof String) {
Expand Down
Loading