xiangfu0 commented on code in PR #19127:
URL: https://github.com/apache/pinot/pull/19127#discussion_r3686951993
##########
pinot-plugins/pinot-input-format/pinot-json/src/main/java/org/apache/pinot/plugin/inputformat/json/format/JsonPayloadParser.java:
##########
@@ -51,4 +54,16 @@ public interface JsonPayloadParser {
/// @throws Exception if the region is not valid for this format
Map<String, Object> parse(byte[] payload, int offset, int length)
throws Exception;
+
+ /// Parses the payload directly into {@code destination}, avoiding a
top-level per-record map when the
Review Comment:
Updated the Markdown Javadoc to use backticks for `destination`, `null`,
`true`, and `false`.
##########
pinot-plugins/pinot-input-format/pinot-json/src/main/java/org/apache/pinot/plugin/inputformat/json/format/JsonPayloadParser.java:
##########
@@ -51,4 +54,16 @@ public interface JsonPayloadParser {
/// @throws Exception if the region is not valid for this format
Map<String, Object> parse(byte[] payload, int offset, int length)
throws Exception;
+
+ /// Parses the payload directly into {@code destination}, avoiding a
top-level per-record map when the
+ /// implementation supports it.
+ ///
+ /// @param fields fields to populate, or {@code null} to populate every
top-level field
+ /// @return {@code true} when the payload was decoded into {@code
destination}; {@code false} when the caller
+ /// should fall back to [#parse]
+ default boolean parseTo(byte[] payload, int offset, int length, @Nullable
Set<String> fields,
Review Comment:
Updated the direct-row overload to `parse(payload, offset, length,
destination, fields)`, with nullable `fields` last.
##########
pinot-plugins/pinot-input-format/pinot-json/src/main/java/org/apache/pinot/plugin/inputformat/json/format/JacksonPayloadParser.java:
##########
@@ -31,15 +37,71 @@
/// The [ObjectReader] is immutable and thread-safe, so a single instance is
shared across all decode calls.
abstract class JacksonPayloadParser implements JsonPayloadParser {
+ private final ObjectMapper _mapper;
Review Comment:
Updated text JSON and PostgreSQL JSONB to reuse `JsonUtils.DEFAULT_READER`.
Smile and CBOR still require their format-specific Jackson factories. Selected
values continue through `JSONRecordExtractor.convert`; the test now asserts
oversized integers are materialized as `BigDecimal`.
##########
pinot-plugins/pinot-input-format/pinot-json/src/main/java/org/apache/pinot/plugin/inputformat/json/JSONMessageDecoder.java:
##########
@@ -72,6 +74,9 @@ public void init(Map<String, String> props, Set<String>
fieldsToRead, String top
}
_jsonRecordExtractor =
PluginManager.get().createInstance(recordExtractorClass);
_jsonRecordExtractor.init(fieldsToRead, null);
+ _fieldsToRead = fieldsToRead == null || fieldsToRead.isEmpty() ? null :
Set.copyOf(fieldsToRead);
Review Comment:
Updated to `CollectionUtils.isNotEmpty(fieldsToRead)`.
##########
pinot-plugins/pinot-input-format/pinot-json/src/main/java/org/apache/pinot/plugin/inputformat/json/JSONMessageDecoder.java:
##########
@@ -72,6 +74,9 @@ public void init(Map<String, String> props, Set<String>
fieldsToRead, String top
}
_jsonRecordExtractor =
PluginManager.get().createInstance(recordExtractorClass);
_jsonRecordExtractor.init(fieldsToRead, null);
+ _fieldsToRead = fieldsToRead == null || fieldsToRead.isEmpty() ? null :
Set.copyOf(fieldsToRead);
+ // A configured extractor can change conversion semantics, so only bypass
Pinot's exact default class.
+ _usesDefaultRecordExtractor = _jsonRecordExtractor.getClass() ==
JSONRecordExtractor.class;
Review Comment:
The direct parser bypasses `RecordExtractor.extract()`, so it is only
semantically safe for the exact default `JSONRecordExtractor`. A configured
extractor or subclass can override extraction behavior; the exact-class guard
preserves that behavior through the map fallback. I expanded the code comment,
and `testCustomExtractorUsesMapFallback` covers this case.
--
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]
