Skip to content

Commit 9c93f82

Browse files
lidavidmlidavidm
authored
ARROW-16902: [C++][FlightRPC] Fix DLL linkage in Flight SQL (apache#13434)
Also, enable Flight SQL in Windows CI builds. Authored-by: David Li <li.davidm96@gmail.com> Signed-off-by: Sutou Kouhei <kou@clear-code.com>
1 parent 3dcf2d8 commit 9c93f82

21 files changed

Lines changed: 205 additions & 58 deletions

‎.github/workflows/cpp.yml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -281,6 +281,7 @@ jobs:
281281
ARROW_BUILD_TYPE: release
282282
ARROW_DATASET: ON
283283
ARROW_FLIGHT: ON
284+
ARROW_FLIGHT_SQL: ON
284285
ARROW_GANDIVA: ON
285286
ARROW_GCS: ON
286287
ARROW_HDFS: OFF

‎appveyor.yml‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ environment:
4646
CLCACHE_COMPRESS: 1
4747
CLCACHE_COMPRESSLEVEL: 6
4848
ARROW_BUILD_FLIGHT: "OFF"
49+
ARROW_BUILD_FLIGHT_SQL: "OFF"
4950
ARROW_BUILD_GANDIVA: "OFF"
5051
ARROW_LLVM_VERSION: "7.0.*"
5152
ARROW_S3: "OFF"
@@ -60,6 +61,7 @@ environment:
6061
ARROW_GCS: "ON"
6162
ARROW_S3: "ON"
6263
ARROW_BUILD_FLIGHT: "ON"
64+
ARROW_BUILD_FLIGHT_SQL: "ON"
6365
ARROW_BUILD_GANDIVA: "ON"
6466
- JOB: "Build_Debug"
6567
GENERATOR: Ninja

‎ci/appveyor-cpp-build.bat‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,7 @@ cmake -G "%GENERATOR%" %CMAKE_ARGS% ^
103103
-DARROW_DATASET=ON ^
104104
-DARROW_ENABLE_TIMING_TESTS=OFF ^
105105
-DARROW_FLIGHT=%ARROW_BUILD_FLIGHT% ^
106+
-DARROW_FLIGHT_SQL=%ARROW_BUILD_FLIGHT_SQL% ^
106107
-DARROW_GANDIVA=%ARROW_BUILD_GANDIVA% ^
107108
-DARROW_MIMALLOC=ON ^
108109
-DARROW_PARQUET=ON ^

‎cpp/cmake_modules/BuildUtils.cmake‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -386,6 +386,7 @@ function(ADD_ARROW_LIB LIB_NAME)
386386
if(WIN32)
387387
target_compile_definitions(${LIB_NAME}_static PUBLIC ARROW_STATIC)
388388
target_compile_definitions(${LIB_NAME}_static PUBLIC ARROW_FLIGHT_STATIC)
389+
target_compile_definitions(${LIB_NAME}_static PUBLIC ARROW_FLIGHT_SQL_STATIC)
389390
endif()
390391

391392
set_target_properties(${LIB_NAME}_static

‎cpp/src/arrow/flight/sql/CMakeLists.txt‎

Lines changed: 26 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -29,19 +29,19 @@ set(PROTO_DEPENDS ${FLIGHT_SQL_PROTO} ${ARROW_PROTOBUF_LIBPROTOBUF})
2929

3030
add_custom_command(OUTPUT ${FLIGHT_SQL_GENERATED_PROTO_FILES}
3131
COMMAND ${ARROW_PROTOBUF_PROTOC} "-I${FLIGHT_SQL_PROTO_PATH}"
32-
"--cpp_out=${CMAKE_CURRENT_BINARY_DIR}" "${FLIGHT_SQL_PROTO}"
32+
"--cpp_out=dllexport_decl=ARROW_FLIGHT_SQL_EXPORT:${CMAKE_CURRENT_BINARY_DIR}"
33+
"${FLIGHT_SQL_PROTO}"
3334
DEPENDS ${PROTO_DEPENDS})
3435

3536
set_source_files_properties(${FLIGHT_SQL_GENERATED_PROTO_FILES} PROPERTIES GENERATED TRUE)
36-
3737
add_custom_target(flight_sql_protobuf_gen ALL DEPENDS ${FLIGHT_SQL_GENERATED_PROTO_FILES})
3838

3939
set(ARROW_FLIGHT_SQL_SRCS
4040
server.cc
4141
sql_info_internal.cc
4242
column_metadata.cc
4343
client.cc
44-
"${CMAKE_CURRENT_BINARY_DIR}/FlightSql.pb.cc")
44+
protocol_internal.cc)
4545

4646
add_arrow_lib(arrow_flight_sql
4747
CMAKE_PACKAGE_NAME
@@ -63,6 +63,14 @@ add_arrow_lib(arrow_flight_sql
6363
PRIVATE_INCLUDES
6464
"${Protobuf_INCLUDE_DIRS}")
6565

66+
if(MSVC)
67+
# Suppress warnings caused by Protobuf (casts)
68+
set_source_files_properties(protocol_internal.cc PROPERTIES COMPILE_FLAGS "/wd4267")
69+
endif()
70+
foreach(LIB_TARGET ${ARROW_FLIGHT_SQL_LIBRARIES})
71+
target_compile_definitions(${LIB_TARGET} PRIVATE ARROW_FLIGHT_SQL_EXPORTING)
72+
endforeach()
73+
6674
if(ARROW_FLIGHT_TEST_LINKAGE STREQUAL "static" AND ARROW_BUILD_STATIC)
6775
set(ARROW_FLIGHT_SQL_TEST_LINK_LIBS arrow_flight_sql_static)
6876
else()
@@ -81,11 +89,18 @@ if(ARROW_BUILD_TESTS OR ARROW_BUILD_EXAMPLES)
8189
example/sqlite_statement_batch_reader.cc
8290
example/sqlite_server.cc
8391
example/sqlite_tables_schema_batch_reader.cc)
92+
set(ARROW_FLIGHT_SQL_TEST_SRCS server_test.cc)
93+
if(NOT MSVC AND NOT MINGW)
94+
# ARROW-16902: getting Protobuf generated code to have all the
95+
# proper dllexport/dllimport declarations is difficult, since
96+
# protoc does not insert them everywhere needed to satisfy both
97+
# MinGW and MSVC, and the Protobuf team recommends against it
98+
list(APPEND ARROW_FLIGHT_SQL_TEST_SRCS client_test.cc)
99+
endif()
84100

85101
add_arrow_test(flight_sql_test
86102
SOURCES
87-
client_test.cc
88-
server_test.cc
103+
${ARROW_FLIGHT_SQL_TEST_SRCS}
89104
${ARROW_FLIGHT_SQL_TEST_SERVER_SRCS}
90105
STATIC_LINK_LIBS
91106
${ARROW_FLIGHT_SQL_TEST_LINK_LIBS}
@@ -102,4 +117,10 @@ if(ARROW_BUILD_TESTS OR ARROW_BUILD_EXAMPLES)
102117
add_executable(flight-sql-test-app test_app_cli.cc)
103118
target_link_libraries(flight-sql-test-app PRIVATE ${ARROW_FLIGHT_SQL_TEST_LINK_LIBS}
104119
${GFLAGS_LIBRARIES})
120+
121+
if(ARROW_FLIGHT_TEST_LINKAGE STREQUAL "static" AND ARROW_BUILD_STATIC)
122+
target_compile_definitions(flight_sql_test
123+
flight-sql-test-server flight-sql-test-app
124+
PUBLIC ARROW_FLIGHT_STATIC ARROW_FLIGHT_SQL_STATIC)
125+
endif()
105126
endif()

‎cpp/src/arrow/flight/sql/client.cc‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,12 +15,15 @@
1515
// specific language governing permissions and limitations
1616
// under the License.
1717

18+
// Platform-specific defines
19+
#include "arrow/flight/platform.h"
20+
1821
#include "arrow/flight/sql/client.h"
1922

2023
#include <google/protobuf/any.pb.h>
2124

2225
#include "arrow/buffer.h"
23-
#include "arrow/flight/sql/FlightSql.pb.h"
26+
#include "arrow/flight/sql/protocol_internal.h"
2427
#include "arrow/flight/types.h"
2528
#include "arrow/io/memory.h"
2629
#include "arrow/ipc/reader.h"

‎cpp/src/arrow/flight/sql/client.h‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222

2323
#include "arrow/flight/client.h"
2424
#include "arrow/flight/sql/types.h"
25+
#include "arrow/flight/sql/visibility.h"
2526
#include "arrow/flight/types.h"
2627
#include "arrow/result.h"
2728
#include "arrow/status.h"
@@ -35,7 +36,7 @@ class PreparedStatement;
3536
/// \brief Flight client with Flight SQL semantics.
3637
///
3738
/// Wraps a Flight client to provide the Flight SQL RPC calls.
38-
class ARROW_EXPORT FlightSqlClient {
39+
class ARROW_FLIGHT_SQL_EXPORT FlightSqlClient {
3940
friend class PreparedStatement;
4041

4142
private:
@@ -202,7 +203,7 @@ class ARROW_EXPORT FlightSqlClient {
202203
};
203204

204205
/// \brief A prepared statement that can be executed.
205-
class ARROW_EXPORT PreparedStatement {
206+
class ARROW_FLIGHT_SQL_EXPORT PreparedStatement {
206207
public:
207208
/// \brief Create a new prepared statement. However, applications
208209
/// should generally use FlightSqlClient::Prepare.

‎cpp/src/arrow/flight/sql/client_test.cc‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,9 @@
1515
// specific language governing permissions and limitations
1616
// under the License.
1717

18+
// Platform-specific defines
19+
#include "arrow/flight/platform.h"
20+
1821
#include "arrow/flight/client.h"
1922

2023
#include <gmock/gmock.h>
@@ -24,8 +27,8 @@
2427
#include <utility>
2528

2629
#include "arrow/buffer.h"
27-
#include "arrow/flight/sql/FlightSql.pb.h"
2830
#include "arrow/flight/sql/api.h"
31+
#include "arrow/flight/sql/protocol_internal.h"
2932
#include "arrow/testing/gtest_util.h"
3033

3134
namespace pb = arrow::flight::protocol;

‎cpp/src/arrow/flight/sql/column_metadata.h‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,14 +19,15 @@
1919

2020
#include <string>
2121

22+
#include "arrow/flight/sql/visibility.h"
2223
#include "arrow/util/key_value_metadata.h"
2324

2425
namespace arrow {
2526
namespace flight {
2627
namespace sql {
2728

2829
/// \brief Helper class to set column metadata.
29-
class ColumnMetadata {
30+
class ARROW_FLIGHT_SQL_EXPORT ColumnMetadata {
3031
private:
3132
std::shared_ptr<const arrow::KeyValueMetadata> metadata_map_;
3233

@@ -114,7 +115,7 @@ class ColumnMetadata {
114115
const std::shared_ptr<const arrow::KeyValueMetadata>& metadata_map() const;
115116

116117
/// \brief A builder class to construct the ColumnMetadata object.
117-
class ColumnMetadataBuilder {
118+
class ARROW_FLIGHT_SQL_EXPORT ColumnMetadataBuilder {
118119
public:
119120
friend class ColumnMetadata;
120121

‎cpp/src/arrow/flight/sql/example/sqlite_server.cc‎

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -249,9 +249,11 @@ class SQLiteFlightSqlServer::Impl {
249249
std::string GenerateRandomString() {
250250
uint32_t length = 16;
251251

252-
std::uniform_int_distribution<char> dist('0', 'z');
252+
// MSVC doesn't support char types here
253+
std::uniform_int_distribution<uint16_t> dist(static_cast<uint16_t>('0'),
254+
static_cast<uint16_t>('z'));
253255
std::string ret(length, 0);
254-
auto get_random_char = [&]() { return dist(gen_); };
256+
auto get_random_char = [&]() { return static_cast<char>(dist(gen_)); };
255257
std::generate_n(ret.begin(), length, get_random_char);
256258
return ret;
257259
}
@@ -384,14 +386,14 @@ class SQLiteFlightSqlServer::Impl {
384386
const ActionCreatePreparedStatementRequest& request) {
385387
std::shared_ptr<SqliteStatement> statement;
386388
ARROW_ASSIGN_OR_RAISE(statement, SqliteStatement::Create(db_, request.query));
387-
const std::string handle = GenerateRandomString();
389+
std::string handle = GenerateRandomString();
388390
prepared_statements_[handle] = statement;
389391

390392
ARROW_ASSIGN_OR_RAISE(auto dataset_schema, statement->GetSchema());
391393

392394
sqlite3_stmt* stmt = statement->GetSqlite3Stmt();
393395
const int parameter_count = sqlite3_bind_parameter_count(stmt);
394-
std::vector<std::shared_ptr<arrow::Field>> parameter_fields;
396+
FieldVector parameter_fields;
395397
parameter_fields.reserve(parameter_count);
396398

397399
// As SQLite doesn't know the parameter types before executing the query, the
@@ -410,13 +412,9 @@ class SQLiteFlightSqlServer::Impl {
410412
parameter_fields.push_back(field(parameter_name, dense_union_type));
411413
}
412414

413-
const std::shared_ptr<Schema>& parameter_schema = arrow::schema(parameter_fields);
414-
415-
ActionCreatePreparedStatementResult result{.dataset_schema = dataset_schema,
416-
.parameter_schema = parameter_schema,
417-
.prepared_statement_handle = handle};
418-
419-
return result;
415+
std::shared_ptr<Schema> parameter_schema = arrow::schema(parameter_fields);
416+
return ActionCreatePreparedStatementResult{
417+
std::move(dataset_schema), std::move(parameter_schema), std::move(handle)};
420418
}
421419

422420
Status ClosePreparedStatement(const ServerCallContext& context,

0 commit comments

Comments
 (0)