rdblue commented on a change in pull request #1963:
URL: https://github.com/apache/iceberg/pull/1963#discussion_r579898254
##########
File path: core/src/main/java/org/apache/iceberg/avro/ValueWriters.java
##########
@@ -358,8 +380,10 @@ public void write(BigDecimal decimal, Encoder encoder)
throws IOException {
private final int nullIndex;
private final int valueIndex;
private final ValueWriter<T> valueWriter;
+ private final Schema.Type type;
+ private long nullValueCount;
- private OptionWriter(int nullIndex, ValueWriter<T> valueWriter) {
+ private OptionWriter(int nullIndex, ValueWriter<T> valueWriter,
Schema.Type type) {
Review comment:
I think that we should not pass the type here. Instead, let's have a
`MetricsOptionWriter` and a non-metrics version and choose which one to use
ahead of time. That way, we don't keep a count that won't be used. There's no
need to check whether the null count should be used when `metrics` is called.
That can be determined ahead of time.
If we do that, then there is no need to pass the type here or into the
`option` method. We can either add a boolean (`collectMetrics`) or have a
separate factory method. I think that would be a bit cleaner.
----------------------------------------------------------------
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]
