yangshangqing95 commented on PR #17522: URL: https://github.com/apache/iceberg/pull/17522#issuecomment-5318925418
> LGTM, thanks @yangshangqing95 for the fix, and @dramaticlly @uros-b for the reviews. > > Agree the pruning is safe to keep when residuals are ignored. `SnapshotEvaluator` only prunes on `reference_snapshot_id` and returns `ROWS_MIGHT_MATCH` for every other reference, and each `ManifestListReadTask` stamps its rows with its own snapshot id, so a pruned snapshot can't contribute a matching row. This also lines up with the `ignoreResiduals()` contract of filtering files but not rows in those files. > > Optional follow-up, not a blocker: the new test covers the pruning direction only. It would be nice to also assert that a filter on a non-snapshot column such as `content`, combined with `ignoreResiduals()`, still returns all snapshots, so we guard against over-pruning too. Hi all, thanks for the reviewing! that's a good idea, I'll submit a separate PR to add the test when this one get merged. -- 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]
