pitrou commented on code in PR #44990: URL: https://github.com/apache/arrow/pull/44990#discussion_r2022813165
########## cpp/src/arrow/dataset/file_parquet_encryption_test.cc: ########## @@ -51,9 +53,35 @@ using arrow::internal::checked_pointer_cast; namespace arrow { namespace dataset { +struct EncryptionTestParam { + bool uniform_encryption; // false is using per-column keys + bool concurrently; + bool crypto_factory; +}; + +std::string PrintParam(const testing::TestParamInfo<EncryptionTestParam>& info) { Review Comment: Wouldn't it be more idiomatic to define a `operator<<` for `EncryptionTestParam`? ########## cpp/src/parquet/file_reader.cc: ########## @@ -638,11 +635,12 @@ class SerializedFile : public ParquetFileReader::Contents { const std::shared_ptr<Buffer>& footer_buffer, const uint32_t metadata_len, std::shared_ptr<InternalFileDecryptor> file_decryptor); - std::string HandleAadPrefix(FileDecryptionProperties* file_decryption_properties, - EncryptionAlgorithm& algo); + std::string HandleAadPrefix( + const std::shared_ptr<FileDecryptionProperties>& file_decryption_properties, + EncryptionAlgorithm& algo); Review Comment: Ok, can we take the opportunity to change the mutable ref argument to const-ref? ```suggestion std::string HandleAadPrefix( const std::shared_ptr<FileDecryptionProperties>& file_decryption_properties, const EncryptionAlgorithm& algo); ``` ########## cpp/src/arrow/dataset/file_parquet_encryption_test.cc: ########## @@ -51,9 +53,35 @@ using arrow::internal::checked_pointer_cast; namespace arrow { namespace dataset { +struct EncryptionTestParam { + bool uniform_encryption; // false is using per-column keys + bool concurrently; + bool crypto_factory; Review Comment: Nit: call this `use_crypto_factory`? -- 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: github-unsubscr...@arrow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org