Skip to content

Commit 649e653

Browse files
committed
Doxygen Error Fixes and Address comments from Kou and James
Address comments from Kou - Updates License.txt - Update cmake toolchain - Move whereami to `vendored` - Use string_view instead of NOLINT std::string Remove `whereami.cc` from arrow util build We are building whereami.cc as part of odbc Fix include headers to replace <> with "" Address comments from James
1 parent d8bd76f commit 649e653

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

57 files changed

+290
-286
lines changed

LICENSE.txt

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2290,3 +2290,46 @@ AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
22902290
LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
22912291
OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
22922292
THE SOFTWARE.
2293+
2294+
--------------------------------------------------------------------------------
2295+
The files cpp/src/arrow/vendored/whereami/whereami.h,
2296+
cpp/src/arrow/vendored/whereami/whereami.cc are adapted from
2297+
Grégory Pakosz's whereami library (https://github.com/gpakosz/whereami)
2298+
It is dual licensed under both the WTFPLv2 and MIT licenses.
2299+
2300+
The WTFPLv2 License
2301+
DO WHAT THE FUCK YOU WANT TO PUBLIC LICENSE
2302+
Version 2, December 2004
2303+
2304+
Copyright (C) 2004 Sam Hocevar <[email protected]>
2305+
2306+
Everyone is permitted to copy and distribute verbatim or modified
2307+
copies of this license document, and changing it is allowed as long
2308+
as the name is changed.
2309+
2310+
DO WHAT THE FUCK YOU WANT TO PUBLIC LICENSE
2311+
TERMS AND CONDITIONS FOR COPYING, DISTRIBUTION AND MODIFICATION
2312+
2313+
0. You just DO WHAT THE FUCK YOU WANT TO.
2314+
1. Bla bla bla
2315+
2. Montesqieu et camembert, vive la France, zut alors!
2316+
2317+
The MIT License (MIT)
2318+
Copyright Gregory Pakosz
2319+
2320+
Permission is hereby granted, free of charge, to any person obtaining a copy of
2321+
this software and associated documentation files (the "Software"), to deal in
2322+
the Software without restriction, including without limitation the rights to
2323+
use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of
2324+
the Software, and to permit persons to whom the Software is furnished to do so,
2325+
subject to the following conditions:
2326+
2327+
The above copyright notice and this permission notice shall be included in all
2328+
copies or substantial portions of the Software.
2329+
2330+
THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
2331+
IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS
2332+
FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR
2333+
COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER
2334+
IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
2335+
CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.

cpp/cmake_modules/ThirdpartyToolchain.cmake

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1288,6 +1288,9 @@ if(ARROW_USE_BOOST)
12881288
endif()
12891289
if(ARROW_BOOST_REQUIRE_LIBRARY)
12901290
set(ARROW_BOOST_COMPONENTS filesystem system)
1291+
if(ARROW_FLIGHT_SQL_ODBC AND MSVC)
1292+
list(APPEND ARROW_BOOST_COMPONENTS locale)
1293+
endif()
12911294
set(ARROW_BOOST_OPTIONAL_COMPONENTS process)
12921295
else()
12931296
set(ARROW_BOOST_COMPONENTS)
@@ -5562,14 +5565,11 @@ if(ARROW_WITH_AZURE_SDK)
55625565
Azure::azure-storage-blobs Azure::azure-identity)
55635566
endif()
55645567

5565-
message(STATUS "All bundled static libraries: ${ARROW_BUNDLED_STATIC_LIBS}")
5566-
55675568
# ----------------------------------------------------------------------
55685569
# Apache Flight SQL ODBC
55695570

55705571
if(ARROW_FLIGHT_SQL_ODBC)
55715572
find_package(ODBC REQUIRED)
5572-
if(MSVC)
5573-
find_package(Boost REQUIRED COMPONENTS locale)
5574-
endif()
55755573
endif()
5574+
5575+
message(STATUS "All bundled static libraries: ${ARROW_BUNDLED_STATIC_LIBS}")

cpp/src/arrow/flight/sql/odbc/flight_sql/accessors/date_array_accessor.cc

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@
1717

1818
#include "arrow/flight/sql/odbc/flight_sql/accessors/date_array_accessor.h"
1919
#include <time.h>
20-
#include "arrow/compute/api.h"
2120
#include "arrow/flight/sql/odbc/odbcabstraction/include/odbcabstraction/calendar_utils.h"
2221

2322
using arrow::Date32Array;

cpp/src/arrow/flight/sql/odbc/flight_sql/accessors/primitive_array_accessor_test.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616
// under the License.
1717

1818
#include "arrow/flight/sql/odbc/flight_sql/accessors/primitive_array_accessor.h"
19-
#include <arrow/flight/sql/odbc/odbcabstraction/include/odbcabstraction/diagnostics.h>
19+
#include "arrow/flight/sql/odbc/odbcabstraction/include/odbcabstraction/diagnostics.h"
2020
#include "arrow/testing/builder.h"
2121
#include "gtest/gtest.h"
2222

cpp/src/arrow/flight/sql/odbc/flight_sql/address_info.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
#include <string>
2121

2222
#include <arrow/flight/sql/odbc/odbcabstraction/include/odbcabstraction/platform.h>
23+
2324
#include <sys/types.h>
2425
#if !_WIN32
2526
# include <netdb.h>

cpp/src/arrow/flight/sql/odbc/flight_sql/config/configuration.cc

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -34,19 +34,20 @@ static const char DEFAULT_USE_CERT_STORE[] = TRUE_STR;
3434
static const char DEFAULT_DISABLE_CERT_VERIFICATION[] = FALSE_STR;
3535

3636
namespace {
37-
std::string ReadDsnString(const std::string& dsn, const std::string& key,
37+
std::string ReadDsnString(const std::string& dsn, const std::string_view& key,
3838
const std::string& dflt = "") {
3939
#define BUFFER_SIZE (1024)
4040
std::vector<char> buf(BUFFER_SIZE);
41-
42-
int ret = SQLGetPrivateProfileString(dsn.c_str(), key.c_str(), dflt.c_str(), buf.data(),
43-
static_cast<int>(buf.size()), "ODBC.INI");
41+
int ret =
42+
SQLGetPrivateProfileString(dsn.c_str(), key.data(), dflt.c_str(), buf.data(),
43+
static_cast<int>(buf.size()), "ODBC.INI");
4444

4545
if (ret > BUFFER_SIZE) {
4646
// If there wasn't enough space, try again with the right size buffer.
4747
buf.resize(ret + 1);
48-
ret = SQLGetPrivateProfileString(dsn.c_str(), key.c_str(), dflt.c_str(), buf.data(),
49-
static_cast<int>(buf.size()), "ODBC.INI");
48+
ret =
49+
SQLGetPrivateProfileString(dsn.c_str(), key.data(), dflt.c_str(), buf.data(),
50+
static_cast<int>(buf.size()), "ODBC.INI");
5051
}
5152

5253
return std::string(buf.data(), ret);
@@ -131,17 +132,18 @@ void Configuration::LoadDsn(const std::string& dsn) {
131132
auto customKeys = ReadAllKeys(dsn);
132133
RemoveAllKnownKeys(customKeys);
133134
for (auto key : customKeys) {
134-
Set(key, ReadDsnString(dsn, key));
135+
std::string_view key_sv(key);
136+
Set(key, ReadDsnString(dsn, key_sv));
135137
}
136138
}
137139

138140
void Configuration::Clear() { this->properties.clear(); }
139141

140-
bool Configuration::IsSet(const std::string& key) const {
142+
bool Configuration::IsSet(const std::string_view& key) const {
141143
return 0 != this->properties.count(key);
142144
}
143145

144-
const std::string& Configuration::Get(const std::string& key) const {
146+
const std::string& Configuration::Get(const std::string_view& key) const {
145147
const auto itr = this->properties.find(key);
146148
if (itr == this->properties.cend()) {
147149
static const std::string empty("");
@@ -150,7 +152,7 @@ const std::string& Configuration::Get(const std::string& key) const {
150152
return itr->second;
151153
}
152154

153-
void Configuration::Set(const std::string& key, const std::string& value) {
155+
void Configuration::Set(const std::string_view& key, const std::string& value) {
154156
const std::string copy = boost::trim_copy(value);
155157
if (!copy.empty()) {
156158
this->properties[key] = value;
@@ -162,12 +164,12 @@ const driver::odbcabstraction::Connection::ConnPropertyMap& Configuration::GetPr
162164
return this->properties;
163165
}
164166

165-
std::vector<std::string> Configuration::GetCustomKeys() const {
167+
std::vector<std::string_view> Configuration::GetCustomKeys() const {
166168
driver::odbcabstraction::Connection::ConnPropertyMap copyProps(properties);
167169
for (auto& key : FlightSqlConnection::ALL_KEYS) {
168170
copyProps.erase(key);
169171
}
170-
std::vector<std::string> keys;
172+
std::vector<std::string_view> keys;
171173
boost::copy(copyProps | boost::adaptors::map_keys, std::back_inserter(keys));
172174
return keys;
173175
}

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

Lines changed: 14 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -62,34 +62,7 @@ using driver::odbcabstraction::DriverException;
6262
using driver::odbcabstraction::OdbcVersion;
6363
using driver::odbcabstraction::Statement;
6464

65-
// clang-format off
66-
const std::string FlightSqlConnection::DSN = "dsn"; // NOLINT(runtime/string)
67-
const std::string FlightSqlConnection::DRIVER = "driver"; // NOLINT(runtime/string)
68-
const std::string FlightSqlConnection::HOST = "host"; // NOLINT(runtime/string)
69-
const std::string FlightSqlConnection::PORT = "port"; // NOLINT(runtime/string)
70-
const std::string FlightSqlConnection::USER = "user"; // NOLINT(runtime/string)
71-
const std::string FlightSqlConnection::USER_ID = "user id"; // NOLINT(runtime/string)
72-
const std::string FlightSqlConnection::UID = "uid"; // NOLINT(runtime/string)
73-
const std::string FlightSqlConnection::PASSWORD = "password"; // NOLINT(runtime/string)
74-
const std::string FlightSqlConnection::PWD = "pwd"; // NOLINT(runtime/string)
75-
const std::string FlightSqlConnection::TOKEN = "token"; // NOLINT(runtime/string)
76-
const std::string FlightSqlConnection::USE_ENCRYPTION = // NOLINT(runtime/string)
77-
"useEncryption";
78-
const std::string FlightSqlConnection::DISABLE_CERTIFICATE_VERIFICATION = // NOLINT
79-
"disableCertificateVerification";
80-
const std::string FlightSqlConnection::TRUSTED_CERTS = // NOLINT(runtime/string)
81-
"trustedCerts";
82-
const std::string FlightSqlConnection::USE_SYSTEM_TRUST_STORE = // NOLINT(runtime/string)
83-
"useSystemTrustStore";
84-
const std::string FlightSqlConnection::STRING_COLUMN_LENGTH = // NOLINT(runtime/string)
85-
"StringColumnLength";
86-
const std::string FlightSqlConnection::USE_WIDE_CHAR = // NOLINT(runtime/string)
87-
"UseWideChar";
88-
const std::string FlightSqlConnection::CHUNK_BUFFER_CAPACITY = // NOLINT(runtime/string)
89-
"ChunkBufferCapacity";
90-
// clang-format on
91-
92-
const std::vector<std::string> FlightSqlConnection::ALL_KEYS = {
65+
const std::vector<std::string_view> FlightSqlConnection::ALL_KEYS = {
9366
FlightSqlConnection::DSN,
9467
FlightSqlConnection::DRIVER,
9568
FlightSqlConnection::HOST,
@@ -138,7 +111,7 @@ inline std::string GetCerts() { return ""; }
138111

139112
#endif
140113

141-
const std::set<std::string, odbcabstraction::CaseInsensitiveComparator>
114+
const std::set<std::string_view, odbcabstraction::CaseInsensitiveComparator>
142115
BUILT_IN_PROPERTIES = {FlightSqlConnection::HOST,
143116
FlightSqlConnection::PORT,
144117
FlightSqlConnection::USER,
@@ -155,8 +128,8 @@ const std::set<std::string, odbcabstraction::CaseInsensitiveComparator>
155128
FlightSqlConnection::USE_WIDE_CHAR};
156129

157130
Connection::ConnPropertyMap::const_iterator TrackMissingRequiredProperty(
158-
const std::string& property, const Connection::ConnPropertyMap& properties,
159-
std::vector<std::string>& missing_attr) {
131+
const std::string_view& property, const Connection::ConnPropertyMap& properties,
132+
std::vector<std::string_view>& missing_attr) {
160133
auto prop_iter = properties.find(property);
161134
if (properties.end() == prop_iter) {
162135
missing_attr.push_back(property);
@@ -186,7 +159,7 @@ std::shared_ptr<FlightSqlSslConfig> LoadFlightSslConfigs(
186159
}
187160

188161
void FlightSqlConnection::Connect(const ConnPropertyMap& properties,
189-
std::vector<std::string>& missing_attr) {
162+
std::vector<std::string_view>& missing_attr) {
190163
try {
191164
auto flight_ssl_configs = LoadFlightSslConfigs(properties);
192165

@@ -242,7 +215,7 @@ boost::optional<int32_t> FlightSqlConnection::GetStringColumnLength(
242215
} catch (const std::exception& e) {
243216
diagnostics_.AddWarning(
244217
std::string("Invalid value for connection property " +
245-
FlightSqlConnection::STRING_COLUMN_LENGTH +
218+
std::string(FlightSqlConnection::STRING_COLUMN_LENGTH) +
246219
". Please ensure it has a valid numeric value. Message: " + e.what()),
247220
"01000", odbcabstraction::ODBCErrorCodes_GENERAL_WARNING);
248221
}
@@ -271,7 +244,7 @@ size_t FlightSqlConnection::GetChunkBufferCapacity(
271244
} catch (const std::exception& e) {
272245
diagnostics_.AddWarning(
273246
std::string("Invalid value for connection property " +
274-
FlightSqlConnection::CHUNK_BUFFER_CAPACITY +
247+
std::string(FlightSqlConnection::CHUNK_BUFFER_CAPACITY) +
275248
". Please ensure it has a valid numeric value. Message: " + e.what()),
276249
"01000", odbcabstraction::ODBCErrorCodes_GENERAL_WARNING);
277250
}
@@ -299,7 +272,7 @@ const FlightCallOptions& FlightSqlConnection::PopulateCallOptions(
299272
// Connection properties containing spaces will crash gRPC, but some tools
300273
// such as the OLE DB to ODBC bridge generate unused properties containing spaces.
301274
diagnostics_.AddWarning(
302-
std::string("Ignoring connection option " + prop.first) +
275+
std::string("Ignoring connection option " + std::string(prop.first)) +
303276
". Server-specific options must be valid HTTP header names and " +
304277
"cannot contain spaces.",
305278
"01000", odbcabstraction::ODBCErrorCodes_GENERAL_WARNING);
@@ -308,15 +281,15 @@ const FlightCallOptions& FlightSqlConnection::PopulateCallOptions(
308281

309282
// Note: header names must be lower case for gRPC.
310283
// gRPC will crash if they are not lower-case.
311-
std::string key_lc = boost::algorithm::to_lower_copy(prop.first);
284+
std::string key_lc = boost::algorithm::to_lower_copy(std::string(prop.first));
312285
call_options_.headers.emplace_back(std::make_pair(key_lc, prop.second));
313286
}
314287

315288
return call_options_;
316289
}
317290

318291
FlightClientOptions FlightSqlConnection::BuildFlightClientOptions(
319-
const ConnPropertyMap& properties, std::vector<std::string>& missing_attr,
292+
const ConnPropertyMap& properties, std::vector<std::string_view>& missing_attr,
320293
const std::shared_ptr<FlightSqlSslConfig>& ssl_config) {
321294
FlightClientOptions options;
322295
// Persist state information using cookies if the FlightProducer supports it.
@@ -343,15 +316,17 @@ FlightClientOptions FlightSqlConnection::BuildFlightClientOptions(
343316
}
344317

345318
Location FlightSqlConnection::BuildLocation(
346-
const ConnPropertyMap& properties, std::vector<std::string>& missing_attr,
319+
const ConnPropertyMap& properties, std::vector<std::string_view>& missing_attr,
347320
const std::shared_ptr<FlightSqlSslConfig>& ssl_config) {
348321
const auto& host_iter = TrackMissingRequiredProperty(HOST, properties, missing_attr);
349322

350323
const auto& port_iter = TrackMissingRequiredProperty(PORT, properties, missing_attr);
351324

352325
if (!missing_attr.empty()) {
326+
std::vector<std::string> missing_attr_string_vec(missing_attr.begin(),
327+
missing_attr.end());
353328
std::string missing_attr_str = std::string("Missing required properties: ") +
354-
boost::algorithm::join(missing_attr, ", ");
329+
boost::algorithm::join(missing_attr_string_vec, ", ");
355330
throw DriverException(missing_attr_str);
356331
}
357332

cpp/src/arrow/flight/sql/odbc/flight_sql/flight_sql_connection.h

Lines changed: 22 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -53,30 +53,31 @@ class FlightSqlConnection : public odbcabstraction::Connection {
5353
void PopulateMetadataSettings(const Connection::ConnPropertyMap& connPropertyMap);
5454

5555
public:
56-
static const std::vector<std::string> ALL_KEYS;
57-
static const std::string DSN;
58-
static const std::string DRIVER;
59-
static const std::string HOST;
60-
static const std::string PORT;
61-
static const std::string USER;
62-
static const std::string UID;
63-
static const std::string USER_ID;
64-
static const std::string PASSWORD;
65-
static const std::string PWD;
66-
static const std::string TOKEN;
67-
static const std::string USE_ENCRYPTION;
68-
static const std::string DISABLE_CERTIFICATE_VERIFICATION;
69-
static const std::string TRUSTED_CERTS;
70-
static const std::string USE_SYSTEM_TRUST_STORE;
71-
static const std::string STRING_COLUMN_LENGTH;
72-
static const std::string USE_WIDE_CHAR;
73-
static const std::string CHUNK_BUFFER_CAPACITY;
56+
static const std::vector<std::string_view> ALL_KEYS;
57+
static constexpr std::string_view DSN = "dsn";
58+
static constexpr std::string_view DRIVER = "driver";
59+
static constexpr std::string_view HOST = "host";
60+
static constexpr std::string_view PORT = "port";
61+
static constexpr std::string_view USER = "user";
62+
static constexpr std::string_view USER_ID = "user id";
63+
static constexpr std::string_view UID = "uid";
64+
static constexpr std::string_view PASSWORD = "password";
65+
static constexpr std::string_view PWD = "pwd";
66+
static constexpr std::string_view TOKEN = "token";
67+
static constexpr std::string_view USE_ENCRYPTION = "useEncryption";
68+
static constexpr std::string_view DISABLE_CERTIFICATE_VERIFICATION =
69+
"disableCertificateVerification";
70+
static constexpr std::string_view TRUSTED_CERTS = "trustedCerts";
71+
static constexpr std::string_view USE_SYSTEM_TRUST_STORE = "useSystemTrustStore";
72+
static constexpr std::string_view STRING_COLUMN_LENGTH = "StringColumnLength";
73+
static constexpr std::string_view USE_WIDE_CHAR = "UseWideChar";
74+
static constexpr std::string_view CHUNK_BUFFER_CAPACITY = "ChunkBufferCapacity";
7475

7576
explicit FlightSqlConnection(odbcabstraction::OdbcVersion odbc_version,
7677
const std::string& driver_version = "0.9.0.0");
7778

7879
void Connect(const ConnPropertyMap& properties,
79-
std::vector<std::string>& missing_attr) override;
80+
std::vector<std::string_view>& missing_attr) override;
8081

8182
void Close() override;
8283

@@ -92,13 +93,13 @@ class FlightSqlConnection : public odbcabstraction::Connection {
9293
/// \brief Builds a Location used for FlightClient connection.
9394
/// \note Visible for testing
9495
static arrow::flight::Location BuildLocation(
95-
const ConnPropertyMap& properties, std::vector<std::string>& missing_attr,
96+
const ConnPropertyMap& properties, std::vector<std::string_view>& missing_attr,
9697
const std::shared_ptr<FlightSqlSslConfig>& ssl_config);
9798

9899
/// \brief Builds a FlightClientOptions used for FlightClient connection.
99100
/// \note Visible for testing
100101
static arrow::flight::FlightClientOptions BuildFlightClientOptions(
101-
const ConnPropertyMap& properties, std::vector<std::string>& missing_attr,
102+
const ConnPropertyMap& properties, std::vector<std::string_view>& missing_attr,
102103
const std::shared_ptr<FlightSqlSslConfig>& ssl_config);
103104

104105
/// \brief Builds a FlightCallOptions used on gRPC calls.

cpp/src/arrow/flight/sql/odbc/flight_sql/flight_sql_connection_test.cc

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@ TEST(MetadataSettingsTest, UseWideCharTest) {
103103
}
104104

105105
TEST(BuildLocationTests, ForTcp) {
106-
std::vector<std::string> missing_attr;
106+
std::vector<std::string_view> missing_attr;
107107
Connection::ConnPropertyMap properties = {
108108
{FlightSqlConnection::HOST, std::string("localhost")},
109109
{FlightSqlConnection::PORT, std::string("32010")},
@@ -129,7 +129,7 @@ TEST(BuildLocationTests, ForTcp) {
129129
}
130130

131131
TEST(BuildLocationTests, ForTls) {
132-
std::vector<std::string> missing_attr;
132+
std::vector<std::string_view> missing_attr;
133133
Connection::ConnPropertyMap properties = {
134134
{FlightSqlConnection::HOST, std::string("localhost")},
135135
{FlightSqlConnection::PORT, std::string("32010")},

cpp/src/arrow/flight/sql/odbc/flight_sql/flight_sql_result_set_accessors.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,9 @@
1717

1818
#pragma once
1919

20-
#include <arrow/flight/sql/odbc/odbcabstraction/include/odbcabstraction/types.h>
21-
#include <arrow/type_fwd.h>
2220
#include <memory>
21+
#include "arrow/flight/sql/odbc/odbcabstraction/include/odbcabstraction/types.h"
22+
#include "arrow/type_fwd.h"
2323

2424
namespace driver {
2525
namespace flight_sql {

0 commit comments

Comments
 (0)