mapleFU commented on code in PR #37461:
URL: https://github.com/apache/arrow/pull/37461#discussion_r1310229837


##########
cpp/src/parquet/page_index.cc:
##########
@@ -88,9 +88,8 @@ class TypedColumnIndexImpl : public TypedColumnIndex<DType> {
  public:
   using T = typename DType::c_type;
 
-  TypedColumnIndexImpl(const ColumnDescriptor& descr,
-                       const format::ColumnIndex& column_index)
-      : column_index_(column_index) {
+  TypedColumnIndexImpl(const ColumnDescriptor& descr, format::ColumnIndex 
column_index)
+      : column_index_(std::move(column_index)) {

Review Comment:
   Actually I just find some remarkable place have similar improvement.
   
   It seems that Parquet C++ metadata system uses some hacking, the thrift is 
deserialized once, and use pointer like `void*` to pass it to other component, 
e.g.:
   
   ```
   FileMetaData <-- will deserialize thrift, and store as member `metadata_`
   RowGroupMetadata: uses reference from `FileMetadata.metadata_`
   ColumnChunkMetadata: uses reference from parent...
   ```
   
   This style make sure the performance ( though tricky ) without copying 
`ContainerT<format::Type>`
   



-- 
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