rdblue commented on code in PR #11324:
URL: https://github.com/apache/iceberg/pull/11324#discussion_r1833006470


##########
api/src/main/java/org/apache/iceberg/types/Types.java:
##########
@@ -412,6 +413,29 @@ public String toString() {
     }
   }
 
+  public static class VariantType extends PrimitiveType {

Review Comment:
   I don't think that variant should be either a primitive type or a nested 
type. There are at least a few places where making variant a primitive creates 
problems. For example, `Schema` checks that a field is a primitive to allow it 
as an identifier field and the `Identity` transform will bind to a type if it 
is a primitive. There are also places that check whether a type is nested and 
make decisions based on whether there are sub-fields.
   
   Neither one of those seems appropriate to me so I'd suggest making this 
extend just `Type`. Then we can adjust the logic in those places without 
assuming either one and potentially introducing bugs when the assumption is bad.



-- 
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: issues-unsubscr...@iceberg.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org

Reply via email to