tanmayrauth commented on PR #1501: URL: https://github.com/apache/iceberg-go/pull/1501#issuecomment-5094163305
Thanks for the review @laskoviymishka. On projectedFields: rather than walking the threaded schema directly, I resolve the projection once in PlanFiles and thread the projected schema into buildScanReport, then walk that (dropping the second scan.Projection() call). The reason for the extra step is that the schema we thread for planning is the full effective schema; walking it directly would report every column as projected instead of the selected subset (diverging from Java's scan.schema()), so threading the projected schema is what actually closes the nil-projection seam without misreporting the fields. On the test: added TestPlanFilesEmitsReportForRealSnapshot, a positive sibling that plans a real snapshot through an InMemoryReporter and asserts one report with the expected SnapshotID and non-zero counters, so the Snapshot() != nil gate and id-population branch are now exercised. Also took the non-blocking SanitizeExpression note — reworded the godoc so the only guarantee is "no user value leaks" (placeholder fo rmat explicitly non-contractual) and added a NOT IN case alongside the IN test. -- 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]
