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



##########
File path: data/src/main/java/org/apache/iceberg/data/orc/GenericOrcWriter.java
##########
@@ -115,53 +116,72 @@ 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);

Review comment:
       Copied the comment from here: 
https://github.com/apache/iceberg/pull/3250#discussion_r725771333
   
   > 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