rdblue commented on code in PR #11904:
URL: https://github.com/apache/iceberg/pull/11904#discussion_r1925920839


##########
parquet/src/main/java/org/apache/iceberg/data/parquet/BaseParquetReaders.java:
##########
@@ -76,6 +80,46 @@ protected ParquetValueReader<T> createReader(
   protected abstract ParquetValueReader<T> createStructReader(
       List<Type> types, List<ParquetValueReader<?>> fieldReaders, 
Types.StructType structType);
 
+  protected ParquetValueReader<?> fixedReader(ColumnDescriptor desc) {
+    return new FixedReader(desc);
+  }
+
+  protected ParquetValueReader<?> dateReader(ColumnDescriptor desc) {
+    return new DateReader(desc);
+  }
+
+  protected ParquetValueReader<?> timeReader(
+      ColumnDescriptor desc, LogicalTypeAnnotation.TimeUnit unit) {
+    switch (unit) {
+      case MICROS:
+        return new TimeReader(desc);
+      case MILLIS:
+        return new TimeMillisReader(desc);
+      default:
+        throw new UnsupportedOperationException("Unsupported Unit: " + unit);
+    }
+  }
+
+  protected ParquetValueReader<?> timestampReader(
+      ColumnDescriptor desc, LogicalTypeAnnotation.TimeUnit unit, boolean 
isAdjustedToUTC) {
+    switch (unit) {
+      case MICROS:
+        return isAdjustedToUTC ? new TimestamptzReader(desc) : new 
TimestampReader(desc);
+      case MILLIS:
+        return isAdjustedToUTC
+            ? new TimestamptzMillisReader(desc)
+            : new TimestampMillisReader(desc);
+      case NANOS:
+        if (isAdjustedToUTC) {
+          return new TimestampInt96Reader(desc);

Review Comment:
   I don't think that we need an extra factory method for int96 timestamps. It 
is a timestamp, it just is a case that uses a different physical type. I like 
minimizing the number of dependencies between the parent and child class.



-- 
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: issues-unsubscr...@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org

Reply via email to