wgtmac commented on code in PR #51685:
URL: https://github.com/apache/arrow/pull/51685#discussion_r4238189968
##########
cpp/src/parquet/column_writer.h:
##########
@@ -199,6 +204,22 @@ class PARQUET_EXPORT ColumnWriter {
int64_t num_levels, const ::arrow::Array&
leaf_array,
ArrowWriteContext* ctx,
bool leaf_field_nullable) = 0;
+
+ private:
+ friend class RowGroupSerializer;
+
+ /// \brief Create a column writer using the given level information and
content
+ /// defined chunker
+ ///
+ /// The file writer computes the level information of a column once and
gives the same
+ /// chunker to the column writers of a column, so the content defined
chunking is
+ /// carried over between the row groups. The chunker is required if the
properties
+ /// enable content defined chunking.
+ static std::shared_ptr<ColumnWriter> Make(
Review Comment:
Why not expanding the existing Make function?
##########
cpp/src/parquet/column_writer.cc:
##########
@@ -744,10 +744,12 @@ class ColumnWriterImpl {
public:
ColumnWriterImpl(ColumnChunkMetaDataBuilder* metadata,
std::unique_ptr<PageWriter> pager, const bool
use_dictionary,
- Encoding::type encoding, const WriterProperties* properties)
+ Encoding::type encoding, const WriterProperties* properties,
+ const internal::LevelInfo& level_info,
+ internal::ContentDefinedChunker* content_defined_chunker)
: metadata_(metadata),
descr_(metadata->descr()),
- level_info_(internal::LevelInfo::ComputeLevelInfo(metadata->descr())),
+ level_info_(level_info),
Review Comment:
Why this is changed? Shouldn't we save an extra input parameter?
##########
cpp/src/parquet/file_writer.cc:
##########
@@ -255,6 +261,8 @@ class RowGroupSerializer : public RowGroupWriter::Contents {
InternalFileEncryptor* file_encryptor_;
PageIndexBuilder* page_index_builder_;
BloomFilterBuilder* bloom_filter_builder_;
+ const std::vector<internal::LevelInfo>& level_infos_;
Review Comment:
Why do we want to cache this? It is expensive to recompute them when
creating the ColumnWriter instances?
##########
cpp/src/parquet/file_writer.cc:
##########
@@ -139,14 +143,16 @@ class RowGroupSerializer : public
RowGroupWriter::Contents {
// Throws an error if more columns are being written
auto col_meta = metadata_->NextColumnChunk();
+ // Keep the ordinal in step with the metadata even if closing the previous
column
Review Comment:
Why adding this comment? This is misleading.
--
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]
