xiangfu0 commented on code in PR #20060:
URL: https://github.com/apache/pinot/pull/20060#discussion_r4392761374
##########
pinot-plugins/pinot-input-format/pinot-parquet/src/main/java/org/apache/pinot/plugin/inputformat/parquet/ParquetAvroRecordExtractor.java:
##########
@@ -66,8 +68,9 @@ public GenericRow extract(GenericRecord from, GenericRow to) {
protected Object convertSingleValue(Schema schema, Object value) {
if (schema.getType() == Schema.Type.FIXED && schema.getFixedSize() ==
INT96_BYTE_SIZE
&& INT96_DOC.equals(schema.getDoc())) {
- long nanos = ParquetUtils.convertInt96ToEpochNanos(((GenericFixed)
value).bytes());
- return _extractRawTimeValues ? nanos :
TimestampUtils.fromNanosSinceEpoch(nanos);
+ byte[] bytes = ((GenericFixed) value).bytes();
+ return _extractRawTimeValues ?
ParquetUtils.convertInt96ToEpochNanos(bytes)
Review Comment:
Fixed in `10245ec94d`. The Avro reader now prefetches in `finally`, so
extraction failures consume the rejected row while preserving
extraction-before-prefetch ordering and the existing `RecordFetchException`
handling. The new file-backed, forced-Avro/raw-mode test reproduces the failure
on the previous head and verifies rejected year-1600 row → valid 7 ns row →
EOF, including after rewind. All 51 focused tests and
formatting/checkstyle/license checks pass; independent review found no
remaining issue. Hosted CI is running on the new head.
##########
pinot-plugins/pinot-input-format/pinot-parquet/src/main/java/org/apache/pinot/plugin/inputformat/parquet/ParquetAvroRecordExtractor.java:
##########
@@ -24,18 +24,20 @@
import org.apache.avro.generic.GenericRecord;
import org.apache.pinot.plugin.inputformat.avro.AvroRecordExtractor;
import org.apache.pinot.spi.data.readers.GenericRow;
-import org.apache.pinot.spi.utils.TimestampUtils;
/// The type matrix is inherited from [AvroRecordExtractor], with two
overrides:
/// - INT96 timestamp (which parquet-avro surfaces as `fixed(12)` with `doc =
"INT96 represented as byte[12]"`) →
/// `Timestamp` (or `Long` epoch nanos when `extractRawTimeValues` is
`true`) via
-/// [ParquetUtils#convertInt96ToEpochNanos].
+/// [ParquetUtils].
Review Comment:
Applied in `10245ec94d`: `[ParquetUtils]` is on the preceding line as
suggested. Formatting, checkstyle and license checks pass.
##########
pinot-plugins/pinot-input-format/pinot-parquet/src/main/java/org/apache/pinot/plugin/inputformat/parquet/ParquetAvroRecordExtractor.java:
##########
@@ -66,8 +68,9 @@ public GenericRow extract(GenericRecord from, GenericRow to) {
protected Object convertSingleValue(Schema schema, Object value) {
if (schema.getType() == Schema.Type.FIXED && schema.getFixedSize() ==
INT96_BYTE_SIZE
&& INT96_DOC.equals(schema.getDoc())) {
- long nanos = ParquetUtils.convertInt96ToEpochNanos(((GenericFixed)
value).bytes());
- return _extractRawTimeValues ? nanos :
TimestampUtils.fromNanosSinceEpoch(nanos);
+ byte[] bytes = ((GenericFixed) value).bytes();
+ return _extractRawTimeValues ?
ParquetUtils.convertInt96ToEpochNanos(bytes)
+ : ParquetUtils.convertInt96ToTimestamp(bytes);
Review Comment:
Applied in `10245ec94d`: both ternary branches are on separate continuation
lines. All 51 focused tests and quality checks pass.
--
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]
