laskoviymishka commented on code in PR #1937:
URL: https://github.com/apache/iceberg-go/pull/1937#discussion_r4119697924
##########
table/arrow_scanner.go:
##########
@@ -746,7 +782,13 @@ func releasePosDeletes(deletes map[string]*arrow.Chunked) {
}
}
-func readDeletes(ctx context.Context, fs iceio.IO, dataFile iceberg.DataFile)
(_ map[string]*arrow.Chunked, err error) {
+func readDeletes(ctx context.Context, fs iceio.IO, dataFile iceberg.DataFile)
(map[string]*arrow.Chunked, error) {
+ return readDeletesForPaths(ctx, fs, dataFile, nil)
Review Comment:
This is the wrapper the lazy loader hits: `lazyPositionDeleteLoader.load()`
(~line 257, behind every `Scan().ToArrowTable()`) still calls `readDeletes`,
which forwards `nil` targets straight through here. So the new pushdown only
ever runs from `readAllDeleteFiles`, the write-side
`makePositionDeleteRecordsForFilter` and the benchmark. For an ordinary scan
the whole delete file still gets read and grouped per path, same as before this
PR.
`newLazyPositionDeleteLoader` already receives the full task slice, so it
could precompute the same `targetsByDelete` set and thread it into the
`once.Do` read instead of `nil`. Might be worth either wiring that through, or
re-scoping the title/description to write-path-only, so this doesn't read as a
general scan-read win the primary read path doesn't see. wdyt?
--
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]
