dontirun commented on code in PR #674:
URL: https://github.com/apache/iceberg-go/pull/674#discussion_r2761009491
##########
table/transaction.go:
##########
@@ -628,65 +638,98 @@ func (t *Transaction)
classifyFilesForFilteredOverwrite(ctx context.Context, fs
return nil, nil, fmt.Errorf("failed to get manifests: %w", err)
}
- for _, manifest := range manifests {
- if manifestEval != nil {
- match, err := manifestEval(manifest)
- if err != nil {
- return nil, nil, fmt.Errorf("failed to evaluate
manifest: %w", err)
- }
- if !match {
- continue
- }
- }
+ type classifiedFiles struct {
+ toDelete []iceberg.DataFile
+ toRewrite []iceberg.DataFile
+ }
- entries, err := manifest.FetchEntries(fs, false)
- if err != nil {
- return nil, nil, fmt.Errorf("failed to fetch manifest
entries: %w", err)
- }
+ var (
+ mu sync.Mutex
+ allFilesToDel []iceberg.DataFile
+ allFilesToRewr []iceberg.DataFile
+ )
- for _, entry := range entries {
- if entry.Status() == iceberg.EntryStatusDELETED {
- continue
- }
+ g, ctx := errgroup.WithContext(ctx)
+ g.SetLimit(min(concurrency, len(manifests)))
- df := entry.DataFile()
- if df.ContentType() != iceberg.EntryContentData {
- continue
+ for _, manifest := range manifests {
+ manifest := manifest // capture loop variable
Review Comment:
I saw this was used elsewhere in the code base. I don't think it's necessary
since it was [fixed in Go 1.22](https://go.dev/blog/loopvar-preview) but
keeping it for consistency
--
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]
