From 21e4bb83b437281b68e955823c7157d9361285a2 Mon Sep 17 00:00:00 2001 From: David Li Date: Fri, 24 Jun 2022 15:18:19 -0400 Subject: [PATCH 1/8] ARROW-16902: [C++][FlightRPC] Fix DLL linkage in Flight SQL --- .github/workflows/cpp.yml | 1 + appveyor.yml | 2 ++ ci/appveyor-cpp-build.bat | 1 + cpp/src/arrow/flight/sql/CMakeLists.txt | 9 +++++ cpp/src/arrow/flight/sql/client.cc | 3 ++ cpp/src/arrow/flight/sql/client.h | 4 +-- cpp/src/arrow/flight/sql/client_test.cc | 3 ++ cpp/src/arrow/flight/sql/column_metadata.h | 5 +-- cpp/src/arrow/flight/sql/server.cc | 3 ++ cpp/src/arrow/flight/sql/server.h | 37 ++++++++++---------- cpp/src/arrow/flight/sql/sql_info_internal.h | 3 +- cpp/src/arrow/flight/sql/types.h | 5 +-- 12 files changed, 51 insertions(+), 25 deletions(-) diff --git a/.github/workflows/cpp.yml b/.github/workflows/cpp.yml index 1d681c96a7579..cbb569074100a 100644 --- a/.github/workflows/cpp.yml +++ b/.github/workflows/cpp.yml @@ -281,6 +281,7 @@ jobs: ARROW_BUILD_TYPE: release ARROW_DATASET: ON ARROW_FLIGHT: ON + ARROW_FLIGHT_SQL: ON ARROW_GANDIVA: ON ARROW_GCS: ON ARROW_HDFS: OFF diff --git a/appveyor.yml b/appveyor.yml index 2699e479b79f9..03a3597c9b7ba 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -46,6 +46,7 @@ environment: CLCACHE_COMPRESS: 1 CLCACHE_COMPRESSLEVEL: 6 ARROW_BUILD_FLIGHT: "OFF" + ARROW_BUILD_FLIGHT_SQL: "OFF" ARROW_BUILD_GANDIVA: "OFF" ARROW_LLVM_VERSION: "7.0.*" ARROW_S3: "OFF" @@ -60,6 +61,7 @@ environment: ARROW_GCS: "ON" ARROW_S3: "ON" ARROW_BUILD_FLIGHT: "ON" + ARROW_BUILD_FLIGHT_SQL: "ON" ARROW_BUILD_GANDIVA: "ON" - JOB: "Build_Debug" GENERATOR: Ninja diff --git a/ci/appveyor-cpp-build.bat b/ci/appveyor-cpp-build.bat index e9441c63e2099..e7fea480b4f43 100644 --- a/ci/appveyor-cpp-build.bat +++ b/ci/appveyor-cpp-build.bat @@ -103,6 +103,7 @@ cmake -G "%GENERATOR%" %CMAKE_ARGS% ^ -DARROW_DATASET=ON ^ -DARROW_ENABLE_TIMING_TESTS=OFF ^ -DARROW_FLIGHT=%ARROW_BUILD_FLIGHT% ^ + -DARROW_FLIGHT_SQL=%ARROW_BUILD_FLIGHT_SQL% ^ -DARROW_GANDIVA=%ARROW_BUILD_GANDIVA% ^ -DARROW_MIMALLOC=ON ^ -DARROW_PARQUET=ON ^ diff --git a/cpp/src/arrow/flight/sql/CMakeLists.txt b/cpp/src/arrow/flight/sql/CMakeLists.txt index 1f5a72b50dad5..62d2c208d0942 100644 --- a/cpp/src/arrow/flight/sql/CMakeLists.txt +++ b/cpp/src/arrow/flight/sql/CMakeLists.txt @@ -33,6 +33,11 @@ add_custom_command(OUTPUT ${FLIGHT_SQL_GENERATED_PROTO_FILES} DEPENDS ${PROTO_DEPENDS}) set_source_files_properties(${FLIGHT_SQL_GENERATED_PROTO_FILES} PROPERTIES GENERATED TRUE) +if(MSVC) + # Suppress warnings caused by Protobuf (casts, dll-interface) + set_source_files_properties(${FLIGHT_SQL_GENERATED_PROTO_FILES} + PROPERTIES COMPILE_FLAGS "/wd4251 /wd4267") +endif() add_custom_target(flight_sql_protobuf_gen ALL DEPENDS ${FLIGHT_SQL_GENERATED_PROTO_FILES}) @@ -63,6 +68,10 @@ add_arrow_lib(arrow_flight_sql PRIVATE_INCLUDES "${Protobuf_INCLUDE_DIRS}") +foreach(LIB_TARGET ${ARROW_FLIGHT_SQL_LIBRARIES}) + target_compile_definitions(${LIB_TARGET} PRIVATE ARROW_FLIGHT_EXPORTING) +endforeach() + if(ARROW_FLIGHT_TEST_LINKAGE STREQUAL "static" AND ARROW_BUILD_STATIC) set(ARROW_FLIGHT_SQL_TEST_LINK_LIBS arrow_flight_sql_static) else() diff --git a/cpp/src/arrow/flight/sql/client.cc b/cpp/src/arrow/flight/sql/client.cc index 89cfb3aad03f6..3ce220491f173 100644 --- a/cpp/src/arrow/flight/sql/client.cc +++ b/cpp/src/arrow/flight/sql/client.cc @@ -15,6 +15,9 @@ // specific language governing permissions and limitations // under the License. +// Platform-specific defines +#include "arrow/flight/platform.h" + #include "arrow/flight/sql/client.h" #include diff --git a/cpp/src/arrow/flight/sql/client.h b/cpp/src/arrow/flight/sql/client.h index 78c162e4cda22..0b83a6edc6243 100644 --- a/cpp/src/arrow/flight/sql/client.h +++ b/cpp/src/arrow/flight/sql/client.h @@ -35,7 +35,7 @@ class PreparedStatement; /// \brief Flight client with Flight SQL semantics. /// /// Wraps a Flight client to provide the Flight SQL RPC calls. -class ARROW_EXPORT FlightSqlClient { +class ARROW_FLIGHT_EXPORT FlightSqlClient { friend class PreparedStatement; private: @@ -202,7 +202,7 @@ class ARROW_EXPORT FlightSqlClient { }; /// \brief A prepared statement that can be executed. -class ARROW_EXPORT PreparedStatement { +class ARROW_FLIGHT_EXPORT PreparedStatement { public: /// \brief Create a new prepared statement. However, applications /// should generally use FlightSqlClient::Prepare. diff --git a/cpp/src/arrow/flight/sql/client_test.cc b/cpp/src/arrow/flight/sql/client_test.cc index 1cfce2520f50d..b29af7c46681d 100644 --- a/cpp/src/arrow/flight/sql/client_test.cc +++ b/cpp/src/arrow/flight/sql/client_test.cc @@ -15,6 +15,9 @@ // specific language governing permissions and limitations // under the License. +// Platform-specific defines +#include "arrow/flight/platform.h" + #include "arrow/flight/client.h" #include diff --git a/cpp/src/arrow/flight/sql/column_metadata.h b/cpp/src/arrow/flight/sql/column_metadata.h index 08e7c64aeec07..7d6c74e012aa2 100644 --- a/cpp/src/arrow/flight/sql/column_metadata.h +++ b/cpp/src/arrow/flight/sql/column_metadata.h @@ -19,6 +19,7 @@ #include +#include "arrow/flight/visibility.h" #include "arrow/util/key_value_metadata.h" namespace arrow { @@ -26,7 +27,7 @@ namespace flight { namespace sql { /// \brief Helper class to set column metadata. -class ColumnMetadata { +class ARROW_FLIGHT_EXPORT ColumnMetadata { private: std::shared_ptr metadata_map_; @@ -114,7 +115,7 @@ class ColumnMetadata { const std::shared_ptr& metadata_map() const; /// \brief A builder class to construct the ColumnMetadata object. - class ColumnMetadataBuilder { + class ARROW_FLIGHT_EXPORT ColumnMetadataBuilder { public: friend class ColumnMetadata; diff --git a/cpp/src/arrow/flight/sql/server.cc b/cpp/src/arrow/flight/sql/server.cc index 5ad1be466e11e..c82146de86da3 100644 --- a/cpp/src/arrow/flight/sql/server.cc +++ b/cpp/src/arrow/flight/sql/server.cc @@ -18,6 +18,9 @@ // Interfaces to use for defining Flight RPC servers. API should be considered // experimental for now +// Platform-specific defines +#include "arrow/flight/platform.h" + #include "arrow/flight/sql/server.h" #include diff --git a/cpp/src/arrow/flight/sql/server.h b/cpp/src/arrow/flight/sql/server.h index 4e6ddce239746..1875ec8f7a769 100644 --- a/cpp/src/arrow/flight/sql/server.h +++ b/cpp/src/arrow/flight/sql/server.h @@ -39,43 +39,43 @@ namespace sql { /// @{ /// \brief A SQL query. -struct StatementQuery { +struct ARROW_FLIGHT_EXPORT StatementQuery { /// \brief The SQL query. std::string query; }; /// \brief A SQL update query. -struct StatementUpdate { +struct ARROW_FLIGHT_EXPORT StatementUpdate { /// \brief The SQL query. std::string query; }; /// \brief A request to execute a query. -struct StatementQueryTicket { +struct ARROW_FLIGHT_EXPORT StatementQueryTicket { /// \brief The server-generated opaque identifier for the query. std::string statement_handle; }; /// \brief A prepared query statement. -struct PreparedStatementQuery { +struct ARROW_FLIGHT_EXPORT PreparedStatementQuery { /// \brief The server-generated opaque identifier for the statement. std::string prepared_statement_handle; }; /// \brief A prepared update statement. -struct PreparedStatementUpdate { +struct ARROW_FLIGHT_EXPORT PreparedStatementUpdate { /// \brief The server-generated opaque identifier for the statement. std::string prepared_statement_handle; }; /// \brief A request to fetch server metadata. -struct GetSqlInfo { +struct ARROW_FLIGHT_EXPORT GetSqlInfo { /// \brief A list of metadata IDs to fetch. std::vector info; }; /// \brief A request to list database schemas. -struct GetDbSchemas { +struct ARROW_FLIGHT_EXPORT GetDbSchemas { /// \brief An optional database catalog to filter on. util::optional catalog; /// \brief An optional database schema to filter on. @@ -83,7 +83,7 @@ struct GetDbSchemas { }; /// \brief A request to list database tables. -struct GetTables { +struct ARROW_FLIGHT_EXPORT GetTables { /// \brief An optional database catalog to filter on. util::optional catalog; /// \brief An optional database schema to filter on. @@ -97,33 +97,33 @@ struct GetTables { }; /// \brief A request to get SQL data type information. -struct GetXdbcTypeInfo { +struct ARROW_FLIGHT_EXPORT GetXdbcTypeInfo { /// \brief A specific SQL type ID to fetch information about. util::optional data_type; }; /// \brief A request to list primary keys of a table. -struct GetPrimaryKeys { +struct ARROW_FLIGHT_EXPORT GetPrimaryKeys { /// \brief The given table. TableRef table_ref; }; /// \brief A request to list foreign key columns referencing primary key /// columns of a table. -struct GetExportedKeys { +struct ARROW_FLIGHT_EXPORT GetExportedKeys { /// \brief The given table. TableRef table_ref; }; /// \brief A request to list foreign keys of a table. -struct GetImportedKeys { +struct ARROW_FLIGHT_EXPORT GetImportedKeys { /// \brief The given table. TableRef table_ref; }; /// \brief A request to list foreign key columns of a table that /// reference columns in a given parent table. -struct GetCrossReference { +struct ARROW_FLIGHT_EXPORT GetCrossReference { /// \brief The parent table (the one containing referenced columns). TableRef pk_table_ref; /// \brief The foreign table (for which foreign key columns will be listed). @@ -131,19 +131,19 @@ struct GetCrossReference { }; /// \brief A request to create a new prepared statement. -struct ActionCreatePreparedStatementRequest { +struct ARROW_FLIGHT_EXPORT ActionCreatePreparedStatementRequest { /// \brief The SQL query. std::string query; }; /// \brief A request to close a prepared statement. -struct ActionClosePreparedStatementRequest { +struct ARROW_FLIGHT_EXPORT ActionClosePreparedStatementRequest { /// \brief The server-generated opaque identifier for the statement. std::string prepared_statement_handle; }; /// \brief The result of creating a new prepared statement. -struct ActionCreatePreparedStatementResult { +struct ARROW_FLIGHT_EXPORT ActionCreatePreparedStatementResult { /// \brief The schema of the query results, if applicable. std::shared_ptr dataset_schema; /// \brief The schema of the query parameters, if applicable. @@ -160,6 +160,7 @@ struct ActionCreatePreparedStatementResult { /// /// \param[in] statement_handle The statement handle that will originate the ticket. /// \return The parsed ticket as an string. +ARROW_FLIGHT_EXPORT arrow::Result CreateStatementQueryTicket( const std::string& statement_handle); @@ -167,7 +168,7 @@ arrow::Result CreateStatementQueryTicket( /// /// Applications should subclass this class and override the virtual /// methods declared on this class. -class ARROW_EXPORT FlightSqlServerBase : public FlightServerBase { +class ARROW_FLIGHT_EXPORT FlightSqlServerBase : public FlightServerBase { private: SqlInfoResultMap sql_info_id_to_result_; @@ -488,7 +489,7 @@ class ARROW_EXPORT FlightSqlServerBase : public FlightServerBase { }; /// \brief Auxiliary class containing all Schemas used on Flight SQL. -class ARROW_EXPORT SqlSchema { +class ARROW_FLIGHT_EXPORT SqlSchema { public: /// \brief Get the Schema used on GetCatalogs response. /// \return The default schema template. diff --git a/cpp/src/arrow/flight/sql/sql_info_internal.h b/cpp/src/arrow/flight/sql/sql_info_internal.h index b18789c25498c..89328e4138866 100644 --- a/cpp/src/arrow/flight/sql/sql_info_internal.h +++ b/cpp/src/arrow/flight/sql/sql_info_internal.h @@ -18,6 +18,7 @@ #pragma once #include "arrow/flight/sql/types.h" +#include "arrow/flight/visibility.h" namespace arrow { namespace flight { @@ -26,7 +27,7 @@ namespace internal { /// \brief Auxiliary class used to populate GetSqlInfo's DenseUnionArray with different /// data types. -class SqlInfoResultAppender { +class ARROW_FLIGHT_EXPORT SqlInfoResultAppender { public: /// \brief Append a string to the DenseUnionBuilder. /// \param[in] value Value to be appended. diff --git a/cpp/src/arrow/flight/sql/types.h b/cpp/src/arrow/flight/sql/types.h index ebfb2ef0eaf18..7f81fea3cd068 100644 --- a/cpp/src/arrow/flight/sql/types.h +++ b/cpp/src/arrow/flight/sql/types.h @@ -22,6 +22,7 @@ #include #include +#include "arrow/flight/visibility.h" #include "arrow/type_fwd.h" #include "arrow/util/optional.h" #include "arrow/util/variant.h" @@ -43,7 +44,7 @@ using SqlInfoResult = using SqlInfoResultMap = std::unordered_map; /// \brief Options to be set in the SqlInfo. -struct SqlInfoOptions { +struct ARROW_FLIGHT_EXPORT SqlInfoOptions { /// \brief Predefined info values for GetSqlInfo. enum SqlInfo { /// \name Server Information @@ -835,7 +836,7 @@ struct SqlInfoOptions { }; /// \brief A SQL %table reference, optionally containing table's catalog and db_schema. -struct TableRef { +struct ARROW_FLIGHT_EXPORT TableRef { /// \brief The table's catalog. util::optional catalog; /// \brief The table's database schema. From 522981a083281c7b4cfb01f74b693de97070315c Mon Sep 17 00:00:00 2001 From: David Li Date: Fri, 24 Jun 2022 20:45:30 -0400 Subject: [PATCH 2/8] Fix invalid syntax --- cpp/src/arrow/flight/sql/example/sqlite_server.cc | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/cpp/src/arrow/flight/sql/example/sqlite_server.cc b/cpp/src/arrow/flight/sql/example/sqlite_server.cc index 2105eb5be0371..b1bdd39d0aaac 100644 --- a/cpp/src/arrow/flight/sql/example/sqlite_server.cc +++ b/cpp/src/arrow/flight/sql/example/sqlite_server.cc @@ -384,14 +384,14 @@ class SQLiteFlightSqlServer::Impl { const ActionCreatePreparedStatementRequest& request) { std::shared_ptr statement; ARROW_ASSIGN_OR_RAISE(statement, SqliteStatement::Create(db_, request.query)); - const std::string handle = GenerateRandomString(); + std::string handle = GenerateRandomString(); prepared_statements_[handle] = statement; ARROW_ASSIGN_OR_RAISE(auto dataset_schema, statement->GetSchema()); sqlite3_stmt* stmt = statement->GetSqlite3Stmt(); const int parameter_count = sqlite3_bind_parameter_count(stmt); - std::vector> parameter_fields; + FieldVector parameter_fields; parameter_fields.reserve(parameter_count); // As SQLite doesn't know the parameter types before executing the query, the @@ -410,13 +410,9 @@ class SQLiteFlightSqlServer::Impl { parameter_fields.push_back(field(parameter_name, dense_union_type)); } - const std::shared_ptr& parameter_schema = arrow::schema(parameter_fields); - - ActionCreatePreparedStatementResult result{.dataset_schema = dataset_schema, - .parameter_schema = parameter_schema, - .prepared_statement_handle = handle}; - - return result; + std::shared_ptr parameter_schema = arrow::schema(parameter_fields); + return ActionCreatePreparedStatementResult{ + std::move(dataset_schema), std::move(parameter_schema), std::move(handle)}; } Status ClosePreparedStatement(const ServerCallContext& context, From 6d78931a5750a16d5fd66dea360fc2a4669c6acf Mon Sep 17 00:00:00 2001 From: David Li Date: Mon, 27 Jun 2022 09:42:03 -0400 Subject: [PATCH 3/8] Introduce ARROW_FLIGHT_SQL_EXPORT --- cpp/cmake_modules/BuildUtils.cmake | 1 + cpp/src/arrow/flight/sql/CMakeLists.txt | 8 +++- cpp/src/arrow/flight/sql/client.h | 5 +- cpp/src/arrow/flight/sql/column_metadata.h | 6 +-- cpp/src/arrow/flight/sql/server.h | 39 ++++++++-------- cpp/src/arrow/flight/sql/sql_info_internal.h | 4 +- cpp/src/arrow/flight/sql/types.h | 6 +-- cpp/src/arrow/flight/sql/visibility.h | 48 ++++++++++++++++++++ docs/source/developers/cpp/windows.rst | 16 +++++-- 9 files changed, 99 insertions(+), 34 deletions(-) create mode 100644 cpp/src/arrow/flight/sql/visibility.h diff --git a/cpp/cmake_modules/BuildUtils.cmake b/cpp/cmake_modules/BuildUtils.cmake index 5ce65ee51d0a4..888ca19af5814 100644 --- a/cpp/cmake_modules/BuildUtils.cmake +++ b/cpp/cmake_modules/BuildUtils.cmake @@ -386,6 +386,7 @@ function(ADD_ARROW_LIB LIB_NAME) if(WIN32) target_compile_definitions(${LIB_NAME}_static PUBLIC ARROW_STATIC) target_compile_definitions(${LIB_NAME}_static PUBLIC ARROW_FLIGHT_STATIC) + target_compile_definitions(${LIB_NAME}_static PUBLIC ARROW_FLIGHT_SQL_STATIC) endif() set_target_properties(${LIB_NAME}_static diff --git a/cpp/src/arrow/flight/sql/CMakeLists.txt b/cpp/src/arrow/flight/sql/CMakeLists.txt index 62d2c208d0942..50a47da44d209 100644 --- a/cpp/src/arrow/flight/sql/CMakeLists.txt +++ b/cpp/src/arrow/flight/sql/CMakeLists.txt @@ -69,7 +69,7 @@ add_arrow_lib(arrow_flight_sql "${Protobuf_INCLUDE_DIRS}") foreach(LIB_TARGET ${ARROW_FLIGHT_SQL_LIBRARIES}) - target_compile_definitions(${LIB_TARGET} PRIVATE ARROW_FLIGHT_EXPORTING) + target_compile_definitions(${LIB_TARGET} PRIVATE ARROW_FLIGHT_SQL_EXPORTING) endforeach() if(ARROW_FLIGHT_TEST_LINKAGE STREQUAL "static" AND ARROW_BUILD_STATIC) @@ -111,4 +111,10 @@ if(ARROW_BUILD_TESTS OR ARROW_BUILD_EXAMPLES) add_executable(flight-sql-test-app test_app_cli.cc) target_link_libraries(flight-sql-test-app PRIVATE ${ARROW_FLIGHT_SQL_TEST_LINK_LIBS} ${GFLAGS_LIBRARIES}) + + if(ARROW_FLIGHT_TEST_LINKAGE STREQUAL "static" AND ARROW_BUILD_STATIC) + target_compile_definitions(flight_sql_test + flight-sql-test-server flight-sql-test-app + PUBLIC ARROW_FLIGHT_STATIC ARROW_FLIGHT_SQL_STATIC) + endif() endif() diff --git a/cpp/src/arrow/flight/sql/client.h b/cpp/src/arrow/flight/sql/client.h index 0b83a6edc6243..7c8cb640e8d13 100644 --- a/cpp/src/arrow/flight/sql/client.h +++ b/cpp/src/arrow/flight/sql/client.h @@ -22,6 +22,7 @@ #include "arrow/flight/client.h" #include "arrow/flight/sql/types.h" +#include "arrow/flight/sql/visibility.h" #include "arrow/flight/types.h" #include "arrow/result.h" #include "arrow/status.h" @@ -35,7 +36,7 @@ class PreparedStatement; /// \brief Flight client with Flight SQL semantics. /// /// Wraps a Flight client to provide the Flight SQL RPC calls. -class ARROW_FLIGHT_EXPORT FlightSqlClient { +class ARROW_FLIGHT_SQL_EXPORT FlightSqlClient { friend class PreparedStatement; private: @@ -202,7 +203,7 @@ class ARROW_FLIGHT_EXPORT FlightSqlClient { }; /// \brief A prepared statement that can be executed. -class ARROW_FLIGHT_EXPORT PreparedStatement { +class ARROW_FLIGHT_SQL_EXPORT PreparedStatement { public: /// \brief Create a new prepared statement. However, applications /// should generally use FlightSqlClient::Prepare. diff --git a/cpp/src/arrow/flight/sql/column_metadata.h b/cpp/src/arrow/flight/sql/column_metadata.h index 7d6c74e012aa2..15b139ec5806a 100644 --- a/cpp/src/arrow/flight/sql/column_metadata.h +++ b/cpp/src/arrow/flight/sql/column_metadata.h @@ -19,7 +19,7 @@ #include -#include "arrow/flight/visibility.h" +#include "arrow/flight/sql/visibility.h" #include "arrow/util/key_value_metadata.h" namespace arrow { @@ -27,7 +27,7 @@ namespace flight { namespace sql { /// \brief Helper class to set column metadata. -class ARROW_FLIGHT_EXPORT ColumnMetadata { +class ARROW_FLIGHT_SQL_EXPORT ColumnMetadata { private: std::shared_ptr metadata_map_; @@ -115,7 +115,7 @@ class ARROW_FLIGHT_EXPORT ColumnMetadata { const std::shared_ptr& metadata_map() const; /// \brief A builder class to construct the ColumnMetadata object. - class ARROW_FLIGHT_EXPORT ColumnMetadataBuilder { + class ARROW_FLIGHT_SQL_EXPORT ColumnMetadataBuilder { public: friend class ColumnMetadata; diff --git a/cpp/src/arrow/flight/sql/server.h b/cpp/src/arrow/flight/sql/server.h index 1875ec8f7a769..f077c5d5d5d1f 100644 --- a/cpp/src/arrow/flight/sql/server.h +++ b/cpp/src/arrow/flight/sql/server.h @@ -27,6 +27,7 @@ #include "arrow/flight/server.h" #include "arrow/flight/sql/server.h" #include "arrow/flight/sql/types.h" +#include "arrow/flight/sql/visibility.h" #include "arrow/util/optional.h" namespace arrow { @@ -39,43 +40,43 @@ namespace sql { /// @{ /// \brief A SQL query. -struct ARROW_FLIGHT_EXPORT StatementQuery { +struct ARROW_FLIGHT_SQL_EXPORT StatementQuery { /// \brief The SQL query. std::string query; }; /// \brief A SQL update query. -struct ARROW_FLIGHT_EXPORT StatementUpdate { +struct ARROW_FLIGHT_SQL_EXPORT StatementUpdate { /// \brief The SQL query. std::string query; }; /// \brief A request to execute a query. -struct ARROW_FLIGHT_EXPORT StatementQueryTicket { +struct ARROW_FLIGHT_SQL_EXPORT StatementQueryTicket { /// \brief The server-generated opaque identifier for the query. std::string statement_handle; }; /// \brief A prepared query statement. -struct ARROW_FLIGHT_EXPORT PreparedStatementQuery { +struct ARROW_FLIGHT_SQL_EXPORT PreparedStatementQuery { /// \brief The server-generated opaque identifier for the statement. std::string prepared_statement_handle; }; /// \brief A prepared update statement. -struct ARROW_FLIGHT_EXPORT PreparedStatementUpdate { +struct ARROW_FLIGHT_SQL_EXPORT PreparedStatementUpdate { /// \brief The server-generated opaque identifier for the statement. std::string prepared_statement_handle; }; /// \brief A request to fetch server metadata. -struct ARROW_FLIGHT_EXPORT GetSqlInfo { +struct ARROW_FLIGHT_SQL_EXPORT GetSqlInfo { /// \brief A list of metadata IDs to fetch. std::vector info; }; /// \brief A request to list database schemas. -struct ARROW_FLIGHT_EXPORT GetDbSchemas { +struct ARROW_FLIGHT_SQL_EXPORT GetDbSchemas { /// \brief An optional database catalog to filter on. util::optional catalog; /// \brief An optional database schema to filter on. @@ -83,7 +84,7 @@ struct ARROW_FLIGHT_EXPORT GetDbSchemas { }; /// \brief A request to list database tables. -struct ARROW_FLIGHT_EXPORT GetTables { +struct ARROW_FLIGHT_SQL_EXPORT GetTables { /// \brief An optional database catalog to filter on. util::optional catalog; /// \brief An optional database schema to filter on. @@ -97,33 +98,33 @@ struct ARROW_FLIGHT_EXPORT GetTables { }; /// \brief A request to get SQL data type information. -struct ARROW_FLIGHT_EXPORT GetXdbcTypeInfo { +struct ARROW_FLIGHT_SQL_EXPORT GetXdbcTypeInfo { /// \brief A specific SQL type ID to fetch information about. util::optional data_type; }; /// \brief A request to list primary keys of a table. -struct ARROW_FLIGHT_EXPORT GetPrimaryKeys { +struct ARROW_FLIGHT_SQL_EXPORT GetPrimaryKeys { /// \brief The given table. TableRef table_ref; }; /// \brief A request to list foreign key columns referencing primary key /// columns of a table. -struct ARROW_FLIGHT_EXPORT GetExportedKeys { +struct ARROW_FLIGHT_SQL_EXPORT GetExportedKeys { /// \brief The given table. TableRef table_ref; }; /// \brief A request to list foreign keys of a table. -struct ARROW_FLIGHT_EXPORT GetImportedKeys { +struct ARROW_FLIGHT_SQL_EXPORT GetImportedKeys { /// \brief The given table. TableRef table_ref; }; /// \brief A request to list foreign key columns of a table that /// reference columns in a given parent table. -struct ARROW_FLIGHT_EXPORT GetCrossReference { +struct ARROW_FLIGHT_SQL_EXPORT GetCrossReference { /// \brief The parent table (the one containing referenced columns). TableRef pk_table_ref; /// \brief The foreign table (for which foreign key columns will be listed). @@ -131,19 +132,19 @@ struct ARROW_FLIGHT_EXPORT GetCrossReference { }; /// \brief A request to create a new prepared statement. -struct ARROW_FLIGHT_EXPORT ActionCreatePreparedStatementRequest { +struct ARROW_FLIGHT_SQL_EXPORT ActionCreatePreparedStatementRequest { /// \brief The SQL query. std::string query; }; /// \brief A request to close a prepared statement. -struct ARROW_FLIGHT_EXPORT ActionClosePreparedStatementRequest { +struct ARROW_FLIGHT_SQL_EXPORT ActionClosePreparedStatementRequest { /// \brief The server-generated opaque identifier for the statement. std::string prepared_statement_handle; }; /// \brief The result of creating a new prepared statement. -struct ARROW_FLIGHT_EXPORT ActionCreatePreparedStatementResult { +struct ARROW_FLIGHT_SQL_EXPORT ActionCreatePreparedStatementResult { /// \brief The schema of the query results, if applicable. std::shared_ptr dataset_schema; /// \brief The schema of the query parameters, if applicable. @@ -160,7 +161,7 @@ struct ARROW_FLIGHT_EXPORT ActionCreatePreparedStatementResult { /// /// \param[in] statement_handle The statement handle that will originate the ticket. /// \return The parsed ticket as an string. -ARROW_FLIGHT_EXPORT +ARROW_FLIGHT_SQL_EXPORT arrow::Result CreateStatementQueryTicket( const std::string& statement_handle); @@ -168,7 +169,7 @@ arrow::Result CreateStatementQueryTicket( /// /// Applications should subclass this class and override the virtual /// methods declared on this class. -class ARROW_FLIGHT_EXPORT FlightSqlServerBase : public FlightServerBase { +class ARROW_FLIGHT_SQL_EXPORT FlightSqlServerBase : public FlightServerBase { private: SqlInfoResultMap sql_info_id_to_result_; @@ -489,7 +490,7 @@ class ARROW_FLIGHT_EXPORT FlightSqlServerBase : public FlightServerBase { }; /// \brief Auxiliary class containing all Schemas used on Flight SQL. -class ARROW_FLIGHT_EXPORT SqlSchema { +class ARROW_FLIGHT_SQL_EXPORT SqlSchema { public: /// \brief Get the Schema used on GetCatalogs response. /// \return The default schema template. diff --git a/cpp/src/arrow/flight/sql/sql_info_internal.h b/cpp/src/arrow/flight/sql/sql_info_internal.h index 89328e4138866..2a3cafdbe61bd 100644 --- a/cpp/src/arrow/flight/sql/sql_info_internal.h +++ b/cpp/src/arrow/flight/sql/sql_info_internal.h @@ -18,7 +18,7 @@ #pragma once #include "arrow/flight/sql/types.h" -#include "arrow/flight/visibility.h" +#include "arrow/flight/sql/visibility.h" namespace arrow { namespace flight { @@ -27,7 +27,7 @@ namespace internal { /// \brief Auxiliary class used to populate GetSqlInfo's DenseUnionArray with different /// data types. -class ARROW_FLIGHT_EXPORT SqlInfoResultAppender { +class ARROW_FLIGHT_SQL_EXPORT SqlInfoResultAppender { public: /// \brief Append a string to the DenseUnionBuilder. /// \param[in] value Value to be appended. diff --git a/cpp/src/arrow/flight/sql/types.h b/cpp/src/arrow/flight/sql/types.h index 7f81fea3cd068..a6c2648e7c49e 100644 --- a/cpp/src/arrow/flight/sql/types.h +++ b/cpp/src/arrow/flight/sql/types.h @@ -22,7 +22,7 @@ #include #include -#include "arrow/flight/visibility.h" +#include "arrow/flight/sql/visibility.h" #include "arrow/type_fwd.h" #include "arrow/util/optional.h" #include "arrow/util/variant.h" @@ -44,7 +44,7 @@ using SqlInfoResult = using SqlInfoResultMap = std::unordered_map; /// \brief Options to be set in the SqlInfo. -struct ARROW_FLIGHT_EXPORT SqlInfoOptions { +struct ARROW_FLIGHT_SQL_EXPORT SqlInfoOptions { /// \brief Predefined info values for GetSqlInfo. enum SqlInfo { /// \name Server Information @@ -836,7 +836,7 @@ struct ARROW_FLIGHT_EXPORT SqlInfoOptions { }; /// \brief A SQL %table reference, optionally containing table's catalog and db_schema. -struct ARROW_FLIGHT_EXPORT TableRef { +struct ARROW_FLIGHT_SQL_EXPORT TableRef { /// \brief The table's catalog. util::optional catalog; /// \brief The table's database schema. diff --git a/cpp/src/arrow/flight/sql/visibility.h b/cpp/src/arrow/flight/sql/visibility.h new file mode 100644 index 0000000000000..2074815e0a246 --- /dev/null +++ b/cpp/src/arrow/flight/sql/visibility.h @@ -0,0 +1,48 @@ +// 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. + +#pragma once + +#if defined(_WIN32) || defined(__CYGWIN__) +#if defined(_MSC_VER) +#pragma warning(push) +#pragma warning(disable : 4251) +#else +#pragma GCC diagnostic ignored "-Wattributes" +#endif + +#ifdef ARROW_FLIGHT_SQL_STATIC +#define ARROW_FLIGHT_SQL_EXPORT +#elif defined(ARROW_FLIGHT_SQL_EXPORTING) +#define ARROW_FLIGHT_SQL_EXPORT __declspec(dllexport) +#else +#define ARROW_FLIGHT_SQL_EXPORT __declspec(dllimport) +#endif + +#define ARROW_FLIGHT_SQL_NO_EXPORT +#else // Not Windows +#ifndef ARROW_FLIGHT_SQL_EXPORT +#define ARROW_FLIGHT_SQL_EXPORT __attribute__((visibility("default"))) +#endif +#ifndef ARROW_FLIGHT_SQL_NO_EXPORT +#define ARROW_FLIGHT_SQL_NO_EXPORT __attribute__((visibility("hidden"))) +#endif +#endif // Non-Windows + +#if defined(_MSC_VER) +#pragma warning(pop) +#endif diff --git a/docs/source/developers/cpp/windows.rst b/docs/source/developers/cpp/windows.rst index 84ba000cb58c2..91562a2c8cbb1 100644 --- a/docs/source/developers/cpp/windows.rst +++ b/docs/source/developers/cpp/windows.rst @@ -363,7 +363,7 @@ 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. +be defined, and similarly for ``-DARROW_FLIGHT_SQL=ON``. .. code-block:: cmake @@ -372,9 +372,17 @@ be defined. find_package(Arrow REQUIRED) add_executable(my_example my_example.cc) - target_link_libraries(my_example PRIVATE arrow_static arrow_flight_static) - - target_compile_definitions(my_example PUBLIC ARROW_STATIC ARROW_FLIGHT_STATIC) + target_link_libraries(my_example + PRIVATE + arrow_static + arrow_flight_static + arrow_flight_sql_static) + + target_compile_definitions(my_example + PUBLIC + ARROW_STATIC + ARROW_FLIGHT_STATIC + ARROW_FLIGHT_SQL_STATIC) Downloading the Timezone Database ================================= From 2ea94b1950507f4b73ff80ebf42ed3c289cdcbb3 Mon Sep 17 00:00:00 2001 From: David Li Date: Mon, 27 Jun 2022 10:40:49 -0400 Subject: [PATCH 4/8] Fix build errors --- .../arrow/flight/sql/example/sqlite_server.cc | 6 ++++-- .../example/sqlite_statement_batch_reader.cc | 21 ++++++++++--------- cpp/src/arrow/flight/sql/server_test.cc | 1 + 3 files changed, 16 insertions(+), 12 deletions(-) diff --git a/cpp/src/arrow/flight/sql/example/sqlite_server.cc b/cpp/src/arrow/flight/sql/example/sqlite_server.cc index b1bdd39d0aaac..35fa05468ba27 100644 --- a/cpp/src/arrow/flight/sql/example/sqlite_server.cc +++ b/cpp/src/arrow/flight/sql/example/sqlite_server.cc @@ -249,9 +249,11 @@ class SQLiteFlightSqlServer::Impl { std::string GenerateRandomString() { uint32_t length = 16; - std::uniform_int_distribution dist('0', 'z'); + // MSVC doesn't support char types here + std::uniform_int_distribution dist(static_cast('0'), + static_cast('z')); std::string ret(length, 0); - auto get_random_char = [&]() { return dist(gen_); }; + auto get_random_char = [&]() { return static_cast(dist(gen_)); }; std::generate_n(ret.begin(), length, get_random_char); return ret; } diff --git a/cpp/src/arrow/flight/sql/example/sqlite_statement_batch_reader.cc b/cpp/src/arrow/flight/sql/example/sqlite_statement_batch_reader.cc index e31d6173ee870..c247eb628757c 100644 --- a/cpp/src/arrow/flight/sql/example/sqlite_statement_batch_reader.cc +++ b/cpp/src/arrow/flight/sql/example/sqlite_statement_batch_reader.cc @@ -63,16 +63,17 @@ break; \ } -#define FLOAT_BUILDER_CASE(TYPE_CLASS, STMT, COLUMN) \ - case TYPE_CLASS##Type::type_id: { \ - auto builder = reinterpret_cast(array_builder); \ - if (sqlite3_column_type(stmt_, i) == SQLITE_NULL) { \ - ARROW_RETURN_NOT_OK(builder->AppendNull()); \ - break; \ - } \ - const double value = sqlite3_column_double(STMT, COLUMN); \ - ARROW_RETURN_NOT_OK(builder->Append(value)); \ - break; \ +#define FLOAT_BUILDER_CASE(TYPE_CLASS, STMT, COLUMN) \ + case TYPE_CLASS##Type::type_id: { \ + auto builder = reinterpret_cast(array_builder); \ + if (sqlite3_column_type(stmt_, i) == SQLITE_NULL) { \ + ARROW_RETURN_NOT_OK(builder->AppendNull()); \ + break; \ + } \ + const double value = sqlite3_column_double(STMT, COLUMN); \ + ARROW_RETURN_NOT_OK( \ + builder->Append(static_cast(value))); \ + break; \ } namespace arrow { diff --git a/cpp/src/arrow/flight/sql/server_test.cc b/cpp/src/arrow/flight/sql/server_test.cc index 746c91c102b3e..69081acdb59c6 100644 --- a/cpp/src/arrow/flight/sql/server_test.cc +++ b/cpp/src/arrow/flight/sql/server_test.cc @@ -21,6 +21,7 @@ #include #include +#include #include #include From 93d518e2481eb7dc3338cf7db37f9553628d19d1 Mon Sep 17 00:00:00 2001 From: David Li Date: Mon, 27 Jun 2022 12:35:29 -0400 Subject: [PATCH 5/8] Try to fix MinGW build --- cpp/src/arrow/flight/sql/CMakeLists.txt | 3 ++- cpp/src/arrow/flight/sql/client_test.cc | 3 +++ cpp/src/arrow/symbols.map | 1 + 3 files changed, 6 insertions(+), 1 deletion(-) diff --git a/cpp/src/arrow/flight/sql/CMakeLists.txt b/cpp/src/arrow/flight/sql/CMakeLists.txt index 50a47da44d209..5dbadd1234bb7 100644 --- a/cpp/src/arrow/flight/sql/CMakeLists.txt +++ b/cpp/src/arrow/flight/sql/CMakeLists.txt @@ -29,7 +29,8 @@ set(PROTO_DEPENDS ${FLIGHT_SQL_PROTO} ${ARROW_PROTOBUF_LIBPROTOBUF}) add_custom_command(OUTPUT ${FLIGHT_SQL_GENERATED_PROTO_FILES} COMMAND ${ARROW_PROTOBUF_PROTOC} "-I${FLIGHT_SQL_PROTO_PATH}" - "--cpp_out=${CMAKE_CURRENT_BINARY_DIR}" "${FLIGHT_SQL_PROTO}" + "--cpp_out=dllexport_decl=ARROW_FLIGHT_SQL_EXPORT:${CMAKE_CURRENT_BINARY_DIR}" + "${FLIGHT_SQL_PROTO}" DEPENDS ${PROTO_DEPENDS}) set_source_files_properties(${FLIGHT_SQL_GENERATED_PROTO_FILES} PROPERTIES GENERATED TRUE) diff --git a/cpp/src/arrow/flight/sql/client_test.cc b/cpp/src/arrow/flight/sql/client_test.cc index b29af7c46681d..c25a3e02b8c60 100644 --- a/cpp/src/arrow/flight/sql/client_test.cc +++ b/cpp/src/arrow/flight/sql/client_test.cc @@ -26,6 +26,9 @@ #include +// Must come before Protobuf include +#include "arrow/flight/sql/visibility.h" + #include "arrow/buffer.h" #include "arrow/flight/sql/FlightSql.pb.h" #include "arrow/flight/sql/api.h" diff --git a/cpp/src/arrow/symbols.map b/cpp/src/arrow/symbols.map index 0c5616c692d0b..7d4f13fa28627 100644 --- a/cpp/src/arrow/symbols.map +++ b/cpp/src/arrow/symbols.map @@ -35,6 +35,7 @@ pyarrow_*; # ARROW-14771: export Protobuf symbol table descriptor_table_Flight_2eproto; + descriptor_table_FlightSql_2eproto; # Symbols marked as 'local' are not exported by the DSO and thus may not # be used by client applications. Everything except the above falls here. From f27fa719532b225ad1e453414a27e1b1943f218f Mon Sep 17 00:00:00 2001 From: David Li Date: Mon, 27 Jun 2022 12:53:57 -0400 Subject: [PATCH 6/8] Try to fix MinGW build (again) --- cpp/src/arrow/flight/sql/CMakeLists.txt | 6 ++--- cpp/src/arrow/flight/sql/client.cc | 2 +- cpp/src/arrow/flight/sql/client_test.cc | 10 +++++-- cpp/src/arrow/flight/sql/protocol_internal.cc | 23 ++++++++++++++++ cpp/src/arrow/flight/sql/protocol_internal.h | 26 +++++++++++++++++++ cpp/src/arrow/flight/sql/server.cc | 2 +- 6 files changed, 62 insertions(+), 7 deletions(-) create mode 100644 cpp/src/arrow/flight/sql/protocol_internal.cc create mode 100644 cpp/src/arrow/flight/sql/protocol_internal.h diff --git a/cpp/src/arrow/flight/sql/CMakeLists.txt b/cpp/src/arrow/flight/sql/CMakeLists.txt index 5dbadd1234bb7..d0cac8c4691f4 100644 --- a/cpp/src/arrow/flight/sql/CMakeLists.txt +++ b/cpp/src/arrow/flight/sql/CMakeLists.txt @@ -36,8 +36,8 @@ add_custom_command(OUTPUT ${FLIGHT_SQL_GENERATED_PROTO_FILES} set_source_files_properties(${FLIGHT_SQL_GENERATED_PROTO_FILES} PROPERTIES GENERATED TRUE) if(MSVC) # Suppress warnings caused by Protobuf (casts, dll-interface) - set_source_files_properties(${FLIGHT_SQL_GENERATED_PROTO_FILES} - PROPERTIES COMPILE_FLAGS "/wd4251 /wd4267") + set_source_files_properties(${FLIGHT_SQL_GENERATED_PROTO_FILES} PROPERTIES COMPILE_FLAGS + "/wd4267") endif() add_custom_target(flight_sql_protobuf_gen ALL DEPENDS ${FLIGHT_SQL_GENERATED_PROTO_FILES}) @@ -47,7 +47,7 @@ set(ARROW_FLIGHT_SQL_SRCS sql_info_internal.cc column_metadata.cc client.cc - "${CMAKE_CURRENT_BINARY_DIR}/FlightSql.pb.cc") + protocol_internal.cc) add_arrow_lib(arrow_flight_sql CMAKE_PACKAGE_NAME diff --git a/cpp/src/arrow/flight/sql/client.cc b/cpp/src/arrow/flight/sql/client.cc index 3ce220491f173..10ff1eea6f4cb 100644 --- a/cpp/src/arrow/flight/sql/client.cc +++ b/cpp/src/arrow/flight/sql/client.cc @@ -23,7 +23,7 @@ #include #include "arrow/buffer.h" -#include "arrow/flight/sql/FlightSql.pb.h" +#include "arrow/flight/sql/protocol_internal.h" #include "arrow/flight/types.h" #include "arrow/io/memory.h" #include "arrow/ipc/reader.h" diff --git a/cpp/src/arrow/flight/sql/client_test.cc b/cpp/src/arrow/flight/sql/client_test.cc index c25a3e02b8c60..54c2b73af0f5a 100644 --- a/cpp/src/arrow/flight/sql/client_test.cc +++ b/cpp/src/arrow/flight/sql/client_test.cc @@ -26,11 +26,17 @@ #include +// XXX(ARROW-16902): Protobuf doesn't insert dllimport declarations in +// all the right places for MinGW (causing an error "redeclared +// without dllimport attribute"). Instead, just omit them and rely on +// symbols being found implicitly. +#if defined(__MINGW32__) +#define ARROW_FLIGHT_SQL_STATIC +#endif // Must come before Protobuf include -#include "arrow/flight/sql/visibility.h" +#include "arrow/flight/sql/protocol_internal.h" #include "arrow/buffer.h" -#include "arrow/flight/sql/FlightSql.pb.h" #include "arrow/flight/sql/api.h" #include "arrow/testing/gtest_util.h" diff --git a/cpp/src/arrow/flight/sql/protocol_internal.cc b/cpp/src/arrow/flight/sql/protocol_internal.cc new file mode 100644 index 0000000000000..0d5e3c4c60b8d --- /dev/null +++ b/cpp/src/arrow/flight/sql/protocol_internal.cc @@ -0,0 +1,23 @@ +// 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 + +#include "arrow/flight/sql/protocol_internal.h" + +// NOTE(lidavidm): Normally this is forbidden, but on Windows to get +// the dllexport/dllimport macro in the right places, we need to +// ensure our header gets included (and Protobuf will not insert the +// include for you) +#include "arrow/flight/sql/FlightSql.pb.cc" // NOLINT diff --git a/cpp/src/arrow/flight/sql/protocol_internal.h b/cpp/src/arrow/flight/sql/protocol_internal.h new file mode 100644 index 0000000000000..ce50ad2f61b1e --- /dev/null +++ b/cpp/src/arrow/flight/sql/protocol_internal.h @@ -0,0 +1,26 @@ +// 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 + +#pragma once + +// This addresses platform-specific defines, e.g. on Windows +#include "arrow/flight/platform.h" // IWYU pragma: keep + +// This header holds the Flight SQL definitions. + +#include "arrow/flight/sql/visibility.h" + +#include "arrow/flight/sql/FlightSql.pb.h" // IWYU pragma: export diff --git a/cpp/src/arrow/flight/sql/server.cc b/cpp/src/arrow/flight/sql/server.cc index c82146de86da3..0ebe647ba1490 100644 --- a/cpp/src/arrow/flight/sql/server.cc +++ b/cpp/src/arrow/flight/sql/server.cc @@ -27,7 +27,7 @@ #include "arrow/buffer.h" #include "arrow/builder.h" -#include "arrow/flight/sql/FlightSql.pb.h" +#include "arrow/flight/sql/protocol_internal.h" #include "arrow/flight/sql/sql_info_internal.h" #include "arrow/type.h" #include "arrow/util/checked_cast.h" From 5368d61b28a03588c9c159a8d4e46d5cd76b5dde Mon Sep 17 00:00:00 2001 From: David Li Date: Mon, 27 Jun 2022 13:04:25 -0400 Subject: [PATCH 7/8] Fix MSVC warning --- cpp/src/arrow/flight/sql/CMakeLists.txt | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/cpp/src/arrow/flight/sql/CMakeLists.txt b/cpp/src/arrow/flight/sql/CMakeLists.txt index d0cac8c4691f4..3546e3c79ca8a 100644 --- a/cpp/src/arrow/flight/sql/CMakeLists.txt +++ b/cpp/src/arrow/flight/sql/CMakeLists.txt @@ -34,12 +34,6 @@ add_custom_command(OUTPUT ${FLIGHT_SQL_GENERATED_PROTO_FILES} DEPENDS ${PROTO_DEPENDS}) set_source_files_properties(${FLIGHT_SQL_GENERATED_PROTO_FILES} PROPERTIES GENERATED TRUE) -if(MSVC) - # Suppress warnings caused by Protobuf (casts, dll-interface) - set_source_files_properties(${FLIGHT_SQL_GENERATED_PROTO_FILES} PROPERTIES COMPILE_FLAGS - "/wd4267") -endif() - add_custom_target(flight_sql_protobuf_gen ALL DEPENDS ${FLIGHT_SQL_GENERATED_PROTO_FILES}) set(ARROW_FLIGHT_SQL_SRCS @@ -69,6 +63,10 @@ add_arrow_lib(arrow_flight_sql PRIVATE_INCLUDES "${Protobuf_INCLUDE_DIRS}") +if(MSVC) + # Suppress warnings caused by Protobuf (casts) + set_source_files_properties(protocol_internal.cc PROPERTIES COMPILE_FLAGS "/wd4267") +endif() foreach(LIB_TARGET ${ARROW_FLIGHT_SQL_LIBRARIES}) target_compile_definitions(${LIB_TARGET} PRIVATE ARROW_FLIGHT_SQL_EXPORTING) endforeach() From 09ee32b832d6b08e411e156c5f5557b30a05e621 Mon Sep 17 00:00:00 2001 From: David Li Date: Tue, 28 Jun 2022 07:58:46 -0400 Subject: [PATCH 8/8] Disable client_test.cc on Windows --- cpp/src/arrow/flight/sql/CMakeLists.txt | 11 +++++++++-- cpp/src/arrow/flight/sql/client_test.cc | 11 +---------- 2 files changed, 10 insertions(+), 12 deletions(-) diff --git a/cpp/src/arrow/flight/sql/CMakeLists.txt b/cpp/src/arrow/flight/sql/CMakeLists.txt index 3546e3c79ca8a..0c789fd71c525 100644 --- a/cpp/src/arrow/flight/sql/CMakeLists.txt +++ b/cpp/src/arrow/flight/sql/CMakeLists.txt @@ -89,11 +89,18 @@ if(ARROW_BUILD_TESTS OR ARROW_BUILD_EXAMPLES) example/sqlite_statement_batch_reader.cc example/sqlite_server.cc example/sqlite_tables_schema_batch_reader.cc) + set(ARROW_FLIGHT_SQL_TEST_SRCS server_test.cc) + if(NOT MSVC AND NOT MINGW) + # ARROW-16902: getting Protobuf generated code to have all the + # proper dllexport/dllimport declarations is difficult, since + # protoc does not insert them everywhere needed to satisfy both + # MinGW and MSVC, and the Protobuf team recommends against it + list(APPEND ARROW_FLIGHT_SQL_TEST_SRCS client_test.cc) + endif() add_arrow_test(flight_sql_test SOURCES - client_test.cc - server_test.cc + ${ARROW_FLIGHT_SQL_TEST_SRCS} ${ARROW_FLIGHT_SQL_TEST_SERVER_SRCS} STATIC_LINK_LIBS ${ARROW_FLIGHT_SQL_TEST_LINK_LIBS} diff --git a/cpp/src/arrow/flight/sql/client_test.cc b/cpp/src/arrow/flight/sql/client_test.cc index 54c2b73af0f5a..b9eeda76b00ad 100644 --- a/cpp/src/arrow/flight/sql/client_test.cc +++ b/cpp/src/arrow/flight/sql/client_test.cc @@ -26,18 +26,9 @@ #include -// XXX(ARROW-16902): Protobuf doesn't insert dllimport declarations in -// all the right places for MinGW (causing an error "redeclared -// without dllimport attribute"). Instead, just omit them and rely on -// symbols being found implicitly. -#if defined(__MINGW32__) -#define ARROW_FLIGHT_SQL_STATIC -#endif -// Must come before Protobuf include -#include "arrow/flight/sql/protocol_internal.h" - #include "arrow/buffer.h" #include "arrow/flight/sql/api.h" +#include "arrow/flight/sql/protocol_internal.h" #include "arrow/testing/gtest_util.h" namespace pb = arrow::flight::protocol;