linliu-code opened a new pull request, #708: URL: https://github.com/apache/hudi-rs/pull/708
## Description Three related changes to base-file predicate pushdown in `reader_v2`, smallest first. Each commit stands alone. ### 1. A base-only slice may push, whatever the table type A file slice with no log files has no merge: no log record can supersede a base row and no delete block can remove one, so nothing can flip a pushed predicate's outcome. The gate on `ReaderContext` answered only the table-level question — CoW, or MOR whose predicate is confined to immutable primary keys — so on a MOR table with a data-column predicate, **every base-only slice lost pushdown**: the base read returned whole columns for a post-merge filter to discard, having read every byte of them. The declarative harness had been standing in for the missing rule by declaring a base-only slice `COPY_ON_WRITE` regardless of the fixture's real table type. It now passes `MERGE_ON_READ`, which is what those fixtures are, so the eight base-only filter cases exercise the split-level gate rather than an `is_cow()` branch that was never true in production for them. Those cases assert exact rows and poison if the filter is not applied — reverting the gate fails all eight. ### 2. The gate is about whether the read merges, not about the table type Keeping the table-type disjunct hides what the rule is: a CoW slice is safe because it carries no log files, not because of what `hoodie.table.type` says. Java agrees — `SparkFileFormatInternalRowReaderContext.getSchemaAndFiltersForRead` branches on `getHasLogFiles()` and never on the table type. So the gate becomes ``` base_read_pushdown_is_safe() = no log files on the split || mor_pk_safe ``` with the same outcomes, and `ReaderContext::can_push_row_filter` / `is_cow` go away. The doc comment records why the split's own file list is used rather than `ReaderContext::has_log_files` (different fact, different source — a safety gate should not rest on a caller-supplied boolean), and why Java's bootstrap tier has no counterpart here. The parquet log-block gate now reads `mor_pk_safe` directly. A log block only exists on a slice that has log files, so the base gate's first disjunct is false there by construction and the condition already reduced to exactly this. No behaviour change: the disjunct it drops could only be true for a CoW table with log blocks. ### 3. Row groups can be pruned, and reads can be measured A `RowFilter` never skips IO — it decides per row, after the predicate columns have been fetched and decoded. Nothing on this path could skip a row group. - `RowGroupSelector`: a caller-supplied closure over the file's parsed footer returning the row groups to keep, `None` meaning "no opinion". It runs against metadata the reader has already fetched, so consulting it costs nothing, and before the row filter is installed, so the filter only ever sees groups that survived. It reaches a read through `BaseFileReadOptions::with_row_group_selector` or `HoodieFileGroupReaderBuilder::with_row_group_selector`. - Both mechanisms share **one** safety gate, bound to a local so the sharing is structural rather than two call sites that happen to agree. Pruning is the one that must not drift: a row filter still sees every row, while a pruned row group is gone before the merge could have updated one of its rows into a match. - `ReadVolume` on `Storage` (one per file-group read) counts bytes fetched, round trips, row groups scanned vs. row groups the file has, rows the file has, and rows the stream yielded — plus `row_group_selector_calls` and `row_group_selector_suppressed`. Bytes and calls are counted in a `CountingReader` at the `AsyncFileReader` boundary, which makes them exact and cache-independent: a warm re-read reports the same bytes as a cold one, which wall-clock does not. - A selector the gate refuses is counted, because a suppressed selector and an absent one both read as zero calls and only the counter separates them. `BaseFileReader::read_schema` is new (with a default implementation, so no other format changes) for the base read's schema-evolution probe, which was opening a whole stream to take its schema and dropping it. The parquet override answers from the footer instead. This is correctness for the counters, not tidiness: the probe went through the same read path it was measuring, so a three-row-group file reported six row groups scanned before a single data byte moved. ### API changes | Change | Note | |---|---| | removed `ReaderContext::can_push_row_filter`, `ReaderContext::is_cow` | superseded by `base_read_pushdown_is_safe()`; the table type no longer decides anything about a read | | added `RowGroupSelector`, `ReadVolume`, `Storage::read_volume()` | new, in `storage` | | added `BaseFileReadOptions::with_row_group_selector`, `HoodieFileGroupReaderBuilder::with_row_group_selector` | mirror the existing row-filter setters | | added `BaseFileReader::read_schema` | defaulted; parquet overrides it | No behaviour change for `Table` or DataFusion reads: `resolve_reader_context` installs neither a filter nor a selector, so those paths push nothing, exactly as before. ## How are the changes test-covered - [ ] N/A - [x] Automated tests (unit and/or integration tests) - [ ] Manual tests Each change was falsified by reverting it and watching the corresponding tests go red, not only by watching them pass: | Change | Discriminating test | With the change reverted | |---|---|---| | split-level gate | 8 base-only harness cases, exact-row asserts on MOR fixtures | 8 failed, 3 passed | | row-group pruning | 3-row base file written one row per row group, selector keeping group 0 | 2 failed — 3 rows returned instead of 1 | | suppression counter | selector installed on a merging slice with a non-PK-safe predicate | n/a — asserts `calls == 0 && suppressed == 1` | | read volume | a `RowFilter` rejecting every row | `rows_out == 0` while `bytes_read > 0`, which is the case the counters exist for | `cargo test --no-fail-fast --all-targets --all-features --workspace` and `cargo clippy --all-targets --all-features --workspace --no-deps -- -D warnings` are clean. -- 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]
