From ab5cb560714f6f9efe1ca7535b53ed211af9cf62 Mon Sep 17 00:00:00 2001
From: mwish <maplewish117@gmail.com>
Date: Thu, 15 Aug 2024 22:49:29 +0800
Subject: [PATCH] Apply suggestions

---
 cpp/src/parquet/column_writer_test.cc |  9 +++++++--
 cpp/src/parquet/metadata.cc           | 11 ++++++++---
 2 files changed, 15 insertions(+), 5 deletions(-)

diff --git a/cpp/src/parquet/column_writer_test.cc b/cpp/src/parquet/column_writer_test.cc
index 882a6fcb166e8..d2b3aa0dff003 100644
--- a/cpp/src/parquet/column_writer_test.cc
+++ b/cpp/src/parquet/column_writer_test.cc
@@ -1730,13 +1730,18 @@ TEST_F(TestInt32Writer, NoWriteKeyValueMetadata) {
 
 TEST_F(TestInt32Writer, WriteKeyValueMetadata) {
   auto writer = this->BuildWriter();
-  writer->AddKeyValueMetadata(KeyValueMetadata::Make({"hello"}, {"world"}));
+  writer->AddKeyValueMetadata(
+      KeyValueMetadata::Make({"hello", "bye"}, {"world", "earth"}));
+  // overwrite the previous value
+  writer->AddKeyValueMetadata(KeyValueMetadata::Make({"bye"}, {"moon"}));
   writer->Close();
   auto key_value_metadata = metadata_key_value_metadata();
   ASSERT_THAT(key_value_metadata, NotNull());
-  ASSERT_EQ(1, key_value_metadata->size());
+  ASSERT_EQ(2, key_value_metadata->size());
   ASSERT_OK_AND_ASSIGN(auto value, key_value_metadata->Get("hello"));
   ASSERT_EQ("world", value);
+  ASSERT_OK_AND_ASSIGN(value, key_value_metadata->Get("bye"));
+  ASSERT_EQ("moon", value);
 }
 
 TEST_F(TestInt32Writer, ResetKeyValueMetadata) {
diff --git a/cpp/src/parquet/metadata.cc b/cpp/src/parquet/metadata.cc
index d4c4b78f413b3..4f2aa6e37328c 100644
--- a/cpp/src/parquet/metadata.cc
+++ b/cpp/src/parquet/metadata.cc
@@ -135,8 +135,11 @@ std::shared_ptr<Statistics> MakeColumnStats(const format::ColumnMetaData& meta_d
   throw ParquetException("Can't decode page statistics for selected column type");
 }
 
+// Get KeyValueMetadata from parquet Thrift RowGroup or ColumnChunk metadata.
+//
+// Returns nullptr if the metadata is not set.
 template <typename Metadata>
-std::shared_ptr<KeyValueMetadata> CopyKeyValueMetadata(const Metadata& source) {
+std::shared_ptr<KeyValueMetadata> FromThriftKeyValueMetadata(const Metadata& source) {
   std::shared_ptr<KeyValueMetadata> metadata = nullptr;
   if (source.__isset.key_value_metadata) {
     std::vector<std::string> keys;
@@ -380,7 +383,7 @@ class ColumnChunkMetaData::ColumnChunkMetaDataImpl {
 
  private:
   void InitKeyValueMetadata() {
-    key_value_metadata_ = CopyKeyValueMetadata(*column_metadata_);
+    key_value_metadata_ = FromThriftKeyValueMetadata(*column_metadata_);
   }
 
   mutable std::shared_ptr<Statistics> possible_stats_;
@@ -972,7 +975,9 @@ class FileMetaData::FileMetaDataImpl {
     schema_.updateColumnOrders(column_orders);
   }
 
-  void InitKeyValueMetadata() { key_value_metadata_ = CopyKeyValueMetadata(*metadata_); }
+  void InitKeyValueMetadata() {
+    key_value_metadata_ = FromThriftKeyValueMetadata(*metadata_);
+  }
 };
 
 std::shared_ptr<FileMetaData> FileMetaData::Make(