kou commented on code in PR #34616:
URL: https://github.com/apache/arrow/pull/34616#discussion_r1248986239
##########
cpp/src/arrow/dataset/CMakeLists.txt:
##########
@@ -145,7 +145,6 @@ function(ADD_ARROW_DATASET_TEST REL_TEST_NAME)
EXTRA_LINK_LIBS
${ARROW_DATASET_TEST_LINK_LIBS}
PREFIX
- ${PREFIX}
Review Comment:
Is it an intentional change?
##########
cpp/src/parquet/properties.h:
##########
@@ -722,6 +744,10 @@ class PARQUET_EXPORT WriterProperties {
return NULLPTR;
}
}
+ // \brief Returns the default column properties
Review Comment:
```suggestion
// \brief Returns the default column properties
```
##########
cpp/src/arrow/util/config.h.cmake:
##########
@@ -59,3 +59,6 @@
#cmakedefine ARROW_WITH_UCX
#cmakedefine GRPCPP_PP_INCLUDE
+#cmakedefine PARQUET_REQUIRE_ENCRYPTION
+
+
Review Comment:
We may want to group Parquet related definitions in the future:
```suggestion
#cmakedefine PARQUET_REQUIRE_ENCRYPTION
```
##########
python/sample_dataset/2019/part-0.parquet:
##########
Review Comment:
Do we need to add this file?
It seems that this is generated by automatically.
##########
cpp/src/arrow/dataset/file_parquet.h:
##########
@@ -28,6 +28,8 @@
#include "arrow/dataset/discovery.h"
#include "arrow/dataset/file_base.h"
+#include "arrow/dataset/parquet_encryption_config.h"
+
#include "arrow/dataset/type_fwd.h"
#include "arrow/dataset/visibility.h"
#include "arrow/io/caching.h"
Review Comment:
We may need to include `arrow/util/config.h` explicitly?
Could you check whether `PARQUET_REQUIRE_ENCRYPTION` is defined or not in
this file?
##########
cpp/src/arrow/dataset/file_parquet.h:
##########
@@ -28,6 +28,8 @@
#include "arrow/dataset/discovery.h"
#include "arrow/dataset/file_base.h"
+#include "arrow/dataset/parquet_encryption_config.h"
+
Review Comment:
```suggestion
```
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
