Skip to content

Commit af9f125

Browse files
alinaliBQalinaliBQ
authored
apacheGH-46584 Iterate endpoint
* use suggestion from James for one-liner change to return `Status::OK` directly * fix for iterating endpoints, it is based on JDBC's fix for the same bug * save value of `FlightClientOptions` * Use `TestFlightServer` Add `arrow_flight_sql_shared` to enable usages for `TestFlightServer` * Fix build errors from missing `gmock` * Add checks for array data Update flight_sql_stream_chunk_buffer_test.cc Add `FlightStreamChunkBufferTest` unit test Draft test with `FlightSQLODBCMockEndpointTestBase` * Reference apacheGH-47117 and Clean up code * Add driver and DSN to built-in properties to not pass driver/dsn as attributes to server * Allow `=` in values inside connection strings, fixes connectivity problems * Address comments from Rob
1 parent d7c7128 commit af9f125

16 files changed

+280
-78
lines changed

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -291,7 +291,9 @@ if(ARROW_TESTING)
291291
STATIC_INSTALL_INTERFACE_LIBS
292292
${ARROW_FLIGHT_TESTING_STATIC_INSTALL_INTERFACE_LIBS}
293293
PRIVATE_INCLUDES
294-
"${Protobuf_INCLUDE_DIRS}")
294+
"${Protobuf_INCLUDE_DIRS}"
295+
SHARED_PRIVATE_LINK_LIBS
296+
GTest::gmock)
295297

296298
foreach(LIB_TARGET ${ARROW_FLIGHT_TESTING_LIBRARIES})
297299
target_compile_definitions(${LIB_TARGET} PRIVATE ARROW_FLIGHT_EXPORTING)

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -136,9 +136,11 @@ add_arrow_test(arrow_odbc_spi_impl_test
136136
accessors/time_array_accessor_test.cc
137137
accessors/timestamp_array_accessor_test.cc
138138
flight_sql_connection_test.cc
139+
flight_sql_stream_chunk_buffer_test.cc
139140
parse_table_types_test.cc
140141
json_converter_test.cc
141142
record_batch_transformer_test.cc
142143
utils_test.cc
143144
EXTRA_LINK_LIBS
144-
arrow_odbc_spi_impl)
145+
arrow_odbc_spi_impl
146+
arrow_flight_testing_shared)

‎cpp/src/arrow/flight/sql/odbc/flight_sql/flight_sql_auth_method.cc‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,9 +58,9 @@ class NoOpClientAuthHandler : public arrow::flight::ClientAuthHandler {
5858

5959
arrow::Status Authenticate(arrow::flight::ClientAuthSender* outgoing,
6060
arrow::flight::ClientAuthReader* incoming) override {
61-
// Write a blank string. The server should ignore this and just accept any Handshake
61+
// Return OK Status. The server should ignore this and just accept any Handshake
6262
// request.
63-
return outgoing->Write(std::string());
63+
return arrow::Status::OK();
6464
}
6565

6666
arrow::Status GetToken(std::string* token) override {

‎cpp/src/arrow/flight/sql/odbc/flight_sql/flight_sql_connection.cc‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,8 @@ inline std::string GetCerts() { return ""; }
112112
#endif
113113

114114
const std::set<std::string_view, CaseInsensitiveComparatorStrView> BUILT_IN_PROPERTIES = {
115+
FlightSqlConnection::DRIVER,
116+
FlightSqlConnection::DSN,
115117
FlightSqlConnection::HOST,
116118
FlightSqlConnection::PORT,
117119
FlightSqlConnection::USER,
@@ -165,15 +167,15 @@ void FlightSqlConnection::Connect(const ConnPropertyMap& properties,
165167
auto flight_ssl_configs = LoadFlightSslConfigs(properties);
166168

167169
Location location = BuildLocation(properties, missing_attr, flight_ssl_configs);
168-
FlightClientOptions client_options =
170+
client_options_ =
169171
BuildFlightClientOptions(properties, missing_attr, flight_ssl_configs);
170172

171173
const std::shared_ptr<arrow::flight::ClientMiddlewareFactory>& cookie_factory =
172174
arrow::flight::GetCookieFactory();
173-
client_options.middleware.push_back(cookie_factory);
175+
client_options_.middleware.push_back(cookie_factory);
174176

175177
std::unique_ptr<FlightClient> flight_client;
176-
ThrowIfNotOK(FlightClient::Connect(location, client_options).Value(&flight_client));
178+
ThrowIfNotOK(FlightClient::Connect(location, client_options_).Value(&flight_client));
177179

178180
PopulateMetadataSettings(properties);
179181
PopulateCallOptions(properties);
@@ -377,7 +379,7 @@ void FlightSqlConnection::Close() {
377379

378380
std::shared_ptr<Statement> FlightSqlConnection::CreateStatement() {
379381
return std::shared_ptr<Statement>(new FlightSqlStatement(
380-
diagnostics_, *sql_client_, call_options_, metadata_settings_));
382+
diagnostics_, *sql_client_, client_options_, call_options_, metadata_settings_));
381383
}
382384

383385
bool FlightSqlConnection::SetAttribute(Connection::AttributeId attribute,
@@ -423,7 +425,7 @@ FlightSqlConnection::FlightSqlConnection(OdbcVersion odbc_version,
423425
const std::string& driver_version)
424426
: diagnostics_("Apache Arrow", "Flight SQL", odbc_version),
425427
odbc_version_(odbc_version),
426-
info_(call_options_, sql_client_, driver_version),
428+
info_(client_options_, call_options_, sql_client_, driver_version),
427429
closed_(true) {
428430
attribute_[CONNECTION_DEAD] = static_cast<uint32_t>(SQL_TRUE);
429431
attribute_[LOGIN_TIMEOUT] = static_cast<uint32_t>(0);

‎cpp/src/arrow/flight/sql/odbc/flight_sql/flight_sql_result_set.cc‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,13 +44,14 @@ using odbcabstraction::DriverException;
4444

4545
FlightSqlResultSet::FlightSqlResultSet(
4646
FlightSqlClient& flight_sql_client,
47+
const arrow::flight::FlightClientOptions& client_options,
4748
const arrow::flight::FlightCallOptions& call_options,
4849
const std::shared_ptr<FlightInfo>& flight_info,
4950
const std::shared_ptr<RecordBatchTransformer>& transformer,
5051
odbcabstraction::Diagnostics& diagnostics,
5152
const odbcabstraction::MetadataSettings& metadata_settings)
5253
: metadata_settings_(metadata_settings),
53-
chunk_buffer_(flight_sql_client, call_options, flight_info,
54+
chunk_buffer_(flight_sql_client, client_options, call_options, flight_info,
5455
metadata_settings_.chunk_buffer_capacity_),
5556
transformer_(transformer),
5657
metadata_(transformer

‎cpp/src/arrow/flight/sql/odbc/flight_sql/flight_sql_result_set.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@ class FlightSqlResultSet : public ResultSet {
6363
~FlightSqlResultSet() override;
6464

6565
FlightSqlResultSet(FlightSqlClient& flight_sql_client,
66+
const arrow::flight::FlightClientOptions& client_options,
6667
const arrow::flight::FlightCallOptions& call_options,
6768
const std::shared_ptr<FlightInfo>& flight_info,
6869
const std::shared_ptr<RecordBatchTransformer>& transformer,

‎cpp/src/arrow/flight/sql/odbc/flight_sql/flight_sql_statement.cc‎

Lines changed: 29 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -64,11 +64,12 @@ void ClosePreparedStatementIfAny(
6464

6565
FlightSqlStatement::FlightSqlStatement(
6666
const odbcabstraction::Diagnostics& diagnostics, FlightSqlClient& sql_client,
67-
FlightCallOptions call_options,
67+
arrow::flight::FlightClientOptions client_options, FlightCallOptions call_options,
6868
const odbcabstraction::MetadataSettings& metadata_settings)
6969
: diagnostics_("Apache Arrow", diagnostics.GetDataSourceComponent(),
7070
diagnostics.GetOdbcVersion()),
7171
sql_client_(sql_client),
72+
client_options_(std::move(client_options)),
7273
call_options_(std::move(call_options)),
7374
metadata_settings_(metadata_settings) {
7475
attribute_[METADATA_ID] = static_cast<size_t>(SQL_FALSE);
@@ -135,8 +136,8 @@ bool FlightSqlStatement::ExecutePrepared() {
135136
ThrowIfNotOK(result.status());
136137

137138
current_result_set_ = std::make_shared<FlightSqlResultSet>(
138-
sql_client_, call_options_, result.ValueOrDie(), nullptr, diagnostics_,
139-
metadata_settings_);
139+
sql_client_, client_options_, call_options_, result.ValueOrDie(), nullptr,
140+
diagnostics_, metadata_settings_);
140141

141142
return true;
142143
}
@@ -148,8 +149,8 @@ bool FlightSqlStatement::Execute(const std::string& query) {
148149
ThrowIfNotOK(result.status());
149150

150151
current_result_set_ = std::make_shared<FlightSqlResultSet>(
151-
sql_client_, call_options_, result.ValueOrDie(), nullptr, diagnostics_,
152-
metadata_settings_);
152+
sql_client_, client_options_, call_options_, result.ValueOrDie(), nullptr,
153+
diagnostics_, metadata_settings_);
153154

154155
return true;
155156
}
@@ -170,27 +171,29 @@ std::shared_ptr<odbcabstraction::ResultSet> FlightSqlStatement::GetTables(
170171

171172
if ((catalog_name && *catalog_name == "%") && (schema_name && schema_name->empty()) &&
172173
(table_name && table_name->empty())) {
173-
current_result_set_ = GetTablesForSQLAllCatalogs(
174-
column_names, call_options_, sql_client_, diagnostics_, metadata_settings_);
174+
current_result_set_ =
175+
GetTablesForSQLAllCatalogs(column_names, client_options_, call_options_,
176+
sql_client_, diagnostics_, metadata_settings_);
175177
} else if ((catalog_name && catalog_name->empty()) &&
176178
(schema_name && *schema_name == "%") &&
177179
(table_name && table_name->empty())) {
178-
current_result_set_ =
179-
GetTablesForSQLAllDbSchemas(column_names, call_options_, sql_client_, schema_name,
180-
diagnostics_, metadata_settings_);
180+
current_result_set_ = GetTablesForSQLAllDbSchemas(
181+
column_names, client_options_, call_options_, sql_client_, schema_name,
182+
diagnostics_, metadata_settings_);
181183
} else if ((catalog_name && catalog_name->empty()) &&
182184
(schema_name && schema_name->empty()) &&
183185
(table_name && table_name->empty()) && (table_type && *table_type == "%")) {
184-
current_result_set_ = GetTablesForSQLAllTableTypes(
185-
column_names, call_options_, sql_client_, diagnostics_, metadata_settings_);
186+
current_result_set_ =
187+
GetTablesForSQLAllTableTypes(column_names, client_options_, call_options_,
188+
sql_client_, diagnostics_, metadata_settings_);
186189
} else {
187190
if (table_type) {
188191
ParseTableTypes(*table_type, table_types);
189192
}
190193

191194
current_result_set_ = GetTablesForGenericUse(
192-
column_names, call_options_, sql_client_, catalog_name, schema_name, table_name,
193-
table_types, diagnostics_, metadata_settings_);
195+
column_names, client_options_, call_options_, sql_client_, catalog_name,
196+
schema_name, table_name, table_types, diagnostics_, metadata_settings_);
194197
}
195198

196199
return current_result_set_;
@@ -228,9 +231,9 @@ std::shared_ptr<ResultSet> FlightSqlStatement::GetColumns_V2(
228231
auto transformer = std::make_shared<GetColumns_Transformer>(
229232
metadata_settings_, odbcabstraction::V_2, column_name);
230233

231-
current_result_set_ =
232-
std::make_shared<FlightSqlResultSet>(sql_client_, call_options_, flight_info,
233-
transformer, diagnostics_, metadata_settings_);
234+
current_result_set_ = std::make_shared<FlightSqlResultSet>(
235+
sql_client_, client_options_, call_options_, flight_info, transformer, diagnostics_,
236+
metadata_settings_);
234237

235238
return current_result_set_;
236239
}
@@ -249,9 +252,9 @@ std::shared_ptr<ResultSet> FlightSqlStatement::GetColumns_V3(
249252
auto transformer = std::make_shared<GetColumns_Transformer>(
250253
metadata_settings_, odbcabstraction::V_3, column_name);
251254

252-
current_result_set_ =
253-
std::make_shared<FlightSqlResultSet>(sql_client_, call_options_, flight_info,
254-
transformer, diagnostics_, metadata_settings_);
255+
current_result_set_ = std::make_shared<FlightSqlResultSet>(
256+
sql_client_, client_options_, call_options_, flight_info, transformer, diagnostics_,
257+
metadata_settings_);
255258

256259
return current_result_set_;
257260
}
@@ -267,9 +270,9 @@ std::shared_ptr<ResultSet> FlightSqlStatement::GetTypeInfo_V2(int16_t data_type)
267270
auto transformer = std::make_shared<GetTypeInfo_Transformer>(
268271
metadata_settings_, odbcabstraction::V_2, data_type);
269272

270-
current_result_set_ =
271-
std::make_shared<FlightSqlResultSet>(sql_client_, call_options_, flight_info,
272-
transformer, diagnostics_, metadata_settings_);
273+
current_result_set_ = std::make_shared<FlightSqlResultSet>(
274+
sql_client_, client_options_, call_options_, flight_info, transformer, diagnostics_,
275+
metadata_settings_);
273276

274277
return current_result_set_;
275278
}
@@ -285,9 +288,9 @@ std::shared_ptr<ResultSet> FlightSqlStatement::GetTypeInfo_V3(int16_t data_type)
285288
auto transformer = std::make_shared<GetTypeInfo_Transformer>(
286289
metadata_settings_, odbcabstraction::V_3, data_type);
287290

288-
current_result_set_ =
289-
std::make_shared<FlightSqlResultSet>(sql_client_, call_options_, flight_info,
290-
transformer, diagnostics_, metadata_settings_);
291+
current_result_set_ = std::make_shared<FlightSqlResultSet>(
292+
sql_client_, client_options_, call_options_, flight_info, transformer, diagnostics_,
293+
metadata_settings_);
291294

292295
return current_result_set_;
293296
}

‎cpp/src/arrow/flight/sql/odbc/flight_sql/flight_sql_statement.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ class FlightSqlStatement : public odbcabstraction::Statement {
3333
private:
3434
odbcabstraction::Diagnostics diagnostics_;
3535
std::map<StatementAttributeId, Attribute> attribute_;
36+
arrow::flight::FlightClientOptions client_options_;
3637
arrow::flight::FlightCallOptions call_options_;
3738
arrow::flight::sql::FlightSqlClient& sql_client_;
3839
std::shared_ptr<odbcabstraction::ResultSet> current_result_set_;
@@ -48,6 +49,7 @@ class FlightSqlStatement : public odbcabstraction::Statement {
4849
public:
4950
FlightSqlStatement(const odbcabstraction::Diagnostics& diagnostics,
5051
arrow::flight::sql::FlightSqlClient& sql_client,
52+
arrow::flight::FlightClientOptions client_options,
5153
arrow::flight::FlightCallOptions call_options,
5254
const odbcabstraction::MetadataSettings& metadata_settings);
5355
~FlightSqlStatement();

‎cpp/src/arrow/flight/sql/odbc/flight_sql/flight_sql_statement_get_tables.cc‎

Lines changed: 25 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -72,8 +72,9 @@ void ParseTableTypes(const std::string& table_type,
7272
}
7373

7474
std::shared_ptr<ResultSet> GetTablesForSQLAllCatalogs(
75-
const ColumnNames& names, FlightCallOptions& call_options,
76-
FlightSqlClient& sql_client, odbcabstraction::Diagnostics& diagnostics,
75+
const ColumnNames& names, FlightClientOptions& client_options,
76+
FlightCallOptions& call_options, FlightSqlClient& sql_client,
77+
odbcabstraction::Diagnostics& diagnostics,
7778
const odbcabstraction::MetadataSettings& metadata_settings) {
7879
Result<std::shared_ptr<FlightInfo>> result = sql_client.GetCatalogs(call_options);
7980

@@ -92,14 +93,15 @@ std::shared_ptr<ResultSet> GetTablesForSQLAllCatalogs(
9293
.AddFieldOfNulls(names.remarks_column, arrow::utf8())
9394
.Build();
9495

95-
return std::make_shared<FlightSqlResultSet>(
96-
sql_client, call_options, flight_info, transformer, diagnostics, metadata_settings);
96+
return std::make_shared<FlightSqlResultSet>(sql_client, client_options, call_options,
97+
flight_info, transformer, diagnostics,
98+
metadata_settings);
9799
}
98100

99101
std::shared_ptr<ResultSet> GetTablesForSQLAllDbSchemas(
100-
const ColumnNames& names, FlightCallOptions& call_options,
101-
FlightSqlClient& sql_client, const std::string* schema_name,
102-
odbcabstraction::Diagnostics& diagnostics,
102+
const ColumnNames& names, FlightClientOptions& client_options,
103+
FlightCallOptions& call_options, FlightSqlClient& sql_client,
104+
const std::string* schema_name, odbcabstraction::Diagnostics& diagnostics,
103105
const odbcabstraction::MetadataSettings& metadata_settings) {
104106
Result<std::shared_ptr<FlightInfo>> result =
105107
sql_client.GetDbSchemas(call_options, nullptr, schema_name);
@@ -119,13 +121,15 @@ std::shared_ptr<ResultSet> GetTablesForSQLAllDbSchemas(
119121
.AddFieldOfNulls(names.remarks_column, arrow::utf8())
120122
.Build();
121123

122-
return std::make_shared<FlightSqlResultSet>(
123-
sql_client, call_options, flight_info, transformer, diagnostics, metadata_settings);
124+
return std::make_shared<FlightSqlResultSet>(sql_client, client_options, call_options,
125+
flight_info, transformer, diagnostics,
126+
metadata_settings);
124127
}
125128

126129
std::shared_ptr<ResultSet> GetTablesForSQLAllTableTypes(
127-
const ColumnNames& names, FlightCallOptions& call_options,
128-
FlightSqlClient& sql_client, odbcabstraction::Diagnostics& diagnostics,
130+
const ColumnNames& names, FlightClientOptions& client_options,
131+
FlightCallOptions& call_options, FlightSqlClient& sql_client,
132+
odbcabstraction::Diagnostics& diagnostics,
129133
const odbcabstraction::MetadataSettings& metadata_settings) {
130134
Result<std::shared_ptr<FlightInfo>> result = sql_client.GetTableTypes(call_options);
131135

@@ -144,15 +148,16 @@ std::shared_ptr<ResultSet> GetTablesForSQLAllTableTypes(
144148
.AddFieldOfNulls(names.remarks_column, arrow::utf8())
145149
.Build();
146150

147-
return std::make_shared<FlightSqlResultSet>(
148-
sql_client, call_options, flight_info, transformer, diagnostics, metadata_settings);
151+
return std::make_shared<FlightSqlResultSet>(sql_client, client_options, call_options,
152+
flight_info, transformer, diagnostics,
153+
metadata_settings);
149154
}
150155

151156
std::shared_ptr<ResultSet> GetTablesForGenericUse(
152-
const ColumnNames& names, FlightCallOptions& call_options,
153-
FlightSqlClient& sql_client, const std::string* catalog_name,
154-
const std::string* schema_name, const std::string* table_name,
155-
const std::vector<std::string>& table_types,
157+
const ColumnNames& names, FlightClientOptions& client_options,
158+
FlightCallOptions& call_options, FlightSqlClient& sql_client,
159+
const std::string* catalog_name, const std::string* schema_name,
160+
const std::string* table_name, const std::vector<std::string>& table_types,
156161
odbcabstraction::Diagnostics& diagnostics,
157162
const odbcabstraction::MetadataSettings& metadata_settings) {
158163
Result<std::shared_ptr<FlightInfo>> result = sql_client.GetTables(
@@ -173,8 +178,9 @@ std::shared_ptr<ResultSet> GetTablesForGenericUse(
173178
.AddFieldOfNulls(names.remarks_column, arrow::utf8())
174179
.Build();
175180

176-
return std::make_shared<FlightSqlResultSet>(
177-
sql_client, call_options, flight_info, transformer, diagnostics, metadata_settings);
181+
return std::make_shared<FlightSqlResultSet>(sql_client, client_options, call_options,
182+
flight_info, transformer, diagnostics,
183+
metadata_settings);
178184
}
179185

180186
} // namespace flight_sql

‎cpp/src/arrow/flight/sql/odbc/flight_sql/flight_sql_statement_get_tables.h‎

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ namespace driver {
3030
namespace flight_sql {
3131

3232
using arrow::flight::FlightCallOptions;
33+
using arrow::flight::FlightClientOptions;
3334
using arrow::flight::sql::FlightSqlClient;
3435
using odbcabstraction::MetadataSettings;
3536
using odbcabstraction::ResultSet;
@@ -46,26 +47,28 @@ void ParseTableTypes(const std::string& table_type,
4647
std::vector<std::string>& table_types);
4748

4849
std::shared_ptr<ResultSet> GetTablesForSQLAllCatalogs(
49-
const ColumnNames& column_names, FlightCallOptions& call_options,
50-
FlightSqlClient& sql_client, odbcabstraction::Diagnostics& diagnostics,
50+
const ColumnNames& column_names, FlightClientOptions& client_options,
51+
FlightCallOptions& call_options, FlightSqlClient& sql_client,
52+
odbcabstraction::Diagnostics& diagnostics,
5153
const odbcabstraction::MetadataSettings& metadata_settings);
5254

5355
std::shared_ptr<ResultSet> GetTablesForSQLAllDbSchemas(
54-
const ColumnNames& column_names, FlightCallOptions& call_options,
55-
FlightSqlClient& sql_client, const std::string* schema_name,
56-
odbcabstraction::Diagnostics& diagnostics,
56+
const ColumnNames& column_names, FlightClientOptions& client_options,
57+
FlightCallOptions& call_options, FlightSqlClient& sql_client,
58+
const std::string* schema_name, odbcabstraction::Diagnostics& diagnostics,
5759
const odbcabstraction::MetadataSettings& metadata_settings);
5860

5961
std::shared_ptr<ResultSet> GetTablesForSQLAllTableTypes(
60-
const ColumnNames& column_names, FlightCallOptions& call_options,
61-
FlightSqlClient& sql_client, odbcabstraction::Diagnostics& diagnostics,
62+
const ColumnNames& column_names, FlightClientOptions& client_options,
63+
FlightCallOptions& call_options, FlightSqlClient& sql_client,
64+
odbcabstraction::Diagnostics& diagnostics,
6265
const odbcabstraction::MetadataSettings& metadata_settings);
6366

6467
std::shared_ptr<ResultSet> GetTablesForGenericUse(
65-
const ColumnNames& column_names, FlightCallOptions& call_options,
66-
FlightSqlClient& sql_client, const std::string* catalog_name,
67-
const std::string* schema_name, const std::string* table_name,
68-
const std::vector<std::string>& table_types,
68+
const ColumnNames& column_names, FlightClientOptions& client_options,
69+
FlightCallOptions& call_options, FlightSqlClient& sql_client,
70+
const std::string* catalog_name, const std::string* schema_name,
71+
const std::string* table_name, const std::vector<std::string>& table_types,
6972
odbcabstraction::Diagnostics& diagnostics,
7073
const odbcabstraction::MetadataSettings& metadata_settings);
7174
} // namespace flight_sql

0 commit comments

Comments
 (0)