This is an automated email from the ASF dual-hosted git repository.
lidavidm pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow-adbc.git
The following commit(s) were added to refs/heads/main by this push:
new 75ca3d617 test(c/driver_manager): fully test driver/uri/profile
combinations (#4092)
75ca3d617 is described below
commit 75ca3d6174027be802291d3d8e7a4cf8db312d06
Author: David Li <[email protected]>
AuthorDate: Wed Mar 18 10:37:04 2026 +0900
test(c/driver_manager): fully test driver/uri/profile combinations (#4092)
Closes #4085.
---
c/driver_manager/adbc_driver_manager.cc | 123 +++++-
c/driver_manager/adbc_driver_manager_api.cc | 58 +--
c/driver_manager/adbc_driver_manager_internal.h | 6 +
c/driver_manager/adbc_driver_manager_test.cc | 541 +++++++++++++++++++----
go/adbc/drivermgr/adbc_driver_manager.cc | 123 +++++-
go/adbc/drivermgr/adbc_driver_manager_api.cc | 58 +--
go/adbc/drivermgr/adbc_driver_manager_internal.h | 6 +
python/adbc_driver_manager/tests/test_profile.py | 76 +++-
8 files changed, 772 insertions(+), 219 deletions(-)
diff --git a/c/driver_manager/adbc_driver_manager.cc
b/c/driver_manager/adbc_driver_manager.cc
index 5bc76c40c..d55a0d224 100644
--- a/c/driver_manager/adbc_driver_manager.cc
+++ b/c/driver_manager/adbc_driver_manager.cc
@@ -416,16 +416,6 @@ std::optional<ParseDriverUriResult>
InternalAdbcParseDriverUri(std::string_view
return ParseDriverUriResult{d, str.substr(pos + 1), std::nullopt};
}
-// Direct implementations of API methods
-
-int AdbcErrorGetDetailCount(const struct AdbcError* error) {
- if (error->vendor_code == ADBC_ERROR_VENDOR_CODE_PRIVATE_DATA &&
error->private_data &&
- error->private_driver && error->private_driver->ErrorGetDetailCount) {
- return error->private_driver->ErrorGetDetailCount(error);
- }
- return 0;
-}
-
struct ProfileGuard {
AdbcConnectionProfile profile;
explicit ProfileGuard() : profile{} {}
@@ -436,6 +426,107 @@ struct ProfileGuard {
}
};
+ADBC_EXPORT
+AdbcStatusCode InternalAdbcParseOptions(TempDatabase* db, struct AdbcError*
error) {
+ // https://github.com/apache/arrow-adbc/issues/4085
+
+ // init_func always takes precedence
+ if (db->init_func) return ADBC_STATUS_OK;
+
+ std::optional<ParseDriverUriResult> parsed_driver;
+ std::optional<ParseDriverUriResult> parsed_uri;
+
+ if (!db->driver.empty()) {
+ parsed_driver = InternalAdbcParseDriverUri(db->driver);
+ }
+ if (auto it = db->options.find("uri"); it != db->options.end()) {
+ parsed_uri = InternalAdbcParseDriverUri(it->second);
+ }
+
+ // do we have `profile`, `profile://` URI, or `profile://` URI in driver?
+ {
+ const bool have_profile = db->options.find("profile") != db->options.end();
+ const bool have_profile_driver = parsed_driver &&
parsed_driver->profile.has_value();
+ const bool have_profile_uri = parsed_uri &&
parsed_uri->profile.has_value();
+
+ if (static_cast<int>(have_profile) + static_cast<int>(have_profile_uri) +
+ static_cast<int>(have_profile_driver) >
+ 1) {
+ SetError(error,
+ "Multiple profiles specified; only one of `profile` option,
`profile://` "
+ "URI, or `profile://` driver is allowed");
+ return ADBC_STATUS_INVALID_ARGUMENT;
+ }
+
+ // apply profile (may modify options)
+ std::string profile;
+ if (have_profile) {
+ profile = db->options["profile"];
+ db->options.erase("profile");
+ } else if (have_profile_uri) {
+ profile = *parsed_uri->profile;
+ db->options.erase("uri");
+ } else if (have_profile_driver) {
+ profile = *parsed_driver->profile;
+ db->driver.clear();
+ }
+
+ if (!profile.empty()) {
+ auto status = InternalInitializeProfile(db, profile, error);
+ if (status != ADBC_STATUS_OK) {
+ return status;
+ }
+ }
+ }
+
+ // parse driver for URI. we do this even if it was set via profile
+ {
+ // Reparse because profile may have modified driver
+ std::string owned_driver = db->driver;
+ if (auto maybe_driver = InternalAdbcParseDriverUri(owned_driver);
+ maybe_driver.has_value()) {
+ auto parsed = *maybe_driver;
+ // Don't allow recursive profiles (that is the only way we can reach
here)
+ if (parsed.profile.has_value()) {
+ SetError(error, "Profile cannot specify a profile:// URI");
+ return ADBC_STATUS_INVALID_ARGUMENT;
+ }
+ if (parsed.uri.has_value()) {
+ db->driver = std::string{parsed.driver};
+ // maybe only do this if URI is unset?
+ db->options["uri"] = std::string{*parsed.uri};
+ }
+ }
+ }
+
+ // no driver? parse URI
+ if (auto it = db->options.find("uri"); db->driver.empty() && it !=
db->options.end()) {
+ // Reparse because profile may have modified uri
+ std::string owned_uri = it->second;
+ if (auto maybe_uri = InternalAdbcParseDriverUri(it->second);
maybe_uri.has_value()) {
+ auto parsed = *maybe_uri;
+ // Don't allow recursive profiles (that is the only way we can reach
here)
+ if (parsed.profile.has_value()) {
+ SetError(error, "Profile cannot specify a profile:// URI");
+ return ADBC_STATUS_INVALID_ARGUMENT;
+ }
+ if (parsed.uri.has_value()) {
+ db->driver = std::string{parsed.driver};
+ db->options["uri"] = std::string{*parsed.uri};
+ }
+ } else if (db->driver.empty()) {
+ db->driver = std::move(owned_uri);
+ }
+ }
+
+ // still no driver? bail
+ if (db->driver.empty()) {
+ SetError(error, "Must set 'driver' option before AdbcDatabaseInit");
+ return ADBC_STATUS_INVALID_ARGUMENT;
+ }
+ return ADBC_STATUS_OK;
+}
+
AdbcStatusCode InternalInitializeProfile(TempDatabase* args,
const std::string_view profile,
struct AdbcError* error) {
@@ -451,7 +542,17 @@ AdbcStatusCode InternalInitializeProfile(TempDatabase*
args,
AdbcDriverInitFunc init_func = nullptr;
CHECK_STATUS(
guard.profile.GetDriverName(&guard.profile, &driver_name, &init_func,
error));
- if (driver_name != nullptr && strlen(driver_name) > 0) {
+ if (driver_name != nullptr && std::strlen(driver_name) > 0) {
+ // ensure the parsed driver matches the profile driver if both are
specified
+ if (!args->driver.empty() && args->driver != driver_name) {
+ std::string message = "Profile `";
+ message += profile;
+ message += "` specifies driver `";
+ message += driver_name;
+ message += "` which does not match requested driver `" + args->driver +
"`";
+ SetError(error, message);
+ return ADBC_STATUS_INVALID_ARGUMENT;
+ }
args->driver = driver_name;
}
diff --git a/c/driver_manager/adbc_driver_manager_api.cc
b/c/driver_manager/adbc_driver_manager_api.cc
index fb8d543ce..7f49aa65b 100644
--- a/c/driver_manager/adbc_driver_manager_api.cc
+++ b/c/driver_manager/adbc_driver_manager_api.cc
@@ -108,6 +108,14 @@ static void ErrorArrayStreamInit(struct ArrowArrayStream*
out,
// ADBC Error API implementations
//
=============================================================================
+int AdbcErrorGetDetailCount(const struct AdbcError* error) {
+ if (error->vendor_code == ADBC_ERROR_VENDOR_CODE_PRIVATE_DATA &&
error->private_data &&
+ error->private_driver && error->private_driver->ErrorGetDetailCount) {
+ return error->private_driver->ErrorGetDetailCount(error);
+ }
+ return 0;
+}
+
struct AdbcErrorDetail AdbcErrorGetDetail(const struct AdbcError* error, int
index) {
if (error->vendor_code == ADBC_ERROR_VENDOR_CODE_PRIVATE_DATA &&
error->private_data &&
error->private_driver && error->private_driver->ErrorGetDetail) {
@@ -639,55 +647,9 @@ AdbcStatusCode AdbcDatabaseInit(struct AdbcDatabase*
database, struct AdbcError*
return ADBC_STATUS_INVALID_STATE;
}
TempDatabase* args = reinterpret_cast<TempDatabase*>(database->private_data);
- const auto profile_in_use = args->options.find("profile");
- if (profile_in_use != args->options.end()) {
- std::string_view profile = profile_in_use->second;
- auto status = InternalInitializeProfile(args, profile, error);
- if (status != ADBC_STATUS_OK) {
- return status;
- }
- args->options.erase("profile");
- }
-
- if (!args->init_func) {
- const auto uri = args->options.find("uri");
- if (args->driver.empty() && uri != args->options.end()) {
- std::string owned_uri = uri->second;
- auto result = InternalAdbcParseDriverUri(owned_uri);
- if (result) {
- if (result->uri) {
- args->driver = std::string{result->driver};
- args->options["uri"] = std::string{*result->uri};
- } else if (result->profile) {
- args->options.erase("uri");
- auto status = InternalInitializeProfile(args, *result->profile,
error);
- if (status != ADBC_STATUS_OK) {
- return status;
- }
- }
- } else if (args->driver.empty()) {
- args->driver = owned_uri;
- }
- } else if (!args->driver.empty() && uri == args->options.end()) {
- std::string owned_driver = args->driver;
- auto result = InternalAdbcParseDriverUri(owned_driver);
- if (result) {
- args->driver = std::string{result->driver};
- if (result->uri) {
- args->options["uri"] = std::string{*result->uri};
- } else if (result->profile) {
- auto status = InternalInitializeProfile(args, *result->profile,
error);
- if (status != ADBC_STATUS_OK) {
- return status;
- }
- }
- }
- }
- if (args->driver.empty()) {
- SetError(error, "Must set 'driver' option before AdbcDatabaseInit");
- return ADBC_STATUS_INVALID_ARGUMENT;
- }
+ if (auto status = InternalAdbcParseOptions(args, error); status !=
ADBC_STATUS_OK) {
+ return status;
}
// Allocate the underlying driver
diff --git a/c/driver_manager/adbc_driver_manager_internal.h
b/c/driver_manager/adbc_driver_manager_internal.h
index dd0aef128..089e1b405 100644
--- a/c/driver_manager/adbc_driver_manager_internal.h
+++ b/c/driver_manager/adbc_driver_manager_internal.h
@@ -207,6 +207,12 @@ struct TempDatabase {
AdbcConnectionProfileProvider profile_provider = nullptr;
};
+// Parse and validate options like uri/profile, with the result being that
+// `driver` or `init_func` are populated and options like `profile` are
+// removed
+ADBC_EXPORT
+AdbcStatusCode InternalAdbcParseOptions(TempDatabase* db, struct AdbcError*
error);
+
/// Temporary state while the connection is being configured.
struct TempConnection {
std::unordered_map<std::string, std::string> options;
diff --git a/c/driver_manager/adbc_driver_manager_test.cc
b/c/driver_manager/adbc_driver_manager_test.cc
index 3265429e7..91969ad19 100644
--- a/c/driver_manager/adbc_driver_manager_test.cc
+++ b/c/driver_manager/adbc_driver_manager_test.cc
@@ -19,35 +19,25 @@
#include <windows.h>
#endif
-#include <gmock/gmock.h>
-#include <gtest/gtest.h>
-
#include <algorithm>
#include <cstdlib>
#include <filesystem> // NOLINT [build/c++17]
#include <iostream>
#include <string>
-#include <toml++/toml.hpp>
+#include <utility>
#include <vector>
+#include <gmock/gmock.h>
+#include <gtest/gtest.h>
+#include <toml++/toml.hpp>
+
+#include "adbc_driver_manager_internal.h"
#include "arrow-adbc/adbc.h"
#include "arrow-adbc/adbc_driver_manager.h"
#include "current_arch.h"
#include "validation/adbc_validation.h"
#include "validation/adbc_validation_util.h"
-std::string InternalAdbcDriverManagerDefaultEntrypoint(const std::string&
filename);
-std::vector<std::filesystem::path> InternalAdbcParsePath(const
std::string_view path);
-std::filesystem::path InternalAdbcUserConfigDir();
-
-struct ParseDriverUriResult {
- std::string_view driver;
- std::optional<std::string_view> uri;
- std::optional<std::string_view> profile;
-};
-
-std::optional<ParseDriverUriResult>
InternalAdbcParseDriverUri(std::string_view str);
-
// Tests of the SQLite example driver, except using the driver manager
namespace adbc {
@@ -56,6 +46,34 @@ using adbc_validation::Handle;
using adbc_validation::IsOkStatus;
using adbc_validation::IsStatus;
+namespace {
+void SetDriverPath(const char* path) {
+#ifdef _WIN32
+ int size_needed = MultiByteToWideChar(CP_UTF8, 0, path, -1, nullptr, 0);
+ std::wstring wpath(size_needed, 0);
+ MultiByteToWideChar(CP_UTF8, 0, path, -1, &wpath[0], size_needed);
+ ASSERT_TRUE(SetEnvironmentVariableW(L"ADBC_DRIVER_PATH", wpath.c_str()));
+#else
+ setenv("ADBC_DRIVER_PATH", path, 1);
+#endif
+}
+
+void UnsetDriverPath() { SetDriverPath(""); }
+
+void SetProfilePath(const char* path) {
+#ifdef _WIN32
+ int size_needed = MultiByteToWideChar(CP_UTF8, 0, path, -1, nullptr, 0);
+ std::wstring wpath(size_needed, 0);
+ MultiByteToWideChar(CP_UTF8, 0, path, -1, &wpath[0], size_needed);
+ ASSERT_TRUE(SetEnvironmentVariableW(L"ADBC_PROFILE_PATH", wpath.c_str()));
+#else
+ setenv("ADBC_PROFILE_PATH", path, 1);
+#endif
+}
+
+void UnsetProfilePath() { SetProfilePath(""); }
+} // namespace
+
class DriverManager : public ::testing::Test {
public:
void SetUp() override {
@@ -726,19 +744,6 @@ class DriverManifest : public ::testing::Test {
}
protected:
- void SetConfigPath(const char* path) {
-#ifdef _WIN32
- int size_needed = MultiByteToWideChar(CP_UTF8, 0, path, -1, nullptr, 0);
- std::wstring wpath(size_needed, 0);
- MultiByteToWideChar(CP_UTF8, 0, path, -1, &wpath[0], size_needed);
- ASSERT_TRUE(SetEnvironmentVariableW(L"ADBC_DRIVER_PATH", wpath.c_str()));
-#else
- setenv("ADBC_DRIVER_PATH", path, 1);
-#endif
- }
-
- void UnsetConfigPath() { SetConfigPath(""); }
-
struct AdbcDriver driver = {};
struct AdbcError error = {};
@@ -757,7 +762,7 @@ TEST_F(DriverManifest, LoadDriverEnv) {
test_manifest_file << simple_manifest;
test_manifest_file.close();
- SetConfigPath(temp_dir.string().c_str());
+ SetDriverPath(temp_dir.string().c_str());
ASSERT_THAT(AdbcFindLoadDriver("sqlite", nullptr, ADBC_VERSION_1_1_0,
ADBC_LOAD_FLAG_DEFAULT, nullptr, &driver,
&error),
@@ -765,7 +770,7 @@ TEST_F(DriverManifest, LoadDriverEnv) {
ASSERT_TRUE(std::filesystem::remove(temp_dir / "sqlite.toml"));
- UnsetConfigPath();
+ UnsetDriverPath();
}
TEST_F(DriverManifest, LoadNonAsciiPath) {
@@ -786,7 +791,7 @@ TEST_F(DriverManifest, LoadNonAsciiPath) {
test_manifest_file << simple_manifest;
test_manifest_file.close();
- SetConfigPath(non_ascii_dir.string().c_str());
+ SetDriverPath(non_ascii_dir.string().c_str());
ASSERT_THAT(AdbcFindLoadDriver("sqlite", nullptr, ADBC_VERSION_1_1_0,
ADBC_LOAD_FLAG_DEFAULT, nullptr, &driver,
&error),
@@ -794,7 +799,7 @@ TEST_F(DriverManifest, LoadNonAsciiPath) {
ASSERT_TRUE(std::filesystem::remove(non_ascii_dir / "sqlite.toml"));
- UnsetConfigPath();
+ UnsetDriverPath();
}
TEST_F(DriverManifest, DisallowEnvConfig) {
@@ -803,7 +808,7 @@ TEST_F(DriverManifest, DisallowEnvConfig) {
test_manifest_file << simple_manifest;
test_manifest_file.close();
- SetConfigPath(temp_dir.string().c_str());
+ SetDriverPath(temp_dir.string().c_str());
auto load_options = ADBC_LOAD_FLAG_DEFAULT & ~ADBC_LOAD_FLAG_SEARCH_ENV;
ASSERT_THAT(AdbcFindLoadDriver("sqlite", nullptr, ADBC_VERSION_1_1_0,
load_options,
@@ -812,7 +817,7 @@ TEST_F(DriverManifest, DisallowEnvConfig) {
ASSERT_TRUE(std::filesystem::remove(temp_dir / "sqlite.toml"));
- UnsetConfigPath();
+ UnsetDriverPath();
}
TEST_F(DriverManifest, ConfigEntrypoint) {
@@ -1507,19 +1512,6 @@ class ConnectionProfiles : public ::testing::Test {
}
protected:
- void SetConfigPath(const char* path) {
-#ifdef _WIN32
- int size_needed = MultiByteToWideChar(CP_UTF8, 0, path, -1, nullptr, 0);
- std::wstring wpath(size_needed, 0);
- MultiByteToWideChar(CP_UTF8, 0, path, -1, &wpath[0], size_needed);
- ASSERT_TRUE(SetEnvironmentVariableW(L"ADBC_PROFILE_PATH", wpath.c_str()));
-#else
- setenv("ADBC_PROFILE_PATH", path, 1);
-#endif
- }
-
- void UnsetConfigPath() { SetConfigPath(""); }
-
struct AdbcDriver driver = {};
struct AdbcError error = {};
@@ -1557,13 +1549,13 @@ TEST_F(ConnectionProfiles, SetProfileOption) {
ASSERT_THAT(AdbcDatabaseRelease(&database.value, &error),
IsOkStatus(&error));
// find profile by name using ADBC_PROFILE_PATH
- SetConfigPath(temp_dir.string().c_str());
+ SetProfilePath(temp_dir.string().c_str());
ASSERT_THAT(AdbcDatabaseNew(&database.value, &error), IsOkStatus(&error));
ASSERT_THAT(AdbcDatabaseSetOption(&database.value, "profile", "profile",
&error),
IsOkStatus(&error));
ASSERT_THAT(AdbcDatabaseInit(&database.value, &error), IsOkStatus(&error));
ASSERT_THAT(AdbcDatabaseRelease(&database.value, &error),
IsOkStatus(&error));
- UnsetConfigPath();
+ UnsetProfilePath();
}
TEST_F(ConnectionProfiles, HierarchicalProfile) {
@@ -1577,13 +1569,13 @@ TEST_F(ConnectionProfiles, HierarchicalProfile) {
adbc_validation::Handle<struct AdbcDatabase> database;
- SetConfigPath(temp_dir.string().c_str());
+ SetProfilePath(temp_dir.string().c_str());
ASSERT_THAT(AdbcDatabaseNew(&database.value, &error), IsOkStatus(&error));
ASSERT_THAT(AdbcDatabaseSetOption(&database.value, "profile", "dev/profile",
&error),
IsOkStatus(&error));
ASSERT_THAT(AdbcDatabaseInit(&database.value, &error), IsOkStatus(&error));
ASSERT_THAT(AdbcDatabaseRelease(&database.value, &error),
IsOkStatus(&error));
- UnsetConfigPath();
+ UnsetProfilePath();
}
TEST_F(ConnectionProfiles, UriProfileOption) {
@@ -1605,13 +1597,13 @@ TEST_F(ConnectionProfiles, UriProfileOption) {
ASSERT_THAT(AdbcDatabaseRelease(&database.value, &error),
IsOkStatus(&error));
// find profile by name using ADBC_PROFILE_PATH
- SetConfigPath(temp_dir.string().c_str());
+ SetProfilePath(temp_dir.string().c_str());
ASSERT_THAT(AdbcDatabaseNew(&database.value, &error), IsOkStatus(&error));
ASSERT_THAT(AdbcDatabaseSetOption(&database.value, "uri",
"profile://profile", &error),
IsOkStatus(&error));
ASSERT_THAT(AdbcDatabaseInit(&database.value, &error), IsOkStatus(&error));
ASSERT_THAT(AdbcDatabaseRelease(&database.value, &error),
IsOkStatus(&error));
- UnsetConfigPath();
+ UnsetProfilePath();
}
TEST_F(ConnectionProfiles, DriverProfileOption) {
@@ -1633,14 +1625,14 @@ TEST_F(ConnectionProfiles, DriverProfileOption) {
ASSERT_THAT(AdbcDatabaseRelease(&database.value, &error),
IsOkStatus(&error));
// find profile by name using ADBC_PROFILE_PATH
- SetConfigPath(temp_dir.string().c_str());
+ SetProfilePath(temp_dir.string().c_str());
ASSERT_THAT(AdbcDatabaseNew(&database.value, &error), IsOkStatus(&error));
ASSERT_THAT(
AdbcDatabaseSetOption(&database.value, "driver", "profile://profile",
&error),
IsOkStatus(&error));
ASSERT_THAT(AdbcDatabaseInit(&database.value, &error), IsOkStatus(&error));
ASSERT_THAT(AdbcDatabaseRelease(&database.value, &error),
IsOkStatus(&error));
- UnsetConfigPath();
+ UnsetProfilePath();
}
TEST_F(ConnectionProfiles, ExtraStringOption) {
@@ -1655,14 +1647,14 @@ TEST_F(ConnectionProfiles, ExtraStringOption) {
adbc_validation::Handle<struct AdbcDatabase> database;
// find profile by name using ADBC_PROFILE_PATH
- SetConfigPath(temp_dir.string().c_str());
+ SetProfilePath(temp_dir.string().c_str());
ASSERT_THAT(AdbcDatabaseNew(&database.value, &error), IsOkStatus(&error));
ASSERT_THAT(AdbcDatabaseSetOption(&database.value, "profile", "profile",
&error),
IsOkStatus(&error));
ASSERT_THAT(AdbcDatabaseInit(&database.value, &error),
IsStatus(ADBC_STATUS_NOT_IMPLEMENTED, &error));
ASSERT_THAT(error.message, ::testing::HasSubstr("Unknown database option
foo='bar'"));
- UnsetConfigPath();
+ UnsetProfilePath();
}
TEST_F(ConnectionProfiles, ExtraIntOption) {
@@ -1677,14 +1669,14 @@ TEST_F(ConnectionProfiles, ExtraIntOption) {
adbc_validation::Handle<struct AdbcDatabase> database;
// find profile by name using ADBC_PROFILE_PATH
- SetConfigPath(temp_dir.string().c_str());
+ SetProfilePath(temp_dir.string().c_str());
ASSERT_THAT(AdbcDatabaseNew(&database.value, &error), IsOkStatus(&error));
ASSERT_THAT(AdbcDatabaseSetOption(&database.value, "profile", "profile",
&error),
IsOkStatus(&error));
ASSERT_THAT(AdbcDatabaseInit(&database.value, &error),
IsStatus(ADBC_STATUS_NOT_IMPLEMENTED, &error));
ASSERT_THAT(error.message, ::testing::HasSubstr("Unknown database option
foo=42"));
- UnsetConfigPath();
+ UnsetProfilePath();
}
TEST_F(ConnectionProfiles, ExtraDoubleOption) {
@@ -1699,14 +1691,14 @@ TEST_F(ConnectionProfiles, ExtraDoubleOption) {
adbc_validation::Handle<struct AdbcDatabase> database;
// find profile by name using ADBC_PROFILE_PATH
- SetConfigPath(temp_dir.string().c_str());
+ SetProfilePath(temp_dir.string().c_str());
ASSERT_THAT(AdbcDatabaseNew(&database.value, &error), IsOkStatus(&error));
ASSERT_THAT(AdbcDatabaseSetOption(&database.value, "profile", "profile",
&error),
IsOkStatus(&error));
ASSERT_THAT(AdbcDatabaseInit(&database.value, &error),
IsStatus(ADBC_STATUS_NOT_IMPLEMENTED, &error));
ASSERT_THAT(error.message, ::testing::HasSubstr("Unknown database option
foo=42"));
- UnsetConfigPath();
+ UnsetProfilePath();
}
TEST_F(ConnectionProfiles, DotSeparatedKey) {
@@ -1726,7 +1718,7 @@ TEST_F(ConnectionProfiles, DotSeparatedKey) {
adbc_validation::Handle<struct AdbcDatabase> database;
// find profile by name using ADBC_PROFILE_PATH
- SetConfigPath(temp_dir.string().c_str());
+ SetProfilePath(temp_dir.string().c_str());
ASSERT_THAT(AdbcDatabaseNew(&database.value, &error), IsOkStatus(&error));
ASSERT_THAT(AdbcDatabaseSetOption(&database.value, "profile", "profile",
&error),
IsOkStatus(&error));
@@ -1734,7 +1726,7 @@ TEST_F(ConnectionProfiles, DotSeparatedKey) {
IsStatus(ADBC_STATUS_NOT_IMPLEMENTED, &error));
ASSERT_THAT(error.message,
::testing::HasSubstr("Unknown database option
foo.bar.baz='bar'"));
- UnsetConfigPath();
+ UnsetProfilePath();
}
TEST_F(ConnectionProfiles, UseEnvVar) {
@@ -1754,7 +1746,7 @@ TEST_F(ConnectionProfiles, UseEnvVar) {
adbc_validation::Handle<struct AdbcDatabase> database;
// find profile by name using ADBC_PROFILE_PATH
- SetConfigPath(temp_dir.string().c_str());
+ SetProfilePath(temp_dir.string().c_str());
ASSERT_THAT(AdbcDatabaseNew(&database.value, &error), IsOkStatus(&error));
ASSERT_THAT(AdbcDatabaseSetOption(&database.value, "profile", "profile",
&error),
IsOkStatus(&error));
@@ -1762,7 +1754,7 @@ TEST_F(ConnectionProfiles, UseEnvVar) {
IsStatus(ADBC_STATUS_NOT_IMPLEMENTED, &error));
ASSERT_THAT(error.message, ::testing::HasSubstr("Unknown database option
foo='" +
temp_dir.string() + "'"));
- UnsetConfigPath();
+ UnsetProfilePath();
}
TEST_F(ConnectionProfiles, UseEnvVarNotExist) {
@@ -1782,14 +1774,14 @@ TEST_F(ConnectionProfiles, UseEnvVarNotExist) {
adbc_validation::Handle<struct AdbcDatabase> database;
// find profile by name using ADBC_PROFILE_PATH
- SetConfigPath(temp_dir.string().c_str());
+ SetProfilePath(temp_dir.string().c_str());
ASSERT_THAT(AdbcDatabaseNew(&database.value, &error), IsOkStatus(&error));
ASSERT_THAT(AdbcDatabaseSetOption(&database.value, "profile", "profile",
&error),
IsOkStatus(&error));
ASSERT_THAT(AdbcDatabaseInit(&database.value, &error),
IsStatus(ADBC_STATUS_NOT_IMPLEMENTED, &error));
ASSERT_THAT(error.message, ::testing::HasSubstr("Unknown database option
foo=''"));
- UnsetConfigPath();
+ UnsetProfilePath();
}
TEST_F(ConnectionProfiles, UseEnvVarNotExistNoBail) {
@@ -1809,7 +1801,7 @@ TEST_F(ConnectionProfiles, UseEnvVarNotExistNoBail) {
adbc_validation::Handle<struct AdbcDatabase> database;
// find profile by name using ADBC_PROFILE_PATH
- SetConfigPath(temp_dir.string().c_str());
+ SetProfilePath(temp_dir.string().c_str());
ASSERT_THAT(AdbcDatabaseNew(&database.value, &error), IsOkStatus(&error));
ASSERT_THAT(AdbcDatabaseSetOption(&database.value, "profile", "profile",
&error),
IsOkStatus(&error));
@@ -1817,7 +1809,7 @@ TEST_F(ConnectionProfiles, UseEnvVarNotExistNoBail) {
IsStatus(ADBC_STATUS_NOT_IMPLEMENTED, &error));
ASSERT_THAT(error.message,
::testing::HasSubstr("Unknown database option foo='foobar'"));
- UnsetConfigPath();
+ UnsetProfilePath();
}
TEST_F(ConnectionProfiles, UseEnvVarMalformed) {
@@ -1837,7 +1829,7 @@ TEST_F(ConnectionProfiles, UseEnvVarMalformed) {
adbc_validation::Handle<struct AdbcDatabase> database;
// find profile by name using ADBC_PROFILE_PATH
- SetConfigPath(temp_dir.string().c_str());
+ SetProfilePath(temp_dir.string().c_str());
ASSERT_THAT(AdbcDatabaseNew(&database.value, &error), IsOkStatus(&error));
ASSERT_THAT(AdbcDatabaseSetOption(&database.value, "profile", "profile",
&error),
IsOkStatus(&error));
@@ -1847,7 +1839,7 @@ TEST_F(ConnectionProfiles, UseEnvVarMalformed) {
error.message,
::testing::HasSubstr(
"In profile: malformed env_var() in key `foo`: missing closing
parenthesis"));
- UnsetConfigPath();
+ UnsetProfilePath();
}
TEST_F(ConnectionProfiles, UseEnvVarMissingArg) {
@@ -1867,7 +1859,7 @@ TEST_F(ConnectionProfiles, UseEnvVarMissingArg) {
adbc_validation::Handle<struct AdbcDatabase> database;
// find profile by name using ADBC_PROFILE_PATH
- SetConfigPath(temp_dir.string().c_str());
+ SetProfilePath(temp_dir.string().c_str());
ASSERT_THAT(AdbcDatabaseNew(&database.value, &error), IsOkStatus(&error));
ASSERT_THAT(AdbcDatabaseSetOption(&database.value, "profile", "profile",
&error),
IsOkStatus(&error));
@@ -1876,7 +1868,7 @@ TEST_F(ConnectionProfiles, UseEnvVarMissingArg) {
ASSERT_THAT(error.message,
::testing::HasSubstr("In profile: malformed env_var() in key
`foo`: "
"missing environment variable name"));
- UnsetConfigPath();
+ UnsetProfilePath();
}
TEST_F(ConnectionProfiles, UseEnvVarInterpolation) {
@@ -1896,7 +1888,7 @@ TEST_F(ConnectionProfiles, UseEnvVarInterpolation) {
adbc_validation::Handle<struct AdbcDatabase> database;
// find profile by name using ADBC_PROFILE_PATH
- SetConfigPath(temp_dir.string().c_str());
+ SetProfilePath(temp_dir.string().c_str());
ASSERT_THAT(AdbcDatabaseNew(&database.value, &error), IsOkStatus(&error));
ASSERT_THAT(AdbcDatabaseSetOption(&database.value, "profile", "profile",
&error),
IsOkStatus(&error));
@@ -1904,7 +1896,7 @@ TEST_F(ConnectionProfiles, UseEnvVarInterpolation) {
IsStatus(ADBC_STATUS_NOT_IMPLEMENTED, &error));
ASSERT_THAT(error.message, ::testing::HasSubstr("Unknown database option
foo='super " +
temp_dir.string() + "
duper'"));
- UnsetConfigPath();
+ UnsetProfilePath();
}
TEST_F(ConnectionProfiles, UseEnvVarInterpolationMultiple) {
@@ -1924,7 +1916,7 @@ TEST_F(ConnectionProfiles,
UseEnvVarInterpolationMultiple) {
adbc_validation::Handle<struct AdbcDatabase> database;
// find profile by name using ADBC_PROFILE_PATH
- SetConfigPath(temp_dir.string().c_str());
+ SetProfilePath(temp_dir.string().c_str());
ASSERT_THAT(AdbcDatabaseNew(&database.value, &error), IsOkStatus(&error));
ASSERT_THAT(AdbcDatabaseSetOption(&database.value, "profile", "profile",
&error),
IsOkStatus(&error));
@@ -1933,7 +1925,7 @@ TEST_F(ConnectionProfiles,
UseEnvVarInterpolationMultiple) {
ASSERT_THAT(error.message, ::testing::HasSubstr("Unknown database option
foo='super " +
temp_dir.string() + " duper
" +
temp_dir.string() + "
end'"));
- UnsetConfigPath();
+ UnsetProfilePath();
}
TEST_F(ConnectionProfiles, ProfileNotFound) {
@@ -1951,7 +1943,7 @@ TEST_F(ConnectionProfiles, ProfileNotFound) {
ASSERT_THAT(AdbcDatabaseRelease(&database.value, &error),
IsOkStatus(&error));
// find profile by name using ADBC_PROFILE_PATH
- SetConfigPath(temp_dir.string().c_str());
+ SetProfilePath(temp_dir.string().c_str());
ASSERT_THAT(AdbcDatabaseNew(&database.value, &error), IsOkStatus(&error));
ASSERT_THAT(AdbcDatabaseSetOption(&database.value, "profile", "profile",
&error),
IsOkStatus(&error));
@@ -1962,7 +1954,7 @@ TEST_F(ConnectionProfiles, ProfileNotFound) {
"Also searched these paths for
profiles:\n\t" +
"ADBC_PROFILE_PATH: " + temp_dir.string() +
"\n\t"));
ASSERT_THAT(AdbcDatabaseRelease(&database.value, &error),
IsOkStatus(&error));
- UnsetConfigPath();
+ UnsetProfilePath();
}
TEST_F(ConnectionProfiles, CondaProfileTest) {
@@ -2037,4 +2029,393 @@ TEST_F(ConnectionProfiles, CustomProfileProvider) {
ASSERT_THAT(AdbcDatabaseRelease(&database.value, &error),
IsOkStatus(&error));
}
+struct DriverUriProfile {
+ std::string name;
+ std::string driver;
+ std::string uri;
+ std::string profile;
+
+ std::string expected_driver = "";
+ std::string expected_uri = "";
+ AdbcStatusCode expected_error = ADBC_STATUS_OK;
+ std::string expected_message = "";
+
+ DriverUriProfile WithExpected(std::string driver, std::string uri) {
+ expected_driver = std::move(driver);
+ expected_uri = std::move(uri);
+ expected_error = ADBC_STATUS_OK;
+ expected_message.clear();
+ return *this;
+ }
+
+ DriverUriProfile WithExpectedError(AdbcStatusCode error_code,
+ std::string message_substr) {
+ expected_driver.clear();
+ expected_uri.clear();
+ expected_error = error_code;
+ expected_message = std::move(message_substr);
+ return *this;
+ }
+};
+
+class DriverUriProfileTest : public ConnectionProfiles,
+ public
testing::WithParamInterface<DriverUriProfile> {
+ public:
+ void SetUp() override {
+ ConnectionProfiles::SetUp();
+
+ auto profiles = temp_dir / "profiles";
+ std::filesystem::create_directories(profiles);
+ SetProfilePath(profiles.string().c_str());
+
+ {
+ auto filepath = profiles / "prod-sqlite.toml";
+ std::ofstream profile_file(filepath);
+ ASSERT_TRUE(profile_file.is_open());
+ profile_file << toml::table{
+ {"profile_version", 1},
+ {"driver", "sqlite"},
+ {"Options", toml::table{{"uri", "file://:memory:"}}},
+ };
+ }
+ {
+ auto filepath = profiles / "prod-driver-uri.toml";
+ std::ofstream profile_file(filepath);
+ ASSERT_TRUE(profile_file.is_open());
+ profile_file << toml::table{
+ {"profile_version", 1},
+ {"driver", "adbc_driver_sqlite"},
+ {"Options", toml::table{{"uri", "file://:memory:"}}},
+ };
+ }
+ {
+ auto filepath = profiles / "prod-driver.toml";
+ std::ofstream profile_file(filepath);
+ ASSERT_TRUE(profile_file.is_open());
+ profile_file << toml::table{
+ {"profile_version", 1},
+ {"driver", "adbc_driver_sqlite"},
+ {"Options", toml::table{}},
+ };
+ }
+ {
+ auto filepath = profiles / "prod-uri.toml";
+ std::ofstream profile_file(filepath);
+ ASSERT_TRUE(profile_file.is_open());
+ profile_file << toml::table{
+ {"profile_version", 1},
+ {"Options", toml::table{{"uri", "file://:memory:"}}},
+ };
+ }
+ {
+ auto filepath = profiles / "prod-empty.toml";
+ std::ofstream profile_file(filepath);
+ ASSERT_TRUE(profile_file.is_open());
+ profile_file << toml::table{
+ {"profile_version", 1},
+ {"Options", toml::table{{"noturi", "file://:memory:"}}},
+ };
+ }
+
+ auto manifests = temp_dir / "drivers";
+ std::filesystem::create_directories(manifests);
+ SetDriverPath(manifests.string().c_str());
+
+ auto filepath = manifests / "sqlite.toml";
+ std::ofstream manifest_file(filepath);
+ ASSERT_TRUE(manifest_file.is_open());
+ manifest_file << toml::table{
+ {"name", "SQLite3"},
+ {"publisher", "arrow-adbc"},
+ {"version", "X.Y.Z"},
+ {"ADBC",
+ toml::table{
+ {"version", "1.1.0"},
+ }},
+ {"Driver",
+ toml::table{
+ {"shared",
+ toml::table{
+ {adbc::CurrentArch(), driver_path.string()},
+ }},
+ }},
+ };
+ }
+
+ void TearDown() override {
+ ConnectionProfiles::TearDown();
+ UnsetProfilePath();
+ }
+};
+
+TEST_P(DriverUriProfileTest, Load) {
+ const auto& param = GetParam();
+
+ TempDatabase db;
+ if (!param.driver.empty()) {
+ db.driver = param.driver;
+ }
+ if (!param.uri.empty()) {
+ db.options["uri"] = param.uri;
+ }
+ if (!param.profile.empty()) {
+ db.options["profile"] = param.profile;
+ }
+
+ auto status = InternalAdbcParseOptions(&db, &error);
+ if (param.expected_error != ADBC_STATUS_OK) {
+ ASSERT_THAT(status, IsStatus(param.expected_error, &error));
+ ASSERT_THAT(error.message, ::testing::HasSubstr(param.expected_message));
+ } else {
+ ASSERT_THAT(status, IsOkStatus(&error));
+ EXPECT_EQ(db.driver, param.expected_driver);
+ if (param.expected_uri.empty()) {
+ EXPECT_THAT(db.options,
testing::Not(testing::Contains(testing::Key("uri"))));
+ } else {
+ EXPECT_THAT(db.options,
+ testing::Contains(testing::Pair("uri", param.expected_uri)));
+ }
+ }
+}
+
+constexpr char kNoDriverError[] = "Must set 'driver' option before
AdbcDatabaseInit";
+constexpr char kMultipleProfilesError[] = "Multiple profiles specified";
+// TODO(https://github.com/apache/arrow-adbc/issues/3963): designated
initializers
+static const DriverUriProfile kOptionsValues[] = {
+ // nothing is specified => error
+ // ------------------------------------------------------------
+ (DriverUriProfile{"empty_empty_empty", "", "", ""})
+ .WithExpectedError(ADBC_STATUS_INVALID_ARGUMENT, kNoDriverError),
+
+ // no driver + no URI + profile: use profile's values
+ // ------------------------------------------------------------
+ (DriverUriProfile{"empty_empty_profile1", "", "", "prod-driver-uri"})
+ .WithExpected("adbc_driver_sqlite", "file://:memory:"),
+ (DriverUriProfile{"empty_empty_profile2", "", "", "prod-driver"})
+ .WithExpected("adbc_driver_sqlite", ""),
+ // Profile has no driver, but has a URI, so we parse that
+ (DriverUriProfile{"empty_empty_profile3", "", "", "prod-uri"})
+ .WithExpected("file", "file://:memory:"),
+ (DriverUriProfile{"empty_empty_profile4", "", "", "prod-empty"})
+ .WithExpectedError(ADBC_STATUS_INVALID_ARGUMENT, kNoDriverError),
+
+ // no driver + URI (where URI is not a profile URI) + no
+ // profile: parse URI for driver
+ // ------------------------------------------------------------
+ (DriverUriProfile{"empty_driveruri_empty", "", "sqlite://:memory:", ""})
+ .WithExpected("sqlite", "sqlite://:memory:"),
+
+ // no driver + URI (where URI is not a profile URI) + profile:
+ // it depends on what the profile contains. If it has a driver,
+ // then we use the profile's driver
+ // ------------------------------------------------------------
+ (DriverUriProfile{"empty_driveruri_profile1", "", "bar://:memory:",
"prod-sqlite"})
+ .WithExpected("sqlite", "bar://:memory:"),
+ (DriverUriProfile{"empty_driveruri_profile2", "", "bar://:memory:",
"prod-driver"})
+ .WithExpected("adbc_driver_sqlite", "bar://:memory:"),
+ // here the profile's URI is overridden by the supplied one
+ (DriverUriProfile{"empty_driveruri_profile3", "", "bar://:memory:",
"prod-uri"})
+ .WithExpected("bar", "bar://:memory:"),
+ (DriverUriProfile{"empty_driveruri_profile4", "", "bar://:memory:",
"prod-empty"})
+ .WithExpected("bar", "bar://:memory:"),
+
+ // no driver + profile URI + no profile
+ (DriverUriProfile{"empty_profile_empty", "", "profile://prod-sqlite", ""})
+ .WithExpected("sqlite", "file://:memory:"),
+
+ // no driver + profile URI + profile
+ (DriverUriProfile{"empty_profile_profile", "", "profile://prod-sqlite",
"profile"})
+ .WithExpectedError(ADBC_STATUS_INVALID_ARGUMENT,
kMultipleProfilesError),
+
+ // no driver + URI (not profile or driver) + no profile
+ // ------------------------------------------------------------
+ // there's no driver, so the URI gets treated as the driver (as
+ // otherwise you'd blow up anyways)
+ (DriverUriProfile{"empty_uri_empty", "", "DSN={yes}", ""})
+ .WithExpected("DSN={yes}", "DSN={yes}"),
+
+ // no driver + URI (not profile or driver) + profile
+ // ------------------------------------------------------------
+ (DriverUriProfile{"empty_uri_profile1", "", "DSN={yes}", "prod-driver"})
+ .WithExpected("adbc_driver_sqlite", "DSN={yes}"),
+ (DriverUriProfile{"empty_uri_profile2", "", "DSN={yes}",
"prod-driver-uri"})
+ .WithExpected("adbc_driver_sqlite", "DSN={yes}"),
+ (DriverUriProfile{"empty_uri_profile3", "", "DSN={yes}", "prod-uri"})
+ .WithExpected("DSN={yes}", "DSN={yes}"),
+
+ // driver + no URI + no profile
+ // ------------------------------------------------------------
+ (DriverUriProfile{"driver_empty_empty", "sqlite", "",
""}).WithExpected("sqlite", ""),
+
+ // driver + no URI + profile
+ // ------------------------------------------------------------
+ (DriverUriProfile{"driver_empty_profile1", "sqlite", "", "prod-sqlite"})
+ .WithExpected("sqlite", "file://:memory:"),
+ (DriverUriProfile{"driver_empty_profile2", "sqlite", "",
"prod-driver-uri"})
+ .WithExpectedError(
+ ADBC_STATUS_INVALID_ARGUMENT,
+ "Profile `prod-driver-uri` specifies driver `adbc_driver_sqlite`
which does "
+ "not match requested driver `sqlite`"),
+
+ // driver + URI + no profile
+ // ------------------------------------------------------------
+ (DriverUriProfile{"driver_driveruri_empty", "sqlite", "postgres://bar",
""})
+ .WithExpected("sqlite", "postgres://bar"),
+
+ // driver + URI + profile
+ // ------------------------------------------------------------
+ (DriverUriProfile{"driver_driveruri_profile1", "sqlite", "postgres://bar",
+ "prod-sqlite"})
+ .WithExpected("sqlite", "postgres://bar"),
+ (DriverUriProfile{"driver_driveruri_profile2", "adbc_driver_sqlite",
"postgres://bar",
+ "prod-driver-uri"})
+ .WithExpected("adbc_driver_sqlite", "postgres://bar"),
+
+ // driver + profile URI + no profile
+ // ------------------------------------------------------------
+ (DriverUriProfile{"driver_profile_empty1", "sqlite",
"profile://prod-sqlite", ""})
+ .WithExpected("sqlite", "file://:memory:"),
+ (DriverUriProfile{"driver_profile_empty2", "postgres",
"profile://prod-sqlite", ""})
+ .WithExpectedError(ADBC_STATUS_INVALID_ARGUMENT,
+ "Profile `prod-sqlite` specifies driver `sqlite`
which does "
+ "not match requested driver `postgres`"),
+
+ // driver + profile URI + profile
+ // ------------------------------------------------------------
+ (DriverUriProfile{"driver_profile_profile", "sqlite",
"profile://prod-sqlite",
+ "prod-sqlite"})
+ .WithExpectedError(ADBC_STATUS_INVALID_ARGUMENT,
kMultipleProfilesError),
+
+ // driver + URI (not profile or driver) + no profile
+ // ------------------------------------------------------------
+ (DriverUriProfile{"driver_uri_empty", "sqlite", "DSN={yes}", ""})
+ .WithExpected("sqlite", "DSN={yes}"),
+
+ // driver + URI (not profile or driver) + profile
+ // ------------------------------------------------------------
+ (DriverUriProfile{"driver_uri_profile1", "sqlite", "DSN={yes}",
"prod-sqlite"})
+ .WithExpected("sqlite", "DSN={yes}"),
+ (DriverUriProfile{"driver_uri_profile2", "sqlite", "DSN={yes}",
"prod-driver"})
+ .WithExpectedError(ADBC_STATUS_INVALID_ARGUMENT,
+ "Profile `prod-driver` specifies driver
`adbc_driver_sqlite` "
+ "which does not match requested driver `sqlite`"),
+
+ // URI in driver + no URI + no profile
+ // ------------------------------------------------------------
+ (DriverUriProfile{"driveruri_empty_empty", "sqlite://foo", "", ""})
+ .WithExpected("sqlite", "sqlite://foo"),
+
+ // URI in driver + no URI + profile
+ // ------------------------------------------------------------
+ // this fails because we validate the profile's driver against
+ // the requested driver immediately, without doing further
+ // parsing (they would resolve to the same thing if we did
+ // parse the URIs but that's additional edge cases)
+ (DriverUriProfile{"driveruri_empty_profile1", "sqlite://foo", "",
"prod-sqlite"})
+ .WithExpectedError(ADBC_STATUS_INVALID_ARGUMENT,
+ "Profile `prod-sqlite` specifies driver `sqlite`
which does "
+ "not match requested driver `sqlite://foo`"),
+ (DriverUriProfile{"driveruri_empty_profile2", "sqlite://foo", "",
"prod-uri"})
+ .WithExpected("sqlite", "sqlite://foo"),
+ (DriverUriProfile{"driveruri_empty_profile3", "sqlite://foo", "",
"prod-driver-uri"})
+ .WithExpectedError(
+ ADBC_STATUS_INVALID_ARGUMENT,
+ "Profile `prod-driver-uri` specifies driver `adbc_driver_sqlite`
which does "
+ "not match requested driver `sqlite://foo`"),
+
+ // URI in driver + URI + no profile
+ // ------------------------------------------------------------
+ // TODO: I think URI should take precedence here
+ (DriverUriProfile{"driveruri_driver_empty", "sqlite://foo",
"sqlite://bar", ""})
+ .WithExpected("sqlite", "sqlite://foo"),
+
+ // URI in driver + URI + profile
+ // ------------------------------------------------------------
+ (DriverUriProfile{"driveruri_driver_profile1", "sqlite://foo",
"sqlite://bar",
+ "prod-sqlite"})
+ .WithExpectedError(ADBC_STATUS_INVALID_ARGUMENT,
+ "Profile `prod-sqlite` specifies driver `sqlite`
which does "
+ "not match requested driver `sqlite://foo`"),
+
+ // URI in driver + profile URI + no profile
+ // ------------------------------------------------------------
+ (DriverUriProfile{"driveruri_profile_empty", "sqlite://foo",
"profile://prod-sqlite",
+ ""})
+ .WithExpectedError(ADBC_STATUS_INVALID_ARGUMENT,
+ "Profile `prod-sqlite` specifies driver `sqlite`
which does "
+ "not match requested driver `sqlite://foo`"),
+
+ // URI in driver + profile URI + profile
+ // ------------------------------------------------------------
+ (DriverUriProfile{"driveruri_profile_profile1", "sqlite://foo",
+ "profile://prod-sqlite", "prod-sqlite"})
+ .WithExpectedError(ADBC_STATUS_INVALID_ARGUMENT,
kMultipleProfilesError),
+
+ // URI in driver + URI (not profile or driver) + no profile
+ // ------------------------------------------------------------
+ // TODO: I think URI should take precedence here
+ (DriverUriProfile{"driveruri_uri_empty", "sqlite://foo", "DSN={yes}", ""})
+ .WithExpected("sqlite", "sqlite://foo"),
+
+ // URI in driver + URI (not profile or driver) + profile
+ // ------------------------------------------------------------
+ (DriverUriProfile{"driveruri_uri_profile1", "sqlite://foo", "DSN={yes}",
+ "prod-sqlite"})
+ .WithExpectedError(ADBC_STATUS_INVALID_ARGUMENT,
+ "Profile `prod-sqlite` specifies driver `sqlite`
which does "
+ "not match requested driver `sqlite://foo`"),
+
+ // profile URI in driver + no URI + no profile
+ // ------------------------------------------------------------
+ (DriverUriProfile{"profile_empty_empty", "profile://prod-sqlite", "", ""})
+ .WithExpected("sqlite", "file://:memory:"),
+
+ // profile URI in driver + no URI + profile
+ // ------------------------------------------------------------
+ (DriverUriProfile{"profile_empty_profile", "profile://prod-sqlite", "",
+ "prod-sqlite"})
+ .WithExpectedError(ADBC_STATUS_INVALID_ARGUMENT,
kMultipleProfilesError),
+
+ // profile URI in driver + driver URI + no profile
+ // ------------------------------------------------------------
+ (DriverUriProfile{"profile_driveruri_empty", "profile://prod-sqlite",
"sqlite://foo",
+ ""})
+ .WithExpected("sqlite", "sqlite://foo"),
+
+ // profile URI in driver + driver URI + profile
+ // ------------------------------------------------------------
+ (DriverUriProfile{"profile_driveruri_profile", "profile://prod-sqlite",
+ "sqlite://foo", "prod-sqlite"})
+ .WithExpectedError(ADBC_STATUS_INVALID_ARGUMENT,
kMultipleProfilesError),
+
+ // profile URI in driver + profile URI + no profile
+ // ------------------------------------------------------------
+ (DriverUriProfile{"profile_profile_empty", "profile://prod-sqlite",
+ "profile://prod-sqlite", ""})
+ .WithExpectedError(ADBC_STATUS_INVALID_ARGUMENT,
kMultipleProfilesError),
+
+ // profile URI in driver + profile URI + profile
+ // ------------------------------------------------------------
+ (DriverUriProfile{"profile_profile_profile", "profile://prod-sqlite",
+ "profile://prod-sqlite", "prod-sqlite"})
+ .WithExpectedError(ADBC_STATUS_INVALID_ARGUMENT,
kMultipleProfilesError),
+
+ // profile URI in driver + URI (not profile or driver) + no profile
+ // ------------------------------------------------------------
+ (DriverUriProfile{"profile_uri_empty", "profile://prod-sqlite",
"DSN={yes}", ""})
+ .WithExpected("sqlite", "DSN={yes}"),
+
+ // profile URI in driver + URI (not profile or driver) + profile
+ // ------------------------------------------------------------
+ (DriverUriProfile{"profile_uri_profile", "profile://prod-sqlite",
"DSN={yes}",
+ "prod-sqlite"})
+ .WithExpectedError(ADBC_STATUS_INVALID_ARGUMENT,
kMultipleProfilesError),
+};
+
+INSTANTIATE_TEST_SUITE_P(
+ ValidateOptions, DriverUriProfileTest, testing::ValuesIn(kOptionsValues),
+ [](const testing::TestParamInfo<DriverUriProfileTest::ParamType>& info) {
+ return info.param.name;
+ });
+
} // namespace adbc
diff --git a/go/adbc/drivermgr/adbc_driver_manager.cc
b/go/adbc/drivermgr/adbc_driver_manager.cc
index 5bc76c40c..d55a0d224 100644
--- a/go/adbc/drivermgr/adbc_driver_manager.cc
+++ b/go/adbc/drivermgr/adbc_driver_manager.cc
@@ -416,16 +416,6 @@ std::optional<ParseDriverUriResult>
InternalAdbcParseDriverUri(std::string_view
return ParseDriverUriResult{d, str.substr(pos + 1), std::nullopt};
}
-// Direct implementations of API methods
-
-int AdbcErrorGetDetailCount(const struct AdbcError* error) {
- if (error->vendor_code == ADBC_ERROR_VENDOR_CODE_PRIVATE_DATA &&
error->private_data &&
- error->private_driver && error->private_driver->ErrorGetDetailCount) {
- return error->private_driver->ErrorGetDetailCount(error);
- }
- return 0;
-}
-
struct ProfileGuard {
AdbcConnectionProfile profile;
explicit ProfileGuard() : profile{} {}
@@ -436,6 +426,107 @@ struct ProfileGuard {
}
};
+ADBC_EXPORT
+AdbcStatusCode InternalAdbcParseOptions(TempDatabase* db, struct AdbcError*
error) {
+ // https://github.com/apache/arrow-adbc/issues/4085
+
+ // init_func always takes precedence
+ if (db->init_func) return ADBC_STATUS_OK;
+
+ std::optional<ParseDriverUriResult> parsed_driver;
+ std::optional<ParseDriverUriResult> parsed_uri;
+
+ if (!db->driver.empty()) {
+ parsed_driver = InternalAdbcParseDriverUri(db->driver);
+ }
+ if (auto it = db->options.find("uri"); it != db->options.end()) {
+ parsed_uri = InternalAdbcParseDriverUri(it->second);
+ }
+
+ // do we have `profile`, `profile://` URI, or `profile://` URI in driver?
+ {
+ const bool have_profile = db->options.find("profile") != db->options.end();
+ const bool have_profile_driver = parsed_driver &&
parsed_driver->profile.has_value();
+ const bool have_profile_uri = parsed_uri &&
parsed_uri->profile.has_value();
+
+ if (static_cast<int>(have_profile) + static_cast<int>(have_profile_uri) +
+ static_cast<int>(have_profile_driver) >
+ 1) {
+ SetError(error,
+ "Multiple profiles specified; only one of `profile` option,
`profile://` "
+ "URI, or `profile://` driver is allowed");
+ return ADBC_STATUS_INVALID_ARGUMENT;
+ }
+
+ // apply profile (may modify options)
+ std::string profile;
+ if (have_profile) {
+ profile = db->options["profile"];
+ db->options.erase("profile");
+ } else if (have_profile_uri) {
+ profile = *parsed_uri->profile;
+ db->options.erase("uri");
+ } else if (have_profile_driver) {
+ profile = *parsed_driver->profile;
+ db->driver.clear();
+ }
+
+ if (!profile.empty()) {
+ auto status = InternalInitializeProfile(db, profile, error);
+ if (status != ADBC_STATUS_OK) {
+ return status;
+ }
+ }
+ }
+
+ // parse driver for URI. we do this even if it was set via profile
+ {
+ // Reparse because profile may have modified driver
+ std::string owned_driver = db->driver;
+ if (auto maybe_driver = InternalAdbcParseDriverUri(owned_driver);
+ maybe_driver.has_value()) {
+ auto parsed = *maybe_driver;
+ // Don't allow recursive profiles (that is the only way we can reach
here)
+ if (parsed.profile.has_value()) {
+ SetError(error, "Profile cannot specify a profile:// URI");
+ return ADBC_STATUS_INVALID_ARGUMENT;
+ }
+ if (parsed.uri.has_value()) {
+ db->driver = std::string{parsed.driver};
+ // maybe only do this if URI is unset?
+ db->options["uri"] = std::string{*parsed.uri};
+ }
+ }
+ }
+
+ // no driver? parse URI
+ if (auto it = db->options.find("uri"); db->driver.empty() && it !=
db->options.end()) {
+ // Reparse because profile may have modified uri
+ std::string owned_uri = it->second;
+ if (auto maybe_uri = InternalAdbcParseDriverUri(it->second);
maybe_uri.has_value()) {
+ auto parsed = *maybe_uri;
+ // Don't allow recursive profiles (that is the only way we can reach
here)
+ if (parsed.profile.has_value()) {
+ SetError(error, "Profile cannot specify a profile:// URI");
+ return ADBC_STATUS_INVALID_ARGUMENT;
+ }
+ if (parsed.uri.has_value()) {
+ db->driver = std::string{parsed.driver};
+ db->options["uri"] = std::string{*parsed.uri};
+ }
+ } else if (db->driver.empty()) {
+ db->driver = std::move(owned_uri);
+ }
+ }
+
+ // still no driver? bail
+ if (db->driver.empty()) {
+ SetError(error, "Must set 'driver' option before AdbcDatabaseInit");
+ return ADBC_STATUS_INVALID_ARGUMENT;
+ }
+ return ADBC_STATUS_OK;
+}
+
AdbcStatusCode InternalInitializeProfile(TempDatabase* args,
const std::string_view profile,
struct AdbcError* error) {
@@ -451,7 +542,17 @@ AdbcStatusCode InternalInitializeProfile(TempDatabase*
args,
AdbcDriverInitFunc init_func = nullptr;
CHECK_STATUS(
guard.profile.GetDriverName(&guard.profile, &driver_name, &init_func,
error));
- if (driver_name != nullptr && strlen(driver_name) > 0) {
+ if (driver_name != nullptr && std::strlen(driver_name) > 0) {
+ // ensure the parsed driver matches the profile driver if both are
specified
+ if (!args->driver.empty() && args->driver != driver_name) {
+ std::string message = "Profile `";
+ message += profile;
+ message += "` specifies driver `";
+ message += driver_name;
+ message += "` which does not match requested driver `" + args->driver +
"`";
+ SetError(error, message);
+ return ADBC_STATUS_INVALID_ARGUMENT;
+ }
args->driver = driver_name;
}
diff --git a/go/adbc/drivermgr/adbc_driver_manager_api.cc
b/go/adbc/drivermgr/adbc_driver_manager_api.cc
index fb8d543ce..7f49aa65b 100644
--- a/go/adbc/drivermgr/adbc_driver_manager_api.cc
+++ b/go/adbc/drivermgr/adbc_driver_manager_api.cc
@@ -108,6 +108,14 @@ static void ErrorArrayStreamInit(struct ArrowArrayStream*
out,
// ADBC Error API implementations
//
=============================================================================
+int AdbcErrorGetDetailCount(const struct AdbcError* error) {
+ if (error->vendor_code == ADBC_ERROR_VENDOR_CODE_PRIVATE_DATA &&
error->private_data &&
+ error->private_driver && error->private_driver->ErrorGetDetailCount) {
+ return error->private_driver->ErrorGetDetailCount(error);
+ }
+ return 0;
+}
+
struct AdbcErrorDetail AdbcErrorGetDetail(const struct AdbcError* error, int
index) {
if (error->vendor_code == ADBC_ERROR_VENDOR_CODE_PRIVATE_DATA &&
error->private_data &&
error->private_driver && error->private_driver->ErrorGetDetail) {
@@ -639,55 +647,9 @@ AdbcStatusCode AdbcDatabaseInit(struct AdbcDatabase*
database, struct AdbcError*
return ADBC_STATUS_INVALID_STATE;
}
TempDatabase* args = reinterpret_cast<TempDatabase*>(database->private_data);
- const auto profile_in_use = args->options.find("profile");
- if (profile_in_use != args->options.end()) {
- std::string_view profile = profile_in_use->second;
- auto status = InternalInitializeProfile(args, profile, error);
- if (status != ADBC_STATUS_OK) {
- return status;
- }
- args->options.erase("profile");
- }
-
- if (!args->init_func) {
- const auto uri = args->options.find("uri");
- if (args->driver.empty() && uri != args->options.end()) {
- std::string owned_uri = uri->second;
- auto result = InternalAdbcParseDriverUri(owned_uri);
- if (result) {
- if (result->uri) {
- args->driver = std::string{result->driver};
- args->options["uri"] = std::string{*result->uri};
- } else if (result->profile) {
- args->options.erase("uri");
- auto status = InternalInitializeProfile(args, *result->profile,
error);
- if (status != ADBC_STATUS_OK) {
- return status;
- }
- }
- } else if (args->driver.empty()) {
- args->driver = owned_uri;
- }
- } else if (!args->driver.empty() && uri == args->options.end()) {
- std::string owned_driver = args->driver;
- auto result = InternalAdbcParseDriverUri(owned_driver);
- if (result) {
- args->driver = std::string{result->driver};
- if (result->uri) {
- args->options["uri"] = std::string{*result->uri};
- } else if (result->profile) {
- auto status = InternalInitializeProfile(args, *result->profile,
error);
- if (status != ADBC_STATUS_OK) {
- return status;
- }
- }
- }
- }
- if (args->driver.empty()) {
- SetError(error, "Must set 'driver' option before AdbcDatabaseInit");
- return ADBC_STATUS_INVALID_ARGUMENT;
- }
+ if (auto status = InternalAdbcParseOptions(args, error); status !=
ADBC_STATUS_OK) {
+ return status;
}
// Allocate the underlying driver
diff --git a/go/adbc/drivermgr/adbc_driver_manager_internal.h
b/go/adbc/drivermgr/adbc_driver_manager_internal.h
index dd0aef128..089e1b405 100644
--- a/go/adbc/drivermgr/adbc_driver_manager_internal.h
+++ b/go/adbc/drivermgr/adbc_driver_manager_internal.h
@@ -207,6 +207,12 @@ struct TempDatabase {
AdbcConnectionProfileProvider profile_provider = nullptr;
};
+// Parse and validate options like uri/profile, with the result being that
+// `driver` or `init_func` are populated and options like `profile` are
+// removed
+ADBC_EXPORT
+AdbcStatusCode InternalAdbcParseOptions(TempDatabase* db, struct AdbcError*
error);
+
/// Temporary state while the connection is being configured.
struct TempConnection {
std::unordered_map<std::string, std::string> options;
diff --git a/python/adbc_driver_manager/tests/test_profile.py
b/python/adbc_driver_manager/tests/test_profile.py
index f8eda8027..008e06dc4 100644
--- a/python/adbc_driver_manager/tests/test_profile.py
+++ b/python/adbc_driver_manager/tests/test_profile.py
@@ -57,6 +57,17 @@ def test_profile_option(sqlitedev) -> None:
assert cursor.fetchone() is not None
+def test_profile_redundant(sqlitedev) -> None:
+ # Test that proving the profile twice is rejected
+ with pytest.raises(dbapi.ProgrammingError, match="Multiple profiles
specified"):
+ with dbapi.connect(uri=f"profile://{sqlitedev}", profile=sqlitedev):
+ pass
+
+ with pytest.raises(dbapi.ProgrammingError, match="Multiple profiles
specified"):
+ with dbapi.connect(driver=f"profile://{sqlitedev}", profile=sqlitedev):
+ pass
+
+
def test_option_env_var(subtests, tmp_path, monkeypatch) -> None:
# Test a profile that uses env var substitution for option values
monkeypatch.setenv("ADBC_PROFILE_PATH", str(tmp_path))
@@ -230,12 +241,11 @@ profile_version = 1
with dbapi.connect("profile://nodriver"):
pass
- # TODO(https://github.com/apache/arrow-adbc/issues/4085): do we want to
allow this?
- # with subtests.test(msg="uri"):
- # with dbapi.connect("adbc_driver_sqlite", uri="profile://nodriver")
as conn:
- # with conn.cursor() as cursor:
- # cursor.execute("SELECT sqlite_version()")
- # assert cursor.fetchone() is not None
+ with subtests.test(msg="uri"):
+ with dbapi.connect("adbc_driver_sqlite", uri="profile://nodriver") as
conn:
+ with conn.cursor() as cursor:
+ cursor.execute("SELECT sqlite_version()")
+ assert cursor.fetchone() is not None
with subtests.test(msg="profile"):
with dbapi.connect("adbc_driver_sqlite", profile="nodriver") as conn:
@@ -244,23 +254,47 @@ profile_version = 1
assert cursor.fetchone() is not None
-# TODO(https://github.com/apache/arrow-adbc/issues/4085): do we want to allow
this?
+def test_driver_override(subtests, tmp_path, monkeypatch) -> None:
+ # Test that the driver cannot be overridden by an option
+ monkeypatch.setenv("ADBC_PROFILE_PATH", str(tmp_path))
+
+ with (tmp_path / "foo.toml").open("w") as sink:
+ sink.write("""
+profile_version = 1
+driver = "adbc_driver_sqlite"
+[Options]
+""")
+
+ monkeypatch.setenv("ADBC_PROFILE_PATH", str(tmp_path))
+ with subtests.test(msg="with redundant (URI)"):
+ with dbapi.connect("adbc_driver_sqlite", "profile://foo") as conn:
+ with conn.cursor() as cursor:
+ cursor.execute("SELECT sqlite_version()")
+ assert cursor.fetchone() is not None
+
+ with subtests.test(msg="with redundant (profile)"):
+ with dbapi.connect("adbc_driver_sqlite", profile="foo") as conn:
+ with conn.cursor() as cursor:
+ cursor.execute("SELECT sqlite_version()")
+ assert cursor.fetchone() is not None
-# @pytest.mark.xfail
-# def test_driver_override(subtests, tmp_path, monkeypatch) -> None:
-# # Test that the driver can be overridden by an option
-# monkeypatch.setenv("ADBC_PROFILE_PATH", str(tmp_path))
-# with subtests.test(msg="with override (URI)"):
-# with dbapi.connect("adbc_driver_sqlite", "profile://nonexistent") as
conn:
-# with conn.cursor() as cursor:
-# cursor.execute("SELECT sqlite_version()")
-# assert cursor.fetchone() is not None
+ with subtests.test(msg="with override (URI)"):
+ with pytest.raises(
+ dbapi.ProgrammingError,
+ match="Profile `foo` specifies driver `adbc_driver_sqlite`"
+ " which does not match requested driver
`adbc_driver_somethingelse`",
+ ):
+ with dbapi.connect("adbc_driver_somethingelse", "profile://foo"):
+ pass
-# with subtests.test(msg="with override (profile)"):
-# with dbapi.connect("adbc_driver_sqlite", profile="nonexistent") as
conn:
-# with conn.cursor() as cursor:
-# cursor.execute("SELECT sqlite_version()")
-# assert cursor.fetchone() is not None
+ with subtests.test(msg="with override (profile)"):
+ with pytest.raises(
+ dbapi.ProgrammingError,
+ match="Profile `foo` specifies driver `adbc_driver_sqlite`"
+ " which does not match requested driver
`adbc_driver_somethingelse`",
+ ):
+ with dbapi.connect("adbc_driver_somethingelse", profile="foo"):
+ pass
def test_driver_invalid(subtests, tmp_path, monkeypatch) -> None: