lidavidm commented on code in PR #2918:
URL: https://github.com/apache/arrow-adbc/pull/2918#discussion_r2125262141
##########
c/driver_manager/adbc_driver_manager_test.cc:
##########
@@ -28,6 +30,7 @@
#include "validation/adbc_validation_util.h"
std::string AdbcDriverManagerDefaultEntrypoint(const std::string& filename);
+std::vector<std::filesystem::path> AdbcParsePath(const std::string_view& path);
Review Comment:
string_view should generally be passed by value
##########
c/driver_manager/adbc_driver_manager.cc:
##########
@@ -114,7 +129,175 @@ struct OwnedError {
}
};
+std::filesystem::path UserConfigDir() {
+ std::filesystem::path config_dir;
+#if defined(_WIN32)
+ auto dir = std::getenv("AppData");
+ if (dir) {
+ config_dir = std::filesystem::path(dir);
+ config_dir /= "ADBC/drivers";
+ }
+#elif defined(__APPLE__)
+ auto dir = std::getenv("HOME");
+ if (dir) {
+ config_dir = std::filesystem::path(dir);
+ config_dir /= "Library/Application Support/ADBC";
+ }
+#elif defined(__linux__)
+ auto dir = std::getenv("XDG_CONFIG_HOME");
+ if (!dir) {
+ dir = std::getenv("HOME");
+ if (dir) {
+ config_dir = std::filesystem::path(dir);
+ }
+ } else {
+ config_dir = std::filesystem::path(dir);
+ }
+
+ if (!config_dir.empty()) {
+ config_dir /= "adbc";
+ }
+#endif
+
+ return config_dir;
+}
+
// Driver state
+struct DriverInfo {
+ std::string manifest_file;
+ std::string driver_name;
+ std::string lib_path;
+ std::string entrypoint;
+
+ std::string version;
+ std::string source;
+};
+
+#ifdef _WIN32
+class RegistryKey {
+ public:
+ RegistryKey(HKEY root, const std::string_view subkey) noexcept
+ : root_(root), key_(nullptr), is_open_(false) {
+ auto result = RegOpenKeyExA(root_, subkey.data(), 0, KEY_READ, &key_);
+ if (result == ERROR_SUCCESS) {
+ is_open_ = true;
+ }
+ }
+
+ ~RegistryKey() {
+ if (is_open_ && key_ != nullptr) {
+ RegCloseKey(key_);
+ key_ = nullptr;
+ is_open_ = false;
+ }
+ }
+
+ HKEY key() const { return key_; }
+ bool is_open() const { return is_open_; }
+
+ std::string GetString(const std::string& name, const std::string&
default_value = "") {
+ if (!is_open_) return default_value;
+
+ DWORD type = REG_SZ;
+ DWORD size = 0;
+ auto result = RegQueryValueExA(key_, name.c_str(), nullptr, &type,
nullptr, &size);
+ if (result != ERROR_SUCCESS) return default_value;
+ if (type != REG_SZ) return default_value;
+
+ std::string value(size, '\0');
+ result = RegQueryValueExA(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_;
+ bool is_open_;
+};
+
+AdbcStatusCode LoadDriverFromRegistry(HKEY root, const std::string&
driver_name,
+ DriverInfo& info, struct AdbcError*
error) {
+ static const LPCSTR kAdbcDriverRegistry = "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 = dkey.GetString("name");
+ info.entrypoint = dkey.GetString("entrypoint");
+ info.version = dkey.GetString("version");
+ info.source = dkey.GetString("source");
+ info.lib_path = dkey.GetString("driver");
+ if (info.lib_path.empty()) {
+ SetError(error, "Driver path not found in registry");
+ 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_STATE;
Review Comment:
Also maybe `INVALID_ARGUMENT` is more reasonable?
##########
c/driver_manager/adbc_driver_manager.cc:
##########
@@ -15,26 +15,41 @@
// 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> // strncasecmp
+#include <strsafe.h>
+
+#ifdef _MSC_VER
+#define strncasecmp _strnicmp
+#endif // _MSC_VER
+
+#else
+#include <dlfcn.h>
+#endif // defined(_WIN32)
+
+#include <toml++/toml.hpp>
#include "arrow-adbc/adbc.h"
+#include "arrow-adbc/adbc_driver_manager.h"
#include <algorithm>
#include <array>
#include <cctype>
#include <cerrno>
#include <cstring>
+#include <filesystem> // NOLINT [build/c++17]
#include <string>
#include <unordered_map>
#include <utility>
+#include <vector>
-#if defined(_WIN32)
-#include <windows.h> // Must come first
+using namespace std::string_literals; // NOLINT [build/namespaces]
-#include <libloaderapi.h>
-#include <strsafe.h>
-#else
-#include <dlfcn.h>
-#endif // defined(_WIN32)
+ADBC_EXPORT
+std::vector<std::filesystem::path> AdbcParsePath(const std::string_view& path);
Review Comment:
Can we use `InternalAdbc` instead?
##########
c/driver_manager/adbc_driver_manager.cc:
##########
@@ -1658,8 +2041,76 @@ const char* AdbcStatusCodeMessage(AdbcStatusCode code) {
#undef CASE
}
+AdbcStatusCode AdbcFindLoadDriver(const char* driver_name, const char*
entrypoint,
+ const int version, const uint16_t
load_options,
+ void* raw_driver, struct AdbcError* error) {
+ AdbcDriverInitFunc init_func = nullptr;
+ std::string error_message;
+
+ switch (version) {
+ case ADBC_VERSION_1_0_0:
+ case ADBC_VERSION_1_1_0:
+ break;
+ default:
+ SetError(error, "Only ADBC 1.0.0 and 1.1.0 are supported");
+ return ADBC_STATUS_NOT_IMPLEMENTED;
+ }
+
+ if (!raw_driver) {
+ SetError(error, "Driver pointer is null");
+ return ADBC_STATUS_INVALID_STATE;
+ }
+ auto* driver = reinterpret_cast<struct AdbcDriver*>(raw_driver);
+ if (!driver_name) {
+ SetError(error, "Driver name is null");
+ return ADBC_STATUS_INVALID_STATE;
+ }
+
+ ManagedLibrary library;
+ DriverInfo info;
+ if (entrypoint) {
+ info.entrypoint = entrypoint;
+ }
+
+ AdbcStatusCode status = library.GetDriverInfo(driver_name, load_options,
info, error);
+ if (status != ADBC_STATUS_OK) {
+ driver->release = nullptr;
+ return status;
+ }
+
+ void* load_handle = nullptr;
+ if (!info.entrypoint.empty()) {
+ status = library.Lookup(info.entrypoint.c_str(), &load_handle, error);
+ } else {
+ auto name = AdbcDriverManagerDefaultEntrypoint(info.lib_path);
+ status = library.Lookup(name.c_str(), &load_handle, error);
+ if (status != ADBC_STATUS_OK) {
+ status = library.Lookup(kDefaultEntrypoint, &load_handle, error);
+ }
+ }
+
+ if (status != ADBC_STATUS_OK) {
+ library.Release();
+ return status;
+ }
+ init_func = reinterpret_cast<AdbcDriverInitFunc>(load_handle);
+
+ status = AdbcLoadDriverFromInitFunc(init_func, version, driver, error);
+ if (status == ADBC_STATUS_OK) {
+ ManagerDriverState* state = new ManagerDriverState;
+ state->driver_release = driver->release;
+ state->handle = std::move(library);
+ driver->release = &ReleaseDriver;
+ driver->private_manager = state;
+ } else {
+ library.Release();
+ }
+ return status;
+}
+
AdbcStatusCode AdbcLoadDriver(const char* driver_name, const char* entrypoint,
int version, void* raw_driver, struct AdbcError*
error) {
+ // should this just be AdbcFindLoadDriver(...., kAdbcDefaultLoadOptions,
...)?
Review Comment:
yeah, probably
##########
c/driver_manager/adbc_driver_manager.cc:
##########
@@ -632,6 +985,36 @@ static const char kDefaultEntrypoint[] = "AdbcDriverInit";
// Other helpers (intentionally not in an anonymous namespace so they can be
tested)
+std::vector<std::filesystem::path> AdbcParsePath(const std::string_view& path)
{
+ std::vector<std::filesystem::path> result;
+ if (path.empty()) {
+ return result;
+ }
+
+#ifdef _WIN32
+ constexpr char delimiter = ';';
+#else
+ constexpr char delimiter = ':';
+#endif // _WIN32
+
+ bool in_quotes = false;
Review Comment:
Hmm, is escaping actually a thing?
https://unix.stackexchange.com/questions/579081/why-assign-export-ld-library-path
suggests it conventionally isn't. (Also it notes that Linux can use `;` too.)
##########
c/driver_manager/adbc_driver_manager.cc:
##########
@@ -114,7 +129,175 @@ struct OwnedError {
}
};
+std::filesystem::path UserConfigDir() {
+ std::filesystem::path config_dir;
+#if defined(_WIN32)
+ auto dir = std::getenv("AppData");
+ if (dir) {
+ config_dir = std::filesystem::path(dir);
+ config_dir /= "ADBC/drivers";
+ }
+#elif defined(__APPLE__)
+ auto dir = std::getenv("HOME");
+ if (dir) {
+ config_dir = std::filesystem::path(dir);
+ config_dir /= "Library/Application Support/ADBC";
+ }
+#elif defined(__linux__)
+ auto dir = std::getenv("XDG_CONFIG_HOME");
+ if (!dir) {
+ dir = std::getenv("HOME");
+ if (dir) {
+ config_dir = std::filesystem::path(dir);
+ }
+ } else {
+ config_dir = std::filesystem::path(dir);
+ }
+
+ if (!config_dir.empty()) {
+ config_dir /= "adbc";
+ }
+#endif
+
+ return config_dir;
+}
+
// Driver state
+struct DriverInfo {
+ std::string manifest_file;
+ std::string driver_name;
+ std::string lib_path;
+ std::string entrypoint;
+
+ std::string version;
+ std::string source;
+};
+
+#ifdef _WIN32
+class RegistryKey {
+ public:
+ RegistryKey(HKEY root, const std::string_view subkey) noexcept
+ : root_(root), key_(nullptr), is_open_(false) {
+ auto result = RegOpenKeyExA(root_, subkey.data(), 0, KEY_READ, &key_);
+ if (result == ERROR_SUCCESS) {
+ is_open_ = true;
+ }
+ }
+
+ ~RegistryKey() {
+ if (is_open_ && key_ != nullptr) {
+ RegCloseKey(key_);
+ key_ = nullptr;
+ is_open_ = false;
+ }
+ }
+
+ HKEY key() const { return key_; }
+ bool is_open() const { return is_open_; }
+
+ std::string GetString(const std::string& name, const std::string&
default_value = "") {
+ if (!is_open_) return default_value;
+
+ DWORD type = REG_SZ;
+ DWORD size = 0;
+ auto result = RegQueryValueExA(key_, name.c_str(), nullptr, &type,
nullptr, &size);
+ if (result != ERROR_SUCCESS) return default_value;
+ if (type != REG_SZ) return default_value;
+
+ std::string value(size, '\0');
+ result = RegQueryValueExA(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_;
+ bool is_open_;
+};
+
+AdbcStatusCode LoadDriverFromRegistry(HKEY root, const std::string&
driver_name,
+ DriverInfo& info, struct AdbcError*
error) {
+ static const LPCSTR kAdbcDriverRegistry = "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 = dkey.GetString("name");
+ info.entrypoint = dkey.GetString("entrypoint");
+ info.version = dkey.GetString("version");
+ info.source = dkey.GetString("source");
+ info.lib_path = dkey.GetString("driver");
+ if (info.lib_path.empty()) {
+ SetError(error, "Driver path not found in registry");
+ 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());
Review Comment:
Can we have the usual `[DriverManager]` prefix to the error messages?
--
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]
