-
Notifications
You must be signed in to change notification settings - Fork 3.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ARROW-17817: [C++] Let ORC compile on MSVC if it is activated #14208
Conversation
Could you enable ORC in our CI jobs? diff --git a/.github/workflows/cpp.yml b/.github/workflows/cpp.yml
index 2642a6ec1a..07243ffc4e 100644
--- a/.github/workflows/cpp.yml
+++ b/.github/workflows/cpp.yml
@@ -213,6 +213,7 @@ jobs:
ARROW_HOME: /usr
ARROW_JEMALLOC: OFF
ARROW_MIMALLOC: ON
+ ARROW_ORC: ON
ARROW_PARQUET: ON
ARROW_USE_GLOG: OFF
ARROW_VERBOSE_THIRDPARTY_BUILD: OFF
diff --git a/ci/appveyor-cpp-build.bat b/ci/appveyor-cpp-build.bat
index 4b90e49257..d8eb9d2c11 100644
--- a/ci/appveyor-cpp-build.bat
+++ b/ci/appveyor-cpp-build.bat
@@ -68,6 +68,7 @@ cmake -G "%GENERATOR%" %CMAKE_ARGS% ^
-DARROW_FLIGHT_SQL=%ARROW_BUILD_FLIGHT_SQL% ^
-DARROW_GANDIVA=%ARROW_BUILD_GANDIVA% ^
-DARROW_MIMALLOC=ON ^
+ -DARROW_ORC=ON ^
-DARROW_PARQUET=ON ^
-DARROW_PYTHON=ON ^
-DARROW_S3=%ARROW_S3% ^ |
|
Hello, I made the changes you suggested. I don't know if this will compile fine as my only compile tests were with vcpkg. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks.
It seems that there are some warnings with Visual C++:
https://github.com/apache/arrow/actions/runs/3111153495/jobs/5043117996#step:11:1325
D:\a\arrow\arrow\cpp\src\arrow\adapters\orc\adapter_test.cc(49): error C2220: the following warning is treated as an error
D:\a\arrow\arrow\cpp\src\arrow\adapters\orc\adapter_test.cc(49): warning C4244: 'initializing': conversion from 'double' to 'int64_t', possible loss of data
D:\a\arrow\arrow\cpp\src\arrow\adapters\orc\adapter_test.cc(49): warning C4244: 'initializing': conversion from 'double' to 'const int64_t', possible loss of data
D:\a\arrow\arrow\cpp\src\arrow\adapters\orc\adapter_test.cc(50): warning C4244: 'initializing': conversion from 'double' to 'int64_t', possible loss of data
D:\a\arrow\arrow\cpp\src\arrow\adapters\orc\adapter_test.cc(50): warning C4244: 'initializing': conversion from 'double' to 'const int64_t', possible loss of data
D:\a\arrow\arrow\cpp\src\arrow\adapters\orc\adapter_test.cc(51): warning C4244: 'initializing': conversion from 'double' to 'int64_t', possible loss of data
D:\a\arrow\arrow\cpp\src\arrow\adapters\orc\adapter_test.cc(51): warning C4244: 'initializing': conversion from 'double' to 'const int64_t', possible loss of data
D:\a\arrow\arrow\cpp\src\arrow\adapters\orc\adapter_test.cc(52): warning C4244: 'initializing': conversion from 'double' to 'int64_t', possible loss of data
D:\a\arrow\arrow\cpp\src\arrow\adapters\orc\adapter_test.cc(52): warning C4244: 'initializing': conversion from 'double' to 'const int64_t', possible loss of data
D:\a\arrow\arrow\cpp\src\arrow\adapters\orc\adapter_test.cc(53): warning C4244: 'initializing': conversion from 'double' to 'int64_t', possible loss of data
D:\a\arrow\arrow\cpp\src\arrow\adapters\orc\adapter_test.cc(53): warning C4244: 'initializing': conversion from 'double' to 'const int64_t', possible loss of data
D:\a\arrow\arrow\cpp\src\arrow\adapters\orc\adapter_test.cc(54): warning C4244: 'initializing': conversion from 'double' to 'int64_t', possible loss of data
D:\a\arrow\arrow\cpp\src\arrow\adapters\orc\adapter_test.cc(54): warning C4244: 'initializing': conversion from 'double' to 'const int64_t', possible loss of data
D:\a\arrow\arrow\cpp\src\arrow\adapters\orc\adapter_test.cc(60): error C2061: syntax error: identifier 'ssize_t'
D:\a\arrow\arrow\cpp\src\arrow\adapters\orc\adapter_test.cc(61): error C2065: 'capacity': undeclared identifier
D:\a\arrow\arrow\cpp\src\arrow\adapters\orc\adapter_test.cc(89): error C2131: expression did not evaluate to a constant
D:\a\arrow\arrow\cpp\src\arrow\adapters\orc\adapter_test.cc(89): note: failure was caused by a read of a variable outside its lifetime
D:\a\arrow\arrow\cpp\src\arrow\adapters\orc\adapter_test.cc(89): note: see usage of 'length'
D:\a\arrow\arrow\cpp\src\arrow\adapters\orc\adapter_test.cc(92): error C3863: array type 'int32_t [length]' is not assignable
D:\a\arrow\arrow\cpp\src\arrow\adapters\orc\adapter_test.cc(292): error C2664: 'arrow::MemoryOutputStream::MemoryOutputStream(const arrow::MemoryOutputStream &)': cannot convert argument 1 from 'const int' to 'const arrow::MemoryOutputStream &'
D:\a\arrow\arrow\cpp\src\arrow\adapters\orc\adapter_test.cc(292): note: Reason: cannot convert from 'const int' to 'const arrow::MemoryOutputStream'
D:\a\arrow\arrow\cpp\src\arrow\adapters\orc\adapter_test.cc(292): note: No constructor could take the source type, or constructor overload resolution was ambiguous
D:\a\arrow\arrow\cpp\src\arrow\adapters\orc\adapter_test.cc(84): note: see declaration of 'arrow::MemoryOutputStream::MemoryOutputStream'
Could you fix them? Or do you want me to fix them?
I can try to fix them, at least for the easy ones. How's that these warnings did not fire before ? It is specific to Visual C++ ? |
I think so. |
Could you apply this for the AppVeyor job failure? diff --git a/ci/conda_env_cpp.txt b/ci/conda_env_cpp.txt
index dd313f19d7..40871f560d 100644
--- a/ci/conda_env_cpp.txt
+++ b/ci/conda_env_cpp.txt
@@ -37,6 +37,7 @@ ninja
# Required by google-cloud-cpp, the Conda package is missing the dependency:
# https://github.com/conda-forge/google-cloud-cpp-feedstock/issues/28
nlohmann_json
+orc
pkg-config
python
rapidjson https://ci.appveyor.com/project/ApacheSoftwareFoundation/arrow/builds/44867881#L740
|
I made the changes for the AppVeyor job failure. However this still does not work because it can't find a correct version of protobuf (from looking at the logs). |
cf1b358
to
c3aff64
Compare
I've fixed CI failures on AppVeyor and GitHub Actions. I'll merge this tomorrow if nobody objects it. |
Co-authored-by: Sutou Kouhei <kou@cozmixng.org>
90338ab
to
f5ede2c
Compare
@github-actions crossbow submit java-jars |
This comment was marked as outdated.
This comment was marked as outdated.
@github-actions crossbow submit java-jars |
Revision: b7b3f6e Submitted crossbow builds: ursacomputing/crossbow @ actions-e1f7effb08
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1
@davisusanibar FYI: This PR also enables ORC for Windows' JNI.
Benchmark runs are scheduled for baseline = f0cf5c2 and contender = aeba616. aeba616 is a master commit associated with this PR. Results will be available as each benchmark for each run completes. |
['Python', 'R'] benchmarks have high level of regressions. |
No description provided.