This is an automated email from the ASF dual-hosted git repository.
pitrou pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow.git
The following commit(s) were added to refs/heads/main by this push:
new 5f75a12e29d GH-55124: [C++][Parquet] Allow masked nulls in required
fields when writing (#55136)
5f75a12e29d is described below
commit 5f75a12e29d2ccf1c5527db0750c8b5ce6aa9ce1
Author: Zehua Zou <[email protected]>
AuthorDate: Tue Oct 27 16:59:22 2026 +0800
GH-55124: [C++][Parquet] Allow masked nulls in required fields when writing
(#55136)
### Rationale for this change
The Parquet writer checks the leaf array's physical null count without
accounting for the parent struct's validity bitmap. As a result, it rejects
nulls in required child fields even when they are masked by null parent rows.
### What changes are included in this PR?
Corrected the logic used by the column writer to validate the null values
for required leaf fields.
### Are these changes tested?
Yes.
### Are there any user-facing changes?
No.
### Was AI used for this PR?
In accordance to the [AI generation
guidelines](https://arrow.apache.org/docs/dev/developers/overview.html#ai-generated-code),
please disclose below whether and how AI was used in this PR.
**PR code and description written by:**
- [x] Human
- [x] AI
**Reviewed before submission by:**
- [x] Human
- [ ] AI
- [ ] Not reviewed
* GitHub Issue: #55124
Authored-by: Zehua Zou <[email protected]>
Signed-off-by: Antoine Pitrou <[email protected]>
---
cpp/src/parquet/arrow/arrow_reader_writer_test.cc | 45 +++++++++++++++++++++++
cpp/src/parquet/arrow/path_internal.cc | 4 +-
cpp/src/parquet/column_writer.cc | 23 +++++++++++-
3 files changed, 69 insertions(+), 3 deletions(-)
diff --git a/cpp/src/parquet/arrow/arrow_reader_writer_test.cc
b/cpp/src/parquet/arrow/arrow_reader_writer_test.cc
index d858962d472..421769521d1 100644
--- a/cpp/src/parquet/arrow/arrow_reader_writer_test.cc
+++ b/cpp/src/parquet/arrow/arrow_reader_writer_test.cc
@@ -3262,6 +3262,47 @@ TEST(ArrowReadWrite, NestedRequiredField) {
/*row_group_size=*/8);
}
+TEST(ArrowReadWrite, NestedRequiredFieldWithMaskedNulls) {
+ // A required child null is valid when its parent is null.
+ // Reuse the child's bitmap to construct this case explicitly.
+ auto type =
+ ::arrow::struct_({::arrow::field("inner", ::arrow::int32(),
/*nullable=*/false)});
+ auto inner = ::arrow::ArrayFromJSON(::arrow::int32(), "[42, 42, null, 42]");
+ ASSERT_OK_AND_ASSIGN(auto array, ::arrow::StructArray::Make({inner},
type->fields(),
+
inner->null_bitmap()));
+ ASSERT_EQ(array->null_count(), 1);
+ ASSERT_EQ(array->field(0)->null_count(), 1);
+
+ auto table = ::arrow::Table::Make(
+ ::arrow::schema({::arrow::field("outer", type, /*nullable=*/true)}),
{array});
+ ASSERT_OK(table->ValidateFull());
+ CheckSimpleRoundtrip(table, /*row_group_size=*/4);
+
+ // Also test masked child nulls after null/empty lists in the same row group.
+ // Null/empty lists add definition levels without corresponding child values.
+ auto offsets = ::arrow::ArrayFromJSON(::arrow::int32(), "[null, 0, 0, 4]");
+ ASSERT_OK_AND_ASSIGN(auto list_array,
::arrow::ListArray::FromArrays(*offsets, *array));
+ auto list_type = list_array->type();
+ auto list_table = ::arrow::Table::Make(
+ ::arrow::schema({::arrow::field("outer", std::move(list_type))}),
+ {std::move(list_array)});
+ ASSERT_OK(list_table->ValidateFull());
+ CheckSimpleRoundtrip(list_table, /*row_group_size=*/3);
+
+ // Exposing the same child null under a valid parent must fail.
+ ASSERT_OK_AND_ASSIGN(auto unmasked,
+ ::arrow::StructArray::Make({std::move(inner)},
type->fields()));
+ auto invalid_table = ::arrow::Table::Make(table->schema(),
{std::move(unmasked)});
+ // Test partially null and all-null child arrays.
+ for (const auto& invalid_slice : {invalid_table, invalid_table->Slice(2,
1)}) {
+ ARROW_SCOPED_TRACE("num_rows = ", invalid_slice->num_rows());
+ auto sink = CreateOutputStream();
+ EXPECT_RAISES_WITH_MESSAGE_THAT(
+ Invalid, ::testing::HasSubstr("is declared non-nullable but contains
nulls"),
+ WriteTable(*invalid_slice, ::arrow::default_memory_pool(), sink));
+ }
+}
+
TEST(ArrowReadWrite, Decimal) {
using ::arrow::field;
@@ -3843,6 +3884,10 @@ TEST(TestArrowWrite, InvalidSchema) {
CheckWritingNonNullableColumnWithNulls(
::arrow::field("a", ::arrow::utf8(), /*nullable=*/false),
R"([{"a": "foo"}, {"a": null}])");
+ CheckWritingNonNullableColumnWithNulls(
+ ::arrow::field("a", ::arrow::list(::arrow::field("item",
::arrow::int32(),
+ /*nullable=*/false))),
+ R"([{"a": null}, {"a": []}, {"a": [42, null]}])");
}
void DoNestedValidate(const std::shared_ptr<::arrow::DataType>& inner_type,
diff --git a/cpp/src/parquet/arrow/path_internal.cc
b/cpp/src/parquet/arrow/path_internal.cc
index c626dcbf48b..48e054e145a 100644
--- a/cpp/src/parquet/arrow/path_internal.cc
+++ b/cpp/src/parquet/arrow/path_internal.cc
@@ -724,7 +724,9 @@ class PathBuilder {
// and the array does in fact contain nulls, we will end up
// traversing the null bitmap twice (once here and once when calculating
// rep/def levels).
- if (LazyNoNulls(array)) {
+ // Required leaf nulls must not lower definition levels: they may be
masked by
+ // an ancestor, but otherwise WriteArrow() must reject them as invalid.
+ if (!nullable_in_parent_ || LazyNoNulls(array)) {
info_.path.emplace_back(AllPresentTerminalNode{info_.max_def_level});
} else if (LazyNullCount(array) == array.length()) {
info_.path.emplace_back(AllNullsTerminalNode(info_.max_def_level - 1));
diff --git a/cpp/src/parquet/column_writer.cc b/cpp/src/parquet/column_writer.cc
index 3b4a873b519..9d5b3b7bcd0 100644
--- a/cpp/src/parquet/column_writer.cc
+++ b/cpp/src/parquet/column_writer.cc
@@ -1430,8 +1430,27 @@ class TypedColumnWriterImpl : public ColumnWriterImpl,
(level_info_.def_level == level_info_.repeated_ancestor_def_level + 1)
&&
leaf_field_nullable;
if (!leaf_field_nullable && leaf_array.null_count() != 0) {
- return Status::Invalid("Column '", descr_->name(),
- "' is declared non-nullable but contains nulls");
+ bool has_unmasked_nulls = true;
+ if (level_info_.HasNullableValues()) {
+ has_unmasked_nulls = false;
+ int64_t leaf_index = 0;
+ for (int64_t i = 0; i < num_levels; ++i) {
+ // Null or empty lists have no corresponding slot in the leaf array.
+ if (def_levels[i] < level_info_.repeated_ancestor_def_level) {
+ continue;
+ }
+ // A required leaf only needs to be valid when all ancestors are
present.
+ if (def_levels[i] == level_info_.def_level &&
leaf_array.IsNull(leaf_index)) {
+ has_unmasked_nulls = true;
+ break;
+ }
+ ++leaf_index;
+ }
+ }
+ if (has_unmasked_nulls) {
+ return Status::Invalid("Column '", descr_->name(),
+ "' is declared non-nullable but contains
nulls");
+ }
}
bool maybe_parent_nulls = level_info_.HasNullableValues() &&
!single_nullable_element;
if (maybe_parent_nulls) {
