Address remaining feedback

Author: justing-bq

cpp/src/arrow/flight/sql/odbc/odbc_api.cc

@@ -34,7 +34,7 @@
 #if defined _WIN32
 // For displaying DSN Window
 #  include "arrow/flight/sql/odbc/odbc_impl/system_dsn.h"
-#endif
+#endif  // defined(_WIN32)
 
 namespace arrow::flight::sql::odbc {
 SQLRETURN SQLAllocHandle(SQLSMALLINT type, SQLHANDLE parent, SQLHANDLE* result) {
@@ -780,9 +780,11 @@ SQLRETURN SQLDriverConnect(SQLHDBC conn, SQLHWND window_handle,
     std::string connection_string =
         ODBC::SqlWcharToString(in_connection_string, in_connection_string_len);
     Connection::ConnPropertyMap properties;
-    std::string dsn = ODBCConnection::GetDsnIfExists(connection_string);
-    if (!dsn.empty()) {
-      LoadPropertiesFromDSN(dsn, properties);
+    std::string dsn_value = "";
+    std::optional<std::string> dsn = ODBCConnection::GetDsnIfExists(connection_string);
+    if (dsn.has_value()) {
+      dsn_value = dsn.value();
+      LoadPropertiesFromDSN(dsn_value, properties);
     }
     ODBCConnection::GetPropertiesFromConnString(connection_string, properties);
 
@@ -798,11 +800,11 @@ SQLRETURN SQLDriverConnect(SQLHDBC conn, SQLHWND window_handle,
       if (!DisplayConnectionWindow(window_handle, config, properties)) {
         return static_cast<SQLRETURN>(SQL_NO_DATA);
       }
-      connection->Connect(dsn, properties, missing_properties);
+      connection->Connect(dsn_value, properties, missing_properties);
     } else if (driver_completion == SQL_DRIVER_COMPLETE ||
                driver_completion == SQL_DRIVER_COMPLETE_REQUIRED) {
       try {
-        connection->Connect(dsn, properties, missing_properties);
+        connection->Connect(dsn_value, properties, missing_properties);
       } catch (const DriverException&) {
         // If first connection fails due to missing attributes, load
         // the DSN window and try to connect again
@@ -813,14 +815,14 @@ SQLRETURN SQLDriverConnect(SQLHDBC conn, SQLHWND window_handle,
           if (!DisplayConnectionWindow(window_handle, config, properties)) {
             return static_cast<SQLRETURN>(SQL_NO_DATA);
           }
-          connection->Connect(dsn, properties, missing_properties);
+          connection->Connect(dsn_value, properties, missing_properties);
         } else {
           throw;
         }
       }
     } else {
       // Default case: attempt connection without showing DSN window
-      connection->Connect(dsn, properties, missing_properties);
+      connection->Connect(dsn_value, properties, missing_properties);
     }
 #else
     // Attempt connection without loading DSN window on macOS/Linux

cpp/src/arrow/flight/sql/odbc/odbc_impl/attribute_utils.h

@@ -27,6 +27,7 @@
 #include "arrow/flight/sql/odbc/odbc_impl/exceptions.h"
 #include "arrow/flight/sql/odbc/odbc_impl/platform.h"
 
+// GH-48083 TODO: replace `namespace ODBC` with `namespace arrow::flight::sql::odbc`
 namespace ODBC {
 
 using arrow::flight::sql::odbc::Diagnostics;

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

@@ -85,7 +85,7 @@ inline size_t ConvertToSqlWChar(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, SQLINTEGER msg_len = SQL_NTS) {
-  if (!wchar_msg || wchar_msg[0] == 0 || msg_len == 0) {
+  if (msg_len == 0 || !wchar_msg || wchar_msg[0] == 0) {
     return std::string();
   }
 

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

@@ -684,7 +684,7 @@ void ODBCConnection::DropDescriptor(ODBCDescriptor* desc) {
 
 // Public Static
 // ===================================================================================
-std::string ODBCConnection::GetDsnIfExists(const std::string& conn_str) {
+std::optional<std::string> ODBCConnection::GetDsnIfExists(const std::string& conn_str) {
   const int groups[] = {1, 2};  // CONNECTION_STR_REGEX has two groups. key: 1, value: 2
   boost::xpressive::sregex_token_iterator regex_iter(conn_str.begin(), conn_str.end(),
                                                      CONNECTION_STR_REGEX, groups),
@@ -703,7 +703,7 @@ std::string ODBCConnection::GetDsnIfExists(const std::string& conn_str) {
   if (boost::iequals(key, "DSN")) {
     return value;
   } else if (boost::iequals(key, "Driver")) {
-    return std::string("");
+    return std::nullopt;
   } else {
     throw DriverException(
         "Connection string is faulty. The first key should be DSN or Driver.", "HY000");

cpp/src/arrow/flight/sql/odbc/odbc_impl/odbc_connection.h

@@ -23,6 +23,7 @@
 #include <sql.h>
 #include <map>
 #include <memory>
+#include <optional>
 #include <vector>
 
 namespace ODBC {
@@ -75,9 +76,8 @@ class ODBCConnection : public ODBCHandle<ODBCConnection> {
 
   inline bool IsOdbc2Connection() const { return is_2x_connection_; }
 
-  /// @return the DSN or an empty string if the DSN is not found or is found after the
-  /// driver
-  static std::string GetDsnIfExists(const std::string& conn_str);
+  /// \return an optional DSN
+  static std::optional<std::string> GetDsnIfExists(const std::string& conn_str);
 
   /// Read properties from connection string, but does not read values from DSN
   static void GetPropertiesFromConnString(

cpp/src/arrow/flight/sql/odbc/odbc_impl/system_dsn.h

@@ -19,7 +19,7 @@
 #include "arrow/flight/sql/odbc/odbc_impl/platform.h"
 
 #include "arrow/flight/sql/odbc/odbc_impl/config/configuration.h"
-#include "arrow/result.h"
+#include "arrow/status.h"
 
 namespace arrow::flight::sql::odbc {