tanmayrauth commented on PR #1501:
URL: https://github.com/apache/iceberg-go/pull/1501#issuecomment-5049325101
Thanks for the thorough review, @laskoviymishka, all issues have been
addressed. buildScanReport now sets Filter from scan.rowFilter (serialized via
the expression MarshalJSON, falling back to always-true on marshal error;
literal sanitization left as a follow-up). The DV accounting now keys by
ReferencedDataFile() and sizes by ContentSizeInBytes() so multiple DVs sharing
one Puffin file no longer collapse, and the local map is renamed to dvByRef to
stop shadowing acc.dvs; added a test with two DVs in one Puffin path.
Report emission is now skipped when scan.Snapshot() == nil so we don't push
zero-snapshot records. The planning schema is resolved once in PlanFiles and
threaded into both planFilesLocal and buildScanReport (dropping the redundant
Schemas() scan and the silent fallback), and TotalPlanningDuration is now
snapped right after planning returns, before report assembly.
Added a real-snapshot test that exercises partition pruning and asserts
scanned + skipped == total with delete manifests landing in the delete
counters. For the minor items: the Combine(NopReporter{}) case is now handled
via a new metrics.IsNop (an unexported isNop() the composite answers); the
throwaway-accumulator wrapper, the Java per-task resultDeleteFiles divergence,
and planFilesLocal's planIO reset + nil-vs-empty return are all documented. go
build, go vet, gofmt, and the table/metrics suites (incl. -race) pass.
--
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]
