openinx commented on a change in pull request #3250:
URL: https://github.com/apache/iceberg/pull/3250#discussion_r725771333



##########
File path: orc/src/main/java/org/apache/iceberg/data/orc/GenericOrcWriter.java
##########
@@ -115,53 +116,77 @@ private WriteBuilder() {
   }
 
   @Override
-  @SuppressWarnings("unchecked")
   public void write(Record value, VectorizedRowBatch output) {
-    Preconditions.checkArgument(writer instanceof RecordWriter, "writer must 
be a RecordWriter.");
+    Preconditions.checkArgument(value != null, "value must not be null");
 
     int row = output.size;
     output.size += 1;
-    List<OrcValueWriter<?>> writers = ((RecordWriter) writer).writers();
-    for (int c = 0; c < writers.size(); ++c) {
-      OrcValueWriter child = writers.get(c);
-      child.write(row, value.get(c, child.getJavaClass()), output.cols[c]);
-    }
+
+    writer.rootNonNullWrite(row, value, output);
+  }
+
+  @Override
+  public List<OrcValueWriter<?>> writers() {
+    return this.writer.writers();
   }
 
   @Override
   public Stream<FieldMetrics<?>> metrics() {
     return writer.metrics();
   }
 
-  private static class RecordWriter implements OrcValueWriter<Record> {
+  public abstract static class StructWriter<S> implements OrcValueWriter<S> {

Review comment:
       This generic `StructWriter` doesn't belongs to the concrete 
`OrcRowWriter<Record>`,  moving this to the writer definition class 
`GenericOrcWriters` looks more reasonable to me. 




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

To unsubscribe, e-mail: [email protected]

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