Clean up ODBC code

Author: alinaliBQ

.github/workflows/cpp_extra.yml

@@ -367,11 +367,10 @@ services:
     volumes: &debian-volumes
       - .:/arrow:delegated
       - ${DOCKER_VOLUME_PREFIX}debian-ccache:/ccache:delegated
-    command: &cpp-command > # -AL- &cpp-command is defined here and calls the build + test sh scripts 
+    command: &cpp-command >
       /bin/bash -c "
         /arrow/ci/scripts/cpp_build.sh /arrow /build &&
         /arrow/ci/scripts/cpp_test.sh /arrow /build"
-# -AL- so I can define a new one for odbc-cpp too.
 
   ubuntu-cpp:
     # Usage:

compose.yaml

@@ -50,8 +50,6 @@ case "$(uname)" in
     ;;
 esac
 
-echo "-AL- USER_ODBCINST_FILE: $USER_ODBCINST_FILE"
-
 DRIVER_NAME="Apache Arrow Flight SQL ODBC Driver"
 
 mkdir -p "$HOME"/Library/ODBC

cpp/src/arrow/flight/sql/odbc/install/unix/install_odbc.sh

@@ -137,11 +137,7 @@ if(WIN32)
                         PUBLIC arrow_flight_sql_shared arrow_compute_shared Boost::locale
                                ${ODBCINST})
 else()
-  # -AL- Linux build. share with apple
-  if(NOT APPLE)
-    message(status "-AL- ODBC_INCLUDE_DIR on Linux: ${ODBC_INCLUDE_DIR}")
-  endif()
-
+  # Unix
   target_include_directories(arrow_odbc_spi_impl SYSTEM BEFORE PUBLIC ${ODBC_INCLUDE_DIR})
   target_link_libraries(arrow_odbc_spi_impl
                         PUBLIC arrow_flight_sql_static

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

@@ -31,8 +31,6 @@
 #include <iterator>
 #include <sstream>
 
-#include "arrow/util/logging.h"  // -AL- TEMP
-
 using ODBC::SetAttributeSQLWCHAR;
 
 namespace arrow::flight::sql::odbc {
@@ -49,14 +47,7 @@ std::string ReadDsnString(const std::string& dsn, std::string_view key,
   CONVERT_WIDE_STR(const std::wstring wkey, key);
   CONVERT_WIDE_STR(const std::wstring wdflt, dflt);
 
-  // -AL- found workaround for `cannot convert 'const wchar_t*' to 'LPCWSTR' {aka
-  // 'const short unsigned int*'}` on Linux. Notes in this file for reference only.
-
-  // Via CONVERT_WIDE_STR, Arrow correctly converts to UFT-32 on Unix systems,
-  // so the conversion from wchar_t to short unsigned int* will work on Linux.
-
-  // -AL- I just need to wrap `reinterpret_cast<LPCWSTR>()` on all string args for
-  // SQLGetPrivateProfileString.
+  // TODO: implement proper Linux unicode support in separate PR
 
 #define BUFFER_SIZE (1024)
   std::vector<SQLWCHAR> buf(BUFFER_SIZE);
@@ -75,11 +66,7 @@ std::string ReadDsnString(const std::string& dsn, std::string_view key,
   }
 
   std::string result("");
-  ARROW_LOG(DEBUG) << "-AL- ReadDsnString key: " << key;
-  ARROW_LOG(DEBUG) << "-AL- ReadDsnString result before: " << result;
   SetAttributeSQLWCHAR(buf.data(), ret * GetSqlWCharSize(), result);
-  ARROW_LOG(DEBUG) << "-AL- ReadDsnString result: " << result;
-  ARROW_LOG(DEBUG) << "-AL- ReadDsnString ret: " << ret;
   return result;
 }
 
@@ -127,7 +114,6 @@ std::vector<std::string> ReadAllKeys(const std::string& dsn) {
     SQLINTEGER key_len = cur - begin;
     SetAttributeSQLWCHAR(begin, key_len * GetSqlWCharSize(), key);
     keys.emplace_back(key);
-    ARROW_LOG(DEBUG) << "-AL- ReadAllKeys key: " << key;
     begin = ++cur;
   }
   return keys;

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

@@ -117,4 +117,12 @@ inline std::string SqlStringToString(const unsigned char* sql_str,
 
   return res;
 }
+
+inline std::vector<SQLWCHAR> ToSqlWCharVector(const std::wstring& ws) {
+  std::vector<SQLWCHAR> buf;
+  // buf.assign(ws.begin(), ws.end());
+  // TODO implement in separate PR
+  return buf;
+}
+
 }  // namespace ODBC

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

@@ -395,9 +395,8 @@ bool GetInfoCache::LoadInfoFromServer() {
               // Unused by ODBC.
               break;
             case SqlInfoOptions::SQL_DDL_SCHEMA: {
-              // bool supports_schema_ddl =
-              //     reinterpret_cast<BooleanScalar*>(scalar->child_value().get())->value;
-              // -AL- todo raise GitHub issues for finishing this work.
+              // GH-49500 TODO: use scalar bool to determine `SQL_CREATE_SCHEMA` and
+              // `SQL_DROP_SCHEMA` values
 
               // Note: this is a bitmask and we can't describe cascade or restrict
               // flags.
@@ -409,9 +408,8 @@ bool GetInfoCache::LoadInfoFromServer() {
               break;
             }
             case SqlInfoOptions::SQL_DDL_TABLE: {
-              // bool supports_table_ddl =
-              //     reinterpret_cast<BooleanScalar*>(scalar->child_value().get())->value;
-              // -AL- todo raise GitHub issues for finishing this work.
+              // GH-49500 TODO: use scalar bool to determine `SQL_CREATE_TABLE` and
+              // `SQL_DROP_TABLE` values
 
               // This is a bitmask and we cannot describe all clauses.
               info_[SQL_CREATE_TABLE] = static_cast<uint32_t>(SQL_CT_CREATE_TABLE);

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

@@ -15,8 +15,7 @@
 // specific language governing permissions and limitations
 // under the License.
 
-// platform.h platform.h includes windows.h so it needs to be included first 
-// -AL- this is an attempt to fix the windows build
+// platform.h platform.h includes windows.h so it needs to be included first
 #include "arrow/flight/sql/odbc/odbc_impl/platform.h"
 
 #include "arrow/type.h"

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

@@ -20,13 +20,14 @@
 #include "arrow/result.h"
 #include "arrow/util/utf8.h"
 
+#include "arrow/flight/sql/odbc/odbc_impl/encoding_utils.h"
+
 #include <sstream>
 
 #ifdef __linux__
-#  define GET_SQWCHAR_PTR(wstring_var) (ToSqlWCharVector(wstring_var).data())
+#  define GET_SQWCHAR_PTR(wstring_var) (ODBC::ToSqlWCharVector(wstring_var).data())
 #else
-// Windows and macOS // -AL- TODO raise GitHub issues for Linux functions
-// Can't test it right now anyways without the tests.
+// Windows and macOS
 #  define GET_SQWCHAR_PTR(wstring_var) (wstring_var.c_str())
 #endif
 
@@ -71,13 +72,6 @@ void PostLastInstallerError() {
   PostError(code, (LPWSTR)error_msg.c_str());
 }
 
-std::vector<SQLWCHAR> ToSqlWCharVector(const std::wstring& ws) {
-  std::vector<SQLWCHAR> buf;
-  // buf.assign(ws.begin(), ws.end());
-  // -AL- GitHub issue to implement. Also need to move this function else where like encoding_utils.h?
-  return buf;
-}
-
 /**
  * Unregister specified DSN.
  *