fix: gracefully handle scalar values under object-typed fields when decoding results (#5685) by ahkcs · Pull Request #5732 · opensearch-project/sql

ahkcs · 2026-08-31T21:08:59Z

Description

On the Calcite path, PPL dedup pushes down as a composite aggregation whose surviving row is fetched via top_hits and decoded from the nested _source. When a query spans a wildcard/alias over indices that disagree on whether a path is an object or a scalar, the cross-index mapping merge types the path as an object (STRUCT) while some documents store a scalar there. Decoding that scalar as a struct threw and failed the whole query:

java.sql.SQLException: ... class java.lang.String cannot be cast to class java.util.Map

This guards the STRUCT branch of OpenSearchExprValueFactory.parse to degrade a scalar-under-object value to null, completing the graceful handling #5618 added for the geo_point and scalar branches (it did not cover the object/STRUCT branch). The catch is scoped to ClassCastException — this branch is also the whole-document parse entry point and each nested object level is wrapped independently, so a broader catch would swallow legitimate errors raised deeper in the recursion.

Issues Resolved

Closes #5685

Behavior (before → after)

Scenario	Before	After
`dedup` over wildcard, object-vs-scalar conflict on an object path	500 `String cannot be cast to Map`	succeeds; conflicting field → `null`
`dedup` over wildcard, scalar-vs-object conflict (mirror)	already handled by #5618	unchanged (regression-guarded)
Unsupported-type parse, deeply nested	`IllegalStateException` (correct)	unchanged (not masked)

Testing

Unit: OpenSearchExprValueFactoryTest.constructStructWithScalarValueReturnsNull
YAML REST: rest-api-spec/test/issues/5685.yml — object-vs-scalar conflict and its mirror (scalar-vs-object).
Full OpenSearchExprValueFactoryTest and the :opensearch unit suite pass; integ-test compiles.

Check List

New functionality includes testing.
Commits are signed per the DCO using --signoff.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

github-actions · 2026-08-31T21:10:09Z

PR Reviewer Guide 🔍

(Review updated until commit `4d3a6d5`)

Here are some key observations to aid the review process:

🧪 PR contains tests
🔒 No security concerns identified
✅ No TODO sections
🔀 No multiple PR themes
⚡ No major issues detected

github-actions · 2026-08-31T21:10:21Z

PR Code Suggestions ✨

Latest suggestions up to 4d3a6d5

Explore these optional code suggestions:

Category	Suggestion	Impact
General	Add logging for caught exceptions Consider logging the `ClassCastException` at debug or trace level before returning null. This would help diagnose mapping conflicts in production environments while maintaining the graceful degradation behavior. opensearch/src/main/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprValueFactory.java [228-232] try { return parseStruct(content, field, supportArrays); } catch (ClassCastException e) { + log.debug("Scalar value encountered under object-typed field '{}', returning null", field, e); return ExprNullValue.of(); } Suggestion importance[1-10]: 5 __ Why: While logging the `ClassCastException` could aid debugging, the suggestion doesn't address a critical issue. The PR already handles the exception gracefully by returning null. Adding logging is a minor enhancement that improves observability but isn't essential for correctness.	Low

Previous suggestions

Suggestions up to commit 3f0f955

Category	Suggestion	Impact
General	Add logging for caught exceptions Consider logging the `ClassCastException` at debug or trace level before returning null. This would help diagnose mapping conflicts in production environments while maintaining the graceful degradation behavior. opensearch/src/main/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprValueFactory.java [228-232] try { return parseStruct(content, field, supportArrays); } catch (ClassCastException e) { + log.debug("Scalar value found under object-typed field '{}', returning null", field, e); return ExprNullValue.of(); } Suggestion importance[1-10]: 5 __ Why: While logging the `ClassCastException` could aid debugging, the suggestion doesn't address a critical issue. The PR already handles the exception gracefully, and adding logging is a minor enhancement for observability rather than a functional improvement.	Low

Suggestions up to commit 22f7212

Category	Suggestion	Impact
General	Log caught ClassCastException for debugging Catching `ClassCastException` silently may hide legitimate bugs in `parseStruct` or its callees. Consider logging the exception with context (field name, content type) to aid debugging while still returning null for the known scalar-under-object scenario. opensearch/src/main/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprValueFactory.java [228-232] try { return parseStruct(content, field, supportArrays); } catch (ClassCastException e) { + log.debug("Scalar value encountered under object-typed field '{}': {}", field, e.getMessage()); return ExprNullValue.of(); } Suggestion importance[1-10]: 5 __ Why: Adding logging for the caught `ClassCastException` would help with debugging and understanding when this edge case occurs. However, the comment in the code already explains this is a specific fix for issue #5685 (scalar under object-typed field), and the catch is intentionally scoped to avoid hiding other errors. The suggestion is valid but offers moderate improvement since the code already has explanatory comments.	Low

Suggestions up to commit 7d340e8

Category	Suggestion	Impact
General	Add logging for mapping conflicts Consider logging the `ClassCastException` at debug or trace level before returning null. This would help diagnose mapping conflicts in production without failing queries, providing visibility into when scalar-to-struct coercion occurs. opensearch/src/main/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprValueFactory.java [234-238] try { return parseStruct(content, field, supportArrays); } catch (ClassCastException e) { + log.debug("Field '{}' mapped as object but contains scalar value, returning null", field, e); return ExprNullValue.of(); } Suggestion importance[1-10]: 5 __ Why: While logging the `ClassCastException` would provide useful diagnostic information for mapping conflicts, this is a minor enhancement rather than a critical fix. The suggestion is valid and would improve observability, but the current implementation already handles the error gracefully by returning null.	Low

github-actions · 2026-08-31T22:07:20Z

Persistent review updated to latest commit 22f7212

RyanL1997 · 2026-08-31T22:32:49Z

        || type == STRUCT) {
-      return parseStruct(content, field, supportArrays);
+      // A scalar under an object-typed field (wildcard over indices with conflicting mappings)
+      // would throw here; return null instead (#5685). CCE-scoped: this is also the whole-document


lets just remove the inline PR link - instead if it is necessary lets to the yaml test with issue id.

Updated to use yaml test as IT

github-actions · 2026-08-31T22:41:11Z

Persistent review updated to latest commit 3f0f955

dai-chen · 2026-08-31T22:47:21Z

Related to #5610

…ch-project#5685) On the Calcite path, `dedup` pushes down as a composite aggregation whose surviving row is fetched via top_hits and decoded from the nested _source. When a wildcard spans indices that disagree on whether a path is an object or a scalar, the merged mapping types the path as an object (STRUCT) while some documents store a scalar there. Decoding that scalar as a struct threw `java.sql.SQLException: ... class java.lang.String cannot be cast to class java.util.Map`, failing the whole query (100% on the reporting customer's `source=logs-pr172502-*`). Guard the STRUCT branch of OpenSearchExprValueFactory.parse to degrade a scalar-under-object value to null, completing the graceful handling opensearch-project#5618 added for the geo_point and scalar branches. The catch is scoped to ClassCastException because this branch is also the whole-document parse entry point and each nested object level is wrapped independently, so a broader catch would swallow legitimate errors raised deeper in the recursion. Adds a factory unit regression and a yamlRestTest (issues/5685.yml) covering the object-vs-scalar conflict and its mirror (scalar-vs-object, guarding opensearch-project#5618). Signed-off-by: Kai Huang <ahkcs@amazon.com>

github-actions · 2026-09-01T16:55:15Z

Persistent review updated to latest commit 4d3a6d5

dai-chen · 2026-09-01T18:07:14Z

+        Content-Type: 'application/json'
+      ppl:
+        body:
+          query: "source=dedup_conflict_a_5685,dedup_conflict_b_5685 | dedup host | fields host | sort host"


Could you confirm is specifying index list in source deterministic to reproduce the issue? Because we have LatestRule which relies on the order and always choose the last one.

Yes, it's deterministic and does reproduce pre-fix. The determinism isn't from source order — only _b maps resource.attributes.k8s.namespace (as object) while _a (dynamic:false) never maps it, so the merged type is always object with no LatestRule tiebreak. _a's document still holds a scalar there, so decode hits String cannot be cast to Map every run without the fix. wildcard vs explicit list behaves the same.

ahkcs requested review from LantaoJin, RyanL1997, Swiddis, acarbonetto, anirudha, dai-chen, joshuali925, mengweieric, noCharger, penghuo, ps48, qianheng-aws, songkant-aws, vamsimanohar, ykmr1224 and yuancu as code owners August 31, 2026 21:09

ahkcs added the bugFix label Aug 31, 2026

ahkcs force-pushed the fix/dedup-object-scalar-mapping-5685 branch from 7d340e8 to 22f7212 Compare August 31, 2026 22:06

RyanL1997 reviewed Aug 31, 2026

View reviewed changes

ahkcs force-pushed the fix/dedup-object-scalar-mapping-5685 branch from 22f7212 to 3f0f955 Compare August 31, 2026 22:40

ahkcs force-pushed the fix/dedup-object-scalar-mapping-5685 branch from 3f0f955 to 4d3a6d5 Compare September 1, 2026 16:54

ahkcs requested a review from RyanL1997 September 1, 2026 17:24

dai-chen reviewed Sep 1, 2026

View reviewed changes

ahkcs requested a review from dai-chen September 1, 2026 18:31

Navigation Menu

Uh oh!

fix: gracefully handle scalar values under object-typed fields when decoding results (#5685) - #5732

fix: gracefully handle scalar values under object-typed fields when decoding results (#5685)#5732
ahkcs wants to merge 1 commit into
opensearch-project:mainfrom
ahkcs:fix/dedup-object-scalar-mapping-5685

ahkcs commented Aug 31, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Aug 31, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Aug 31, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Aug 31, 2026

Uh oh!

RyanL1997 Aug 31, 2026

Uh oh!

ahkcs Aug 31, 2026

Uh oh!

github-actions Bot commented Aug 31, 2026

Uh oh!

dai-chen commented Aug 31, 2026

Uh oh!

github-actions Bot commented Sep 1, 2026

Uh oh!

dai-chen Sep 1, 2026

Uh oh!

ahkcs Sep 1, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

ahkcs commented Aug 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Issues Resolved

Behavior (before → after)

Testing

Check List

Uh oh!

github-actions Bot commented Aug 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Reviewer Guide 🔍

(Review updated until commit 4d3a6d5)

Uh oh!

github-actions Bot commented Aug 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Code Suggestions ✨

Previous suggestions

Uh oh!

github-actions Bot commented Aug 31, 2026

Uh oh!

RyanL1997 Aug 31, 2026

Choose a reason for hiding this comment

Uh oh!

ahkcs Aug 31, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Aug 31, 2026

Uh oh!

dai-chen commented Aug 31, 2026

Uh oh!

github-actions Bot commented Sep 1, 2026

Uh oh!

dai-chen Sep 1, 2026

Choose a reason for hiding this comment

Uh oh!

ahkcs Sep 1, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ahkcs commented Aug 31, 2026 •

edited

Loading

github-actions Bot commented Aug 31, 2026 •

edited

Loading

(Review updated until commit `4d3a6d5`)

github-actions Bot commented Aug 31, 2026 •

edited

Loading