kou commented on code in PR #40939: URL: https://github.com/apache/arrow/pull/40939#discussion_r2083290457
########## cpp/src/arrow/flight/sql/odbc/CMakeLists.txt: ########## @@ -0,0 +1,32 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +cmake_minimum_required(VERSION 3.16) Review Comment: We don't need this because we have this in the top-level `CMakeLists.txt`. ########## cpp/src/arrow/flight/sql/odbc/CMakeLists.txt: ########## @@ -0,0 +1,32 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +cmake_minimum_required(VERSION 3.16) +set(CMAKE_CXX_STANDARD 17) + +add_custom_target(arrow_flight_sql_odbc) + +if(CMAKE_BUILD_TYPE STREQUAL "Release") + add_compile_definitions(NDEBUG) +endif() + +# Add Boost dependencies. Should be pre-installed (Brew on Mac). +find_package(Boost REQUIRED) Review Comment: Could you remove this and update https://github.com/apache/arrow/blob/598938711a8376cbfdceaf5c77ab0fd5057e6c02/cpp/cmake_modules/ThirdpartyToolchain.cmake#L1251-L1273 instead? BTW, do we really need Boost? For example, `boost/optional.h` can be replaced with `std::optional` in C++17. ########## cpp/src/arrow/flight/sql/odbc/flight_sql/CMakeLists.txt: ########## @@ -0,0 +1,176 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +cmake_minimum_required(VERSION 3.16) +set(CMAKE_CXX_STANDARD 17) + +set(CMAKE_POSITION_INDEPENDENT_CODE ON) + +include_directories(include include/flight_sql + ${CMAKE_SOURCE_DIR}/odbcabstraction/include) + +if(DEFINED CMAKE_TOOLCHAIN_FILE) + include(${CMAKE_TOOLCHAIN_FILE}) +endif() + +find_package(RapidJSON CONFIG REQUIRED) + +if(MSVC) + # the following definitions stop arrow from using __declspec when statically + # linking and will break on Windows without them + add_compile_definitions(ARROW_STATIC ARROW_FLIGHT_STATIC) +endif() + +enable_testing() + +add_library(arrow_odbc_spi_impl + include/flight_sql/flight_sql_driver.h + accessors/binary_array_accessor.cc + accessors/binary_array_accessor.h + accessors/boolean_array_accessor.cc + accessors/boolean_array_accessor.h + accessors/common.h + accessors/date_array_accessor.cc + accessors/date_array_accessor.h + accessors/decimal_array_accessor.cc + accessors/decimal_array_accessor.h + accessors/main.h + accessors/primitive_array_accessor.cc + accessors/primitive_array_accessor.h + accessors/string_array_accessor.cc + accessors/string_array_accessor.h + accessors/time_array_accessor.cc + accessors/time_array_accessor.h + accessors/timestamp_array_accessor.cc + accessors/timestamp_array_accessor.h + address_info.cc + address_info.h + flight_sql_auth_method.cc + flight_sql_auth_method.h + flight_sql_connection.cc + flight_sql_connection.h + flight_sql_driver.cc + flight_sql_get_tables_reader.cc + flight_sql_get_tables_reader.h + flight_sql_get_type_info_reader.cc + flight_sql_get_type_info_reader.h + flight_sql_result_set.cc + flight_sql_result_set.h + flight_sql_result_set_accessors.cc + flight_sql_result_set_accessors.h + flight_sql_result_set_column.cc + flight_sql_result_set_column.h + flight_sql_result_set_metadata.cc + flight_sql_result_set_metadata.h + flight_sql_ssl_config.cc + flight_sql_ssl_config.h + flight_sql_statement.cc + flight_sql_statement.h + flight_sql_statement_get_columns.cc + flight_sql_statement_get_columns.h + flight_sql_statement_get_tables.cc + flight_sql_statement_get_tables.h + flight_sql_statement_get_type_info.cc + flight_sql_statement_get_type_info.h + flight_sql_stream_chunk_buffer.cc + flight_sql_stream_chunk_buffer.h + get_info_cache.cc + get_info_cache.h + json_converter.cc + json_converter.h + record_batch_transformer.cc + record_batch_transformer.h + scalar_function_reporter.cc + scalar_function_reporter.h + system_trust_store.cc + system_trust_store.h + utils.cc) +target_include_directories(arrow_odbc_spi_impl PUBLIC ${CMAKE_CURRENT_LIST_DIR}) + +if(WIN32) + target_sources(arrow_odbc_spi_impl + PRIVATE include/flight_sql/config/configuration.h + include/flight_sql/config/connection_string_parser.h + include/flight_sql/ui/add_property_window.h + include/flight_sql/ui/custom_window.h + include/flight_sql/ui/dsn_configuration_window.h + include/flight_sql/ui/window.h + config/configuration.cc + config/connection_string_parser.cc + ui/custom_window.cc + ui/window.cc + ui/dsn_configuration_window.cc + ui/add_property_window.cc + system_dsn.cc) +endif() + +find_package(ArrowFlightSql) + +target_link_libraries(arrow_odbc_spi_impl PUBLIC odbcabstraction arrow_flight_sql_shared) + +if(MSVC) + set(CMAKE_CXX_FLAGS_RELEASE "/MD") + set(CMAKE_CXX_FLAGS_DEBUG "/MDd") + # else + # target_link_libraries(arrow_odbc_spi_impl PUBLIC ArrowFlightSql::arrow_flight_sql_static) +endif() + +# set(ARROW_ODBC_SPI_THIRDPARTY_LIBS +# ${ARROW_LIBS} gRPC::grpc++ ${ZLIB_LIBRARIES} ${Protobuf_LIBRARIES} +# ${OPENSSL_LIBRARIES} ${RapidJSON_LIBRARIES}) Review Comment: Can we remove this? ########## cpp/CMakePresets.json: ########## @@ -122,6 +122,14 @@ "ARROW_FLIGHT_SQL": "ON" } }, + { + "name": "features-flight-sql-odbc", + "inherits": "features-flight-sql", + "hidden": true, + "cacheVariables": { + "ARROW_FLIGHT_SQL_ODBC": "ON" + } + }, Review Comment: Could you remove this and add only `"ARROW_FLIGHT_SQL_ODBC": "ON"` to `features-maximal` instead? ########## cpp/src/arrow/flight/sql/odbc/CMakeLists.txt: ########## @@ -0,0 +1,32 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +cmake_minimum_required(VERSION 3.16) +set(CMAKE_CXX_STANDARD 17) + +add_custom_target(arrow_flight_sql_odbc) + +if(CMAKE_BUILD_TYPE STREQUAL "Release") + add_compile_definitions(NDEBUG) +endif() Review Comment: Could you remove this? CMake does this by default. ########## cpp/src/arrow/flight/sql/odbc/flight_sql/CMakeLists.txt: ########## @@ -0,0 +1,176 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +cmake_minimum_required(VERSION 3.16) +set(CMAKE_CXX_STANDARD 17) + +set(CMAKE_POSITION_INDEPENDENT_CODE ON) Review Comment: This is enabled by default: https://github.com/apache/arrow/blob/598938711a8376cbfdceaf5c77ab0fd5057e6c02/cpp/cmake_modules/DefineOptions.cmake#L161-L162 Do we need this? ########## cpp/src/arrow/flight/sql/odbc/flight_sql/CMakeLists.txt: ########## @@ -0,0 +1,176 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +cmake_minimum_required(VERSION 3.16) +set(CMAKE_CXX_STANDARD 17) + +set(CMAKE_POSITION_INDEPENDENT_CODE ON) + +include_directories(include include/flight_sql + ${CMAKE_SOURCE_DIR}/odbcabstraction/include) + +if(DEFINED CMAKE_TOOLCHAIN_FILE) + include(${CMAKE_TOOLCHAIN_FILE}) +endif() Review Comment: Can we remove this because this is not a top-level `CMakeLists.txt`? ########## cpp/src/arrow/flight/sql/odbc/CMakeLists.txt: ########## @@ -0,0 +1,32 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +cmake_minimum_required(VERSION 3.16) +set(CMAKE_CXX_STANDARD 17) Review Comment: Could you remove this because this is configured in other place? ########## cpp/build-support/lint_cpp_cli.py: ########## @@ -77,6 +77,9 @@ def lint_file(path): EXCLUSIONS = _paths('''\ arrow/arrow-config.cmake + arrow/flight/sql/odbc/flight_sql/get_info_cache.h + arrow/flight/sql/odbc/odbcabstraction/include/odbcabstraction/blocking_queue.h + arrow/flight/sql/odbc/odbcabstraction/include/odbcabstraction/odbc_impl/odbc_handle.h Review Comment: Why do we need them? Are they copied from somewhere? ########## cpp/src/arrow/flight/sql/odbc/CMakeLists.txt: ########## @@ -0,0 +1,32 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +cmake_minimum_required(VERSION 3.16) +set(CMAKE_CXX_STANDARD 17) + +add_custom_target(arrow_flight_sql_odbc) + +if(CMAKE_BUILD_TYPE STREQUAL "Release") + add_compile_definitions(NDEBUG) +endif() + +# Add Boost dependencies. Should be pre-installed (Brew on Mac). +find_package(Boost REQUIRED) +find_package(ODBC REQUIRED) Review Comment: Could you do this in `cpp/cmake_modules/ThirdpartyToolchain.cmake` instead? ########## cpp/src/arrow/flight/sql/odbc/flight_sql/CMakeLists.txt: ########## @@ -0,0 +1,176 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +cmake_minimum_required(VERSION 3.16) +set(CMAKE_CXX_STANDARD 17) Review Comment: Ditto. ########## cpp/src/arrow/flight/sql/odbc/flight_sql/CMakeLists.txt: ########## @@ -0,0 +1,176 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +cmake_minimum_required(VERSION 3.16) +set(CMAKE_CXX_STANDARD 17) + +set(CMAKE_POSITION_INDEPENDENT_CODE ON) + +include_directories(include include/flight_sql + ${CMAKE_SOURCE_DIR}/odbcabstraction/include) + +if(DEFINED CMAKE_TOOLCHAIN_FILE) + include(${CMAKE_TOOLCHAIN_FILE}) +endif() + +find_package(RapidJSON CONFIG REQUIRED) + +if(MSVC) + # the following definitions stop arrow from using __declspec when statically + # linking and will break on Windows without them + add_compile_definitions(ARROW_STATIC ARROW_FLIGHT_STATIC) +endif() Review Comment: We don't need this because `arrow_static`/`arrow_flight_static` CMake targets do this automatically. ########## cpp/src/arrow/flight/sql/odbc/flight_sql/CMakeLists.txt: ########## @@ -0,0 +1,176 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +cmake_minimum_required(VERSION 3.16) +set(CMAKE_CXX_STANDARD 17) + +set(CMAKE_POSITION_INDEPENDENT_CODE ON) + +include_directories(include include/flight_sql + ${CMAKE_SOURCE_DIR}/odbcabstraction/include) + +if(DEFINED CMAKE_TOOLCHAIN_FILE) + include(${CMAKE_TOOLCHAIN_FILE}) +endif() + +find_package(RapidJSON CONFIG REQUIRED) + +if(MSVC) + # the following definitions stop arrow from using __declspec when statically + # linking and will break on Windows without them + add_compile_definitions(ARROW_STATIC ARROW_FLIGHT_STATIC) +endif() + +enable_testing() + +add_library(arrow_odbc_spi_impl + include/flight_sql/flight_sql_driver.h + accessors/binary_array_accessor.cc + accessors/binary_array_accessor.h + accessors/boolean_array_accessor.cc + accessors/boolean_array_accessor.h + accessors/common.h + accessors/date_array_accessor.cc + accessors/date_array_accessor.h + accessors/decimal_array_accessor.cc + accessors/decimal_array_accessor.h + accessors/main.h + accessors/primitive_array_accessor.cc + accessors/primitive_array_accessor.h + accessors/string_array_accessor.cc + accessors/string_array_accessor.h + accessors/time_array_accessor.cc + accessors/time_array_accessor.h + accessors/timestamp_array_accessor.cc + accessors/timestamp_array_accessor.h + address_info.cc + address_info.h + flight_sql_auth_method.cc + flight_sql_auth_method.h + flight_sql_connection.cc + flight_sql_connection.h + flight_sql_driver.cc + flight_sql_get_tables_reader.cc + flight_sql_get_tables_reader.h + flight_sql_get_type_info_reader.cc + flight_sql_get_type_info_reader.h + flight_sql_result_set.cc + flight_sql_result_set.h + flight_sql_result_set_accessors.cc + flight_sql_result_set_accessors.h + flight_sql_result_set_column.cc + flight_sql_result_set_column.h + flight_sql_result_set_metadata.cc + flight_sql_result_set_metadata.h + flight_sql_ssl_config.cc + flight_sql_ssl_config.h + flight_sql_statement.cc + flight_sql_statement.h + flight_sql_statement_get_columns.cc + flight_sql_statement_get_columns.h + flight_sql_statement_get_tables.cc + flight_sql_statement_get_tables.h + flight_sql_statement_get_type_info.cc + flight_sql_statement_get_type_info.h + flight_sql_stream_chunk_buffer.cc + flight_sql_stream_chunk_buffer.h + get_info_cache.cc + get_info_cache.h + json_converter.cc + json_converter.h + record_batch_transformer.cc + record_batch_transformer.h + scalar_function_reporter.cc + scalar_function_reporter.h + system_trust_store.cc + system_trust_store.h + utils.cc) +target_include_directories(arrow_odbc_spi_impl PUBLIC ${CMAKE_CURRENT_LIST_DIR}) + +if(WIN32) + target_sources(arrow_odbc_spi_impl + PRIVATE include/flight_sql/config/configuration.h + include/flight_sql/config/connection_string_parser.h + include/flight_sql/ui/add_property_window.h + include/flight_sql/ui/custom_window.h + include/flight_sql/ui/dsn_configuration_window.h + include/flight_sql/ui/window.h + config/configuration.cc + config/connection_string_parser.cc + ui/custom_window.cc + ui/window.cc + ui/dsn_configuration_window.cc + ui/add_property_window.cc + system_dsn.cc) +endif() + +find_package(ArrowFlightSql) Review Comment: We don't need this, right? ########## cpp/src/arrow/flight/sql/odbc/flight_sql/CMakeLists.txt: ########## @@ -0,0 +1,176 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +cmake_minimum_required(VERSION 3.16) +set(CMAKE_CXX_STANDARD 17) + +set(CMAKE_POSITION_INDEPENDENT_CODE ON) + +include_directories(include include/flight_sql + ${CMAKE_SOURCE_DIR}/odbcabstraction/include) Review Comment: Could you use target based CMake API (`target_include_directories()`) instead of traditional global CMake API? ########## cpp/src/arrow/flight/sql/odbc/flight_sql/CMakeLists.txt: ########## @@ -0,0 +1,176 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +cmake_minimum_required(VERSION 3.16) +set(CMAKE_CXX_STANDARD 17) + +set(CMAKE_POSITION_INDEPENDENT_CODE ON) + +include_directories(include include/flight_sql + ${CMAKE_SOURCE_DIR}/odbcabstraction/include) + +if(DEFINED CMAKE_TOOLCHAIN_FILE) + include(${CMAKE_TOOLCHAIN_FILE}) +endif() + +find_package(RapidJSON CONFIG REQUIRED) + +if(MSVC) + # the following definitions stop arrow from using __declspec when statically + # linking and will break on Windows without them + add_compile_definitions(ARROW_STATIC ARROW_FLIGHT_STATIC) +endif() + +enable_testing() + +add_library(arrow_odbc_spi_impl + include/flight_sql/flight_sql_driver.h + accessors/binary_array_accessor.cc + accessors/binary_array_accessor.h + accessors/boolean_array_accessor.cc + accessors/boolean_array_accessor.h + accessors/common.h + accessors/date_array_accessor.cc + accessors/date_array_accessor.h + accessors/decimal_array_accessor.cc + accessors/decimal_array_accessor.h + accessors/main.h + accessors/primitive_array_accessor.cc + accessors/primitive_array_accessor.h + accessors/string_array_accessor.cc + accessors/string_array_accessor.h + accessors/time_array_accessor.cc + accessors/time_array_accessor.h + accessors/timestamp_array_accessor.cc + accessors/timestamp_array_accessor.h + address_info.cc + address_info.h + flight_sql_auth_method.cc + flight_sql_auth_method.h + flight_sql_connection.cc + flight_sql_connection.h + flight_sql_driver.cc + flight_sql_get_tables_reader.cc + flight_sql_get_tables_reader.h + flight_sql_get_type_info_reader.cc + flight_sql_get_type_info_reader.h + flight_sql_result_set.cc + flight_sql_result_set.h + flight_sql_result_set_accessors.cc + flight_sql_result_set_accessors.h + flight_sql_result_set_column.cc + flight_sql_result_set_column.h + flight_sql_result_set_metadata.cc + flight_sql_result_set_metadata.h + flight_sql_ssl_config.cc + flight_sql_ssl_config.h + flight_sql_statement.cc + flight_sql_statement.h + flight_sql_statement_get_columns.cc + flight_sql_statement_get_columns.h + flight_sql_statement_get_tables.cc + flight_sql_statement_get_tables.h + flight_sql_statement_get_type_info.cc + flight_sql_statement_get_type_info.h + flight_sql_stream_chunk_buffer.cc + flight_sql_stream_chunk_buffer.h + get_info_cache.cc + get_info_cache.h + json_converter.cc + json_converter.h + record_batch_transformer.cc + record_batch_transformer.h + scalar_function_reporter.cc + scalar_function_reporter.h + system_trust_store.cc + system_trust_store.h + utils.cc) +target_include_directories(arrow_odbc_spi_impl PUBLIC ${CMAKE_CURRENT_LIST_DIR}) + +if(WIN32) + target_sources(arrow_odbc_spi_impl + PRIVATE include/flight_sql/config/configuration.h + include/flight_sql/config/connection_string_parser.h + include/flight_sql/ui/add_property_window.h + include/flight_sql/ui/custom_window.h + include/flight_sql/ui/dsn_configuration_window.h + include/flight_sql/ui/window.h + config/configuration.cc + config/connection_string_parser.cc + ui/custom_window.cc + ui/window.cc + ui/dsn_configuration_window.cc + ui/add_property_window.cc + system_dsn.cc) +endif() + +find_package(ArrowFlightSql) + +target_link_libraries(arrow_odbc_spi_impl PUBLIC odbcabstraction arrow_flight_sql_shared) + +if(MSVC) + set(CMAKE_CXX_FLAGS_RELEASE "/MD") + set(CMAKE_CXX_FLAGS_DEBUG "/MDd") + # else + # target_link_libraries(arrow_odbc_spi_impl PUBLIC ArrowFlightSql::arrow_flight_sql_static) +endif() Review Comment: Do we need this? I think that they are default. ########## cpp/src/arrow/flight/sql/odbc/flight_sql/CMakeLists.txt: ########## @@ -0,0 +1,176 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +cmake_minimum_required(VERSION 3.16) +set(CMAKE_CXX_STANDARD 17) + +set(CMAKE_POSITION_INDEPENDENT_CODE ON) + +include_directories(include include/flight_sql + ${CMAKE_SOURCE_DIR}/odbcabstraction/include) + +if(DEFINED CMAKE_TOOLCHAIN_FILE) + include(${CMAKE_TOOLCHAIN_FILE}) +endif() + +find_package(RapidJSON CONFIG REQUIRED) Review Comment: Could you update https://github.com/apache/arrow/blob/598938711a8376cbfdceaf5c77ab0fd5057e6c02/cpp/cmake_modules/ThirdpartyToolchain.cmake#L374-L434 instead of this? ########## cpp/src/arrow/flight/sql/odbc/flight_sql/CMakeLists.txt: ########## @@ -0,0 +1,176 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +cmake_minimum_required(VERSION 3.16) +set(CMAKE_CXX_STANDARD 17) + +set(CMAKE_POSITION_INDEPENDENT_CODE ON) + +include_directories(include include/flight_sql + ${CMAKE_SOURCE_DIR}/odbcabstraction/include) + +if(DEFINED CMAKE_TOOLCHAIN_FILE) + include(${CMAKE_TOOLCHAIN_FILE}) +endif() + +find_package(RapidJSON CONFIG REQUIRED) + +if(MSVC) + # the following definitions stop arrow from using __declspec when statically + # linking and will break on Windows without them + add_compile_definitions(ARROW_STATIC ARROW_FLIGHT_STATIC) +endif() + +enable_testing() Review Comment: Unfortunately, Apache Arrow C++ doesn't use CMake's standard testing mechanism for now... Could you remove this for now? ########## cpp/src/arrow/flight/sql/odbc/flight_sql/CMakeLists.txt: ########## @@ -0,0 +1,176 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +cmake_minimum_required(VERSION 3.16) +set(CMAKE_CXX_STANDARD 17) + +set(CMAKE_POSITION_INDEPENDENT_CODE ON) + +include_directories(include include/flight_sql + ${CMAKE_SOURCE_DIR}/odbcabstraction/include) + +if(DEFINED CMAKE_TOOLCHAIN_FILE) + include(${CMAKE_TOOLCHAIN_FILE}) +endif() + +find_package(RapidJSON CONFIG REQUIRED) + +if(MSVC) + # the following definitions stop arrow from using __declspec when statically + # linking and will break on Windows without them + add_compile_definitions(ARROW_STATIC ARROW_FLIGHT_STATIC) +endif() + +enable_testing() + +add_library(arrow_odbc_spi_impl + include/flight_sql/flight_sql_driver.h + accessors/binary_array_accessor.cc + accessors/binary_array_accessor.h + accessors/boolean_array_accessor.cc + accessors/boolean_array_accessor.h + accessors/common.h + accessors/date_array_accessor.cc + accessors/date_array_accessor.h + accessors/decimal_array_accessor.cc + accessors/decimal_array_accessor.h + accessors/main.h + accessors/primitive_array_accessor.cc + accessors/primitive_array_accessor.h + accessors/string_array_accessor.cc + accessors/string_array_accessor.h + accessors/time_array_accessor.cc + accessors/time_array_accessor.h + accessors/timestamp_array_accessor.cc + accessors/timestamp_array_accessor.h + address_info.cc + address_info.h + flight_sql_auth_method.cc + flight_sql_auth_method.h + flight_sql_connection.cc + flight_sql_connection.h + flight_sql_driver.cc + flight_sql_get_tables_reader.cc + flight_sql_get_tables_reader.h + flight_sql_get_type_info_reader.cc + flight_sql_get_type_info_reader.h + flight_sql_result_set.cc + flight_sql_result_set.h + flight_sql_result_set_accessors.cc + flight_sql_result_set_accessors.h + flight_sql_result_set_column.cc + flight_sql_result_set_column.h + flight_sql_result_set_metadata.cc + flight_sql_result_set_metadata.h + flight_sql_ssl_config.cc + flight_sql_ssl_config.h + flight_sql_statement.cc + flight_sql_statement.h + flight_sql_statement_get_columns.cc + flight_sql_statement_get_columns.h + flight_sql_statement_get_tables.cc + flight_sql_statement_get_tables.h + flight_sql_statement_get_type_info.cc + flight_sql_statement_get_type_info.h + flight_sql_stream_chunk_buffer.cc + flight_sql_stream_chunk_buffer.h + get_info_cache.cc + get_info_cache.h + json_converter.cc + json_converter.h + record_batch_transformer.cc + record_batch_transformer.h + scalar_function_reporter.cc + scalar_function_reporter.h + system_trust_store.cc + system_trust_store.h + utils.cc) +target_include_directories(arrow_odbc_spi_impl PUBLIC ${CMAKE_CURRENT_LIST_DIR}) + +if(WIN32) + target_sources(arrow_odbc_spi_impl + PRIVATE include/flight_sql/config/configuration.h + include/flight_sql/config/connection_string_parser.h + include/flight_sql/ui/add_property_window.h + include/flight_sql/ui/custom_window.h + include/flight_sql/ui/dsn_configuration_window.h + include/flight_sql/ui/window.h + config/configuration.cc + config/connection_string_parser.cc + ui/custom_window.cc + ui/window.cc + ui/dsn_configuration_window.cc + ui/add_property_window.cc + system_dsn.cc) +endif() + +find_package(ArrowFlightSql) + +target_link_libraries(arrow_odbc_spi_impl PUBLIC odbcabstraction arrow_flight_sql_shared) + +if(MSVC) + set(CMAKE_CXX_FLAGS_RELEASE "/MD") + set(CMAKE_CXX_FLAGS_DEBUG "/MDd") + # else + # target_link_libraries(arrow_odbc_spi_impl PUBLIC ArrowFlightSql::arrow_flight_sql_static) +endif() + +# set(ARROW_ODBC_SPI_THIRDPARTY_LIBS +# ${ARROW_LIBS} gRPC::grpc++ ${ZLIB_LIBRARIES} ${Protobuf_LIBRARIES} +# ${OPENSSL_LIBRARIES} ${RapidJSON_LIBRARIES}) + +if(MSVC) + find_package(Boost REQUIRED COMPONENTS locale) + target_link_libraries(arrow_odbc_spi_impl PUBLIC Boost::locale) +endif() + +set_target_properties(arrow_odbc_spi_impl + PROPERTIES ARCHIVE_OUTPUT_DIRECTORY + ${CMAKE_BINARY_DIR}/$<CONFIG>/lib + LIBRARY_OUTPUT_DIRECTORY + ${CMAKE_BINARY_DIR}/$<CONFIG>/lib + RUNTIME_OUTPUT_DIRECTORY + ${CMAKE_BINARY_DIR}/$<CONFIG>/lib) Review Comment: We may be able to remove this by `add_arrow_lib()`. ########## cpp/src/arrow/flight/sql/odbc/flight_sql/CMakeLists.txt: ########## @@ -0,0 +1,176 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +cmake_minimum_required(VERSION 3.16) +set(CMAKE_CXX_STANDARD 17) + +set(CMAKE_POSITION_INDEPENDENT_CODE ON) + +include_directories(include include/flight_sql + ${CMAKE_SOURCE_DIR}/odbcabstraction/include) + +if(DEFINED CMAKE_TOOLCHAIN_FILE) + include(${CMAKE_TOOLCHAIN_FILE}) +endif() + +find_package(RapidJSON CONFIG REQUIRED) + +if(MSVC) + # the following definitions stop arrow from using __declspec when statically + # linking and will break on Windows without them + add_compile_definitions(ARROW_STATIC ARROW_FLIGHT_STATIC) +endif() + +enable_testing() + +add_library(arrow_odbc_spi_impl Review Comment: Could you use our `add_arrow_lib()` instead? https://github.com/apache/arrow/blob/598938711a8376cbfdceaf5c77ab0fd5057e6c02/cpp/cmake_modules/BuildUtils.cmake#L190 ########## cpp/vcpkg.json: ########## @@ -16,11 +16,16 @@ }, "benchmark", "boost-cmake", + "boost-beast", Review Comment: Could you keep this list in alphabetical order? ########## cpp/src/arrow/flight/sql/odbc/flight_sql/CMakeLists.txt: ########## @@ -0,0 +1,176 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +cmake_minimum_required(VERSION 3.16) +set(CMAKE_CXX_STANDARD 17) + +set(CMAKE_POSITION_INDEPENDENT_CODE ON) + +include_directories(include include/flight_sql + ${CMAKE_SOURCE_DIR}/odbcabstraction/include) + +if(DEFINED CMAKE_TOOLCHAIN_FILE) + include(${CMAKE_TOOLCHAIN_FILE}) +endif() + +find_package(RapidJSON CONFIG REQUIRED) + +if(MSVC) + # the following definitions stop arrow from using __declspec when statically + # linking and will break on Windows without them + add_compile_definitions(ARROW_STATIC ARROW_FLIGHT_STATIC) +endif() + +enable_testing() + +add_library(arrow_odbc_spi_impl + include/flight_sql/flight_sql_driver.h + accessors/binary_array_accessor.cc + accessors/binary_array_accessor.h + accessors/boolean_array_accessor.cc + accessors/boolean_array_accessor.h + accessors/common.h + accessors/date_array_accessor.cc + accessors/date_array_accessor.h + accessors/decimal_array_accessor.cc + accessors/decimal_array_accessor.h + accessors/main.h + accessors/primitive_array_accessor.cc + accessors/primitive_array_accessor.h + accessors/string_array_accessor.cc + accessors/string_array_accessor.h + accessors/time_array_accessor.cc + accessors/time_array_accessor.h + accessors/timestamp_array_accessor.cc + accessors/timestamp_array_accessor.h + address_info.cc + address_info.h + flight_sql_auth_method.cc + flight_sql_auth_method.h + flight_sql_connection.cc + flight_sql_connection.h + flight_sql_driver.cc + flight_sql_get_tables_reader.cc + flight_sql_get_tables_reader.h + flight_sql_get_type_info_reader.cc + flight_sql_get_type_info_reader.h + flight_sql_result_set.cc + flight_sql_result_set.h + flight_sql_result_set_accessors.cc + flight_sql_result_set_accessors.h + flight_sql_result_set_column.cc + flight_sql_result_set_column.h + flight_sql_result_set_metadata.cc + flight_sql_result_set_metadata.h + flight_sql_ssl_config.cc + flight_sql_ssl_config.h + flight_sql_statement.cc + flight_sql_statement.h + flight_sql_statement_get_columns.cc + flight_sql_statement_get_columns.h + flight_sql_statement_get_tables.cc + flight_sql_statement_get_tables.h + flight_sql_statement_get_type_info.cc + flight_sql_statement_get_type_info.h + flight_sql_stream_chunk_buffer.cc + flight_sql_stream_chunk_buffer.h + get_info_cache.cc + get_info_cache.h + json_converter.cc + json_converter.h + record_batch_transformer.cc + record_batch_transformer.h + scalar_function_reporter.cc + scalar_function_reporter.h + system_trust_store.cc + system_trust_store.h + utils.cc) +target_include_directories(arrow_odbc_spi_impl PUBLIC ${CMAKE_CURRENT_LIST_DIR}) + +if(WIN32) + target_sources(arrow_odbc_spi_impl + PRIVATE include/flight_sql/config/configuration.h + include/flight_sql/config/connection_string_parser.h + include/flight_sql/ui/add_property_window.h + include/flight_sql/ui/custom_window.h + include/flight_sql/ui/dsn_configuration_window.h + include/flight_sql/ui/window.h + config/configuration.cc + config/connection_string_parser.cc + ui/custom_window.cc + ui/window.cc + ui/dsn_configuration_window.cc + ui/add_property_window.cc + system_dsn.cc) +endif() + +find_package(ArrowFlightSql) + +target_link_libraries(arrow_odbc_spi_impl PUBLIC odbcabstraction arrow_flight_sql_shared) + +if(MSVC) + set(CMAKE_CXX_FLAGS_RELEASE "/MD") + set(CMAKE_CXX_FLAGS_DEBUG "/MDd") + # else + # target_link_libraries(arrow_odbc_spi_impl PUBLIC ArrowFlightSql::arrow_flight_sql_static) +endif() + +# set(ARROW_ODBC_SPI_THIRDPARTY_LIBS +# ${ARROW_LIBS} gRPC::grpc++ ${ZLIB_LIBRARIES} ${Protobuf_LIBRARIES} +# ${OPENSSL_LIBRARIES} ${RapidJSON_LIBRARIES}) + +if(MSVC) + find_package(Boost REQUIRED COMPONENTS locale) Review Comment: Could you do this in `ThirdpartyToolchain.cmake`? ########## cpp/src/arrow/flight/sql/odbc/flight_sql/CMakeLists.txt: ########## @@ -0,0 +1,176 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +cmake_minimum_required(VERSION 3.16) +set(CMAKE_CXX_STANDARD 17) + +set(CMAKE_POSITION_INDEPENDENT_CODE ON) + +include_directories(include include/flight_sql + ${CMAKE_SOURCE_DIR}/odbcabstraction/include) + +if(DEFINED CMAKE_TOOLCHAIN_FILE) + include(${CMAKE_TOOLCHAIN_FILE}) +endif() + +find_package(RapidJSON CONFIG REQUIRED) + +if(MSVC) + # the following definitions stop arrow from using __declspec when statically + # linking and will break on Windows without them + add_compile_definitions(ARROW_STATIC ARROW_FLIGHT_STATIC) +endif() + +enable_testing() + +add_library(arrow_odbc_spi_impl + include/flight_sql/flight_sql_driver.h + accessors/binary_array_accessor.cc + accessors/binary_array_accessor.h + accessors/boolean_array_accessor.cc + accessors/boolean_array_accessor.h + accessors/common.h + accessors/date_array_accessor.cc + accessors/date_array_accessor.h + accessors/decimal_array_accessor.cc + accessors/decimal_array_accessor.h + accessors/main.h + accessors/primitive_array_accessor.cc + accessors/primitive_array_accessor.h + accessors/string_array_accessor.cc + accessors/string_array_accessor.h + accessors/time_array_accessor.cc + accessors/time_array_accessor.h + accessors/timestamp_array_accessor.cc + accessors/timestamp_array_accessor.h + address_info.cc + address_info.h + flight_sql_auth_method.cc + flight_sql_auth_method.h + flight_sql_connection.cc + flight_sql_connection.h + flight_sql_driver.cc + flight_sql_get_tables_reader.cc + flight_sql_get_tables_reader.h + flight_sql_get_type_info_reader.cc + flight_sql_get_type_info_reader.h + flight_sql_result_set.cc + flight_sql_result_set.h + flight_sql_result_set_accessors.cc + flight_sql_result_set_accessors.h + flight_sql_result_set_column.cc + flight_sql_result_set_column.h + flight_sql_result_set_metadata.cc + flight_sql_result_set_metadata.h + flight_sql_ssl_config.cc + flight_sql_ssl_config.h + flight_sql_statement.cc + flight_sql_statement.h + flight_sql_statement_get_columns.cc + flight_sql_statement_get_columns.h + flight_sql_statement_get_tables.cc + flight_sql_statement_get_tables.h + flight_sql_statement_get_type_info.cc + flight_sql_statement_get_type_info.h + flight_sql_stream_chunk_buffer.cc + flight_sql_stream_chunk_buffer.h + get_info_cache.cc + get_info_cache.h + json_converter.cc + json_converter.h + record_batch_transformer.cc + record_batch_transformer.h + scalar_function_reporter.cc + scalar_function_reporter.h + system_trust_store.cc + system_trust_store.h + utils.cc) +target_include_directories(arrow_odbc_spi_impl PUBLIC ${CMAKE_CURRENT_LIST_DIR}) + +if(WIN32) + target_sources(arrow_odbc_spi_impl + PRIVATE include/flight_sql/config/configuration.h + include/flight_sql/config/connection_string_parser.h + include/flight_sql/ui/add_property_window.h + include/flight_sql/ui/custom_window.h + include/flight_sql/ui/dsn_configuration_window.h + include/flight_sql/ui/window.h + config/configuration.cc + config/connection_string_parser.cc + ui/custom_window.cc + ui/window.cc + ui/dsn_configuration_window.cc + ui/add_property_window.cc + system_dsn.cc) +endif() + +find_package(ArrowFlightSql) + +target_link_libraries(arrow_odbc_spi_impl PUBLIC odbcabstraction arrow_flight_sql_shared) + +if(MSVC) + set(CMAKE_CXX_FLAGS_RELEASE "/MD") + set(CMAKE_CXX_FLAGS_DEBUG "/MDd") + # else + # target_link_libraries(arrow_odbc_spi_impl PUBLIC ArrowFlightSql::arrow_flight_sql_static) +endif() + +# set(ARROW_ODBC_SPI_THIRDPARTY_LIBS +# ${ARROW_LIBS} gRPC::grpc++ ${ZLIB_LIBRARIES} ${Protobuf_LIBRARIES} +# ${OPENSSL_LIBRARIES} ${RapidJSON_LIBRARIES}) + +if(MSVC) + find_package(Boost REQUIRED COMPONENTS locale) + target_link_libraries(arrow_odbc_spi_impl PUBLIC Boost::locale) +endif() + +set_target_properties(arrow_odbc_spi_impl + PROPERTIES ARCHIVE_OUTPUT_DIRECTORY + ${CMAKE_BINARY_DIR}/$<CONFIG>/lib + LIBRARY_OUTPUT_DIRECTORY + ${CMAKE_BINARY_DIR}/$<CONFIG>/lib + RUNTIME_OUTPUT_DIRECTORY + ${CMAKE_BINARY_DIR}/$<CONFIG>/lib) + +# target_include_directories(arrow_odbc_spi_impl +# PUBLIC ${CMAKE_CURRENT_SOURCE_DIR}) Review Comment: Can we remove 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
