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 03:36:21 UTC
[PR] test(core): port the merge-on-read reader's end-to-end harness [hudi-rs]
linliu-code opened a new pull request, #661:
URL: https://github.com/apache/hudi-rs/pull/661
**Stacked on #639–#659** — their commits appear here until they merge. **Review only the last commit.**
## What
The reader's unit tests came across with it. Its **integration tests did not** — I only compared the reader module when I checked port completeness. This brings the remaining three files.
| File | Lines | What |
|---|---|---|
| `harness.rs` | 1015 | Declarative case harness — builds a table from a `FgReaderCase`, runs the reader, asserts the **full** output dataset (never just counts) |
| `harness_tests.rs` | 2255 | 57 cases through that harness, plus component tests for schema pruning and the builder path |
| `memory_limit_tests.rs` | 291 | Peak-cap parsing, loud rejection of an oversized insertion, footprint accounting |
Upstream these live in `tests/` because the reader module is public there. Here it is `pub(crate)`, so they live beside the code as `#[cfg(test)]` modules — the shape the fixture tests (#657) already use.
## What had to be adapted rather than copied
- The parquet footer read hangs off `ParquetBaseFileReader` here, not `Storage`
- The ordering config is named after the table property Hudi actually writes (`OrderingFields`, with the deprecated `precombine.field` still honored — so both spellings work)
- The gold snapshot is addressed inside the extracted fixture
- No `env_logger` in this workspace; the harness reports through returned errors anyway
## 28 of 57 cases do not pass, and none were weakened
Each is pinned with the harness's own `ignore = "<finding>"` variant. **The case bodies are untouched**, so `cargo test -- --ignored` still runs them and a gap that closes surfaces as a test that starts passing rather than as silence.
| # | Finding |
|---|---|
| 11 | The delete record's ordering value has two shapes in the wild; this crate carries only the older primitive union, so the wrapper-record shape reads as an out-of-range union index |
| 10 | **Parquet `RowFilter` pushdown is built but never installed on the base read** |
| 3 | Avro maps modelled as `Dictionary(Utf8, V)` — not a valid Arrow type (dictionary keys must be integers) |
| 2 | Corrupt log blocks are not detected (`create_corrupted_block_if_needed` returns `None`) |
| 1 | `int -> long` promotion does not reconcile in the merged-batch evolution path |
| 1 | A case asserting an HFile log block is *rejected* — true upstream, but this crate has HFile support and the read completes. The expectation needs re-deriving, not inverting, so I left it pinned rather than guess |
Groups 1, 3 and 4 are the shared crate gaps already documented in #657 — they fail on the existing reader too. Group 5 is new information.
## The pushdown group is the one that matters
`engine.rs` builds the filter and then drops it:
```rust
let _row_filter = if self.reader_context.can_push_row_filter() { ... };
```
**This is not a pre-existing gap — it is a capability lost in the port.** Upstream threads the filter into the base read; here base reads were routed through this crate's existing parquet reader, which takes no filter, so it goes nowhere. I had previously described this as "ported but unreachable, and no worse than the existing reader" — that was understated. It is no worse than the *existing* reader, but it is worse than the code it was ported from, and these ten cases are what make that visible.
Fixing it means giving the base-file read a way to accept a `RowFilter`. That is its own change.
## Verification
Full workspace green: 1151 lib + 79 table-read + 39 datafusion + 21 + 12; 29 ignored (28 above + the pre-existing fixture gap list). `cargo fmt --check` clean.
I checked the pinning two ways, because the rewrite that applied it was regex-based and got it wrong on the first pass — it pinned a case that had never failed. The ignored set now matches the failing set exactly (diffed against the test runner's own `--ignored --list`), and every reason was re-verified against its case by paren-matching the macro invocations rather than by regex.
🤖 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] test(core): port the merge-on-read reader's end-to-end harness [hudi-rs]
Posted by "linliu-code (via GitHub)" <gi...@apache.org>.
linliu-code closed pull request #661: test(core): port the merge-on-read reader's end-to-end harness
URL: https://github.com/apache/hudi-rs/pull/661
--
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] test(core): port the merge-on-read reader's end-to-end harness [hudi-rs]
Posted by "linliu-code (via GitHub)" <gi...@apache.org>.
linliu-code commented on PR #661:
URL: https://github.com/apache/hudi-rs/pull/661#issuecomment-5232823285
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