Skip to content

Commit

Permalink
address comments
Browse files Browse the repository at this point in the history
  • Loading branch information
yyanyy committed Dec 7, 2020
1 parent e2da2be commit 4bb427f
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 14 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -28,39 +28,35 @@
* This wrapper ensures that metrics not being updated by those writers will not be incorrectly used, by throwing
* exceptions when they are accessed.
*/
public class NaNFieldMetrics extends FieldMetrics {
public class FloatFieldMetrics extends FieldMetrics {

/**
* Constructor for creating a FieldMetrics with only NaN counter.
* @param id field id being tracked by the writer
* @param nanValueCount number of NaN values, will only be non-0 for double or float field.
*/
public NaNFieldMetrics(int id,
long nanValueCount) {
public FloatFieldMetrics(int id,
long nanValueCount) {
super(id, 0L, 0L, nanValueCount, null, null);
}

@Override
public long valueCount() {
throw new IllegalStateException(
"Shouldn't access valueCount() within NaNOnlyFieldMetrics, as this metric is tracked in file statistics. ");
throw new IllegalStateException("Shouldn't access this method, as this metric is tracked in file statistics. ");
}

@Override
public long nullValueCount() {
throw new IllegalStateException(
"Shouldn't access nullValueCount() within NaNOnlyFieldMetrics, as this metric is tracked in file statistics. ");
throw new IllegalStateException("Shouldn't access this method, as this metric is tracked in file statistics. ");
}

@Override
public ByteBuffer lowerBound() {
throw new IllegalStateException(
"Shouldn't access lowerBound() within NaNOnlyFieldMetrics, as this metric is tracked in file statistics. ");
throw new IllegalStateException("Shouldn't access this method, as this metric is tracked in file statistics. ");
}

@Override
public ByteBuffer upperBound() {
throw new IllegalStateException(
"Shouldn't access upperBound() within NaNOnlyFieldMetrics, as this metric is tracked in file statistics. ");
throw new IllegalStateException("Shouldn't access this method, as this metric is tracked in file statistics. ");
}
}
12 changes: 12 additions & 0 deletions core/src/main/java/org/apache/iceberg/MetricsUtil.java
Original file line number Diff line number Diff line change
Expand Up @@ -22,15 +22,21 @@
import java.util.Map;
import java.util.stream.Collectors;
import java.util.stream.Stream;
import org.apache.iceberg.relocated.com.google.common.base.Preconditions;
import org.apache.iceberg.relocated.com.google.common.collect.Maps;

public class MetricsUtil {

private MetricsUtil() {
}

/**
* Construct mapping relationship between column id to NaN value counts from input metrics and metrics config.
*/
public static Map<Integer, Long> createNanValueCounts(
Stream<FieldMetrics> fieldMetrics, MetricsConfig metricsConfig, Schema inputSchema) {
Preconditions.checkNotNull(metricsConfig, "metricsConfig is required");

if (fieldMetrics == null || inputSchema == null) {
return Maps.newHashMap();
}
Expand All @@ -40,7 +46,13 @@ public static Map<Integer, Long> createNanValueCounts(
.collect(Collectors.toMap(FieldMetrics::id, FieldMetrics::nanValueCount));
}

/**
* Extract MetricsMode for the given field id from metrics config.
*/
public static MetricsModes.MetricsMode metricsMode(Schema inputSchema, MetricsConfig metricsConfig, int fieldId) {
Preconditions.checkNotNull(inputSchema, "inputSchema is required");
Preconditions.checkNotNull(metricsConfig, "metricsConfig is required");

String columnName = inputSchema.findColumnName(fieldId);
return metricsConfig.columnMode(columnName);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
import java.util.stream.Stream;
import org.apache.avro.util.Utf8;
import org.apache.iceberg.FieldMetrics;
import org.apache.iceberg.NaNFieldMetrics;
import org.apache.iceberg.FloatFieldMetrics;
import org.apache.iceberg.deletes.PositionDelete;
import org.apache.iceberg.relocated.com.google.common.base.Preconditions;
import org.apache.iceberg.relocated.com.google.common.collect.ImmutableList;
Expand Down Expand Up @@ -189,7 +189,7 @@ public void write(int repetitionLevel, Float value) {

@Override
public Stream<FieldMetrics> metrics() {
return Stream.of(new NaNFieldMetrics(id, nanCount));
return Stream.of(new FloatFieldMetrics(id, nanCount));
}
}

Expand All @@ -213,7 +213,7 @@ public void write(int repetitionLevel, Double value) {

@Override
public Stream<FieldMetrics> metrics() {
return Stream.of(new NaNFieldMetrics(id, nanCount));
return Stream.of(new FloatFieldMetrics(id, nanCount));
}
}

Expand Down

0 comments on commit 4bb427f

Please sign in to comment.