xiangfu0 commented on code in PR #19171:
URL: https://github.com/apache/pinot/pull/19171#discussion_r3789901194
##########
pinot-spi/src/main/java/org/apache/pinot/spi/utils/MapUtils.java:
##########
@@ -183,13 +187,139 @@ public static Map<String, Object>
deserializeMap(ByteBuffer byteBuffer) {
return map;
}
+ /// Deserializes only the value for the requested key from a length-prefixed
MAP frame.
+ /// Non-matching keys and values are skipped without allocating byte arrays
or invoking Jackson.
+ ///
+ /// @param bytes Serialized MAP frame
+ /// @param key Key whose value should be deserialized
+ /// @return Deserialized value, or `null` if the key is missing, has a null
value, or cannot be deserialized
+ @Nullable
+ public static Object deserializeMapValue(byte[] bytes, String key) {
+ return deserializeMapValue(ByteBuffer.wrap(bytes), key);
+ }
+
+ /// Variant of [#deserializeMapValue(byte[], String)] that reads from the
supplied buffer without copying the
+ /// complete MAP frame.
+ ///
+ /// Consumes the buffer from its current position and forces
[ByteOrder#BIG_ENDIAN] on it — the write path frames
+ /// lengths through a big-endian [ByteBuffer], while an off-heap view
inherits the platform's native order.
+ @Nullable
+ public static Object deserializeMapValue(ByteBuffer byteBuffer, String key) {
+ byte[] valueBytes = findValueBytes(byteBuffer, key);
+ if (valueBytes == null) {
+ return null;
+ }
+ try {
+ return JsonUtils.bytesToObject(valueBytes, Object.class);
+ } catch (IOException e) {
+ LOGGER.error("Caught exception while deserializing value for key: {}",
key, e);
+ return null;
+ }
+ }
+
+ /// Reads the value for a key as a string, skipping Jackson when the stored
value is a plain JSON string.
+ ///
+ /// A `MAP` column with a `STRING` value type stores `"pinot-server"`, and
parsing that into a `String` only to
+ /// call `toString()` on it costs a parser instantiation per access. Values
that are not plain strings - numbers,
+ /// booleans, objects, arrays - still go through Jackson so the rendering
matches [#deserializeMapValue] exactly.
+ ///
+ /// @return The value as a string, or `null` if the key is missing, its
value is null, or it cannot be deserialized
+ @Nullable
+ public static String deserializeMapValueAsString(ByteBuffer byteBuffer,
String key) {
+ byte[] valueBytes = findValueBytes(byteBuffer, key);
+ if (valueBytes == null) {
+ return null;
+ }
+ String unquoted = unquotePlainJsonString(valueBytes);
+ if (unquoted != null) {
+ return unquoted;
+ }
+ try {
+ Object value = JsonUtils.bytesToObject(valueBytes, Object.class);
+ return value == null ? null : value.toString();
+ } catch (IOException e) {
+ LOGGER.error("Caught exception while deserializing value for key: {}",
key, e);
+ return null;
+ }
+ }
+
+ /// Decodes a JSON string literal that carries no escapes, otherwise returns
`null` so the caller falls back to
+ /// Jackson. A lone backslash anywhere means an escape sequence is present
and the literal is not its own value.
+ @Nullable
+ private static String unquotePlainJsonString(byte[] valueBytes) {
+ int length = valueBytes.length;
+ if (length < 2 || valueBytes[0] != '"' || valueBytes[length - 1] != '"') {
+ return null;
+ }
+ for (int i = 1; i < length - 1; i++) {
+ if (valueBytes[i] == '\\') {
+ return null;
+ }
+ }
+ return new String(valueBytes, 1, length - 2, StandardCharsets.UTF_8);
+ }
+
+ /// Scans a frame for `key` and returns its raw value bytes, or `null` when
the key is absent.
+ @Nullable
+ private static byte[] findValueBytes(ByteBuffer byteBuffer, String key) {
+ byteBuffer.order(ByteOrder.BIG_ENDIAN);
+ int size = byteBuffer.getInt();
+ if (size == 0) {
+ return null;
+ }
+ byte[] keyBytes = Utf8Utils.encode(key);
+ int keyBytesLength = keyBytes.length;
+ for (int i = 0; i < size; i++) {
+ int keyLength = byteBuffer.getInt();
+ // Bounds-check up front so the absolute gets below are provably in
range, and so a truncated frame still
+ // surfaces as BufferUnderflowException rather than
IndexOutOfBoundsException.
+ checkLength(byteBuffer, keyLength);
+ // Compare through absolute gets so a length mismatch or a differing
byte skips the rest of the key outright,
+ // rather than walking it one relative get at a time just to advance the
position.
+ boolean matches = keyLength == keyBytesLength;
+ if (matches) {
+ int keyOffset = byteBuffer.position();
+ for (int j = 0; j < keyLength; j++) {
+ if (byteBuffer.get(keyOffset + j) != keyBytes[j]) {
+ matches = false;
+ break;
+ }
+ }
+ }
+ byteBuffer.position(byteBuffer.position() + keyLength);
+
+ int valueLength = byteBuffer.getInt();
+ if (!matches) {
+ skip(byteBuffer, valueLength);
+ continue;
+ }
+ // Keys within a frame are unique - the write path iterates a Map - so
the first match is the only match and
+ // the remaining entries never need to be scanned.
+ byte[] valueBytes = new byte[valueLength];
+ byteBuffer.get(valueBytes);
+ return valueBytes;
Review Comment:
No change here - `checkLength(byteBuffer, valueLength)` already runs
*before* the `if (!matches)` branch, so it guards the allocating path and the
skipping path alike, and it rejects `length < 0` as well as `length >
remaining`. So `new byte[valueLength]` cannot see a negative or oversized
length. `MapUtilsTest#testDeserializeMapEntryValueRejectsInvalidValueLength`
covers this. Happy to revisit if you were pointing at a path I have missed.
_🤖 Addressed by [Claude Code](https://claude.com/claude-code)_
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
