zeroshade commented on code in PR #2002:
URL: https://github.com/apache/iceberg-go/pull/2002#discussion_r4064617836
##########
table/arrow_utils.go:
##########
@@ -436,7 +436,7 @@ func (c convertToIceberg) Primitive(dt arrow.DataType)
(result iceberg.NestedFie
switch dt.ExtensionName() {
case "arrow.uuid":
result.Type = iceberg.PrimitiveTypes.UUID
- case "parquet.variant":
+ case extensions.VariantExtensionName,
extensions.LegacyVariantExtensionName:
Review Comment:
Following up on my own question so it does not sit here as a gate: **release
notes alone are sufficient, and the shim question should not hold this merge.**
Reasoning, for the record. iceberg-go is pre-1.0 and variant support is
recent, so the exposure is narrow: it needs a mixed 18.7/18.8 fleet, *and*
variant columns written by the new version, *and* those columns read back by
the old one, all inside the upgrade window. Emitting the legacy extension name
for a release would buy a little safety in that window at the cost of writing a
deprecated name into files that outlive the shim, and I would rather not do
that for a window this narrow.
@nssalian nothing further needed from you on this thread beyond the
release-note item at release time. Sorry for the extra round trip.
--
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]
