Skip to content

Commit c696e50

Browse files
justing-bqjusting-bq
committed
Address review feedback
1 parent a1bc183 commit c696e50

4 files changed

Lines changed: 29 additions & 17 deletions

File tree

‎cpp/src/arrow/flight/sql/odbc/tests/connection_info_test.cc‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -370,7 +370,8 @@ TYPED_TEST(ConnectionInfoTest, TestSQLGetInfoServerName) {
370370
SQLWCHAR value[kOdbcBufferSize] = {};
371371
GetInfoSQLWCHAR(conn, SQL_SERVER_NAME, value);
372372

373-
EXPECT_GT(wcslen(reinterpret_cast<wchar_t*>(value)), 0);
373+
std::wstring result = ConvertToWString(value);
374+
EXPECT_GT(result.length(), 0);
374375
}
375376

376377
TYPED_TEST(ConnectionInfoTest, TestSQLGetInfoStaticCursorAttributes1) {
@@ -401,14 +402,16 @@ TYPED_TEST(ConnectionInfoTest, TestSQLGetInfoDbmsName) {
401402
SQLWCHAR value[kOdbcBufferSize] = {};
402403
GetInfoSQLWCHAR(conn, SQL_DBMS_NAME, value);
403404

404-
EXPECT_GT(wcslen(reinterpret_cast<wchar_t*>(value)), 0);
405+
std::wstring result = ConvertToWString(value);
406+
EXPECT_GT(result.length(), 0);
405407
}
406408

407409
TYPED_TEST(ConnectionInfoTest, TestSQLGetInfoDbmsVer) {
408410
SQLWCHAR value[kOdbcBufferSize] = {};
409411
GetInfoSQLWCHAR(conn, SQL_DBMS_VER, value);
410412

411-
EXPECT_GT(wcslen(reinterpret_cast<wchar_t*>(value)), 0);
413+
std::wstring result = ConvertToWString(value);
414+
EXPECT_GT(result.length(), 0);
412415
}
413416

414417
// Data Source Information
@@ -848,7 +851,8 @@ TYPED_TEST(ConnectionInfoTest, TestSQLGetInfoKeywords) {
848851
SQLWCHAR value[info_len] = {};
849852
GetInfoSQLWCHAR(conn, SQL_KEYWORDS, value, info_len);
850853

851-
EXPECT_GT(wcslen(reinterpret_cast<wchar_t*>(value)), 0);
854+
std::wstring result = ConvertToWString(value, -1, info_len);
855+
EXPECT_GT(result.length(), 0);
852856
}
853857

854858
TYPED_TEST(ConnectionInfoTest, TestSQLGetInfoLikeEscapeClause) {

‎cpp/src/arrow/flight/sql/odbc/tests/odbc_test_suite.cc‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -490,7 +490,7 @@ bool WriteDSN(Connection::ConnPropertyMap properties) {
490490
}
491491

492492
std::string driver = config.Get(FlightSqlConnection::DRIVER);
493-
std::wstring w_driver = arrow::util::UTF8ToWideString(driver).ValueOr(L"");
493+
CONVERT_SQLWCHAR_STR(w_driver, driver);
494494
return RegisterDsn(config, reinterpret_cast<LPCWSTR>(w_driver.c_str()));
495495
}
496496

@@ -521,7 +521,8 @@ size_t SqlWCharArrLen(const SQLWCHAR* str_val) {
521521
return static_cast<size_t>(p - str_val);
522522
}
523523

524-
std::wstring ConvertToWString(const SQLWCHAR* str_val, SQLSMALLINT str_len) {
524+
std::wstring ConvertToWString(const SQLWCHAR* str_val, SQLSMALLINT str_len,
525+
SQLSMALLINT buffer_size) {
525526
if (str_len == -1) {
526527
#ifdef __linux__
527528
str_len = SqlWCharArrLen(str_val);
@@ -534,19 +535,20 @@ std::wstring ConvertToWString(const SQLWCHAR* str_val, SQLSMALLINT str_len) {
534535
attr_str = L"";
535536
} else {
536537
assert(str_val != nullptr);
537-
assert(str_len > 0 && str_len <= static_cast<SQLSMALLINT>(kOdbcBufferSize));
538+
assert(str_len > 0 && str_len <= buffer_size);
538539
attr_str.assign(str_val, str_val + str_len);
539540
}
540541
return attr_str;
541542
}
542543

543-
std::wstring ConvertToWString(const std::vector<SQLWCHAR>& str_val, SQLSMALLINT str_len) {
544+
std::wstring ConvertToWString(const std::vector<SQLWCHAR>& str_val, SQLSMALLINT str_len,
545+
SQLSMALLINT buffer_size) {
544546
std::wstring attr_str;
545547
if (str_len == 0) {
546548
attr_str = L"";
547549
} else {
548550
EXPECT_GT(str_len, 0);
549-
EXPECT_LE(str_len, static_cast<SQLSMALLINT>(kOdbcBufferSize));
551+
EXPECT_LE(str_len, buffer_size);
550552
attr_str =
551553
std::wstring(str_val.begin(), str_val.begin() + str_len / GetSqlWCharSize());
552554
}

‎cpp/src/arrow/flight/sql/odbc/tests/odbc_test_suite.h‎

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -39,17 +39,20 @@
3939
#include "arrow/flight/sql/odbc/odbc_impl/system_dsn.h"
4040

4141
#ifdef __linux__
42-
# define ASSIGN_SQLWCHAR_ARR(name, wstring_literal) \
43-
auto name##_vec = ODBC::ToSqlWCharVector(std::wstring(wstring_literal)); \
42+
# define ASSIGN_SQLWCHAR_ARR(name, wstring_literal) \
43+
auto name##_vec = ODBC::ToSqlWCharVector(std::wstring(wstring_literal)); \
44+
if (name##_vec.empty() || name##_vec.back() != static_cast<SQLWCHAR>(0)) { \
45+
name##_vec.push_back(static_cast<SQLWCHAR>(0)); \
46+
} \
4447
SQLWCHAR* name = name##_vec.data();
4548
# define ASSIGN_SQLWCHAR_ARR_AND_LEN(name, wstring_literal) \
4649
ASSIGN_SQLWCHAR_ARR(name, wstring_literal) \
47-
size_t name##_len = std::wstring(wstring_literal).length();
50+
SQLSMALLINT name##_len = static_cast<SQLSMALLINT>(name##_vec.size() - 1);
4851
#else // Windows & Mac
4952
# define ASSIGN_SQLWCHAR_ARR(name, wstring_literal) SQLWCHAR name[] = wstring_literal;
5053
# define ASSIGN_SQLWCHAR_ARR_AND_LEN(name, wstring_literal) \
5154
ASSIGN_SQLWCHAR_ARR(name, wstring_literal) \
52-
size_t name##_len = std::wcslen(name);
55+
SQLSMALLINT name##_len = static_cast<SQLSMALLINT>(std::wcslen(name));
5356
#endif
5457

5558
static constexpr std::string_view kTestConnectStr = "ARROW_FLIGHT_SQL_ODBC_CONN";
@@ -289,14 +292,18 @@ size_t SqlWCharArrLen(const SQLWCHAR* str_val);
289292
/// \brief Check wide char array and convert into wstring
290293
/// \param[in] str_val Array of SQLWCHAR.
291294
/// \param[in] str_len length of string, in number of characters.
295+
/// \param[in] buffer_size size of underlying buffer, in number of characters.
292296
/// \return wstring
293-
std::wstring ConvertToWString(const SQLWCHAR* str_val, SQLSMALLINT str_len = -1);
297+
std::wstring ConvertToWString(const SQLWCHAR* str_val, SQLSMALLINT str_len = -1,
298+
SQLSMALLINT buffer_size = kOdbcBufferSize);
294299

295300
/// \brief Check wide char vector and convert into wstring
296301
/// \param[in] str_val Vector of SQLWCHAR.
297302
/// \param[in] str_len length of string, in bytes.
303+
/// \param[in] buffer_size size of underlying buffer, in number of characters.
298304
/// \return wstring
299-
std::wstring ConvertToWString(const std::vector<SQLWCHAR>& str_val, SQLSMALLINT str_len);
305+
std::wstring ConvertToWString(const std::vector<SQLWCHAR>& str_val, SQLSMALLINT str_len,
306+
SQLSMALLINT buffer_size = kOdbcBufferSize);
300307

301308
/// \brief Check wide string column.
302309
/// \param[in] stmt Statement.

‎cpp/src/arrow/flight/sql/odbc/tests/statement_test.cc‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2007,8 +2007,7 @@ TYPED_TEST(StatementTest, TestSQLNativeSqlReturnsTruncatedString) {
20072007

20082008
// Create expected return string based on buf size
20092009
SQLWCHAR expected_string_buf[small_buf_size_in_char];
2010-
wcsncpy(reinterpret_cast<wchar_t*>(expected_string_buf),
2011-
reinterpret_cast<const wchar_t*>(input_str), 10);
2010+
std::copy(input_str, input_str + 10, expected_string_buf);
20122011
expected_string_buf[10] = L'\0';
20132012
std::wstring expected_string(expected_string_buf,
20142013
expected_string_buf + small_buf_size_in_char);

0 commit comments

Comments
 (0)