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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
