Skip to content

Commit 682bab2

Browse files
justing-bqjusting-bq
committed
Address review feedback
1 parent 9c88ee5 commit 682bab2

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
@@ -372,7 +372,8 @@ TYPED_TEST(ConnectionInfoTest, TestSQLGetInfoServerName) {
372372
SQLWCHAR value[kOdbcBufferSize] = {};
373373
GetInfoSQLWCHAR(this->conn, SQL_SERVER_NAME, value);
374374

375-
EXPECT_GT(wcslen(reinterpret_cast<wchar_t*>(value)), 0);
375+
std::wstring result = ConvertToWString(value);
376+
EXPECT_GT(result.length(), 0);
376377
}
377378

378379
TYPED_TEST(ConnectionInfoTest, TestSQLGetInfoStaticCursorAttributes1) {
@@ -403,14 +404,16 @@ TYPED_TEST(ConnectionInfoTest, TestSQLGetInfoDbmsName) {
403404
SQLWCHAR value[kOdbcBufferSize] = {};
404405
GetInfoSQLWCHAR(this->conn, SQL_DBMS_NAME, value);
405406

406-
EXPECT_GT(wcslen(reinterpret_cast<wchar_t*>(value)), 0);
407+
std::wstring result = ConvertToWString(value);
408+
EXPECT_GT(result.length(), 0);
407409
}
408410

409411
TYPED_TEST(ConnectionInfoTest, TestSQLGetInfoDbmsVer) {
410412
SQLWCHAR value[kOdbcBufferSize] = {};
411413
GetInfoSQLWCHAR(this->conn, SQL_DBMS_VER, value);
412414

413-
EXPECT_GT(wcslen(reinterpret_cast<wchar_t*>(value)), 0);
415+
std::wstring result = ConvertToWString(value);
416+
EXPECT_GT(result.length(), 0);
414417
}
415418

416419
// Data Source Information
@@ -850,7 +853,8 @@ TYPED_TEST(ConnectionInfoTest, TestSQLGetInfoKeywords) {
850853
SQLWCHAR value[info_len] = {};
851854
GetInfoSQLWCHAR(this->conn, SQL_KEYWORDS, value, info_len);
852855

853-
EXPECT_GT(wcslen(reinterpret_cast<wchar_t*>(value)), 0);
856+
std::wstring result = ConvertToWString(value, -1, info_len);
857+
EXPECT_GT(result.length(), 0);
854858
}
855859

856860
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
@@ -539,7 +539,7 @@ bool WriteDSN(Connection::ConnPropertyMap properties) {
539539
}
540540

541541
std::string driver = config.Get(FlightSqlConnection::DRIVER);
542-
std::wstring w_driver = arrow::util::UTF8ToWideString(driver).ValueOr(L"");
542+
CONVERT_SQLWCHAR_STR(w_driver, driver);
543543
return RegisterDsn(config, reinterpret_cast<LPCWSTR>(w_driver.c_str()));
544544
}
545545

@@ -570,7 +570,8 @@ size_t SqlWCharArrLen(const SQLWCHAR* str_val) {
570570
return static_cast<size_t>(p - str_val);
571571
}
572572

573-
std::wstring ConvertToWString(const SQLWCHAR* str_val, SQLSMALLINT str_len) {
573+
std::wstring ConvertToWString(const SQLWCHAR* str_val, SQLSMALLINT str_len,
574+
SQLSMALLINT buffer_size) {
574575
if (str_len == -1) {
575576
#ifdef __linux__
576577
str_len = SqlWCharArrLen(str_val);
@@ -583,19 +584,20 @@ std::wstring ConvertToWString(const SQLWCHAR* str_val, SQLSMALLINT str_len) {
583584
attr_str = L"";
584585
} else {
585586
assert(str_val != nullptr);
586-
assert(str_len > 0 && str_len <= static_cast<SQLSMALLINT>(kOdbcBufferSize));
587+
assert(str_len > 0 && str_len <= buffer_size);
587588
attr_str.assign(str_val, str_val + str_len);
588589
}
589590
return attr_str;
590591
}
591592

592-
std::wstring ConvertToWString(const std::vector<SQLWCHAR>& str_val, SQLSMALLINT str_len) {
593+
std::wstring ConvertToWString(const std::vector<SQLWCHAR>& str_val, SQLSMALLINT str_len,
594+
SQLSMALLINT buffer_size) {
593595
std::wstring attr_str;
594596
if (str_len == 0) {
595597
attr_str = L"";
596598
} else {
597599
EXPECT_GT(str_len, 0);
598-
EXPECT_LE(str_len, static_cast<SQLSMALLINT>(kOdbcBufferSize));
600+
EXPECT_LE(str_len, buffer_size);
599601
attr_str =
600602
std::wstring(str_val.begin(), str_val.begin() + str_len / GetSqlWCharSize());
601603
}

‎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";
@@ -300,14 +303,18 @@ size_t SqlWCharArrLen(const SQLWCHAR* str_val);
300303
/// \brief Check wide char array and convert into wstring
301304
/// \param[in] str_val Array of SQLWCHAR.
302305
/// \param[in] str_len length of string, in number of characters.
306+
/// \param[in] buffer_size size of underlying buffer, in number of characters.
303307
/// \return wstring
304-
std::wstring ConvertToWString(const SQLWCHAR* str_val, SQLSMALLINT str_len = -1);
308+
std::wstring ConvertToWString(const SQLWCHAR* str_val, SQLSMALLINT str_len = -1,
309+
SQLSMALLINT buffer_size = kOdbcBufferSize);
305310

306311
/// \brief Check wide char vector and convert into wstring
307312
/// \param[in] str_val Vector of SQLWCHAR.
308313
/// \param[in] str_len length of string, in bytes.
314+
/// \param[in] buffer_size size of underlying buffer, in number of characters.
309315
/// \return wstring
310-
std::wstring ConvertToWString(const std::vector<SQLWCHAR>& str_val, SQLSMALLINT str_len);
316+
std::wstring ConvertToWString(const std::vector<SQLWCHAR>& str_val, SQLSMALLINT str_len,
317+
SQLSMALLINT buffer_size = kOdbcBufferSize);
311318

312319
/// \brief Check wide string column.
313320
/// \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
@@ -2050,8 +2050,7 @@ TYPED_TEST(StatementTest, TestSQLNativeSqlReturnsTruncatedString) {
20502050

20512051
// Create expected return string based on buf size
20522052
SQLWCHAR expected_string_buf[small_buf_size_in_char];
2053-
wcsncpy(reinterpret_cast<wchar_t*>(expected_string_buf),
2054-
reinterpret_cast<const wchar_t*>(input_str), 10);
2053+
std::copy(input_str, input_str + 10, expected_string_buf);
20552054
expected_string_buf[10] = L'\0';
20562055
std::wstring expected_string(expected_string_buf,
20572056
expected_string_buf + small_buf_size_in_char);

0 commit comments

Comments
 (0)