Skip to content

fix(core): read the delete record's wrapped ordering value - #663

Open
linliu-code wants to merge 2 commits into
apache:mainfrom
linliu-code:fix/delete-record-wrapper-schema
Open

fix(core): read the delete record's wrapped ordering value#663
linliu-code wants to merge 2 commits into
apache:mainfrom
linliu-code:fix/delete-record-wrapper-schema

Conversation

@linliu-code

Copy link
Copy Markdown

Stacked on #639#662 — review only the last commit.

The bug

A delete block whose ordering value is anything but a small integer fails to read:

Union index 1490 out of bounds: 2
Cannot convert i64 to usize: -26

"out of bounds: 2" is the tell — the only 2-branch unions are recordKey and partitionPath. So the decoder is reading a record key's union index and getting 1490. The byte stream is misaligned; the branch wasn't chosen wrongly.

This is a pre-existing hudi-rs defect, not a port regression. It fails on the current reader too, through Table::read.

Cause

orderingVal is declared here as a union of primitives. Hudi writes a union of per-type wrapper records, and inserted BooleanWrapper at position 1 — so every position from int onward names a different type:

Position This crate Hudi (writer)
1 int BooleanWrapper
2 long IntWrapper
3 float LongWrapper
4 double FloatWrapper

Evidence

The delete block in table_delete_ord_long, 26 bytes, is exactly self-consistent under Hudi's schema:

04 | 02 02 34 | 02 00 | 06 c0 3e | 02 02 33 | 02 00 | 06 f0 2e | 00
  • 04 → 2 records
  • keys "4" and "3", empty partitions, ordering position 3, varints 4000 and 3000

Decoded here, position 3 is a float — 4 fixed bytes — so c0 3e 02 02 is swallowed and 33 is read as the next record's key union index: zigzag(0x33) = −26, the reported error, byte for byte.

What changed

  1. The schema — Hudi's wrapper union, for decoding.
  2. The narrowing template — position i now holds the primitive that wrapper i carries, so the existing "keep [null, i]" logic still yields a primitive Arrow column and the output type is unchanged.
  3. UnwrappingUnion(i, Record[("value", v)])Union(1, v), applied after narrowing, since narrowing reads the position Hudi wrote and unwrapping rewrites it.
  4. A cast — the wrapper is chosen for the value, not the column, so a long ordering column can carry an IntWrapper for a small value. The delete batch's ordering column is cast to the type the data schema declares. The old schema hid this by calling position 2 a long regardless.
  5. ArrayWrapper is rejected in both places that read the position, rather than mapped to something that disagrees with its value.

A correction to an earlier claim

I previously reported that both shapes exist in the wild and could not be distinguished, after an attempt to swap the schema broke v6_trips_8i3d and v8_trips_8i3u1d. That was wrong. Both use position 2, which is long here and IntWrapper{int} there — and a wrapper's field is a bare primitive, so both decode the same bytes to the same number. Those fixtures never distinguished the schemas. They broke on the Arrow type of the result, which is what item 4 addresses.

All 16 fixtures with delete blocks are consistent with the wrapper schema; none requires the primitive form.

Older tables

Not supported, deliberately. No fixture uses the primitive layout, and the two cannot be told apart from the bytes: where they differ, decoding usually fails (this bug), and at position 2 both succeed with the same value. If you know of tables written with the older layout, please say so — the alternative is a decode-and-retry fallback, which I did not take.

Tests

11 previously-ignored cases un-ignored and passing: ordering by long, decimal, timestamp, plus multi-log, watermark and instant-range cases that were blocked behind the same decode.

The schema's own unit tests asserted the old index space; they now assert the wire order, which is what a delete block's index actually refers to. Two new tests cover unwrapping and the ArrayWrapper rejection.

Full workspace green: 1177 lib + 79 table-read + 39 datafusion + 21 + 12. Ignored drops 19 → 8.

🤖 Generated with Claude Code

linliu-code and others added 2 commits August 3, 2026 22:01
Squashed view of apache#639-apache#662 for review. Not for merge — the reviewable
increments are those PRs; this is the same code in one diff.

Ports the merge-on-read file group reader from onehouseinc/hudi-rs-internal
into hudi-core, wires it behind a switch that defaults to the reader that has
always served reads, and brings its end-to-end test harness across.

The ported reader is `pub(crate)` and reached only through
`hoodie.read.merge.engine = v2`. Nothing changes for anyone who does not set
it. It is not at parity yet: the gaps are pinned as ignored cases carrying
their findings, and the outstanding decisions are in the PR descriptions.

Four fixes land on paths the existing reader shares: decimals had no Arrow
conversion, Avro timestamp logical types lost their UTC zone, the
properties-escaped create schema was not being unescaped by one of its two
consumers, and the base file reader had no way to accept a pushdown predicate.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A delete block whose ordering value is anything but a small integer fails to
read, with errors like `Union index 1490 out of bounds: 2`. The index is
nonsense because the byte stream is misaligned, not because a branch was chosen
wrongly.

`orderingVal` is declared here as a union of primitives. Hudi writes a union of
per-type wrapper records, and inserted `BooleanWrapper` at position 1, so every
position from `int` onward names a different type than this crate assumes.
Position 3 is `float` here and `LongWrapper` there. Reading a long as a float
consumes four bytes instead of two, and the next record's key length is read
from the middle of the previous value.

The delete block in `table_delete_ord_long` is exactly self-consistent under
Hudi's schema and not under this one:

    04 | 02 02 34 | 02 00 | 06 c0 3e | 02 02 33 | 02 00 | 06 f0 2e | 00
    two records, keys "4" and "3", ordering position 3, values 4000 and 3000

Decoded here, position 3 is a float, so `c0 3e 02 02` is eaten and `33` becomes
the next key's union index: zigzag 0x33 is -26, which is the reported error.

So this takes Hudi's schema. Two things follow from it:

The Arrow side wants a scalar, not a record with one field, so the wrapper is
unwrapped after the schema is narrowed — narrowing reads the position Hudi
wrote, and unwrapping rewrites it, so the order matters.

The wrapper is chosen for the value rather than for the column, so a table
whose ordering column is a long can still carry an `IntWrapper` for a small
value. The delete batch's ordering column is now cast to the type the data
schema declares. The old schema hid this by calling position 2 a long
regardless, which happened to match the two fixtures that exercise it.

`ArrayWrapper` orders by a list and is rejected in both places that read the
position, rather than mapped to something that would disagree with its value.

Older tables written with the primitive union are not supported. No fixture
here uses one, and the two shapes cannot be told apart from the bytes: where
they differ, decoding usually fails, and at position 2 both succeed and yield
the same number.

Un-ignores the eleven cases that pinned this.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant