Skip to content

Commit a9eae88

Browse files
committed
Change map to use std::string
std::string_view doesn't work well with std::map because once the std::string_view object is out of scope, the key is no longer valid. The map then has undefined behavior. Enabling transparent comparing only helps with the `find` function, not inserts. For inserts we still need to use a std::string constructor. Ran ODBC tests on MSVC locally and they passed.
1 parent bc7ed1e commit a9eae88

File tree

4 files changed

+24
-24
lines changed

4 files changed

+24
-24
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -171,7 +171,7 @@ void Configuration::Set(std::string_view key, const std::wstring& wvalue) {
171171
void Configuration::Set(std::string_view key, const std::string& value) {
172172
const std::string copy = boost::trim_copy(value);
173173
if (!copy.empty()) {
174-
this->properties_[key] = value;
174+
this->properties_[std::string(key)] = value;
175175
}
176176
}
177177

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

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -65,10 +65,10 @@ TEST(MetadataSettingsTest, StringColumnLengthTest) {
6565
const int32_t expected_string_column_length = 100000;
6666

6767
const Connection::ConnPropertyMap properties = {
68-
{FlightSqlConnection::HOST, "localhost"}, // expect not used
69-
{FlightSqlConnection::PORT, "32010"}, // expect not used
70-
{FlightSqlConnection::USE_ENCRYPTION, "false"}, // expect not used
71-
{FlightSqlConnection::STRING_COLUMN_LENGTH,
68+
{std::string(FlightSqlConnection::HOST), "localhost"}, // expect not used
69+
{std::string(FlightSqlConnection::PORT), "32010"}, // expect not used
70+
{std::string(FlightSqlConnection::USE_ENCRYPTION), "false"}, // expect not used
71+
{std::string(FlightSqlConnection::STRING_COLUMN_LENGTH),
7272
std::to_string(expected_string_column_length)},
7373
};
7474

@@ -86,10 +86,10 @@ TEST(MetadataSettingsTest, UseWideCharTest) {
8686
connection.SetClosed(false);
8787

8888
const Connection::ConnPropertyMap properties1 = {
89-
{FlightSqlConnection::USE_WIDE_CHAR, "true"},
89+
{std::string(FlightSqlConnection::USE_WIDE_CHAR), "true"},
9090
};
9191
const Connection::ConnPropertyMap properties2 = {
92-
{FlightSqlConnection::USE_WIDE_CHAR, "false"},
92+
{std::string(FlightSqlConnection::USE_WIDE_CHAR), "false"},
9393
};
9494

9595
EXPECT_EQ(true, connection.GetUseWideChar(properties1));
@@ -101,9 +101,9 @@ TEST(MetadataSettingsTest, UseWideCharTest) {
101101
TEST(BuildLocationTests, ForTcp) {
102102
std::vector<std::string_view> missing_attr;
103103
Connection::ConnPropertyMap properties = {
104-
{FlightSqlConnection::HOST, "localhost"},
105-
{FlightSqlConnection::PORT, "32010"},
106-
{FlightSqlConnection::USE_ENCRYPTION, "false"},
104+
{std::string(FlightSqlConnection::HOST), "localhost"},
105+
{std::string(FlightSqlConnection::PORT), "32010"},
106+
{std::string(FlightSqlConnection::USE_ENCRYPTION), "false"},
107107
};
108108

109109
const std::shared_ptr<FlightSqlSslConfig>& ssl_config =
@@ -113,8 +113,8 @@ TEST(BuildLocationTests, ForTcp) {
113113
FlightSqlConnection::BuildLocation(properties, missing_attr, ssl_config);
114114
const Location& actual_location2 = FlightSqlConnection::BuildLocation(
115115
{
116-
{FlightSqlConnection::HOST, "localhost"},
117-
{FlightSqlConnection::PORT, "32011"},
116+
{std::string(FlightSqlConnection::HOST), "localhost"},
117+
{std::string(FlightSqlConnection::PORT), "32011"},
118118
},
119119
missing_attr, ssl_config);
120120

@@ -127,9 +127,9 @@ TEST(BuildLocationTests, ForTcp) {
127127
TEST(BuildLocationTests, ForTls) {
128128
std::vector<std::string_view> missing_attr;
129129
Connection::ConnPropertyMap properties = {
130-
{FlightSqlConnection::HOST, "localhost"},
131-
{FlightSqlConnection::PORT, "32010"},
132-
{FlightSqlConnection::USE_ENCRYPTION, "1"},
130+
{std::string(FlightSqlConnection::HOST), "localhost"},
131+
{std::string(FlightSqlConnection::PORT), "32010"},
132+
{std::string(FlightSqlConnection::USE_ENCRYPTION), "1"},
133133
};
134134

135135
const std::shared_ptr<FlightSqlSslConfig>& ssl_config =
@@ -139,9 +139,9 @@ TEST(BuildLocationTests, ForTls) {
139139
FlightSqlConnection::BuildLocation(properties, missing_attr, ssl_config);
140140

141141
Connection::ConnPropertyMap second_properties = {
142-
{FlightSqlConnection::HOST, "localhost"},
143-
{FlightSqlConnection::PORT, "32011"},
144-
{FlightSqlConnection::USE_ENCRYPTION, "1"},
142+
{std::string(FlightSqlConnection::HOST), "localhost"},
143+
{std::string(FlightSqlConnection::PORT), "32011"},
144+
{std::string(FlightSqlConnection::USE_ENCRYPTION), "1"},
145145
};
146146

147147
const std::shared_ptr<FlightSqlSslConfig>& second_ssl_config =

cpp/src/arrow/flight/sql/odbc/odbc_impl/main.cc

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -202,11 +202,11 @@ int main() {
202202
driver.CreateConnection(arrow::flight::sql::odbc::OdbcVersion::V_3);
203203

204204
Connection::ConnPropertyMap properties = {
205-
{FlightSqlConnection::HOST, "automaster.apache"},
206-
{FlightSqlConnection::PORT, "32010"},
207-
{FlightSqlConnection::USER, "apache"},
208-
{FlightSqlConnection::PASSWORD, "apache123"},
209-
{FlightSqlConnection::USE_ENCRYPTION, "false"},
205+
{std::string(FlightSqlConnection::HOST), "automaster.apache"},
206+
{std::string(FlightSqlConnection::PORT), "32010"},
207+
{std::string(FlightSqlConnection::USER), "apache"},
208+
{std::string(FlightSqlConnection::PASSWORD), "apache123"},
209+
{std::string(FlightSqlConnection::USE_ENCRYPTION), "false"},
210210
};
211211
std::vector<std::string_view> missing_attr;
212212
connection->Connect(properties, missing_attr);

cpp/src/arrow/flight/sql/odbc/odbc_impl/spi/connection.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ struct CaseInsensitiveComparator {
4040
};
4141

4242
// PropertyMap is case-insensitive for keys.
43-
typedef std::map<std::string_view, std::string, CaseInsensitiveComparator> PropertyMap;
43+
typedef std::map<std::string, std::string, CaseInsensitiveComparator> PropertyMap;
4444

4545
class Statement;
4646

0 commit comments

Comments
 (0)