diff --git a/cpp/src/parquet/arrow/arrow_statistics_test.cc b/cpp/src/parquet/arrow/arrow_statistics_test.cc index a6817f1cbc2d..ee8b0ce6f49e 100644 --- a/cpp/src/parquet/arrow/arrow_statistics_test.cc +++ b/cpp/src/parquet/arrow/arrow_statistics_test.cc @@ -22,6 +22,7 @@ #include "arrow/array/builder_time.h" #include "arrow/compute/api.h" #include "arrow/table.h" +#include "arrow/testing/builder.h" #include "arrow/testing/gtest_util.h" #include "parquet/api/reader.h" @@ -74,6 +75,21 @@ std::string GetManyEmptyLists() { return many_empty_lists; } +// A dictionary-encoded leaf under a struct with null rows. The child slots under the +// null rows are valid in the Arrow array (and hold "zzz"), but they are nulls in +// Parquet: they count as nulls and must not end up in min/max. +std::shared_ptr<::arrow::Table> GetDictionaryUnderNullStruct() { + auto dict_type = ::arrow::dictionary(::arrow::int32(), ::arrow::utf8()); + auto values = ::arrow::DictArrayFromJSON(dict_type, "[0, 1, 0, 1]", R"(["b", "zzz"])"); + std::shared_ptr null_bitmap; + ABORT_NOT_OK( + ::arrow::GetBitmapFromVector({true, false, true, false}, &null_bitmap)); + auto array = ::arrow::StructArray::Make({values}, {::arrow::field("x", dict_type)}, + std::move(null_bitmap)) + .ValueOrDie(); + return Table::Make(::arrow::schema({::arrow::field("a", array->type())}), {array}); +} + // PARQUET-2067: Tests that nulls from parent fields are included in null statistics. TEST_P(ParameterizedStatisticsTest, NoNullCountWrittenForRepeatedFields) { std::shared_ptr<::arrow::ResizableBuffer> serialized_data = AllocateBuffer(); @@ -159,7 +175,23 @@ INSTANTIATE_TEST_SUITE_P( /*expected_null_count=*/5, /*expected_value_count=*/2, /*expected_min=*/"z", - /*expected_max=*/"z"})); + /*expected_max=*/"z"}, + StatisticsTestParam{/*table=*/GetDictionaryUnderNullStruct(), + /*expected_null_count=*/2, + /*expected_value_count=*/2, + /*expected_min=*/"b", + /*expected_max=*/"b"}, + StatisticsTestParam{ + // "zzz" is in the dictionary but no index references it + /*table=*/Table::Make( + ::arrow::schema({::arrow::field("a", dictionary(::arrow::int32(), + ::arrow::utf8()))}), + {::arrow::DictArrayFromJSON(dictionary(::arrow::int32(), ::arrow::utf8()), + "[0, null, 0]", R"(["b", "zzz"])")}), + /*expected_null_count=*/1, + /*expected_value_count=*/2, + /*expected_min=*/"b", + /*expected_max=*/"b"})); TEST(StatisticsTest, FixedWidthLeafUnderListStructNullCount) { // Null counts for leaves under list> must include null and empty diff --git a/cpp/src/parquet/column_writer.cc b/cpp/src/parquet/column_writer.cc index 3296af62f0c4..d64a24c236ac 100644 --- a/cpp/src/parquet/column_writer.cc +++ b/cpp/src/parquet/column_writer.cc @@ -2002,8 +2002,10 @@ Status TypedColumnWriterImpl::WriteArrowDictionary( PARQUET_ASSIGN_OR_THROW(::arrow::Datum referenced_indices, ::arrow::compute::Unique(*chunk_indices, &exec_ctx)); - // On first run, we might be able to re-use the existing dictionary - if (referenced_indices.length() == dictionary->length()) { + // On first run, we might be able to re-use the existing dictionary. + // Null indices show up as one null in referenced_indices: don't count it. + if (referenced_indices.length() - referenced_indices.null_count() == + dictionary->length()) { referenced_dictionary = dictionary; } else { PARQUET_ASSIGN_OR_THROW( @@ -2043,12 +2045,14 @@ Status TypedColumnWriterImpl::WriteArrowDictionary( AddIfNotNull(rep_levels, offset)); std::shared_ptr writeable_indices = indices->Slice(value_offset, batch_num_spaced_values); - if (page_statistics_ || bloom_filter_writer_) { - update_stats(/*num_chunk_levels=*/batch_size, writeable_indices); - } + // Statistics need the recomputed validity too: an index under a null parent may + // be valid in the leaf array. PARQUET_ASSIGN_OR_THROW( writeable_indices, MaybeReplaceValidity(writeable_indices, null_count, ctx->memory_pool)); + if (page_statistics_ || bloom_filter_writer_) { + update_stats(/*num_chunk_levels=*/batch_size, writeable_indices); + } dict_encoder->PutIndices(*writeable_indices); // Update unencoded byte array data size to size statistics UpdateUnencodedDataBytes();