Repository navigation
[core] Normalize Arrow layouts for upserts and data evolution updates - #1144
Conversation
JeonDaehong
left a comment
There was a problem hiding this comment.
Tried a couple of edge cases on def6d03 with probe tests appended to write_batch_normalize.rs. Two comments inline.
| // Java has one STRING value representation. Arrow offset widths do | ||
| // not change the logical values, including in keys and nested fields. | ||
| // This specific cast checks offset overflow without enabling value casts. | ||
| (DataType::LargeUtf8, DataType::Utf8) => { |
There was a problem hiding this comment.
arrow_cast checks the slice's offsets as absolute values, so a small slice taken past the 2 GiB mark of a LargeUtf8 array fails even though the slice itself is tiny. I built a 2-row array (2^31 + 16 bytes, then "abc") and normalized slice(1, 1):
LargeString array too large to cast to String array
The same "abc" as a fresh array goes through. pyarrow to_batches() hands over zero-copy slices, so a large_string chunk over 2 GiB split into small batches would hit this. Rebasing the offsets (subtract the first one, slice the values) before the cast should fix it, like visible_children already does for lists.
There was a problem hiding this comment.
Fixed in 1a0d621. The conversion now subtracts the first visible offset and slices the shared value buffer before narrowing offsets to i32. This accepts a small slice starting beyond 2 GiB without copying its payload, while still rejecting a visible byte range that exceeds the 32-bit limit.
Added a sparse-mmap regression covering the high starting offset, Unicode, NULLs, empty slices, buffer sharing and actual range overflow. The 23 normalization tests, 2 core upsert tests, Clippy and rustfmt checks pass.
| } | ||
|
|
||
| match (array.data_type(), expected) { | ||
| // Java has one STRING value representation. Arrow offset widths do |
There was a problem hiding this comment.
Question: the same reasoning applies to LargeBinary -> Binary for BYTES columns, and that still fails on this head with Arrow write type LargeBinary is incompatible with table type Binary. Is that planned for a follow-up?
There was a problem hiding this comment.
Keeping LargeBinary/Binary conversions out of this PR. PyPaimon maps binary to BYTES and large_binary to BLOB; accepting them interchangeably would require coordinated handling across schema validation and the write/update paths, expanding the scope considerably.
The existing rules remain in place, with regression assertions that neither direction is implicitly accepted by write normalization.
JeonDaehong
left a comment
There was a problem hiding this comment.
Re-ran the probes on 1a0d621. The slice past 2 GiB now normalizes (["abc", null, "de"], offsets [0, 3, 3, 5]), the same slice inside a struct works too, and the whole 2 GiB+ array still fails with the 32-bit offsets error. The module's 23 tests and clippy pass.
Makes sense to keep LargeBinary out given the BYTES/BLOB split. Thanks!
Purpose
Keep PyPaimon's existing large-string upserts on Native execution, including strings used as match keys, partition fields and nested values. PyPaimon currently falls back because core write normalization rejects
LargeUtf8for the canonicalUtf8STRING representation.PyPaimon integration: apache/paimon#10624. Its Native CI continues to build Rust main and requires this change first.
Testing this path also exposes a DE update failure for ROW/ARRAY/MAP types with required children: runtime read schemas relax nested nullability, while the partial-file writer uses the declared table schema. Interleaving against the runtime schema leaves incompatible child types in the output.
Brief change log
LargeUtf8toUtf8in the existing core write normalizer. This is an Arrow layout conversion for Java/Paimon's STRING values, with checked offset bounds; it does not introduce implicit numeric or binary casts.Tests
cargo +1.98.0 test --quiet --locked -p paimon --lib: 4151 passed, 6 ignored.cargo +1.98.0 clippy --quiet --locked -p paimon --lib --tests -- -D warnings.cargo +1.98.0 fmt --all --check.API and Format
No new public interfaces, options, dependencies or storage formats. Java STRING values and the existing declared write constraints remain authoritative.
Documentation
No documentation changes are required.