linliu-code opened a new pull request, #675:
URL: https://github.com/apache/hudi-rs/pull/675
Stacked on #673 — only the top commit is new here.
### The problem
#673 registered position-based merge as a *silent* gap: a read that asks to
merge by position is merged by key instead, succeeds, and returns the same
rows
in every case except a file group holding duplicate keys — and there,
different
ones.
Almost everything needed to close it was already ported: the position buffer,
the `RECORD_POSITIONS` decoder, the loader's buffer selection. What was
missing
is that nothing produced the row-position column they all read. Enabling the
path failed with `non-nullable column '_tmp_metadata_row_index' absent from
source batch`.
Three other things each defeat the path on their own, so all four had to land
together:
- The config key did not exist; the resolver hardcoded `false`.
- The adapter passed `None` for the base file's commit time. A log block's
positions are only meaningful against the base file they were computed
over,
so the loader's gate never opened and the read fell back to key merge. **A
test written without fixing this passes while proving nothing.**
- The engine dropped the column rather than attaching it.
### The change
**The base reader emits row positions.**
`BaseFileReadOptions::row_index_column`
asks for a synthetic `Int64` column, served by parquet-rs's virtual
`RowNumber`
column. They are physical file indices, so a pushed-down `RowFilter` leaves
gaps
rather than renumbering — which is what makes them comparable to positions a
writer recorded over the unfiltered file.
parquet-rs 57.3.0 has a wrinkle here: with an explicit `ProjectionMask` it
reports the stream's schema without the virtual column while still emitting
it
in every batch, because `build()` cuts the projected fields at the number of
parquet leaves and the virtual leaf sits past that. `schema_with_row_index`
restores it from the builder's own schema.
**The config is Hudi's own key**, `hoodie.merge.use.record.positions`, not a
`hoodie.read.*` one — a table written with record positions should be read
with
the spelling the writer was given. Parsed strictly: a typo errors rather than
quietly merging by key, which is the one outcome this setting exists to
change.
**The gap entry is narrowed, not deleted.** A read that asks for position
merge
and is *declined* — unparseable base-file name, non-parquet base — still
warns.
Hudi is handed the instant by the file slice and never has to decline.
### The fixture
`table_duplicate_keys` is the one fixture that can tell the two strategies
apart: one file group holding `k1` twice and `k2` three times, an upsert of
`k1`
and a delete of `k2`. Generator checked in beside the zip.
Hudi's writer tags an incoming record with *every* base row its index
matches,
so those become two and three log records. Keyed by record key they collapse
to
one entry each and the buffer removes the entry once used, so only the
*first*
base row of each key is merged. Hudi returns **5 rows** merging by key (a
stale
`k1` row, two undeleted `k2` rows) and **3** merging by position. Both
snapshots
ship in the zip; the sweep picks by strategy.
### Testing
- `every_fixture_matches_hudi_when_merging_by_record_position` — all 62
fixtures
match Hudi with positions on. 27 of them actually take the path.
- `merging_by_position_and_by_key_disagree_on_duplicate_record_keys` — pins
that
the two golds differ, and that the reader lands on whichever it was asked
for.
Without this the position sweep would still pass if the path silently fell
back.
- Reader-level: positions survive a `RowFilter` as `[1, 3]` rather than `[0,
1]`.
- Engine-level: the base source carries the column for a position merge and
not
for a key merge, on both the eager and streaming paths.
- Poison-tested twice — dropping the column, and forcing
`use_record_position()`
false. Both make the position tests fail.
The existing reader returns 2 rows on the new fixture; it sorts and dedups
whole
batches by record key, so duplicates collapse. Added to the
known-disagreement
list with that cause.
Full workspace suite green.
🤖 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]
