You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2022/04/04 17:07:41 UTC
[GitHub] [arrow] mcraealex-bq opened a new pull request, #12789: MINOR: [C++] Fix flightsql static build on windows
mcraealex-bq opened a new pull request, #12789:
URL: https://github.com/apache/arrow/pull/12789
* Removes non-std initializer list
* Sets ARROW_FLIGHT_STATIC to match ARROW_STATIC when ARROW_BUILD_STATIC
and WIN32
* Updates the documentation arrow staticly linking on windows to include
the ARROW_FLIGHT_STATIC definition, including an example
The goal of this is to allow flightsql to build easily on Windows.
--
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-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] lidavidm commented on a diff in pull request #12789: ARROW-16128: [C++][FlightRPC] Fix Flight SQL static build on Windows
Posted by GitBox <gi...@apache.org>.
lidavidm commented on code in PR #12789:
URL: https://github.com/apache/arrow/pull/12789#discussion_r843103573
##########
cpp/src/arrow/flight/sql/client.cc:
##########
@@ -369,8 +369,8 @@ arrow::Result<int64_t> PreparedStatement::ExecuteUpdate() {
} else {
const std::shared_ptr<Schema> schema = arrow::schema({});
ARROW_RETURN_NOT_OK(client_->DoPut(options_, descriptor, schema, &writer, &reader));
- const auto& record_batch =
- arrow::RecordBatch::Make(schema, 0, (std::vector<std::shared_ptr<Array>>){});
+ const ArrayVector columns = std::vector<std::shared_ptr<Array>>{};
Review Comment:
(There should be no need for the explicit initializer, FWIW - default-initializing it should be good enough.)
```suggestion
const ArrayVector columns;
```
--
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-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] ursabot commented on pull request #12789: ARROW-16128: [C++][FlightRPC] Fix Flight SQL static build on Windows
Posted by GitBox <gi...@apache.org>.
ursabot commented on PR #12789:
URL: https://github.com/apache/arrow/pull/12789#issuecomment-1093285267
Benchmark runs are scheduled for baseline = aa0d96b12b39493c5f73e145e852552009623ae6 and contender = b7f7bad55a172b416c99640003a2e0eb9414c6c9. b7f7bad55a172b416c99640003a2e0eb9414c6c9 is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
Conbench compare runs links:
[Finished :arrow_down:0.0% :arrow_up:0.0%] [ec2-t3-xlarge-us-east-2](https://conbench.ursa.dev/compare/runs/8a32487f811a4082afe45bef84c0b567...6182414d953f443584928f6ba50341a2/)
[Finished :arrow_down:0.0% :arrow_up:0.08%] [test-mac-arm](https://conbench.ursa.dev/compare/runs/75ff3ff904c445a5a9a6a3a8b29fb172...4cc9ca65c3964de2a290b3b601f55d06/)
[Failed :arrow_down:0.0% :arrow_up:0.0%] [ursa-i9-9960x](https://conbench.ursa.dev/compare/runs/8ba0b80988514fc796ed04121473acc4...a7ede7e4e7724d68bf352352a4db7bdc/)
[Finished :arrow_down:0.0% :arrow_up:0.0%] [ursa-thinkcentre-m75q](https://conbench.ursa.dev/compare/runs/85bcc8edbf3e4c3590e983211dffc998...d16404951fa54850a3870e55ab9cddec/)
Buildkite builds:
[Finished] <https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ec2-t3-xlarge-us-east-2/builds/471| `b7f7bad5` ec2-t3-xlarge-us-east-2>
[Finished] <https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-test-mac-arm/builds/456| `b7f7bad5` test-mac-arm>
[Failed] <https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-i9-9960x/builds/457| `b7f7bad5` ursa-i9-9960x>
[Finished] <https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-thinkcentre-m75q/builds/466| `b7f7bad5` ursa-thinkcentre-m75q>
[Finished] <https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ec2-t3-xlarge-us-east-2/builds/470| `aa0d96b1` ec2-t3-xlarge-us-east-2>
[Finished] <https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-test-mac-arm/builds/455| `aa0d96b1` test-mac-arm>
[Finished] <https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-i9-9960x/builds/456| `aa0d96b1` ursa-i9-9960x>
[Finished] <https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-thinkcentre-m75q/builds/465| `aa0d96b1` ursa-thinkcentre-m75q>
Supported benchmarks:
ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python, R. Runs only benchmarks with cloud = True
test-mac-arm: Supported benchmark langs: C++, Python, R
ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java
--
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-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] lidavidm commented on a diff in pull request #12789: ARROW-16128: [C++][FlightRPC] Fix Flight SQL static build on Windows
Posted by GitBox <gi...@apache.org>.
lidavidm commented on code in PR #12789:
URL: https://github.com/apache/arrow/pull/12789#discussion_r843045927
##########
docs/source/developers/cpp/windows.rst:
##########
@@ -362,6 +362,20 @@ suppress dllimport/dllexport marking of symbols. Projects that statically link
against Arrow on Windows additionally need this definition. The Unix builds do
not use the macro.
+In addition if using ``-DARROW_FLIGHT=ON``, ``ARROW_FLIGHT_STATIC`` needs to
+be defined.
+
+.. code-block:: cmake
+
+ project(MyExample)
+
+ find_package(Arrow REQUIRED)
+
+ add_executable(my_example my_example.cc)
+ target_link_libraries(my_example PRIVATE arrow_static arrow_flight_static)
+
+ add_compile_definitions(ARROW_STATIC ARROW_FLIGHT_STATIC)
Review Comment:
It's more idiomatic to use `target_compile_definitions` right?
##########
cpp/src/arrow/flight/sql/client.cc:
##########
@@ -369,8 +369,9 @@ arrow::Result<int64_t> PreparedStatement::ExecuteUpdate() {
} else {
const std::shared_ptr<Schema> schema = arrow::schema({});
ARROW_RETURN_NOT_OK(client_->DoPut(options_, descriptor, schema, &writer, &reader));
+ const auto& columns = std::vector<std::shared_ptr<Array>>{};
Review Comment:
nit (assuming `arrow/type_fwd.h` is included):
```suggestion
ArrayVector columns;
```
--
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-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] kou commented on pull request #12789: MINOR: [C++] Fix flightsql static build on windows
Posted by GitBox <gi...@apache.org>.
kou commented on PR #12789:
URL: https://github.com/apache/arrow/pull/12789#issuecomment-1087989083
Could you open a JIRA issue and replace `MINOR:` in the title of this pull request with `ARROW-XXX` because this is not a MINOR change?
See also: https://github.com/apache/arrow/blob/master/CONTRIBUTING.md#minor-fixes
--
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-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] mcraealex-bq commented on pull request #12789: ARROW-16128: [C++][FlightRPC] Fix Flight SQL static build on Windows
Posted by GitBox <gi...@apache.org>.
mcraealex-bq commented on PR #12789:
URL: https://github.com/apache/arrow/pull/12789#issuecomment-1089106982
My understanding is that after approval I should squash and rename the base commit to align with the pull request name. Is that correct?
--
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-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] lidavidm closed pull request #12789: ARROW-16128: [C++][FlightRPC] Fix Flight SQL static build on Windows
Posted by GitBox <gi...@apache.org>.
lidavidm closed pull request #12789: ARROW-16128: [C++][FlightRPC] Fix Flight SQL static build on Windows
URL: https://github.com/apache/arrow/pull/12789
--
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-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] github-actions[bot] commented on pull request #12789: ARROW-16128: [C++] Fix flightsql static build on windows
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #12789:
URL: https://github.com/apache/arrow/pull/12789#issuecomment-1089011292
https://issues.apache.org/jira/browse/ARROW-16128
--
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-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] github-actions[bot] commented on pull request #12789: ARROW-16128: [C++] Fix flightsql static build on windows
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #12789:
URL: https://github.com/apache/arrow/pull/12789#issuecomment-1089011360
:warning: Ticket **has not been started in JIRA**, please click 'Start Progress'.
--
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-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] lidavidm commented on pull request #12789: ARROW-16128: [C++][FlightRPC] Fix Flight SQL static build on Windows
Posted by GitBox <gi...@apache.org>.
lidavidm commented on PR #12789:
URL: https://github.com/apache/arrow/pull/12789#issuecomment-1089107851
The merge script will take care of that.
--
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-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org