rdblue commented on a change in pull request #1963:
URL: https://github.com/apache/iceberg/pull/1963#discussion_r579892821



##########
File path: core/src/main/java/org/apache/iceberg/avro/ValueWriters.java
##########
@@ -135,145 +150,145 @@ private NullWriter() {
 
     @Override
     public void write(Void ignored, Encoder encoder) throws IOException {
-      encoder.writeNull();
+      throw new IllegalStateException("[BUG] NullWriter shouldn't be used for 
writing nulls for Avro");
     }
-  }
 
-  private static class BooleanWriter implements ValueWriter<Boolean> {
-    private static final BooleanWriter INSTANCE = new BooleanWriter();
+    @Override
+    public Stream<FieldMetrics> metrics() {
+      throw new IllegalStateException("[BUG] NullWriter shouldn't be used for 
writing nulls for Avro");
+    }
+  }
 
-    private BooleanWriter() {
+  private static class BooleanWriter extends ComparableWriter<Boolean> {
+    private BooleanWriter(int id) {
+      super(id);
     }
 
     @Override
-    public void write(Boolean bool, Encoder encoder) throws IOException {
+    protected void writeVal(Boolean bool, Encoder encoder) throws IOException {
       encoder.writeBoolean(bool);
     }
   }
 
-  private static class ByteToIntegerWriter implements ValueWriter<Byte> {
-    private static final ByteToIntegerWriter INSTANCE = new 
ByteToIntegerWriter();
-
-    private ByteToIntegerWriter() {
+  private static class ByteToIntegerWriter extends 
MetricsAwareTransformWriter<Byte, Integer> {
+    private ByteToIntegerWriter(int id) {
+      super(id, Integer::compareTo, Byte::intValue);
     }
 
     @Override
-    public void write(Byte b, Encoder encoder) throws IOException {
-      encoder.writeInt(b.intValue());
+    protected void writeVal(Integer intVal, Encoder encoder) throws 
IOException {
+      encoder.writeInt(intVal);
     }
   }
 
-  private static class ShortToIntegerWriter implements ValueWriter<Short> {
-    private static final ShortToIntegerWriter INSTANCE = new 
ShortToIntegerWriter();
-
-    private ShortToIntegerWriter() {
+  private static class ShortToIntegerWriter extends 
MetricsAwareTransformWriter<Short, Integer> {
+    private ShortToIntegerWriter(int id) {
+      super(id, Integer::compareTo, Short::intValue);
     }
 
     @Override
-    public void write(Short s, Encoder encoder) throws IOException {
-      encoder.writeInt(s.intValue());
+    protected void writeVal(Integer intValue, Encoder encoder) throws 
IOException {
+      encoder.writeInt(intValue);
     }
   }
 
-  private static class IntegerWriter implements ValueWriter<Integer> {
-    private static final IntegerWriter INSTANCE = new IntegerWriter();
-
-    private IntegerWriter() {
+  private static class IntegerWriter extends ComparableWriter<Integer> {
+    private IntegerWriter(int id) {
+      super(id);
     }
 
     @Override
-    public void write(Integer i, Encoder encoder) throws IOException {
+    protected void writeVal(Integer i, Encoder encoder) throws IOException {
       encoder.writeInt(i);
     }
   }
 
-  private static class LongWriter implements ValueWriter<Long> {
-    private static final LongWriter INSTANCE = new LongWriter();
-
-    private LongWriter() {
+  private static class LongWriter extends ComparableWriter<Long> {
+    private LongWriter(int id) {
+      super(id);
     }
 
     @Override
-    public void write(Long l, Encoder encoder) throws IOException {
+    protected void writeVal(Long l, Encoder encoder) throws IOException {
       encoder.writeLong(l);
     }
   }
 
-  private static class FloatWriter implements ValueWriter<Float> {
-    private static final FloatWriter INSTANCE = new FloatWriter();
-
-    private FloatWriter() {
+  private static class FloatWriter extends FloatingPointWriter<Float> {
+    private FloatWriter(int id) {
+      super(id);
     }
 
     @Override
-    public void write(Float f, Encoder encoder) throws IOException {
+    protected void writeVal(Float f, Encoder encoder) throws IOException {
       encoder.writeFloat(f);
     }
   }
 
-  private static class DoubleWriter implements ValueWriter<Double> {
-    private static final DoubleWriter INSTANCE = new DoubleWriter();
-
-    private DoubleWriter() {
+  private static class DoubleWriter extends FloatingPointWriter<Double> {
+    private DoubleWriter(int id) {
+      super(id);
     }
 
     @Override
-    public void write(Double d, Encoder encoder) throws IOException {
+    protected void writeVal(Double d, Encoder encoder) throws IOException {
       encoder.writeDouble(d);
     }
   }
 
-  private static class StringWriter implements ValueWriter<Object> {
-    private static final StringWriter INSTANCE = new StringWriter();
-
-    private StringWriter() {
+  private static class StringWriter extends MetricsAwareWriter<CharSequence> {
+    private StringWriter(int id) {
+      super(id, Comparators.charSequences());
     }
 
     @Override
-    public void write(Object s, Encoder encoder) throws IOException {
+    public void write(CharSequence s, Encoder encoder) throws IOException {
       // use getBytes because it may return the backing byte array if 
available.
       // otherwise, it copies to a new byte array, which is still cheaper than 
Avro
       // calling toString, which incurs encoding costs
       if (s instanceof Utf8) {
-        encoder.writeString((Utf8) s);
+        super.write(s, encoder);
       } else if (s instanceof String) {
-        encoder.writeString(new Utf8((String) s));
+        super.write(new Utf8((String) s), encoder);
       } else if (s == null) {
         throw new IllegalArgumentException("Cannot write null to required 
string column");
       } else {
         throw new IllegalArgumentException(
             "Cannot write unknown string type: " + s.getClass().getName() + ": 
" + s.toString());
       }
     }
-  }
 
-  private static class Utf8Writer implements ValueWriter<Utf8> {
-    private static final Utf8Writer INSTANCE = new Utf8Writer();
+    @Override
+    protected void writeVal(CharSequence s, Encoder encoder) throws 
IOException {

Review comment:
       Nevermind, `write` delegates to `super.write`.




----------------------------------------------------------------
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]

Reply via email to