zeroshade commented on code in PR #1382:
URL: https://github.com/apache/iceberg-go/pull/1382#discussion_r3523576693
##########
table/table.go:
##########
@@ -103,7 +103,9 @@ func (t Table) Equals(other Table) bool {
t.metadata.Equals(other.metadata)
}
-func (t Table) Identifier() Identifier { return
t.identifier }
+func (t Table) Identifier() Identifier {
+ return slices.Clone(t.identifier)
Review Comment:
This getter clone is correct, but the same identifier slice still crosses
the external CatalogIO boundary. In the current head, Refresh and commit/retry
paths still call LoadTable/CommitTable with t.identifier directly
(table/table.go lines 152, 480, and 536), which lets a catalog implementation
retain or mutate the table's backing array. Please pass
slices.Clone(t.identifier) at each catalog call and add a mutating fake-catalog
regression test proving tbl.Identifier() stays stable across refresh/commit.
--
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]
