jhrotko commented on code in PR #892:
URL: https://github.com/apache/arrow-java/pull/892#discussion_r2514056901


##########
vector/src/main/codegen/templates/PromotableWriter.java:
##########
@@ -325,6 +325,9 @@ protected boolean requiresArrowType(MinorType type) {
 
   @Override
   protected FieldWriter getWriter(MinorType type, ArrowType arrowType) {
+    if(type == MinorType.EXTENSIONTYPE) {

Review Comment:
   This was required to provide the interface shown in tests for all vector 
types. This was specially more tricky for Structs and Union writers where they 
own more than one writer because the fields can be different types (lists, 
ints, nested, etc). Given the way, writers interface works, where first it is 
specified which "type" is going to be written to get the type:
   
   ```java
   writer.extension(UuidType.INSTANCE);
   writer.writeExtension(new UUID(...));
   ```
   The best way to understand the flow is to run the test 
`TestPromotableWriter.testPromoteToUnion`
   
   ```mvn test -Dtest=TestPromotableWriter#testPromoteToUnion \                 
                                                                                
                                                                                
                               
     -Dcheckstyle.skip=true -Dmaven.surefire.debug="-Xdebug 
-Xrunjdwp:transport=dt_socket,server=y,suspend=y,address=5005" \
     -Derrorprone.skip=true
   ```
   
   we will hit the if condition: 
   The `lastExtensionType` is set in `NullableStructWriter` in: 
   ```java
     @Override
     public ExtensionWriter extension(String name, ArrowType arrowType) {
       String finalName = handleCase(name);
       FieldWriter writer = fields.get(finalName);
       if(writer == null){
         ...
       } else {
         if (writer instanceof PromotableWriter) {
           // ensure writers are initialized
           ((PromotableWriter)writer).getWriter(MinorType.EXTENSIONTYPE, 
arrowType);  <---
         }
       }
       return (ExtensionWriter) writer;
     }
   ```
   which will "save" the current "active" Extension type in the line where you 
pointed out your comment.
   Then when writing the extension, it is necessary to know why extension type 
it's required because UnionWriter has several writers (list, map, etc) and the 
extension types can be many (besides UUID). This is the method call at this 
point of execution:
   
   ```java
     @Override
     public void writeExtension(Object value) {
       FieldWriter writer = getWriter(MinorType.EXTENSIONTYPE, 
lastExtensionType);
       if(writer instanceof UnionWriter) { <---
         ((UnionWriter) writer).writeExtension(value, lastExtensionType);
       } else {
         writer.writeExtension(value);
       }
     }
   ```



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

Reply via email to