wgtmac commented on code in PR #44990:
URL: https://github.com/apache/arrow/pull/44990#discussion_r1953688393


##########
cpp/src/parquet/column_reader.cc:
##########
@@ -306,8 +306,13 @@ class SerializedPageReader : public PageReader {
   // Please refer to the encryption specification for more details:
   // 
https://github.com/apache/parquet-format/blob/encryption/Encryption.md#44-additional-authenticated-data
 
-  // The ordinal fields in the context below are used for AAD suffix 
calculation.
+  // The CryptoContext used by this PageReader.
   CryptoContext crypto_ctx_;
+  // This PageReader has its own Decryptor instances in order to be 
thread-safe.

Review Comment:
   Yes, but isn't `CryptoContext` is created every time for a new `PageReader`? 
The old code has the problem because it shares decryptor instances from 
FileReader all the way down to PageReader. However, now we can create 
decryptors when creating the `CryptoContext` and cache them along with 
`CryptoContext`.
   
   It is misleading that `PageReader::Open` takes `const CryptoContext*` but 
actually copies everything it passes.



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