nbruno commented on a change in pull request #9151:
URL: https://github.com/apache/arrow/pull/9151#discussion_r565615777



##########
File path: java/vector/src/main/codegen/templates/UnionFixedSizeListWriter.java
##########
@@ -169,6 +169,29 @@ public StructWriter struct(String name) {
     return structWriter;
   }
 
+  @Override
+  public MapWriter map() {
+    return writer;
+  }
+
+  @Override
+  public MapWriter map(String name) {
+    MapWriter mapWriter = writer.map(name);
+    return mapWriter;
+  }
+
+  @Override
+  public MapWriter map(boolean keysSorted) {
+    writer.map(keysSorted);

Review comment:
       I had originally thought this too, but making this change will cause the 
new unit tests I added for MapWriter to break (it won't correctly write a list 
of maps).
   
   I believe it is because of how List and Map vectors are implemented. Map 
itself does not store any data, it is a subclass of List, delegating storage to 
the parent. When we have a list and want to write a map value to get 
`List<Map<?, ?>>`, we don't need a new writer with a new vector to write into, 
we actually want to reuse the vector of the current list. If we construct a new 
writer and return that, we would end up with `List<List<Map<?, ?>>>` when we 
really want `List<Map<?,?>>`.
   
   My explanation probably isn't 100% clear or correct, but it's how I was able 
to figure out how to implement MapWriter for this PR.




----------------------------------------------------------------
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:
us...@infra.apache.org


Reply via email to