mbutrovich commented on code in PR #2188:
URL: https://github.com/apache/iceberg-rust/pull/2188#discussion_r3326653141
##########
crates/catalog/hms/src/schema.rs:
##########
@@ -139,6 +139,10 @@ impl SchemaVisitor for HiveSchemaBuilder {
Ok(hive_type)
}
+
+ fn variant(&mut self, _v: &iceberg::spec::VariantType) -> Result<Self::T> {
Review Comment:
I see this was already discussed back in March and the call was to use
`"variant"` (@CTTY's suggestion, with the Glue rendering as `unknown:
variant`), citing apache/iceberg#15220 as open. iceberg-java has since landed
on `"unknown"` for VARIANT in `HiveSchemaUtil.java:170` (PR
apache/iceberg#15964, April 28). Worth revisiting? Or is the decision still
that the rendered shape here is fine? Not pushing either way, just flagging
that Java's direction shifted after the original discussion.
##########
crates/catalog/hms/src/schema.rs:
##########
@@ -139,6 +139,10 @@ impl SchemaVisitor for HiveSchemaBuilder {
Ok(hive_type)
}
+
+ fn variant(&mut self, _v: &iceberg::spec::VariantType) -> Result<Self::T> {
Review Comment:
I see this was already discussed back in March and the call was to use
`"variant"` (@CTTY's suggestion, with the Glue rendering as `unknown:
variant`), citing apache/iceberg#15220 as open. iceberg-java has since landed
on `"unknown"` for VARIANT in `HiveSchemaUtil.java:170` (PR
apache/iceberg#15964, April 28). Worth revisiting? Or is the decision still
that the rendered shape here is fine? Not pushing either way, just flagging
that Java's direction shifted after the original discussion. CC @nssalian
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
