Skip to content

Render non-String scalars in jsonPathString without JSON quotes - #19029

Merged
Jackie-Jiang merged 1 commit into
apache:masterfrom
Jackie-Jiang:jsonpathstring-scalar-rendering
Jul 21, 2026
Merged

Render non-String scalars in jsonPathString without JSON quotes#19029
Jackie-Jiang merged 1 commit into
apache:masterfrom
Jackie-Jiang:jsonpathstring-scalar-rendering

Conversation

@Jackie-Jiang

Copy link
Copy Markdown
Contributor

Summary

jsonPathString (and its fast variants jsonPathStringFast / jsonPathStringFirstMatch) stringified any non-String resolved value via JsonUtils.objectToString. Jackson serializes several non-JSON-native Java types — notably java.util.UUID — as a quoted JSON string, so extracting such a value produced a double-quoted result. For example, an Avro uuid logical-type field extracted with JSONPATHSTRING(after, '$.customer_id') yielded "657ae8f8-..." (with literal quotes) instead of the bare UUID.

These types never arise from parsing a JSON string (Jackson only yields Boolean / Integer / Long / Double / String / null / List / Map); they only appear when the function operates on an already-materialized record tree — e.g. an ingestion transform reading an extractor-produced value.

Fix

All entry points now route the resolved value through a shared jsonValueToString helper:

  • String → returned verbatim (unquoted).
  • UUID / LocalDate / LocalTime — the non-JSON-native scalars a record extractor can materialize, which objectToString would wrap in quotes — render via their natural canonical / ISO-8601 toString.
  • Everything else — Number, Boolean, Timestamp (portable epoch millis), byte[], and Map / Collection / array containers — continues through JsonUtils.objectToString.

This keeps jsonPathString consistent with the query-time jsonExtractScalar on every value the query path can produce; the only deliberate divergences are the three quote-stripping overrides for types jsonExtractScalar never encounters.

Compatibility note

This changes the observable output of the released jsonPathString* scalar functions for UUID / LocalDate / LocalTime leaves (old: JSON-quoted / Jackson-default form; new: unquoted canonical form). The window is narrow — these types only occur on already-materialized record trees, not JSON-string input — but during a rolling upgrade a mixed-version cluster can briefly return both forms, and previously-persisted derived-column values will differ from newly-ingested ones. The new form is the intended correctness fix, so no version gate is added.

jsonPathString and its fast variants stringified any non-String value via
JsonUtils.objectToString, which wraps a java.util.UUID (and other non-JSON-native
scalars a record extractor can materialize) in JSON string quotes. Extracting an
Avro uuid field, for example, produced a double-quoted "657ae8f8-..." instead of
the bare UUID.

Route the resolved value through a shared jsonValueToString helper: UUID / LocalDate
/ LocalTime render via their natural (canonical / ISO-8601) toString, and everything
else - numbers, Boolean, Timestamp (epoch millis), byte[], and Map / Collection /
array containers - continues through JsonUtils.objectToString, matching the
json-path-to-string behavior of jsonExtractScalar.
@Jackie-Jiang Jackie-Jiang added bug Something is not working as expected ingestion Related to data ingestion pipeline release-notes Referenced by PRs that need attention when compiling the next release notes backward-incompat Introduces a backward-incompatible API or behavior change labels Jul 21, 2026
@codecov-commenter

codecov-commenter commented Jul 21, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 65.40%. Comparing base (b9e538a) to head (f30f607).
⚠️ Report is 2 commits behind head on master.

Additional details and impacted files
@@             Coverage Diff              @@
##             master   #19029      +/-   ##
============================================
+ Coverage     57.10%   65.40%   +8.29%     
- Complexity        7     1405    +1398     
============================================
  Files          2629     3423     +794     
  Lines        156315   215967   +59652     
  Branches      25533    34184    +8651     
============================================
+ Hits          89271   141245   +51974     
- Misses        59423    63339    +3916     
- Partials       7621    11383    +3762     
Flag Coverage Δ
custom-integration1 100.00% <ø> (ø)
integration 100.00% <ø> (ø)
integration1 100.00% <ø> (ø)
integration2 0.00% <ø> (ø)
java-25 65.40% <100.00%> (+8.29%) ⬆️
temurin 65.40% <100.00%> (+8.29%) ⬆️
unittests 65.39% <100.00%> (+8.29%) ⬆️
unittests1 57.08% <100.00%> (-0.03%) ⬇️
unittests2 37.61% <0.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@Jackie-Jiang
Jackie-Jiang merged commit ed2240e into apache:master Jul 21, 2026
12 of 13 checks passed
@Jackie-Jiang
Jackie-Jiang deleted the jsonpathstring-scalar-rendering branch July 21, 2026 17:58
@xiangfu0

Copy link
Copy Markdown
Contributor

Follow-up docs PR opened in pinot-contrib/pinot-docs: pinot-contrib/pinot-docs#932

xiangfu0 added a commit to pinot-contrib/pinot-docs that referenced this pull request Jul 21, 2026
## Summary
- document the `JSONPATHSTRING` compatibility change from
apache/pinot#19029
- clarify that unquoted `UUID`, `LocalDate`, and `LocalTime` rendering
only applies when ingestion transforms read already-materialized object
trees
- note that `JSONPATHSTRINGFAST` and `JSONPATHSTRINGFIRSTMATCH` keep
identical results

## Validation
- cross-checked the behavior against
`/Users/xiangfu/claude-workspace/pinot-doc-expert/pinot` and merged
upstream PR apache/pinot#19029
- ran `git diff --check`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backward-incompat Introduces a backward-incompatible API or behavior change bug Something is not working as expected ingestion Related to data ingestion pipeline release-notes Referenced by PRs that need attention when compiling the next release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants