rdblue commented on a change in pull request #199:
URL: https://github.com/apache/iceberg/pull/199#discussion_r432005543
##########
File path: orc/src/main/java/org/apache/iceberg/orc/OrcMetrics.java
##########
@@ -134,96 +136,82 @@ private static long toMicros(Timestamp ts) {
private static Optional<ByteBuffer> fromOrcMin(Types.NestedField column,
ColumnStatistics columnStats)
{
- ByteBuffer min = null;
+ Object min = null;
if (columnStats instanceof IntegerColumnStatistics) {
- IntegerColumnStatistics intColStats = (IntegerColumnStatistics)
columnStats;
- min = column.type().typeId() == Type.TypeID.INTEGER ?
- Conversions.toByteBuffer(column.type(), (int)
intColStats.getMinimum()) :
- Conversions.toByteBuffer(column.type(), intColStats.getMinimum());
+ min = ((IntegerColumnStatistics) columnStats).getMinimum();
+ if (column.type().typeId() == Type.TypeID.INTEGER) {
+ min = Math.toIntExact((long) min);
+ }
} else if (columnStats instanceof DoubleColumnStatistics) {
- double minVal = ((DoubleColumnStatistics) columnStats).getMinimum();
- min = column.type().typeId() == Type.TypeID.DOUBLE ?
- Conversions.toByteBuffer(column.type(), minVal) :
- Conversions.toByteBuffer(column.type(), (float) minVal);
+ min = ((DoubleColumnStatistics) columnStats).getMinimum();
+ if (column.type().typeId() == Type.TypeID.FLOAT) {
+ min = ((Double) min).floatValue();
+ }
} else if (columnStats instanceof StringColumnStatistics) {
- min = Conversions.toByteBuffer(column.type(),
- ((StringColumnStatistics) columnStats).getMinimum());
+ min = ((StringColumnStatistics) columnStats).getMinimum();
} else if (columnStats instanceof DecimalColumnStatistics) {
min = Optional
.ofNullable(((DecimalColumnStatistics) columnStats).getMinimum())
- .map(minStats -> {
- BigDecimal minValue = minStats.bigDecimalValue()
- .setScale(((Types.DecimalType) column.type()).scale());
- return Conversions.toByteBuffer(column.type(), minValue);
- })
+ .map(minStats -> minStats.bigDecimalValue()
+ .setScale(((Types.DecimalType) column.type()).scale()))
.orElse(null);
} else if (columnStats instanceof DateColumnStatistics) {
min = Optional.ofNullable(((DateColumnStatistics)
columnStats).getMinimum())
- .map(minStats -> Conversions.toByteBuffer(column.type(),
- (int) ChronoUnit.DAYS.between(EPOCH_DAY,
+ .map(minStats ->
+ Math.toIntExact(ChronoUnit.DAYS.between(EPOCH_DAY,
EPOCH.plus(minStats.getTime(),
ChronoUnit.MILLIS).toLocalDate())))
.orElse(null);
} else if (columnStats instanceof TimestampColumnStatistics) {
TimestampColumnStatistics tColStats = (TimestampColumnStatistics)
columnStats;
- Timestamp minValue = ((Types.TimestampType)
column.type()).shouldAdjustToUTC() ?
- tColStats.getMinimum() : tColStats.getMinimumUTC();
+ Timestamp minValue = tColStats.getMinimumUTC();
min = Optional.ofNullable(minValue)
- .map(v -> Conversions.toByteBuffer(column.type(), toMicros(v)))
+ .map(OrcMetrics::toMicros)
.orElse(null);
} else if (columnStats instanceof BooleanColumnStatistics) {
BooleanColumnStatistics booleanStats = (BooleanColumnStatistics)
columnStats;
- min = booleanStats.getFalseCount() > 0 ?
- Conversions.toByteBuffer(column.type(), false) :
- Conversions.toByteBuffer(column.type(), true);
+ min = booleanStats.getFalseCount() <= 0;
}
- return Optional.ofNullable(min);
+ return Optional.ofNullable(Conversions.toByteBuffer(column.type(), min));
}
private static Optional<ByteBuffer> fromOrcMax(Types.NestedField column,
ColumnStatistics columnStats)
{
- ByteBuffer max = null;
+ Object max = null;
if (columnStats instanceof IntegerColumnStatistics) {
- IntegerColumnStatistics intColStats = (IntegerColumnStatistics)
columnStats;
- max = column.type().typeId() == Type.TypeID.INTEGER ?
- Conversions.toByteBuffer(column.type(), (int)
intColStats.getMaximum()) :
- Conversions.toByteBuffer(column.type(), intColStats.getMaximum());
+ max = ((IntegerColumnStatistics) columnStats).getMaximum();
+ if (column.type().typeId() == Type.TypeID.INTEGER) {
+ max = Math.toIntExact((long) max);
+ }
Review comment:
Minor: I generally prefer not mutating a variable and storing it back so
that it is clear that there are two code paths. The ternary operator seems like
a cleaner approach.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
