abnobdoss commented on code in PR #3658:
URL: https://github.com/apache/iceberg-python/pull/3658#discussion_r3607317474
##########
pyiceberg/table/__init__.py:
##########
@@ -2151,11 +2151,7 @@ def _open_manifest(
Returns:
A list of ManifestEntry that matches the provided filters.
"""
- return [
Review Comment:
This is a great find! Would it be better to incorporate this fused behavior
directly into `fetch_manifest_entry` by allowing it to accept an `entry_filter:
Callable[[ManifestEntry], bool] | None` arg? That way the optimization is
generic and available to all callers that currently materialize the full list
just to discard entries immediately.
The call site in `_open_manifest` would become:
```python
return manifest.fetch_manifest_entry(
io,
discard_deleted=True,
entry_filter=lambda e: partition_filter(e.data_file) and
metrics_evaluator(e.data_file),
)
```
This avoids duplicating the Avro reader setup and `_inherit_from_manifest`
call ordering between `fetch_manifest_entry` and `prune_manifest_entry`, and
there are 6 other call sites that filter after materializing that could benefit
from the same interface:
1. `table/__init__.py` - `_open_manifest` (partition + metrics)
2. `table/inspect.py` - `_get_files_from_manifest` (content type)
3. `table/update/snapshot.py` - `_OverwriteFiles._build_...` (strict +
inclusive metrics)
4. `table/update/validate.py` - `_deleted_data_files` (snapshot_id +
data_filter + partition_set)
5. `table/update/validate.py` - `_added_data_files` (same)
6. `table/update/validate.py` - `_added_delete_files` (same)
Using `Callable[[ManifestEntry], bool]` rather than `Callable[[DataFile],
bool]` covers all of these since the validate callers filter on entry-level
fields like `snapshot_id` and `status`, not just `DataFile`.
--
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]
