kszucs commented on code in PR #51685:
URL: https://github.com/apache/arrow/pull/51685#discussion_r4246085673
##########
cpp/src/parquet/column_writer.h:
##########
@@ -127,10 +131,10 @@ class PARQUET_EXPORT ColumnWriter {
public:
virtual ~ColumnWriter() = default;
- static std::shared_ptr<ColumnWriter> Make(ColumnChunkMetaDataBuilder*,
- std::unique_ptr<PageWriter>,
- const WriterProperties* properties,
- BloomFilter* bloom_filter =
NULLPTR);
+ static std::shared_ptr<ColumnWriter> Make(
+ ColumnChunkMetaDataBuilder*, std::unique_ptr<PageWriter>,
+ const WriterProperties* properties, BloomFilter* bloom_filter = NULLPTR,
+ internal::ContentDefinedChunker* content_defined_chunker = NULLPTR);
Review Comment:
`chunker_internal.h` isn't installed (headers matching `internal` are
skipped), so code outside Arrow can only pass null; the forward declaration
just lets the file writer pass its chunker. Parquet's public headers already
take `::arrow::internal::Executor*` the same way. Making the chunker public
would commit us to its API, which is out of scope here.
##########
cpp/src/parquet/column_writer.cc:
##########
@@ -775,11 +777,11 @@ class ColumnWriterImpl {
compressor_temp_buffer_ =
std::static_pointer_cast<ResizableBuffer>(AllocateBuffer(allocator_,
0));
}
- if (properties_->content_defined_chunking_enabled()) {
- auto cdc_options = properties_->content_defined_chunking_options();
- content_defined_chunker_.emplace(level_info_, cdc_options.min_chunk_size,
- cdc_options.max_chunk_size,
- cdc_options.norm_level);
+ if (properties_->content_defined_chunking_enabled() &&
Review Comment:
Done, the column writer now only checks the chunker, and the file writer
decides from the properties.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
