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