Fix null list of structs being read and written as an empty list - #3834
Fix null list of structs being read and written as an empty list#3834slachiewicz wants to merge 2 commits into
Conversation
from_arrays() takes the offsets buffer alone, which cannot express a null list, so rebuilding the array without a mask collapsed every null list<struct<...>> into an empty one. On write that is unrecoverable: the Parquet file itself then holds the empty list. The rebuild still works around apache/arrow#38809 and stays; only the mask is added. That restores the assertion test_null_list_and_map has carried commented out since apache#252. Closes apache#3833
The comment claimed the branch could go once apache/arrow#38809 was fixed. That is fixed in pyarrow 19, but the branch must stay: Iceberg renames by field-id while cast matches element fields by name, so dropping it nulls out any field renamed inside a list element. Verified: the new test passes, and fails with the rebuild removed.
|
Pushed a second commit that removes the The comment is wrong twice over. apache/arrow#38809 is fixed as of pyarrow 19.0.0 (apache/arrow#44587 taught the struct cast to fill absent output fields with null), and a cast really does now handle a field superset, reordering, widening and a null list element. But the rebuild still cannot go, because Iceberg renames by field-id while That is This comment was created with AI assistance. |
1 participant
A null
list<struct<...>>was rebuilt as an empty list, losing the distinctionbetween the two.
test_null_list_and_maphas documented this since #252, assertingthe corrupted value with the correct assertion commented out pending
apache/arrow#38809.
That dependency turns out not to hold for this symptom:
from_arraystakes amaskargument, so carrying the validity bitmap over fixes the null loss on its own. The
rebuild itself is still needed for #38809 and is unchanged.
The write path is affected too, and there the loss is permanent — the Parquet file
pyiceberg produces contains an empty list, so no reader can tell it from a null. The
new unit test covers that direction; the integration assertion covers the read side.
Closes #3833, which has the full analysis and a standalone reproduction.
This change was created with AI assistance.