pitrou commented on code in PR #35758:
URL: https://github.com/apache/arrow/pull/35758#discussion_r1211686590


##########
cpp/src/parquet/metadata.cc:
##########
@@ -1462,40 +1462,42 @@ class 
ColumnChunkMetaDataBuilder::ColumnChunkMetaDataBuilderImpl {
     column_chunk_->meta_data.__set_total_compressed_size(compressed_size);
 
     std::vector<format::Encoding::type> thrift_encodings;
+    std::vector<format::PageEncodingStats> thrift_encoding_stats;
+    auto add_encoding = [&thrift_encodings](format::Encoding::type value) {
+      auto it = std::find(thrift_encodings.begin(), thrift_encodings.end(), 
value);
+      if (it == thrift_encodings.end()) {
+        thrift_encodings.push_back(value);
+      }
+    };
+    // Add dictionary page encoding stats
     if (has_dictionary) {
-      
thrift_encodings.push_back(ToThrift(properties_->dictionary_index_encoding()));
-      if (properties_->version() == ParquetVersion::PARQUET_1_0) {
-        thrift_encodings.push_back(ToThrift(Encoding::PLAIN));
-      } else {
-        
thrift_encodings.push_back(ToThrift(properties_->dictionary_page_encoding()));
+      for (const auto& entry : dict_encoding_stats) {
+        format::PageEncodingStats dict_enc_stat;
+        dict_enc_stat.__set_page_type(format::PageType::DICTIONARY_PAGE);
+        // Dictionary Encoding would be PLAIN_DICTIONARY in v1 and
+        // PLAIN in v2.
+        format::Encoding::type dict_encoding = ToThrift(entry.first);
+        dict_enc_stat.__set_encoding(dict_encoding);
+        dict_enc_stat.__set_count(entry.second);
+        thrift_encoding_stats.push_back(dict_enc_stat);
+        add_encoding(dict_encoding);
       }
-    } else {  // Dictionary not enabled
-      
thrift_encodings.push_back(ToThrift(properties_->encoding(column_->path())));
     }
+    // Force add encoding for RL/DL.
+    // TODO(mwish): BIT_PACKED encoding is supported, but it will not be
+    //  added here. And when RL/DL is empty, it will not be added.

Review Comment:
   `encodings` represent only the encodings _used_, not the encodings 
_supported_. So I don't understand why you're mentioning BIT_PACKED 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]

Reply via email to