edgarRd commented on a change in pull request #199:
URL: https://github.com/apache/iceberg/pull/199#discussion_r432675374
##########
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:
I agree, I generally avoid mutations, but in this case the ternary
operator does not work since `max` is an `Object` and it could have either an
`int` or `long` (boxed) the JVM defaults to always boxing to a `long`, leading
to conversion issues when the value is actually an `INTEGER`.
----------------------------------------------------------------
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]
