rayokota opened a new pull request, #421:
URL: https://github.com/apache/arrow-dotnet/pull/421
### What
`VariantEncodingHelper` wrote and read the Variant object value header with
`field_id_size_minus_one` and `field_offset_size_minus_one` in each other's
bit positions.
Per `apache/parquet-format` `VariantEncoding.md`, the object `value_header`
— the 6 bits above
the 2 basic-type bits — is laid out as:
```
5 4 3 2 1 0
+---+---+-------+-------+
value_header | R | | | |
+---+---+-------+-------+
^ ^ ^
| | +-- field_offset_size_minus_one
| +-- field_id_size_minus_one
+-- is_large
```
`MakeObjectHeader` and `ParseObjectHeader` had the two 2-bit fields
transposed, and the layout
comment above them documented the same transposition — so the block was
internally consistent
rather than wrong in one expression.
`is_large` was already correct. The array header and metadata header helpers
were checked and
match the spec. This affects the object header only.
### Impact
Reader and writer shared the inverted convention, so arrow-dotnet
round-tripped its own output
correctly. The bug was only observable across implementations, and only when
`fieldIdSize != offsetSize` — when the two are equal, transposing them is a
no-op.
Those sizes are computed independently in `VariantValueWriter`
(`fieldIdSize` from the maximum
field ID, `offsetSize` from the encoded data length), so they diverge
routinely: for example an
object drawn from a >255-entry metadata dictionary (2-byte field IDs) whose
own field data is
under 256 bytes (1-byte offsets).
For `fieldIdSize=2, offsetSize=1, isLarge=false`, the spec-correct header
byte is `0x12`;
before this change we emitted `0x06`, and read `0x12` back as
`fieldIdSize=1, offsetSize=2`.
Such objects were silently misparsed in both directions — field IDs and
offsets read at the
wrong widths, surfacing as garbage field values or out-of-range offsets
rather than a clean
error.
### Changes
- `VariantEncodingHelper.MakeObjectHeader` / `ParseObjectHeader`: swap the
two shifts, and
correct the layout comment. The `out` parameters were already named
correctly, so neither
call site — `VariantValueWriter` or `VariantObjectReader` — needed changes.
- `VariantEncodingHelperTests`: add `MakeObjectHeaderUsesSpecBitLayout` and
Closes #420.
--
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]
