Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 45 additions & 0 deletions cpp/src/parquet/arrow/arrow_reader_writer_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
Comment thread
HuaHuaY marked this conversation as resolved.
::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;

Expand Down Expand Up @@ -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,
Expand Down
4 changes: 3 additions & 1 deletion cpp/src/parquet/arrow/path_internal.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down
23 changes: 21 additions & 2 deletions cpp/src/parquet/column_writer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
Loading