You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "fallintoplace (via GitHub)" <gi...@apache.org> on 2026/07/05 00:09:51 UTC
[PR] fix(manifest): clone partition and key metadata inputs [iceberg-go]
fallintoplace opened a new pull request, #1402:
URL: https://github.com/apache/iceberg-go/pull/1402
Single-purpose defensive copy fix.
Clones partition list and key metadata inputs in ManifestBuilder so caller-owned slices/maps do not alias internal manifest state. Adds regression coverage.
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] fix(manifest): clone partition and key metadata inputs [iceberg-go]
Posted by "zeroshade (via GitHub)" <gi...@apache.org>.
zeroshade merged PR #1402:
URL: https://github.com/apache/iceberg-go/pull/1402
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] fix(manifest): clone partition and key metadata inputs [iceberg-go]
Posted by "zeroshade (via GitHub)" <gi...@apache.org>.
zeroshade commented on code in PR #1402:
URL: https://github.com/apache/iceberg-go/pull/1402#discussion_r3553539658
##########
manifest.go:
##########
@@ -144,13 +144,44 @@ func (b *ManifestBuilder) DeletedRows(cnt int64) *ManifestBuilder {
}
func (b *ManifestBuilder) Partitions(p []FieldSummary) *ManifestBuilder {
- b.m.PartitionList = &p
+ if p == nil {
+ b.m.PartitionList = nil
+
Review Comment:
nit: worth a one-line comment noting the intentional behavior change — previously `Partitions(nil)` stored a non-nil pointer to a nil slice (`&p`), whereas now it stores `nil` (same for `KeyMetadata(nil)`). It's arguably more correct, but Avro serialization sees a nil-pointer vs. empty-value difference, so a note would help future readers.
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org