zeroshade commented on code in PR #1444:
URL: https://github.com/apache/iceberg-go/pull/1444#discussion_r3566743463
##########
table/metadata.go:
##########
@@ -1525,7 +1537,7 @@ func (c *commonMetadata) TableUUID() uuid.UUID {
return c.UUID }
func (c *commonMetadata) Location() string { return c.Loc }
func (c *commonMetadata) LastUpdatedMillis() int64 { return c.LastUpdatedMS }
func (c *commonMetadata) LastColumnID() int { return c.LastColumnId }
-func (c *commonMetadata) Schemas() []*iceberg.Schema { return c.SchemaList }
+func (c *commonMetadata) Schemas() []*iceberg.Schema { return
slices.Clone(c.SchemaList) }
Review Comment:
`slices.Clone` copies the outer slice but not the `*iceberg.Schema`
pointees, so callers still receive the original `Schema` objects and can mutate
exported fields like `ID`/`IdentifierFieldIDs`, corrupting metadata state. To
actually protect it, clone each schema (or return immutable copies).
##########
table/metadata.go:
##########
@@ -1536,7 +1548,7 @@ func (c *commonMetadata) CurrentSchema() *iceberg.Schema {
}
func (c *commonMetadata) PartitionSpecs() []iceberg.PartitionSpec {
- return c.Specs
+ return slices.Clone(c.Specs)
Review Comment:
Same shallow-copy issue: `slices.Clone(c.Specs)` copies the `PartitionSpec`
structs but they still share their internal `fields` slice and each field's
`SourceIDs` backing array, so `specs[i].Field(j).SourceIDs[k] = ...` mutates
the original. A deep clone of each spec (fields + SourceIDs, rebuilding derived
maps) is needed here — you already do the SourceIDs clone in the builder
`clone()` around line 1722.
--
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]
