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) {