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


##########
cpp/src/parquet/page_index.h:
##########
@@ -26,9 +26,12 @@
 namespace parquet {
 
 class ColumnDescriptor;

Review Comment:
   Can we perhaps use the relevant `type_fwd.h` includes instead of repeating 
all those fw-declarations?



##########
cpp/src/parquet/page_index.h:
##########
@@ -43,6 +46,12 @@ class PARQUET_EXPORT ColumnIndex {
                                            uint32_t index_len,
                                            const ReaderProperties& properties);
 
+  static std::unique_ptr<ColumnIndex> Make(const ColumnDescriptor& descr,
+                                           const void* serialized_index,
+                                           uint32_t index_len,
+                                           const ReaderProperties& properties,
+                                           Decryptor* decryptor);

Review Comment:
   Perhaps we can simply `decryptor` an optional argument (defaulting to 
`NULLPTR`) to avoid adding overloads :-)
   



##########
cpp/src/parquet/page_index.cc:
##########
@@ -308,6 +312,34 @@ class RowGroupPageIndexReaderImpl : public 
RowGroupPageIndexReader {
     }
   }
 
+  /// Get decryptor if the file is encrypted
+  std::shared_ptr<Decryptor> GetColumnMetaDecryptor(
+      const ColumnCryptoMetaData* crypto_metadata, int column_ordinal,
+      int8_t module_type) const {
+    if (crypto_metadata == nullptr) {
+      return nullptr;
+    }
+    if (file_decryptor_ == nullptr) {
+      throw ParquetException("RowGroup is noted as encrypted but no file 
decryptor");
+    }
+    std::shared_ptr<Decryptor> decryptor;
+    if (crypto_metadata->encrypted_with_footer_key()) {
+      // The column is encrypted with footer key
+      decryptor = file_decryptor_->GetFooterDecryptorForColumnMeta();
+    } else {
+      // The column is encrypted with its own key
+      const std::string& column_key_metadata = crypto_metadata->key_metadata();
+      const std::string column_path = 
crypto_metadata->path_in_schema()->ToDotString();
+      decryptor =
+          file_decryptor_->GetColumnMetaDecryptor(column_path, 
column_key_metadata);
+    }
+    ARROW_DCHECK(!decryptor->file_aad().empty());
+    decryptor->UpdateAad(encryption::CreateModuleAad(decryptor->file_aad(), 
module_type,
+                                                     row_group_ordinal_, 
column_ordinal,
+                                                     kNonPageOrdinal));
+    return decryptor;
+  }

Review Comment:
   This is not far from the code in `GetColumnPageReader` (in 
`file_reader.cc`). It would be nice to factor out those per-column decryption 
facilities, either in this PR or in a subsequent issue, what do you think?



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