emkornfield commented on a change in pull request #8023:
URL: https://github.com/apache/arrow/pull/8023#discussion_r475351570



##########
File path: cpp/src/parquet/test_encryption_util.h
##########
@@ -65,5 +66,36 @@ inline std::string data_file(const char* file) {
   return ss.str();
 }
 
+static constexpr char DOUBLE_FIELD_NAME[] = "double_field";
+static constexpr char FLOAT_FIELD_NAME[] = "float_field";
+static constexpr char BOOLEAN_FIELD_NAME[] = "boolean_field";
+static constexpr char INT32_FIELD_NAME[] = "int32_field";
+static constexpr char INT64_FIELD_NAME[] = "int64_field";
+static constexpr char INT96_FIELD_NAME[] = "int96_field";
+static constexpr char BA_FIELD_NAME[] = "ba_field";
+static constexpr char FLBA_FIELD_NAME[] = "flba_field";
+
+class FileEncryptor {

Review comment:
       docs please.

##########
File path: cpp/src/parquet/CMakeLists.txt
##########
@@ -363,6 +376,12 @@ if(PARQUET_REQUIRE_ENCRYPTION)
                    encryption_write_configurations_test.cc
                    encryption_read_configurations_test.cc
                    encryption_properties_test.cc
+                   test_encryption_util.cc
+                   test_util.cc)
+  add_parquet_test(encryption-key-management-test

Review comment:
       nit: add blank line

##########
File path: cpp/src/parquet/encryption.h
##########
@@ -47,15 +47,15 @@ using ColumnPathToEncryptionPropertiesMap =
 
 class PARQUET_EXPORT DecryptionKeyRetriever {
  public:
-  virtual std::string GetKey(const std::string& key_metadata) const = 0;
+  virtual std::string GetKey(const std::string& key_metadata) = 0;
   virtual ~DecryptionKeyRetriever() {}
 };
 
 /// Simple integer key retriever
 class PARQUET_EXPORT IntegerKeyIdRetriever : public DecryptionKeyRetriever {
  public:
   void PutKey(uint32_t key_id, const std::string& key);
-  std::string GetKey(const std::string& key_metadata) const;
+  std::string GetKey(const std::string& key_metadata);

Review comment:
       why this removal?

##########
File path: cpp/src/parquet/encryption_internal.h
##########
@@ -45,6 +45,9 @@ constexpr int8_t kOffsetIndex = 7;
 /// Performs AES encryption operations with GCM or CTR ciphers.
 class AesEncryptor {
  public:
+  /// Can serve one key length only. Possible values: 16, 24, 32 bytes.
+  explicit AesEncryptor(ParquetCipher::type alg_id, int key_len, bool 
metadata);

Review comment:
       why isn't static factory sufficient?




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

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to