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



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

Review comment:
       There are two reasons:
   1. This is breaking API change.  Users that were relying on passing this 
through as a const object in places can non longer do so.
   2.  Semantically, I would in most cases to have Get operations be const, 
even if there is caching happening underneath.




----------------------------------------------------------------
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:
us...@infra.apache.org


Reply via email to