wgtmac commented on code in PR #51357:
URL: https://github.com/apache/arrow/pull/51357#discussion_r4044048282
##########
cpp/src/parquet/column_writer.cc:
##########
@@ -1750,7 +1750,8 @@ class TypedColumnWriterImpl : public ColumnWriterImpl,
internal::DefLevelsToBitmap(def_levels, batch_size, level_info_, &io);
*out_values_to_write = io.values_read - io.null_count;
*out_spaced_values_to_write = io.values_read;
- *null_count = io.null_count;
+ // Include nulls from repeated ancestors, which are excluded from
io.null_count.
+ *null_count = batch_size - *out_values_to_write;
Review Comment:
`io.null_count` and the Parquet-level null count represent different domains
here. `io.null_count` counts null slots in the compacted Arrow leaf array and
is required by `MaybeReplaceValidity`, while `batch_size -
*out_values_to_write` also includes repeated-ancestor entries that have no
Arrow slot. The new value now reaches the ByteArray and direct-dictionary
`MaybeReplaceValidity` calls, producing an incorrect `ArrayData::null_count`;
the dictionary path can even size its indices buffer too small. Please keep
both counts separately and use the total count only for Parquet statistics/page
metadata.
--
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]
