You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hudi.apache.org by "linliu-code (via GitHub)" <gi...@apache.org> on 2026/08/04 04:59:38 UTC
[PR] fix(core): push the row filter into the base file read [hudi-rs]
linliu-code opened a new pull request, #662:
URL: https://github.com/apache/hudi-rs/pull/662
**Stacked on #639–#661** — their commits appear here until they merge. **Review only the last commit.**
## The bug
`engine.rs` decides whether a predicate is safe to push, builds the filter, and then throws it away:
```rust
let _row_filter = if self.reader_context.can_push_row_filter() { .. };
```
Every row of every base file is read and fed to the merge; the predicate is left for whoever sits above the reader.
This was not a pre-existing gap in this crate — it is a capability lost in the port. Upstream threads the same filter into the base read. Here base reads go through this crate's `BaseFileReader`, which had nowhere to put one, so it went nowhere. #661's harness cases are what surfaced it: ten of them failed with `actual > expected` because no filtering happened at all.
## The fix
Give it somewhere to go.
- `BaseFileReadOptions` gains an optional `RowFilterBuilder`
- `ParquetBaseFileReader::apply_options` resolves it once the footer is open and installs it on the stream builder
- The three base reads in `make_base_file_source` pass it through
Other base-file formats ignore it, which is why it is a documented parquet-only option rather than a trait method. `RowFilterBuilder` was already a public type in `storage`, and its doc already said "options holding it stay `Clone`" — it was designed to live in a read-options struct.
**A builder rather than a filter**, because the predicate has to be resolved against the *file's own* schema, which the caller does not have until the footer is read. A builder that declines — typically because the file has none of the columns the predicate names — reads every row rather than none. That direction matters: the opposite would silently drop data on a file the predicate cannot speak about.
## The safety gate, which now actually does something
Pushing a predicate below the merge is only sound when the merge cannot change its outcome:
- **copy-on-write** — always safe, there is no merge
- **merge-on-read** — safe only when every column the predicate names is a primary key, since primary keys are immutable across upserts (mirrors Java's `filterIsSafeForPrimaryKey`)
- **otherwise** — do not push; the post-merge filter still runs
That gate was already written. It was untested in practice, because nothing was ever pushed, so `harness_filter_unsafe_not_pushed_mor` passed vacuously.
I checked it is no longer vacuous by forcing the gate open:
```
- let row_filter = if self.reader_context.can_push_row_filter() {
+ let row_filter = if true {
test harness_filter_unsafe_not_pushed_mor ... FAILED (gate forced open)
test harness_filter_unsafe_not_pushed_mor ... ok (gate restored)
```
## Tests
- **10 harness cases un-ignored** and now passing — boolean, date, decimal, float32, timestamp, logical-type and data-column predicates on CoW; primary-key `=`, `<` and `IN` on MOR
- **1 negative case** (`harness_filter_unsafe_not_pushed_mor`) upgraded from vacuous to meaningful, verified by the mutation above
- **2 new unit tests** on `ParquetBaseFileReader`, at the layer the option was added, since `with_row_filter` is public API independent of the merge-on-read engine: an always-false predicate must return 0 rows (a dropped filter would return all of them), and a builder that declines must return every row
Full workspace green: 1164 lib + 79 table-read + 39 datafusion + 21 + 12; 19 ignored, down from 29. `cargo fmt --check` clean; no new clippy findings in the changed files.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(core): push the row filter into the base file read [hudi-rs]
Posted by "linliu-code (via GitHub)" <gi...@apache.org>.
linliu-code closed pull request #662: fix(core): push the row filter into the base file read
URL: https://github.com/apache/hudi-rs/pull/662
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(core): push the row filter into the base file read [hudi-rs]
Posted by "linliu-code (via GitHub)" <gi...@apache.org>.
linliu-code commented on PR #662:
URL: https://github.com/apache/hudi-rs/pull/662#issuecomment-5232823449
Closing: this line is superseded.
The merge-on-read work now lives on a single line (#663-#673, plus #675), which
starts from the port as one squashed commit rather than this decomposition of
it. That squashed commit contains everything this chain built, so nothing here
is lost -- it is the same tree, reached by a different route.
#660 carries the full squash for review, and is now up to date with the tip.
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org