zeroshade commented on code in PR #1238:
URL: https://github.com/apache/iceberg-go/pull/1238#discussion_r3462923923
##########
table/arrow_utils.go:
##########
@@ -1191,8 +1191,22 @@ func (a *arrowProjectionVisitor) Primitive(_
iceberg.PrimitiveType, arr arrow.Ar
return arr
}
+// Variant reassembles a shredded variant back into the unshredded layout so
the
+// projected schema matches; without it the scanner fails to assemble the
table.
func (a *arrowProjectionVisitor) Variant(_ iceberg.VariantType, arr
arrow.Array) arrow.Array {
Review Comment:
Correctness here quietly depends on `VariantType` NOT implementing
`arrow.NestedType`: in `Struct` (line ~1078) the `defer arr.Release()` only
fires for nested types, so the owned +1 from `UnshredVariant` is balanced
instead by `castIfNeeded`s `Retain()`/deferred `Release()`. If arrow-go ever
made `VariantType` a `NestedType`, line 1078 would fire and double-release. The
two checked-allocator leak tests defend this, but a one-line comment noting the
invariant (relies on castIfNeeded retain/release; VariantType is intentionally
not NestedType) would make it explicit for the next reader.
--
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]
