Jackie-Jiang commented on code in PR #18400:
URL: https://github.com/apache/pinot/pull/18400#discussion_r3185793662
##########
pinot-plugins/pinot-input-format/pinot-parquet/src/main/java/org/apache/pinot/plugin/inputformat/parquet/ParquetRecordReaderConfig.java:
##########
@@ -18,34 +18,31 @@
*/
package org.apache.pinot.plugin.inputformat.parquet;
-import org.apache.commons.configuration2.Configuration;
import org.apache.pinot.spi.data.readers.RecordReaderConfig;
-/**
- * Config for ParquetRecordReader
- */
+/// Config for [ParquetRecordReader] (and the underlying
[ParquetAvroRecordReader] / [ParquetNativeRecordReader]).
+/// Three settings, all default `false`:
+/// - `useParquetAvroRecordReader` — force the parquet-avro reader.
+/// - `useParquetNativeRecordReader` — force the native parquet reader. When
neither flag is set, the dispatcher
+/// auto-detects via the file's `avro.schema` metadata (Avro reader if
present, native otherwise).
+/// - `extractRawTimeValues` — opt out of TIMESTAMP / DATE / TIME conversion
at the extractor boundary,
+/// surfacing the raw underlying integer in the column's declared unit
instead of the contract Java type.
+/// DECIMAL and UUID always convert. See [ParquetAvroRecordExtractor] /
[ParquetNativeRecordExtractor] for
+/// the per-type matrix.
public class ParquetRecordReaderConfig implements RecordReaderConfig {
- private static final String USE_PARQUET_AVRO_RECORDER_READER =
"useParquetAvroRecordReader";
- private static final String USE_PARQUET_NATIVE_RECORDER_READER =
"useParquetNativeRecordReader";
-
private boolean _useParquetAvroRecordReader;
private boolean _useParquetNativeRecordReader;
- private Configuration _conf;
-
- public ParquetRecordReaderConfig() {
- }
-
- public ParquetRecordReaderConfig(Configuration conf) {
- _conf = conf;
- _useParquetAvroRecordReader =
conf.getBoolean(USE_PARQUET_AVRO_RECORDER_READER, false);
- _useParquetNativeRecordReader =
conf.getBoolean(USE_PARQUET_NATIVE_RECORDER_READER, false);
- }
+ private boolean _extractRawTimeValues;
Review Comment:
Acknowledged as a deliberate API change, in the same family as the
`BaseRecordExtractor.shouldExtract` and `JsonUtils.jsonNodeToStringMap`
removals already discussed in this PR. The `Configuration`-based constructor
and `getConfig()` accessor were a thin convenience over `commons-configuration`
access; in-tree usage was zero, and external callers can switch to the
property-based constructor or call `setX(...)` directly. Trading the small
breakage now for a cleaner config surface that doesn't pull a
`commons-configuration` Configuration into the SPI.
--
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]
