zeroshade opened a new issue, #2091:
URL: https://github.com/apache/iceberg-go/issues/2091
**Problem**
`MetadataBuilder.buildCommonMetadata` (table/metadata.go:1520-1525) appends
`previousFileEntry` to `b.metadataLog` whenever `HasChanges()` is true. That
mutates the builder, and nothing ever clears it. So every `Build()`
(metadata.go:1681) on a builder that already has changes adds one more
identical log entry.
A transaction's builder is created with the current metadata location
(table/table.go:211), and several paths call `Build()` on it mid-transaction:
- `Transaction.apply`, when validating a new requirement
(transaction.go:211). The clone it builds becomes `t.meta` at line 247.
- `classifyFilesForFilteredDeletions` (transaction.go:2699).
- `rewriteSingleFile`, once per rewritten file (transaction.go:2952).
- `makePositionDeleteRecordsForFilter` (transaction.go:3191) and
`positionDeleteRecordsToDataFiles` (arrow_utils.go:2202).
- `Transaction.Scan` (transaction.go:3276) and `Transaction.StagedTable`
(transaction.go:3316).
Persisted history is not affected. `Commit` sends `meta.updates`
(transaction.go:3356), and catalogs rebuild metadata from the base via
`UpdateTableMetadata(base, updates, loc)` (table/metadata.go:3647,
catalog/internal/utils.go:296, catalog/hadoop/hadoop.go:681). What breaks is
any metadata built from the in-flight transaction (`StagedTable()`,
`Transaction.Scan`, and anything else that reads `metadataLog`
mid-transaction), plus some wasted allocation.
`TestTransactionStagedTableBuildsSinglePreviousMetadataLogEntryAfterNoRequirementApplies`
(transaction_internal_test.go:1306) pins a single entry, but only for the
SetProperties path.
**Reproduction**
On main (dd935d83): a table with a metadata location and two data files (ids
1..5 and 6..8).
```go
tx := tbl.NewTransaction()
_ = tx.SetProperties(iceberg.Properties{table.WriteDeleteModeKey:
table.WriteModeCopyOnWrite})
staged, _ := tx.StagedTable()
// len(PreviousFiles()) == 1 -- correct
tx2 := tbl.NewTransaction()
_ = tx2.SetProperties(iceberg.Properties{table.WriteDeleteModeKey:
table.WriteModeCopyOnWrite})
_ = tx2.Delete(ctx, iceberg.NewOr(
iceberg.EqualTo(iceberg.Reference("id"), int64(2)),
iceberg.EqualTo(iceberg.Reference("id"), int64(7)),
), nil) // CoW rewrite of both files
staged2, _ := tx2.StagedTable()
// len(PreviousFiles()) == 5, all the same metadata file and timestamp
```
Expected: 1 entry. #2046 adds another `Build()` on this path, which brings
it to 6.
**Proposed fix**
Make `Build()` free of side effects on `metadataLog`. `buildCommonMetadata`
should compute the appended and trimmed log on a copy instead of mutating
`b.metadataLog`. Add a test that calls `Build()` repeatedly on a builder with
changes and asserts a single previous-file entry.
Separately, the CoW path could build once per operation instead of once per
file (raised in the #2046 review), but that only hides the root cause.
**Related**
- #2046: review thread on `rewriteScanTasks` calling `meta.Build()`
--
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]
