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