Render MAP projections straight from the serialized frame - #19169
Open
xiangfu0 wants to merge 1 commit into
Open
Render MAP projections straight from the serialized frame#19169xiangfu0 wants to merge 1 commit into
xiangfu0 wants to merge 1 commit into
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #19169 +/- ##
============================================
- Coverage 66.65% 66.64% -0.01%
+ Complexity 1423 1417 -6
============================================
Files 3443 3443
Lines 218632 218694 +62
Branches 34793 34802 +9
============================================
+ Hits 145726 145758 +32
- Misses 61192 61212 +20
- Partials 11714 11724 +10
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Contributor
There was a problem hiding this comment.
Pull request overview
Optimizes MAP column projection (SELECT attributes, LASTWITHTIME(attributes, ...)) by rendering JSON directly from the serialized MAP frame stored in forward indexes, avoiding per-row map materialization and re-serialization.
Changes:
- Added
MapUtils.frameToJsonString(...)to render a serialized MAP frame as JSON without deserializing to aMap. - Introduced
ForwardIndexReader#getMapAsJsonStringand routed MAP SV string reads through it, with overrides in frame-backed forward-index readers. - Added unit tests for the rendering equivalence/round-trip behavior and a JMH benchmark to quantify the improvement.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| pinot-spi/src/test/java/org/apache/pinot/spi/utils/MapUtilsTest.java | Adds coverage validating frameToJsonString equivalence with the legacy deserialize+serialize path and byte-order handling. |
| pinot-spi/src/main/java/org/apache/pinot/spi/utils/MapUtils.java | Implements frame-to-JSON rendering and supporting utilities. |
| pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/index/reader/ForwardIndexReader.java | Adds getMapAsJsonString hook and switches MAP SV reads to use it. |
| pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/readers/forward/VarByteChunkSVForwardIndexReader.java | Overrides MAP JSON rendering to use frameToJsonString for frame-backed storage. |
| pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/readers/forward/VarByteChunkForwardIndexReaderV4.java | Overrides MAP JSON rendering to use frameToJsonString for frame-backed storage. |
| pinot-segment-local/src/main/java/org/apache/pinot/segment/local/realtime/impl/forward/VarByteSVMutableForwardIndex.java | Overrides MAP JSON rendering to use frameToJsonString for frame-backed storage. |
| pinot-perf/src/main/java/org/apache/pinot/perf/BenchmarkMapProjection.java | Adds JMH benchmark comparing legacy vs optimized MAP projection rendering. |
Comment on lines
+203
to
+208
| /// Renders a serialized MAP frame as a JSON object without materializing the map. | ||
| /// | ||
| /// The frame already stores each value as the JSON bytes that [#serializeMap] produced, so the values are copied | ||
| /// through verbatim and only the keys are quoted. That skips the parse-into-`HashMap`-then-serialize-again round | ||
| /// trip [#toString(Map)] performs, and it skips Jackson entirely. | ||
| /// |
Comment on lines
+268
to
+274
| private void appendRaw(ByteBuffer byteBuffer, int length) { | ||
| ensure(length); | ||
| int offset = byteBuffer.position(); | ||
| for (int i = 0; i < length; i++) { | ||
| _bytes[_length++] = byteBuffer.get(offset + i); | ||
| } | ||
| } |
Comment on lines
+219
to
+223
| byteBuffer.order(ByteOrder.BIG_ENDIAN); | ||
| int size = byteBuffer.getInt(); | ||
| if (size == 0) { | ||
| return "{}"; | ||
| } |
xiangfu0
force-pushed
the
agent/map-projection-json
branch
from
August 7, 2026 09:28
6787123 to
6d16753
Compare
Projecting a whole MAP column as a string parsed every entry into a HashMap and then serialized that map back to JSON, once per row. Both steps are avoidable: the frame already stores each value as the JSON bytes serializeMap produced, so the values can be copied through verbatim and only the keys need quoting. Add MapUtils#frameToJsonString and a ForwardIndexReader#getMapAsJsonString hook, overridden by the three readers that hold the map as a frame. The default still materializes the map, so a reader that keeps the map columnar-decomposed is unaffected. Output is unchanged. Both forward-index write paths - ForwardIndexCreator at segment build and MutableSegmentImpl while consuming - frame maps through the key-sorting serializeMap, and nested values are sorted by the same writer, so emitting in frame order reproduces what toString(deserializeMap(frame)) produced. A test pins that equivalence over scalars, nesting, unicode, and keys needing escapes. Isolated JMH, JDK 25: entries shape before after before B/op after B/op 4 flat 0.866 0.258 4768 768 4 nested 1.312 0.274 7320 848 16 flat 3.261 1.018 16936 2688 16 nested 5.352 1.049 26968 3008 64 flat 14.111 3.885 65656 10464 64 nested 24.071 4.227 104312 11744 3.2-5.7x faster, 6-9x less garbage. Nested values cost the old path 71% more than flat ones at 64 entries because Jackson materializes a container per value; the new path is within 9% of flat since it never looks inside a value. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
xiangfu0
force-pushed
the
agent/map-projection-json
branch
from
August 8, 2026 09:36
6d16753 to
d3d2453
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Projecting a whole
MAPcolumn as a string —SELECT attributes,LASTWITHTIME(attributes, ...)— parses every entry into aHashMapand then serializes that map back to JSON, once per row.Both steps are avoidable. The frame already stores each value as the JSON bytes
serializeMapproduced, so values can be copied through verbatim and only keys need quoting.Changes
MapUtils#frameToJsonString, rendering a frame to JSON without Jackson and without materializing the map.ForwardIndexReader#getMapAsJsonStringhook and route thecase MAPbranch ofreadValuesSVthrough it.VarByteSVMutableForwardIndex,VarByteChunkSVForwardIndexReader,VarByteChunkForwardIndexReaderV4. The default still materializes the map, so a reader holding the map columnar-decomposed is unaffected.Output is unchanged
Both forward-index write paths —
ForwardIndexCreator#putValueat segment build andMutableSegmentImplwhile consuming — frame maps through the key-sortingserializeMap(Map), and nested values are sorted by that same writer. So emitting entries in frame order reproduces exactly whattoString(deserializeMap(frame))produced.testFrameToJsonStringMatchesToStringpins that equivalence across scalars, nesting, unicode, and keys that need escaping.Performance
Isolated JMH (
BenchmarkMapProjection), JDK 25, 2 forks x 5x1s,-prof gc:Nested values cost the old path 71% more than flat ones at 64 entries, because Jackson materializes a container per value. The new path stays within 9% of flat since it never looks inside a value. The allocation that remains is the output buffer and the returned
String.This is an isolated forward-index measurement, not an end-to-end query latency result.
Validation
MapUtilsTest25/25 (4 new)TableIndexingTest506,DataBlockBuilderTest97,GenericRowSerDeTest7,DataTableSerDeTest5,OpenStructDataTypeTest5 — all passingspotless:apply,license:check,checkstyle:checkclean onpinot-spi,pinot-segment-spi,pinot-segment-local,pinot-perf