lidavidm commented on code in PR #43077:
URL: https://github.com/apache/arrow/pull/43077#discussion_r1694831816


##########
java/vector/src/main/codegen/templates/AbstractPromotableFieldWriter.java:
##########
@@ -287,6 +298,11 @@ public ListWriter list(String name) {
     return getWriter(MinorType.STRUCT).list(name);
   }
 
+  @Override
+  public ListWriter listView(String name) {
+    return getWriter(MinorType.STRUCT).listView(name);

Review Comment:
   Why is this struct? (Also isn't it a duplicate of above?)



##########
java/vector/src/main/codegen/templates/UnionWriter.java:
##########
@@ -58,6 +59,42 @@ public UnionWriter(UnionVector vector, 
NullableStructWriterFactory nullableStruc
     this.nullableStructWriterFactory = nullableStructWriterFactory;
   }
 
+  /**
+   * Convert the UnionWriter to a UnionViewWriter.
+   *
+   * @return the converted UnionViewWriter
+   */
+  public UnionViewWriter toViewWriter() {
+    UnionViewWriter unionViewWriter = new UnionViewWriter(data, 
nullableStructWriterFactory);
+    unionViewWriter.setStructWriter(structWriter);
+    unionViewWriter.setListWriter(listWriter);
+    unionViewWriter.setListViewWriter(listViewWriter);
+    unionViewWriter.setMapWriter(mapWriter);
+    unionViewWriter.setWriters(writers);
+    unionViewWriter.setPosition(idx());
+    return unionViewWriter;
+  }
+
+  protected void setStructWriter(StructWriter structWriter) {
+    this.structWriter = structWriter;
+  }
+
+  protected void setListWriter(UnionListWriter listWriter) {
+    this.listWriter = listWriter;
+  }
+
+  protected void setListViewWriter(UnionListViewWriter listViewWriter) {
+    this.listViewWriter = listViewWriter;
+  }
+
+  protected void setMapWriter(UnionMapWriter mapWriter) {
+    this.mapWriter = mapWriter;
+  }
+
+  protected void setWriters(List<BaseWriter> writers) {
+    this.writers = writers;
+  }
+

Review Comment:
   Same here.



##########
java/vector/src/test/java/org/apache/arrow/vector/complex/writer/TestComplexWriter.java:
##########


Review Comment:
   Basically, every test here is duplicated for ListView, and the bodies of the 
List tests got refactored into methods so they could be called on ListView as 
well?



##########
java/vector/src/main/codegen/templates/PromotableWriter.java:
##########
@@ -526,4 +538,36 @@ public int getValueCapacity() {
   public void close() throws Exception {
     getWriter().close();
   }
+
+  protected void setState(State state) {
+    this.state = state;
+  }
+
+  protected void setType(MinorType type) {
+    this.type = type;
+  }
+
+  protected void setUnionVector(UnionVector unionVector) {
+    this.unionVector = unionVector;
+  }
+
+  protected void setWriter(FieldWriter writer) {
+    this.writer = writer;
+  }
+
+  /**
+   * Convert the writer to a PromotableViewWriter.
+   *
+   * @return The writer as a PromotableViewWriter.
+   */
+  public PromotableViewWriter toViewWriter() {
+    PromotableViewWriter promotableViewWriter = new 
PromotableViewWriter(unionVector, parentContainer, nullableStructWriterFactory);
+    promotableViewWriter.setPosition(position);
+    promotableViewWriter.setWriter(writer);
+    promotableViewWriter.setState(state);
+    promotableViewWriter.setUnionVector(unionVector);
+    promotableViewWriter.setType(MinorType.LISTVIEW);
+    promotableViewWriter.setPosition(idx());
+    return promotableViewWriter;
+  }

Review Comment:
   Again, are all these setters actually used elsewhere? If not, inline them.



##########
java/vector/src/main/java/org/apache/arrow/vector/ipc/JsonFileWriter.java:
##########
@@ -277,6 +281,14 @@ private void writeFromVectorIntoJson(Field field, 
FieldVector vector) throws IOE
               vectorBufferTmp.setLong(0, 0);
               writeValueToGenerator(bufferType, vectorBufferTmp, null, vector, 
i);
             }
+          } else if (bufferType.equals(SIZE)
+              && vector.getValueCount() == 0
+              && vector.getMinorType() == MinorType.LISTVIEW) {
+            // Empty vectors may not have allocated a sizes buffer
+            try (ArrowBuf vectorBufferTmp = vector.getAllocator().buffer(4)) {
+              vectorBufferTmp.setInt(0, 0);
+              writeValueToGenerator(bufferType, vectorBufferTmp, null, vector, 
i);
+            }

Review Comment:
   I think it should not apply for list view. We shouldn't blindly apply the 
same hacks since that may cover up bugs.



##########
java/vector/src/main/codegen/templates/AbstractPromotableFieldWriter.java:
##########
@@ -314,11 +331,6 @@ public MapWriter map(String name, boolean keysSorted) {
     return getWriter(MinorType.STRUCT).${lowerName}(name);
   }
 
-  @Override
-  public ${capName}Writer ${lowerName}() {
-    return getWriter(MinorType.LIST).${lowerName}();
-  }
-

Review Comment:
   Removing this doesn't change any public APIs?



-- 
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: github-unsubscr...@arrow.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to