kapoisu commented on PR #46202: URL: https://github.com/apache/arrow/pull/46202#issuecomment-2969124537
I noticed that the FileKeyUnwrapper class has public constructors that accept either owned or non-owned pointer to KeyToolkit, originating from [PR #40329](https://github.com/apache/arrow/pull/40329). Internally, the implementation only uses the non-owned pointer, but it keeps a copy of shared_ptr when the owned version is used, to avoid dropping the reference count to 0. If this class is intended for internal use only, there doesn’t seem to be a strong reason to keep the owned versions. Removing them would make the semantics clearer, as users would then understand they are responsible for managing the lifetime of the object. It would also simplify future additions by avoiding the need to introduce two constructors for every new parameter combination. @adamreeve any thoughts on this? -- 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
