linliu-code opened a new pull request, #703:
URL: https://github.com/apache/hudi-rs/pull/703
## Description
hudi-rs refused to read a merge-on-read table whose base file and log blocks
are HFile unless the table sat at a metadata-table path. The version two engine
reads such a table correctly — the refusals gated on **where the table lived**,
not on what the reader could do.
This was established before anything was changed, by building such a table
and reading it. The sharpest demonstration: the only difference between the
read that was refused and the read that succeeded was the directory the table
sat in. Identical bytes, identical `hoodie.properties`, identical reader.
The configuration is reachable rather than hypothetical. Java's
`HoodieFileWriterFactory.getFileWriterByFormat` dispatches `HFILE` by extension
to `newHFileFileWriter`, and `HoodieTableConfig.BASE_FILE_FORMAT` has a
`PARQUET` default with no valid-values restriction.
Three gates decided this, and **all three are needed** for the read to work
— fixing any subset leaves it broken at a different line, which is why they
move together:
| Site | Was | Now |
| --- | --- | --- |
| `table/validation.rs` | refused the format when the base path was not a
metadata-table path, before any reader existed | removed |
| `file_group/base_file/reader.rs` | the shared factory returned an error
for HFile | returns `HFileBaseFileReader` |
| `file_group/reader.rs` | diverted every HFile slice to file group reader
version 1 | removed; version 2 serves it |
Version 1 has no HFile base file reader and its log decoder refuses HFile
blocks outright, so that third fallback turned a readable slice into
`Unexpected HFile records in regular table log file`.
Two refusals are deliberately kept. Version 1's log-block refusal stays,
because that reader genuinely cannot decode an HFile block into Arrow — it is
simply no longer reached for these tables. The metadata-table refusal in
`reader_v2/adapter.rs` stays too, but its message claimed the engine had no
HFile support, which this change contradicts; it now gives the real reason,
that the metadata table's record key, merge rule and partition come from its
own configuration.
Removing the format gate also lost the only consumer of `base_file_path` in
`version_two_unsupported_reason`, so that parameter is gone and its call sites
updated.
**Known gap, filed rather than bundled:** schema resolution still has no
HFile arm (`schema/resolver.rs`), so an HFile table whose commit metadata
carries no schema reports "no file path found" — false, since a path was found.
This PR makes that reachable. The one-line arm does not compile: `get_schema`
lives on the concrete parquet and Lance readers, not on the `BaseFileReader`
trait, so `HFileBaseFileReader` needs its own accessor doing its own I/O.
Tracked separately.
## How are the changes test-covered
- [ ] N/A
- [x] Automated tests (unit and/or integration tests)
- [ ] Manual tests
- [ ] Details are described below
A new fixture, `crates/test/data/hfile_base_file_table`: a non-metadata MOR
table with an HFile base file and an HFile log block, shaped so that **the
failure modes look different from each other**.
| key | base fare / `ts` | log fare / `ts` | `COMMIT_TIME` → | `EVENT_TIME`
→ |
| --- | --- | --- | --- | --- |
| `uuid0001` | 11 / 5001 | — | 11 | 11 |
| `uuid0002` | 12 / 5002 | — | 12 | 12 |
| `uuid0003` | 13 / 5003 | 102 / **1000** | **102** | **13** |
| `uuid0004` | 14 / 5004 | 103 / **9000** | 103 | 103 |
| `uuid0005` | — | 104 / 5005 | 104 | 104 |
| `uuid0006` | — | 105 / 5006 | 105 | 105 |
The log block is always the later commit. `uuid0003` is the discriminator
between merge modes: commit-time takes the log's value, event-time keeps the
base's because the log's event time is older. `uuid0004` runs the other way, so
a read that ignored the log block entirely still fails. Values are asserted,
never a row count — dropping the log block returns four rows at 11–14, and
losing the base returns four rows from `uuid0003`.
Mutation-checked, every mutation with a positive control on the restored
tree:
| mutation | caught by |
| --- | --- |
| re-divert HFile to version 1 | the merge tests, with the old log-block
error |
| drop the log block from the read | values: `uuid0003` = 13 instead of 102 |
| read event-time as commit-time | `uuid0003` = 102 against expected 13 |
| flatten the fixture's event times so the modes converge | the mode tests |
| restore the format refusal, error and swallow arm together |
`an_hfile_base_file_is_not_refused_for_its_format` |
Five existing tests pinned the old refusals and were **inverted rather than
deleted**, so each now pins the opposite claim.
A fresh-agent correctness review ran before this PR was opened. It found no
case where removing the three refusals produces a panic, a silent wrong result,
or a lost guard — verified by building a complete table-version-6 HFile table
with a real timeline and reading it end-to-end through `Table`, and by probing
copy-on-write, read-optimized, streaming, layout version 1, and
`drop.partition.columns` paths. Its findings are fixed here, including one test
that could not fail: it asserted the *absence* of a retired error string, and
passed even with the pre-change refusal fully restored. It now asserts what the
failure is.
```
cargo test -p hudi-core --lib 1391
passed, 0 failed
cargo test -p hudi-core --lib --no-default-features 1371
passed, 0 failed
cargo test --workspace --all-targets --all-features 0
failing suites
cargo clippy -p hudi-core --all-targets -- -D warnings clean
cargo clippy -p hudi-core --lib --no-default-features -- -D warnings clean
cargo fmt --all -- --check clean
```
**Not covered, stated rather than implied:** partitioned HFile tables,
multiple log blocks in one log file, and a compacted slice. The fixture is one
non-partitioned single-slice table, and the removed fallback used to be those
shapes' safety net.
--
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]
