ODBC Test Segfault Fix

.gitignore

@@ -33,6 +33,7 @@ dependency-reduced-pom.xml
 MANIFEST
 compile_commands.json
 build.ninja
+build*/
 
 # Generated Visual Studio files
 *.vcxproj

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

@@ -34,6 +34,15 @@ endif()
 
 add_definitions(-DUNICODE=1)
 
+include(FetchContent)
+fetchcontent_declare(spdlog
+                     URL https://github.com/gabime/spdlog/archive/76fb40d95455f249bd70824ecfcae7a8f0930fa3.zip
+                         CONFIGURE_COMMAND
+                         ""
+                         BUILD_COMMAND
+                         "")
+fetchcontent_makeavailable(spdlog)
+
 add_subdirectory(flight_sql)
 add_subdirectory(odbcabstraction)
 add_subdirectory(tests)
@@ -71,7 +80,8 @@ add_arrow_lib(arrow_flight_sql_odbc
               ${ODBC_LIBRARIES}
               ${ODBCINST}
               odbcabstraction
-              arrow_odbc_spi_impl)
+              arrow_odbc_spi_impl
+              spdlog::spdlog)
 
 foreach(LIB_TARGET ${ARROW_FLIGHT_SQL_ODBC_LIBRARIES})
   target_compile_definitions(${LIB_TARGET} PRIVATE ARROW_FLIGHT_SQL_ODBC_EXPORTING)

cpp/src/arrow/flight/sql/odbc/flight_sql/ui/dsn_configuration_window.cc

@@ -94,7 +94,7 @@ void DsnConfigurationWindow::Create() {
     throw odbcabstraction::DriverException(buf.str());
   }
 }
-//-AL- fix this file next
+
 void DsnConfigurationWindow::OnCreate() {
   tabControl = CreateTabControl(ChildId::TAB_CONTROL);
   tabControl->AddTab(L"Common", COMMON_TAB);

cpp/src/arrow/flight/sql/odbc/odbcabstraction/CMakeLists.txt

@@ -63,19 +63,4 @@ set_target_properties(odbcabstraction
                                  RUNTIME_OUTPUT_DIRECTORY
                                  ${CMAKE_BINARY_DIR}/$<CONFIG>/lib)
 
-include(FetchContent)
-fetchcontent_declare(spdlog
-                     URL https://github.com/gabime/spdlog/archive/76fb40d95455f249bd70824ecfcae7a8f0930fa3.zip
-                         CONFIGURE_COMMAND
-                         ""
-                         BUILD_COMMAND
-                         "")
-fetchcontent_getproperties(spdlog)
-if(NOT spdlog_POPULATED)
-  fetchcontent_populate(spdlog)
-endif()
-
-add_library(spdlog INTERFACE)
-target_include_directories(spdlog INTERFACE ${spdlog_SOURCE_DIR}/include)
-
-target_link_libraries(odbcabstraction PUBLIC spdlog)
+target_link_libraries(odbcabstraction PUBLIC spdlog::spdlog)

cpp/src/arrow/flight/sql/odbc/odbcabstraction/include/odbcabstraction/odbc_impl/encoding_utils.h

@@ -83,7 +83,7 @@ inline size_t ConvertToSqlWChar(const std::string_view& str, SQLWCHAR* buffer,
 /// \param[in] msg_len Number of characters in wchar_msg
 /// \return wchar_msg in std::string format
 inline std::string SqlWcharToString(SQLWCHAR* wchar_msg, SQLSMALLINT msg_len = SQL_NTS) {
-  if (!wchar_msg || wchar_msg[0] == 0) {
+  if (!wchar_msg || wchar_msg[0] == 0 || msg_len == 0) {
     return std::string();
   }
 

cpp/src/arrow/flight/sql/odbc/tests/CMakeLists.txt

@@ -29,10 +29,11 @@ set(ARROW_FLIGHT_SQL_MOCK_SERVER_SRCS
     ../../example/sqlite_server.cc
     ../../example/sqlite_tables_schema_batch_reader.cc)
 
-add_arrow_test(connection_test
+add_arrow_test(flight_sql_odbc_test
                SOURCES
-               connection_test.cc
                connection_attr_test.cc
+               # Connection test needs to be put last to resolve segfault issue
+               connection_test.cc
                odbc_test_suite.cc
                odbc_test_suite.h
                ${ARROW_FLIGHT_SQL_MOCK_SERVER_SRCS}

cpp/src/arrow/flight/sql/odbc/tests/connection_attr_test.cc

@@ -26,10 +26,8 @@
 
 #include "gtest/gtest.h"
 
-namespace arrow {
-namespace flight {
-namespace odbc {
-namespace integration_tests {
+namespace arrow::flight::sql::odbc {
+
 TYPED_TEST(FlightSQLODBCTestBase, TestSQLSetConnectAttrAsyncDbcEventUnsupported) {
   this->connect();
 
@@ -272,6 +270,9 @@ TYPED_TEST(FlightSQLODBCTestBase, TestSQLGetConnectAttrTraceFileDMOnly) {
                                     ODBC_BUFFER_SIZE, &outstrlen);
 
   EXPECT_EQ(ret, SQL_SUCCESS);
+  // Length is returned in bytes for SQLGetConnectAttr,
+  // we want the number of characters
+  outstrlen /= driver::odbcabstraction::GetSqlWCharSize();
   std::string out_connection_string =
       ODBC::SqlWcharToString(outstr, static_cast<SQLSMALLINT>(outstrlen));
   EXPECT_TRUE(!out_connection_string.empty());
@@ -429,7 +430,7 @@ TYPED_TEST(FlightSQLODBCTestBase, TestSQLGetConnectAttrEnlistInDtcDefault) {
 TYPED_TEST(FlightSQLODBCTestBase, TestSQLGetConnectAttrQuietModeDefault) {
   this->connect();
 
-  SQLPOINTER ptr = NULL;
+  HWND ptr = NULL;
   SQLRETURN ret = SQLGetConnectAttr(this->conn, SQL_ATTR_QUIET_MODE, ptr, 0, 0);
 
   EXPECT_EQ(ret, SQL_SUCCESS);
@@ -561,7 +562,4 @@ TYPED_TEST(FlightSQLODBCTestBase, TestSQLSetConnectAttrPacketSizeValid) {
   this->disconnect();
 }
 
-}  // namespace integration_tests
-}  // namespace odbc
-}  // namespace flight
-}  // namespace arrow
+}  // namespace arrow::flight::sql::odbc

cpp/src/arrow/flight/sql/odbc/tests/connection_test.cc

@@ -26,10 +26,8 @@
 
 #include "gtest/gtest.h"
 
-namespace arrow {
-namespace flight {
-namespace odbc {
-namespace integration_tests {
+namespace arrow::flight::sql::odbc {
+
 TEST(SQLAllocHandle, TestSQLAllocHandleEnv) {
   // ODBC Environment
   SQLHENV env;
@@ -1076,10 +1074,7 @@ TYPED_TEST(FlightSQLODBCTestBase, TestCloseConnectionWithOpenStatement) {
   EXPECT_EQ(ret, SQL_SUCCESS);
 }
 
-}  // namespace integration_tests
-}  // namespace odbc
-}  // namespace flight
-}  // namespace arrow
+}  // namespace arrow::flight::sql::odbc
 
 int main(int argc, char** argv) {
   ::testing::InitGoogleTest(&argc, argv);

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

@@ -25,10 +25,7 @@
 #include "arrow/flight/sql/odbc/flight_sql/include/flight_sql/config/configuration.h"
 #include "arrow/flight/sql/odbc/odbcabstraction/include/odbcabstraction/odbc_impl/odbc_connection.h"
 
-namespace arrow {
-namespace flight {
-namespace odbc {
-namespace integration_tests {
+namespace arrow::flight::sql::odbc {
 
 void FlightSQLODBCRemoteTestBase::allocEnvConnHandles() {
   // Allocate an environment handle
@@ -272,7 +269,5 @@ bool writeDSN(Connection::ConnPropertyMap properties) {
   std::wstring wDriver = arrow::util::UTF8ToWideString(driver).ValueOr(L"");
   return RegisterDsn(config, wDriver.c_str());
 }
-}  // namespace integration_tests
-}  // namespace odbc
-}  // namespace flight
-}  // namespace arrow
+
+}  // namespace arrow::flight::sql::odbc

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

@@ -42,10 +42,7 @@
 #define TEST_CONNECT_STR "ARROW_FLIGHT_SQL_ODBC_CONN"
 #define TEST_DSN "Apache Arrow Flight SQL Test DSN"
 
-namespace arrow {
-namespace flight {
-namespace odbc {
-namespace integration_tests {
+namespace arrow::flight::sql::odbc {
 using driver::odbcabstraction::Connection;
 
 class FlightSQLODBCRemoteTestBase : public ::testing::Test {
@@ -177,7 +174,4 @@ bool writeDSN(std::string connection_str);
 /// \param[in] properties map.
 /// \return true on success
 bool writeDSN(Connection::ConnPropertyMap properties);
-}  // namespace integration_tests
-}  // namespace odbc
-}  // namespace flight
-}  // namespace arrow
+}  // namespace arrow::flight::sql::odbc