mapleFU commented on code in PR #41580:
URL: https://github.com/apache/arrow/pull/41580#discussion_r1664379656
##########
cpp/src/parquet/column_writer.cc:
##########
@@ -1104,6 +1106,7 @@ int64_t ColumnWriterImpl::Close() {
if (rows_written_ > 0 && chunk_statistics.is_set()) {
metadata_->SetStatistics(chunk_statistics);
}
+ metadata_->SetKeyValueMetadata(std::move(key_value_metadata_));
Review Comment:
🤔 should for not moved in to help some debugging?
##########
cpp/src/parquet/column_writer.cc:
##########
@@ -1405,6 +1408,25 @@ class TypedColumnWriterImpl : public ColumnWriterImpl,
public TypedColumnWriter<
return pages_change_on_record_boundaries_;
}
+ void AddKeyValueMetadata(
+ const std::shared_ptr<const KeyValueMetadata>& key_value_metadata)
override {
+ if (closed_) {
+ throw ParquetException("Cannot add key-value metadata to closed column");
+ }
Review Comment:
nit: should we throw if `key_value_metadata == nullptr`?
##########
cpp/src/parquet/column_writer.h:
##########
@@ -53,6 +54,8 @@ class Encryptor;
class OffsetIndexBuilder;
class WriterProperties;
+using KeyValueMetadata = ::arrow::KeyValueMetadata;
Review Comment:
IMO just `::arrow::KeyValueMetadata` is ok here
--
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]
