From 2aa540e5cc42e26bf8a95f721412a746db3e899d Mon Sep 17 00:00:00 2001 From: Robert Kruszewski Date: Wed, 7 Oct 2026 15:03:10 +0100 Subject: [PATCH 1/2] Add support for string/binary view to content defined chunking writer --- cpp/src/parquet/chunker_internal.cc | 14 ++++++++++++++ cpp/src/parquet/chunker_internal_test.cc | 17 +++++++++++++++-- 2 files changed, 29 insertions(+), 2 deletions(-) diff --git a/cpp/src/parquet/chunker_internal.cc b/cpp/src/parquet/chunker_internal.cc index 794075b73379..c45bf0c97ea8 100644 --- a/cpp/src/parquet/chunker_internal.cc +++ b/cpp/src/parquet/chunker_internal.cc @@ -21,6 +21,7 @@ #include #include #include +#include #include #include "arrow/array.h" @@ -349,6 +350,17 @@ class ContentDefinedChunker::Impl { }); } + std::vector CalculateBinaryView(const int16_t* def_levels, + const int16_t* rep_levels, int64_t num_levels, + const ::arrow::Array& values) { + const auto& array = checked_cast(values); + return Calculate(def_levels, rep_levels, num_levels, [&](int64_t i) { + const std::string_view value = array.GetView(i); + Roll(reinterpret_cast(value.data()), + static_cast(value.size())); + }); + } + std::vector GetChunks(const int16_t* def_levels, const int16_t* rep_levels, int64_t num_levels, const ::arrow::Array& values) { auto handle_type = [&](auto&& type) -> std::vector { @@ -380,6 +392,8 @@ class ContentDefinedChunker::Impl { } else if constexpr (::arrow::is_large_binary_like(ArrowType::type_id)) { return CalculateBinaryLike<::arrow::LargeBinaryArray>(def_levels, rep_levels, num_levels, values); + } else if constexpr (::arrow::is_binary_view_like(ArrowType::type_id)) { + return CalculateBinaryView(def_levels, rep_levels, num_levels, values); } else if constexpr (::arrow::is_dictionary(ArrowType::type_id)) { return GetChunks(def_levels, rep_levels, num_levels, *static_cast(values).indices()); diff --git a/cpp/src/parquet/chunker_internal_test.cc b/cpp/src/parquet/chunker_internal_test.cc index 2469d54afbf1..5acfa5d68ee3 100644 --- a/cpp/src/parquet/chunker_internal_test.cc +++ b/cpp/src/parquet/chunker_internal_test.cc @@ -170,6 +170,10 @@ Result> GenerateArray(const std::shared_ptr& field std::string("bin_") + std::to_string(val)) GENERATE_CASE(LARGE_BINARY, ::arrow::LargeBinaryBuilder, std::string("bin_") + std::to_string(val)) + GENERATE_CASE(STRING_VIEW, ::arrow::StringViewBuilder, + std::string("str_") + std::to_string(val)) + GENERATE_CASE(BINARY_VIEW, ::arrow::BinaryViewBuilder, + std::string("bin_") + std::to_string(val)) case ::arrow::Type::FIXED_SIZE_BINARY: { auto size = checked_cast(type.get())->byte_width(); @@ -742,6 +746,13 @@ Result CalculateCdcSize(const std::shared_ptr& array, bool nulla } else if (::arrow::is_large_binary_like(type_id)) { auto binary_array = checked_cast(array.get()); result += binary_array->total_values_length(); + } else if (::arrow::is_binary_view_like(type_id)) { + auto view_array = checked_cast(array.get()); + for (int64_t i = 0; i < view_array->length(); i++) { + if (view_array->IsValid(i)) { + result += static_cast(view_array->GetView(i).size()); + } + } } else { return Status::NotImplemented("CDC size calculation for type ", array->type()->ToString(), " is not implemented"); @@ -784,7 +795,8 @@ void AssertContentDefinedChunkSizes(const std::shared_ptr<::arrow::ChunkedArray> ASSERT_EQ(column_info.has_dictionary_page, expect_dictionary_page); } - if (::arrow::is_fixed_width(type_id) || ::arrow::is_base_binary_like(type_id)) { + if (::arrow::is_fixed_width(type_id) || ::arrow::is_base_binary_like(type_id) || + ::arrow::is_binary_view_like(type_id)) { int64_t offset = 0; auto page_lengths = column_info.page_lengths; @@ -1451,7 +1463,7 @@ INSTANTIATE_TEST_SUITE_P( testing::Values( CaseConfig{::arrow::boolean(), false}, CaseConfig{::arrow::int64(), true}, // Binary-like - CaseConfig{::arrow::utf8(), false}, + CaseConfig{::arrow::utf8(), false}, CaseConfig{::arrow::utf8_view(), true}, CaseConfig{::arrow::fixed_size_binary(16), true}, // Nested types CaseConfig{::arrow::list(::arrow::int32()), false}, @@ -1475,6 +1487,7 @@ INSTANTIATE_TEST_SUITE_P( // Binary-like CaseConfig{::arrow::utf8(), false}, CaseConfig{::arrow::binary(), true}, CaseConfig{::arrow::fixed_size_binary(16), true}, + CaseConfig{::arrow::utf8_view(), false}, CaseConfig{::arrow::binary_view(), true}, // Temporal CaseConfig{::arrow::date32(), false}, CaseConfig{::arrow::time32(::arrow::TimeUnit::MILLI), true}, From d69a2d95b47d52118bfa03f1fdcc3dea71920854 Mon Sep 17 00:00:00 2001 From: Robert Kruszewski Date: Thu, 8 Oct 2026 22:58:07 +0100 Subject: [PATCH 2/2] fixes Signed-off-by: Robert Kruszewski --- cpp/src/parquet/chunker_internal_test.cc | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/cpp/src/parquet/chunker_internal_test.cc b/cpp/src/parquet/chunker_internal_test.cc index 5acfa5d68ee3..8de4136cfab6 100644 --- a/cpp/src/parquet/chunker_internal_test.cc +++ b/cpp/src/parquet/chunker_internal_test.cc @@ -30,6 +30,7 @@ #include "arrow/testing/generator.h" #include "arrow/type_fwd.h" #include "arrow/util/float16.h" +#include "arrow/visit_data_inline.h" #include "parquet/arrow/reader.h" #include "parquet/arrow/reader_internal.h" #include "parquet/arrow/test_util.h" @@ -747,12 +748,9 @@ Result CalculateCdcSize(const std::shared_ptr& array, bool nulla auto binary_array = checked_cast(array.get()); result += binary_array->total_values_length(); } else if (::arrow::is_binary_view_like(type_id)) { - auto view_array = checked_cast(array.get()); - for (int64_t i = 0; i < view_array->length(); i++) { - if (view_array->IsValid(i)) { - result += static_cast(view_array->GetView(i).size()); - } - } + ::arrow::VisitArraySpanInline<::arrow::BinaryViewType>( + *array->data(), + [&](std::string_view v) { result += static_cast(v.size()); }, [] {}); } else { return Status::NotImplemented("CDC size calculation for type ", array->type()->ToString(), " is not implemented");