Copilot commented on code in PR #51685:
URL: https://github.com/apache/arrow/pull/51685#discussion_r4166413494
##########
cpp/src/parquet/column_writer.cc:
##########
@@ -2756,4 +2761,12 @@ std::shared_ptr<ColumnWriter>
ColumnWriter::Make(ColumnChunkMetaDataBuilder* met
return std::shared_ptr<ColumnWriter>(nullptr);
}
+std::shared_ptr<ColumnWriter> ColumnWriter::Make(ColumnChunkMetaDataBuilder*
metadata,
+ std::unique_ptr<PageWriter>
pager,
+ const WriterProperties*
properties,
+ BloomFilter* bloom_filter) {
+ return Make(metadata, std::move(pager), properties, bloom_filter,
+ /*content_defined_chunker=*/nullptr);
Review Comment:
The public factory now always supplies a null chunker, so callers using
`ColumnWriter::Make` directly silently lose content-defined chunking even when
it is enabled in `properties` (and `WriteBatch` is no longer rejected). This
contradicts the stated requirement that the public factory keep creating its
own chunker. Please retain an owned chunker for this overload while the
file-writer overload borrows the file-owned chunker, and cover the
public-factory path with a regression test.
--
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]
