kou commented on code in PR #2918:
URL: https://github.com/apache/arrow-adbc/pull/2918#discussion_r2155952693
##########
c/cmake_modules/AdbcDefines.cmake:
##########
@@ -90,6 +89,8 @@ if(MSVC)
add_compile_options(/wd4868)
add_compile_options(/wd4710)
add_compile_options(/wd4711)
+ # Don't warn about enforcing left-to-right evaluation order for operator[]
+ add_compile_options(/wd4866)
# Don't warn about padding added after members
add_compile_options(/wd4820)
Review Comment:
We may want to sort by warning ID:
```suggestion
# Don't warn about padding added after members
add_compile_options(/wd4820)
# Don't warn about enforcing left-to-right evaluation order for operator[]
add_compile_options(/wd4866)
```
##########
c/driver_manager/adbc_driver_manager.cc:
##########
@@ -114,7 +130,186 @@ struct OwnedError {
}
};
+#ifdef _WIN32
+using char_type = wchar_t;
+
+std::string Utf8Encode(const std::wstring& wstr) {
+ if (wstr.empty()) return std::string();
+ int size_needed = WideCharToMultiByte(
+ CP_UTF8, 0, &wstr[0], static_cast<int>(wstr.size()), NULL, 0, NULL,
NULL);
+ std::string str_to(size_needed, 0);
+ WideCharToMultiByte(CP_UTF8, 0, &wstr[0], static_cast<int>(wstr.size()),
&str_to[0],
+ size_needed, NULL, NULL);
+ return str_to;
+}
+
+std::wstring Utf8Decode(const std::string& str) {
+ if (str.empty()) return std::wstring();
+ int size_needed =
+ MultiByteToWideChar(CP_UTF8, 0, &str[0], static_cast<int>(str.size()),
NULL, 0);
+ std::wstring wstr_to(size_needed, 0);
+ MultiByteToWideChar(CP_UTF8, 0, &str[0], static_cast<int>(str.size()),
&wstr_to[0],
+ size_needed);
+ return wstr_to;
+}
+
+#else
+using char_type = char;
+#endif
+
// Driver state
+struct DriverInfo {
+ std::string manifest_file;
+ std::string driver_name;
+ std::filesystem::path lib_path;
+ std::string entrypoint;
+
+ std::string version;
+ std::string source;
+};
+
+#ifdef _WIN32
+class RegistryKey {
+ public:
+ RegistryKey(HKEY root, const std::wstring_view subkey) noexcept
+ : root_(root), key_(nullptr) {
+ status_ = RegOpenKeyExW(root_, subkey.data(), 0, KEY_READ, &key_);
+ }
+
+ ~RegistryKey() {
+ if (is_open() && key_ != nullptr) {
+ RegCloseKey(key_);
+ key_ = nullptr;
+ status_ = ERROR_REGISTRY_IO_FAILED;
+ }
+ }
+
+ HKEY key() const { return key_; }
+ bool is_open() const { return status_ == ERROR_SUCCESS; }
+ LSTATUS status() const { return status_; }
+
+ std::wstring GetString(const std::wstring& name, std::wstring default_value)
{
+ if (!is_open()) return default_value;
+
+ DWORD type = REG_SZ;
+ DWORD size = 0;
+ auto result = RegQueryValueExW(key_, name.c_str(), nullptr, &type,
nullptr, &size);
Review Comment:
We can use `data()` with C++11 or later because `data()` with C++11 or later
returns `NULL` terminated C string:
```suggestion
auto result = RegQueryValueExW(key_, name.data(), nullptr, &type,
nullptr, &size);
```
##########
c/driver_manager/adbc_driver_manager.cc:
##########
@@ -15,26 +15,42 @@
// specific language governing permissions and limitations
// under the License.
-#include "arrow-adbc/adbc_driver_manager.h"
+#if defined(_WIN32)
+#pragma comment(lib, "advapi32.lib") // for registry stuff
Review Comment:
Can we move this to `CMakeiLists.txt`?
##########
c/driver_manager/adbc_driver_manager.cc:
##########
@@ -15,26 +15,42 @@
// specific language governing permissions and limitations
// under the License.
-#include "arrow-adbc/adbc_driver_manager.h"
+#if defined(_WIN32)
+#pragma comment(lib, "advapi32.lib") // for registry stuff
+#include <windows.h> // Must come first
+
+#include <libloaderapi.h>
+#include <string.h> // _wcsnicmp
+#include <strsafe.h>
+
+#include <codecvt>
+#include <locale>
+#else
+#include <dlfcn.h>
+#endif // defined(_WIN32)
+
+#include <toml++/toml.hpp>
#include "arrow-adbc/adbc.h"
+#include "arrow-adbc/adbc_driver_manager.h"
+#include "current_arch.h"
#include <algorithm>
#include <array>
#include <cctype>
#include <cerrno>
#include <cstring>
+#include <filesystem> // NOLINT [build/c++17]
Review Comment:
Can we remove this comment by #2989?
##########
c/driver_manager/adbc_driver_manager.cc:
##########
@@ -15,26 +15,42 @@
// specific language governing permissions and limitations
// under the License.
-#include "arrow-adbc/adbc_driver_manager.h"
+#if defined(_WIN32)
+#pragma comment(lib, "advapi32.lib") // for registry stuff
+#include <windows.h> // Must come first
+
+#include <libloaderapi.h>
+#include <string.h> // _wcsnicmp
+#include <strsafe.h>
+
+#include <codecvt>
Review Comment:
This is deprecated in C++17...
##########
c/driver_manager/adbc_driver_manager.cc:
##########
@@ -138,22 +333,189 @@ struct ManagedLibrary {
// release() from the DLL - how to handle this?
}
- AdbcStatusCode Load(const char* library, struct AdbcError* error) {
+ AdbcStatusCode GetDriverInfo(const std::string_view driver_name,
+ const AdbcLoadFlags load_options, DriverInfo&
info,
+ struct AdbcError* error) {
+ if (driver_name.empty()) {
+ SetError(error, "Driver name is empty");
+ return ADBC_STATUS_INVALID_ARGUMENT;
+ }
+
+ std::filesystem::path driver_path(driver_name);
+ const bool allow_relative_paths = load_options &
ADBC_LOAD_FLAG_ALLOW_RELATIVE_PATHS;
+ if (driver_path.has_extension()) {
+ if (driver_path.is_relative() && !allow_relative_paths) {
+ SetError(error, "Driver path is relative and relative paths are not
allowed");
+ return ADBC_STATUS_INVALID_ARGUMENT;
+ }
+
+ if (HasExtension(driver_path, ".toml")) {
+ // if the extension is .toml, attempt to load the manifest
+ // erroring if we fail
+
+ auto status = LoadDriverManifest(driver_path, info, error);
+ if (status == ADBC_STATUS_OK) {
+ return Load(info.lib_path.c_str(), error);
Review Comment:
```suggestion
return Load(info.lib_path.data(), error);
```
##########
c/driver_manager/adbc_driver_manager.cc:
##########
@@ -114,7 +130,186 @@ struct OwnedError {
}
};
+#ifdef _WIN32
+using char_type = wchar_t;
+
+std::string Utf8Encode(const std::wstring& wstr) {
+ if (wstr.empty()) return std::string();
+ int size_needed = WideCharToMultiByte(
+ CP_UTF8, 0, &wstr[0], static_cast<int>(wstr.size()), NULL, 0, NULL,
NULL);
Review Comment:
```suggestion
CP_UTF8, 0, wstr.data(), static_cast<int>(wstr.size()), NULL, 0, NULL,
NULL);
```
##########
c/driver_manager/adbc_driver_manager.cc:
##########
@@ -114,7 +130,186 @@ struct OwnedError {
}
};
+#ifdef _WIN32
+using char_type = wchar_t;
+
+std::string Utf8Encode(const std::wstring& wstr) {
+ if (wstr.empty()) return std::string();
+ int size_needed = WideCharToMultiByte(
+ CP_UTF8, 0, &wstr[0], static_cast<int>(wstr.size()), NULL, 0, NULL,
NULL);
+ std::string str_to(size_needed, 0);
+ WideCharToMultiByte(CP_UTF8, 0, &wstr[0], static_cast<int>(wstr.size()),
&str_to[0],
+ size_needed, NULL, NULL);
+ return str_to;
+}
+
+std::wstring Utf8Decode(const std::string& str) {
+ if (str.empty()) return std::wstring();
+ int size_needed =
+ MultiByteToWideChar(CP_UTF8, 0, &str[0], static_cast<int>(str.size()),
NULL, 0);
Review Comment:
```suggestion
MultiByteToWideChar(CP_UTF8, 0, str.data(),
static_cast<int>(str.size()), NULL, 0);
```
##########
c/driver_manager/adbc_driver_manager.cc:
##########
@@ -114,7 +130,186 @@ struct OwnedError {
}
};
+#ifdef _WIN32
+using char_type = wchar_t;
+
+std::string Utf8Encode(const std::wstring& wstr) {
+ if (wstr.empty()) return std::string();
+ int size_needed = WideCharToMultiByte(
+ CP_UTF8, 0, &wstr[0], static_cast<int>(wstr.size()), NULL, 0, NULL,
NULL);
+ std::string str_to(size_needed, 0);
+ WideCharToMultiByte(CP_UTF8, 0, &wstr[0], static_cast<int>(wstr.size()),
&str_to[0],
+ size_needed, NULL, NULL);
+ return str_to;
+}
+
+std::wstring Utf8Decode(const std::string& str) {
+ if (str.empty()) return std::wstring();
+ int size_needed =
+ MultiByteToWideChar(CP_UTF8, 0, &str[0], static_cast<int>(str.size()),
NULL, 0);
+ std::wstring wstr_to(size_needed, 0);
+ MultiByteToWideChar(CP_UTF8, 0, &str[0], static_cast<int>(str.size()),
&wstr_to[0],
Review Comment:
```suggestion
MultiByteToWideChar(CP_UTF8, 0, str.data(), static_cast<int>(str.size()),
wstr_to.data(),
```
##########
c/driver_manager/adbc_driver_manager.cc:
##########
@@ -114,7 +130,186 @@ struct OwnedError {
}
};
+#ifdef _WIN32
+using char_type = wchar_t;
+
+std::string Utf8Encode(const std::wstring& wstr) {
+ if (wstr.empty()) return std::string();
+ int size_needed = WideCharToMultiByte(
+ CP_UTF8, 0, &wstr[0], static_cast<int>(wstr.size()), NULL, 0, NULL,
NULL);
+ std::string str_to(size_needed, 0);
+ WideCharToMultiByte(CP_UTF8, 0, &wstr[0], static_cast<int>(wstr.size()),
&str_to[0],
Review Comment:
```suggestion
WideCharToMultiByte(CP_UTF8, 0, wstr.data(),
static_cast<int>(wstr.size()), str_to.data(),
```
##########
c/driver_manager/adbc_driver_manager.cc:
##########
@@ -114,7 +130,186 @@ struct OwnedError {
}
};
+#ifdef _WIN32
+using char_type = wchar_t;
+
+std::string Utf8Encode(const std::wstring& wstr) {
+ if (wstr.empty()) return std::string();
+ int size_needed = WideCharToMultiByte(
+ CP_UTF8, 0, &wstr[0], static_cast<int>(wstr.size()), NULL, 0, NULL,
NULL);
+ std::string str_to(size_needed, 0);
+ WideCharToMultiByte(CP_UTF8, 0, &wstr[0], static_cast<int>(wstr.size()),
&str_to[0],
+ size_needed, NULL, NULL);
+ return str_to;
+}
+
+std::wstring Utf8Decode(const std::string& str) {
+ if (str.empty()) return std::wstring();
+ int size_needed =
+ MultiByteToWideChar(CP_UTF8, 0, &str[0], static_cast<int>(str.size()),
NULL, 0);
+ std::wstring wstr_to(size_needed, 0);
+ MultiByteToWideChar(CP_UTF8, 0, &str[0], static_cast<int>(str.size()),
&wstr_to[0],
+ size_needed);
+ return wstr_to;
+}
+
+#else
+using char_type = char;
+#endif
+
// Driver state
+struct DriverInfo {
+ std::string manifest_file;
+ std::string driver_name;
+ std::filesystem::path lib_path;
+ std::string entrypoint;
+
+ std::string version;
+ std::string source;
+};
+
+#ifdef _WIN32
+class RegistryKey {
+ public:
+ RegistryKey(HKEY root, const std::wstring_view subkey) noexcept
+ : root_(root), key_(nullptr) {
+ status_ = RegOpenKeyExW(root_, subkey.data(), 0, KEY_READ, &key_);
+ }
+
+ ~RegistryKey() {
+ if (is_open() && key_ != nullptr) {
+ RegCloseKey(key_);
+ key_ = nullptr;
+ status_ = ERROR_REGISTRY_IO_FAILED;
+ }
+ }
+
+ HKEY key() const { return key_; }
+ bool is_open() const { return status_ == ERROR_SUCCESS; }
+ LSTATUS status() const { return status_; }
+
+ std::wstring GetString(const std::wstring& name, std::wstring default_value)
{
+ if (!is_open()) return default_value;
+
+ DWORD type = REG_SZ;
+ DWORD size = 0;
+ auto result = RegQueryValueExW(key_, name.c_str(), nullptr, &type,
nullptr, &size);
+ if (result != ERROR_SUCCESS) return default_value;
+ if (type != REG_SZ) return default_value;
+
+ std::wstring value(size, '\0');
+ result = RegQueryValueExW(key_, name.c_str(), nullptr, &type,
Review Comment:
```suggestion
result = RegQueryValueExW(key_, name.data(), nullptr, &type,
```
##########
c/driver_manager/adbc_driver_manager.cc:
##########
@@ -114,7 +130,186 @@ struct OwnedError {
}
};
+#ifdef _WIN32
+using char_type = wchar_t;
+
+std::string Utf8Encode(const std::wstring& wstr) {
+ if (wstr.empty()) return std::string();
+ int size_needed = WideCharToMultiByte(
+ CP_UTF8, 0, &wstr[0], static_cast<int>(wstr.size()), NULL, 0, NULL,
NULL);
+ std::string str_to(size_needed, 0);
+ WideCharToMultiByte(CP_UTF8, 0, &wstr[0], static_cast<int>(wstr.size()),
&str_to[0],
+ size_needed, NULL, NULL);
+ return str_to;
+}
+
+std::wstring Utf8Decode(const std::string& str) {
+ if (str.empty()) return std::wstring();
+ int size_needed =
+ MultiByteToWideChar(CP_UTF8, 0, &str[0], static_cast<int>(str.size()),
NULL, 0);
+ std::wstring wstr_to(size_needed, 0);
+ MultiByteToWideChar(CP_UTF8, 0, &str[0], static_cast<int>(str.size()),
&wstr_to[0],
+ size_needed);
+ return wstr_to;
+}
+
+#else
+using char_type = char;
+#endif
+
// Driver state
+struct DriverInfo {
+ std::string manifest_file;
+ std::string driver_name;
+ std::filesystem::path lib_path;
+ std::string entrypoint;
+
+ std::string version;
+ std::string source;
+};
+
+#ifdef _WIN32
+class RegistryKey {
+ public:
+ RegistryKey(HKEY root, const std::wstring_view subkey) noexcept
+ : root_(root), key_(nullptr) {
+ status_ = RegOpenKeyExW(root_, subkey.data(), 0, KEY_READ, &key_);
+ }
+
+ ~RegistryKey() {
+ if (is_open() && key_ != nullptr) {
+ RegCloseKey(key_);
+ key_ = nullptr;
+ status_ = ERROR_REGISTRY_IO_FAILED;
+ }
+ }
+
+ HKEY key() const { return key_; }
+ bool is_open() const { return status_ == ERROR_SUCCESS; }
+ LSTATUS status() const { return status_; }
+
+ std::wstring GetString(const std::wstring& name, std::wstring default_value)
{
+ if (!is_open()) return default_value;
+
+ DWORD type = REG_SZ;
+ DWORD size = 0;
+ auto result = RegQueryValueExW(key_, name.c_str(), nullptr, &type,
nullptr, &size);
+ if (result != ERROR_SUCCESS) return default_value;
+ if (type != REG_SZ) return default_value;
+
+ std::wstring value(size, '\0');
+ result = RegQueryValueExW(key_, name.c_str(), nullptr, &type,
+ reinterpret_cast<LPBYTE>(value.data()), &size);
+ if (result != ERROR_SUCCESS) return default_value;
+ return value;
+ }
+
+ private:
+ HKEY root_;
+ HKEY key_;
+ LSTATUS status_;
+};
+
+AdbcStatusCode LoadDriverFromRegistry(HKEY root, const std::wstring&
driver_name,
+ DriverInfo& info, struct AdbcError*
error) {
+ static const LPCWSTR kAdbcDriverRegistry = L"SOFTWARE\\ADBC\\Drivers";
+ RegistryKey drivers_key(root, kAdbcDriverRegistry);
+ if (!drivers_key.is_open()) {
+ return ADBC_STATUS_NOT_FOUND;
+ }
+
+ RegistryKey dkey(drivers_key.key(), driver_name);
+ if (!dkey.is_open()) {
+ return ADBC_STATUS_NOT_FOUND;
+ }
+
+ info.driver_name = Utf8Encode(dkey.GetString(L"name", L""));
+ info.entrypoint = Utf8Encode(dkey.GetString(L"entrypoint", L""));
+ info.version = Utf8Encode(dkey.GetString(L"version", L""));
+ info.source = Utf8Encode(dkey.GetString(L"source", L""));
+ info.lib_path = std::filesystem::path(dkey.GetString(L"driver", L""));
+ if (info.lib_path.empty()) {
+ SetError(error, "Driver path not found in registry for \""s +
+ Utf8Encode(driver_name) + "\"");
+ return ADBC_STATUS_NOT_FOUND;
+ }
+ return ADBC_STATUS_OK;
+}
+#endif
+
+AdbcStatusCode LoadDriverManifest(const std::filesystem::path& driver_manifest,
+ DriverInfo& info, struct AdbcError* error) {
+ toml::table config;
+ try {
+ config = toml::parse_file(driver_manifest.native());
+ } catch (const toml::parse_error& err) {
+ SetError(error, err.what());
+ return ADBC_STATUS_INVALID_ARGUMENT;
+ }
+
+ info.manifest_file = driver_manifest.string();
+ info.driver_name = config["name"].value_or(""s);
+ info.entrypoint = config["entrypoint"].value_or(""s);
+ info.version = config["version"].value_or(""s);
+ info.source = config["source"].value_or(""s);
+
+ auto driver = config.at_path("Driver.shared");
+ if (toml::table* platforms = driver.as_table()) {
+ info.lib_path = platforms->at_path(adbc::CurrentArch()).value_or(""s);
+ } else if (auto* path = driver.as_string()) {
+ info.lib_path = path->get();
+ }
+
+ if (info.lib_path.empty()) {
+ SetError(error, "Driver path not found in manifest '"s +
driver_manifest.string() +
+ "' for current architecture '" + adbc::CurrentArch() +
"'");
+ return ADBC_STATUS_NOT_FOUND;
+ }
+
+ return ADBC_STATUS_OK;
+}
+
+std::vector<std::filesystem::path> GetSearchPaths(const AdbcLoadFlags levels) {
+ std::vector<std::filesystem::path> paths;
+ if (levels & ADBC_LOAD_FLAG_SEARCH_ENV) {
+ // Check the ADBC_CONFIG_PATH environment variable
+ const char* env_path = std::getenv("ADBC_CONFIG_PATH");
+ if (env_path) {
+ paths = InternalAdbcParsePath(env_path);
+ }
+ }
+
+ if (levels & ADBC_LOAD_FLAG_SEARCH_USER) {
+ // Check the user configuration directory
+ std::filesystem::path user_config_dir = InternalAdbcUserConfigDir();
+ if (!user_config_dir.empty() && std::filesystem::exists(user_config_dir)) {
+ paths.push_back(user_config_dir);
+ }
+ }
+
+ if (levels & ADBC_LOAD_FLAG_SEARCH_SYSTEM) {
+#ifndef _WIN32
+ // system level for windows is to search the registry keys
+ const std::filesystem::path system_config_dir("/etc/adbc");
+ if (std::filesystem::exists(system_config_dir)) {
+ paths.push_back(system_config_dir);
+ }
+#endif
+ }
+
+ return paths;
+}
+
+bool HasExtension(const std::filesystem::path& path, const std::string& ext) {
+#ifdef _WIN32
+ auto wext = Utf8Decode(ext);
+ auto path_ext = path.extension().native();
+ return path_ext.size() == wext.size() &&
+ _wcsnicmp(path_ext.c_str(), wext.c_str(), wext.size()) == 0;
Review Comment:
```suggestion
_wcsnicmp(path_ext.data(), wext.data(), wext.size()) == 0;
```
##########
c/driver_manager/adbc_driver_manager.cc:
##########
@@ -138,22 +333,189 @@ struct ManagedLibrary {
// release() from the DLL - how to handle this?
}
- AdbcStatusCode Load(const char* library, struct AdbcError* error) {
+ AdbcStatusCode GetDriverInfo(const std::string_view driver_name,
+ const AdbcLoadFlags load_options, DriverInfo&
info,
+ struct AdbcError* error) {
+ if (driver_name.empty()) {
+ SetError(error, "Driver name is empty");
+ return ADBC_STATUS_INVALID_ARGUMENT;
+ }
+
+ std::filesystem::path driver_path(driver_name);
+ const bool allow_relative_paths = load_options &
ADBC_LOAD_FLAG_ALLOW_RELATIVE_PATHS;
+ if (driver_path.has_extension()) {
+ if (driver_path.is_relative() && !allow_relative_paths) {
+ SetError(error, "Driver path is relative and relative paths are not
allowed");
+ return ADBC_STATUS_INVALID_ARGUMENT;
+ }
+
+ if (HasExtension(driver_path, ".toml")) {
+ // if the extension is .toml, attempt to load the manifest
+ // erroring if we fail
+
+ auto status = LoadDriverManifest(driver_path, info, error);
+ if (status == ADBC_STATUS_OK) {
+ return Load(info.lib_path.c_str(), error);
Review Comment:
(We have more `.c_str()`.)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
