Thanks Ben, I left a few comments there.
Cheers, Gidon
On Mon, Nov 16, 2020 at 2:58 AM Benjamin Kietzman
wrote:
> @Gidon
>
> Copied the gist to a google doc for commenting:
>
> https://docs.google.com/document/d/11qz84ajysvVo5ZAV9mXKOeh6ay4-xgkBrubggCP5220/edit#
>
> @Micah
>
> > it would be
@Gidon
Copied the gist to a google doc for commenting:
https://docs.google.com/document/d/11qz84ajysvVo5ZAV9mXKOeh6ay4-xgkBrubggCP5220/edit#
@Micah
> it would be preferable to put them in an internal namespace to ensure
adequate unit testing is in place.
Agreed; my intent was only to indicate
Hi Ben,
Can you reconsider this point:
> Properties (including FileDecryptionProperties) are immutable after
construction and thus can be safely shared between threads with no further
synchronization.
As I see, the returned FileDecryptionProperties object from
PropertiesDrivenCryptoFactory
is
Hi all,
Glad to see the parquet-cpp progress on this! Can I suggest creating a
googledoc for the technical discussion? The current md doc format seems to
be harder for pinpointed comments. I got a few, but they are too minor for
sending to the two mailing lists.
Cheers, Gidon
On Fri, Nov 13,
I skimmed through and this seems like a clean design (I would have to
reread the PR to do a comparison. A few thoughts of the top of my head:
> - Multiple internal classes are left public in header files, where it
> would be
> preferred that public classes be kept to a minimum.
I think some
In the context of https://issues.apache.org/jira/browse/ARROW-9318 /
https://github.com/apache/arrow/pull/8023 which port the parquet-mr
design to c++: there's some question whether that design is consistent
with the style and conventions of the c++ implementation of parquet.
Here is a Gist with
