mzzz-zzm commented on code in PR #982:
URL: https://github.com/apache/iceberg-go/pull/982#discussion_r3202207361
##########
table/table.go:
##########
@@ -428,6 +421,20 @@ func (t Table) doCommit(ctx context.Context, updates
[]Update, reqs []Requiremen
}
current = fresh.metadata
reqs = rewriteRefSnapshotRequirements(reqs, co.branch,
current)
+
+ // Rebuild snapshot manifest lists to inherit all files
committed
+ // by concurrent writers since the snapshot was
originally built.
+ // Without this, the new snapshot's manifest list would
only
+ // contain its own files and callers scanning the
current snapshot
+ // would miss every concurrent writer's data.
+ if wfs, ok := fs.(icebergio.WriteFileIO); ok {
Review Comment:
The `WriteFileIO` type assertion is now hoisted to the top of `doCommit`,
before the retry loop. A non-`WriteFileIO` filesystem returns an error
immediately — there is no silent fallback:
```go
wfs, ok := fs.(icebergio.WriteFileIO)
if !ok {
return nil, fmt.Errorf("commit: file system does not implement
WriteFileIO: "+
"manifest list rebuild requires write access")
}
```
Both inline `if wfs, ok := fs.(icebergio.WriteFileIO); ok { ... }` guards in
the original code are removed. The rest of `doCommit` uses `wfs` directly.
Test added: `TestDoCommit_NonWriteFileIOReturnsError` — passes a read-only
FS that does not implement `WriteFileIO`; asserts an immediate error before any
commit attempt.
--
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]
