lidavidm commented on code in PR #3876:
URL: https://github.com/apache/arrow-adbc/pull/3876#discussion_r2791122120
##########
c/driver_manager/adbc_driver_manager.cc:
##########
@@ -1600,22 +1951,26 @@ std::optional<ParseDriverUriResult>
InternalAdbcParseDriverUri(std::string_view
std::string_view d = str.substr(0, pos);
if (str.size() <= pos + 1) {
- return ParseDriverUriResult{d, std::nullopt};
+ return ParseDriverUriResult{d, std::nullopt, std::nullopt};
}
#ifdef _WIN32
if (std::filesystem::exists(std::filesystem::path(str))) {
// No scheme, just a path
- return ParseDriverUriResult{str, std::nullopt};
+ return ParseDriverUriResult{str, std::nullopt, std::nullopt};
}
#endif
if (str[pos + 1] == '/') { // scheme is also driver
- return ParseDriverUriResult{d, str};
+ if (d == "profile" && str.size() > pos + 2) {
+ // found a profile URI "profile://"
+ return ParseDriverUriResult{"", std::nullopt, str.substr(pos + 3)};
Review Comment:
not necessarily something we have to do in this PR, but I think this should
become a std::variant (driver+URI or profile)
##########
c/include/arrow-adbc/adbc_driver_manager.h:
##########
@@ -175,6 +175,183 @@ AdbcStatusCode
AdbcDriverManagerDatabaseSetAdditionalSearchPathList(
ADBC_EXPORT
const char* AdbcStatusCodeMessage(AdbcStatusCode code);
+/// \defgroup adbc-driver-manager-connection-profile Connection Profiles
+/// Similar to odbc.ini, the ADBC driver manager can support "connection
profiles"
Review Comment:
This should also be updated? (If anything I'd like to maybe not mention ODBC
at all in the actual header?)
##########
docs/source/format/connection_profiles.rst:
##########
@@ -0,0 +1,568 @@
+.. Licensed to the Apache Software Foundation (ASF) under one
+.. or more contributor license agreements. See the NOTICE file
+.. distributed with this work for additional information
+.. regarding copyright ownership. The ASF licenses this file
+.. to you under the Apache License, Version 2.0 (the
+.. "License"); you may not use this file except in compliance
+.. with the License. You may obtain a copy of the License at
+..
+.. http://www.apache.org/licenses/LICENSE-2.0
+..
+.. Unless required by applicable law or agreed to in writing,
+.. software distributed under the License is distributed on an
+.. "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+.. KIND, either express or implied. See the License for the
+.. specific language governing permissions and limitations
+.. under the License.
+
+==================================
+Driver Manager Connection Profiles
+==================================
+
+Overview
+========
+
+Similar to ODBC's ``odbc.ini``, the ADBC driver manager supports **connection
profiles**
+that specify a driver and connection options in a reusable configuration. This
allows users to:
+
+- Define connection information in files or environment variables
+- Share connection configurations across applications
+- Distribute standardized connection settings
+- Avoid hardcoding driver names and credentials in application code
+
+Profiles are loaded during ``AdbcDatabaseInit()`` before initializing the
driver. Options
+from the profile are applied automatically but do not override options already
set via ``AdbcDatabaseSetOption()``.
+
+Quick Start
+===========
+
+Using a Profile via URI
+-----------------------
+
+The simplest way to use a profile is through a URI:
+
+.. code-block:: c
+
+ AdbcDatabase database;
+ AdbcDatabaseNew(&database, &error);
+ AdbcDatabaseSetOption(&database, "uri", "profile://my_snowflake_prod",
&error);
+ AdbcDatabaseInit(&database, &error);
+
+Using a Profile via Option
+---------------------------
+
+Alternatively, specify the profile name directly:
+
+.. code-block:: c
+
+ AdbcDatabase database;
+ AdbcDatabaseNew(&database, &error);
+ AdbcDatabaseSetOption(&database, "profile", "my_snowflake_prod", &error);
+ AdbcDatabaseInit(&database, &error);
+
+Profile File Format
+===================
+
+Filesystem-based profiles use TOML format with the following structure:
+
+.. code-block:: toml
+
+ version = 1
+ driver = "snowflake"
+
+ [options]
+ # String options
+ adbc.snowflake.sql.account = "mycompany"
+ adbc.snowflake.sql.warehouse = "COMPUTE_WH"
+ adbc.snowflake.sql.database = "PRODUCTION"
+ adbc.snowflake.sql.schema = "PUBLIC"
+
+ # Integer options
+ adbc.snowflake.sql.client_session_keep_alive_heartbeat_frequency = 3600
+
+ # Double options
+ adbc.snowflake.sql.client_timeout = 30.5
+
+ # Boolean options (converted to "true" or "false" strings)
+ adbc.snowflake.sql.client_session_keep_alive = true
+
+version
+-------
+
+- **Required**: Yes
+- **Type**: Integer
+- **Supported values**: ``1``
+
+The ``version`` field specifies the profile format version. Currently, only
version 1 is supported.
+This will enable future changes while maintaining backward compatibility.
+
+driver
+------
+
+- **Required**: No
+- **Type**: String
+
+The ``driver`` field specifies which ADBC driver to load. This can be:
+
+- A driver name (e.g., ``"snowflake"``)
+- A path to a shared library (e.g.,
``"/usr/local/lib/libadbc_driver_snowflake.so"``)
+- A path to a driver manifest (e.g., ``"/etc/adbc/drivers/snowflake.toml"``)
+
+If omitted, the driver must be specified through other means (e.g., the
``driver`` option or ``uri`` parameter).
+The driver will be loaded identically to if it was specified via
``AdbcDatabaseSetOption("driver", "<driver>")``.
+For more detils, see :doc:`driver_manifests`.
+
+Options Section
+---------------
+
+The ``[options]`` section contains driver-specific configuration options.
Options can be of the following types:
+
+**String values**
+ Applied using ``AdbcDatabaseSetOption()``
+
+ .. code-block:: toml
+
+ adbc.snowflake.sql.account = "mycompany"
+ adbc.snowflake.sql.warehouse = "COMPUTE_WH"
+
+**Integer values**
+ Applied using ``AdbcDatabaseSetOptionInt()``
+
+ .. code-block:: toml
+
+ adbc.snowflake.sql.client_session_keep_alive_heartbeat_frequency = 3600
+
+**Double values**
+ Applied using ``AdbcDatabaseSetOptionDouble()``
+
+ .. code-block:: toml
+
+ adbc.snowflake.sql.client_timeout = 30.5
+
+**Boolean values**
+ Converted to strings ``"true"`` or ``"false"`` and applied using
``AdbcDatabaseSetOption()``
+
+ .. code-block:: toml
+
+ adbc.snowflake.sql.client_session_keep_alive = true
+
+Environment Variable Substitution
+----------------------------------
+
+Profile values can reference environment variables using the ``env_var()``
syntax:
+
+.. code-block:: toml
+
+ version = 1
+ driver = "adbc_driver_snowflake"
+
+ [options]
+ adbc.snowflake.sql.account = "env_var(SNOWFLAKE_ACCOUNT)"
Review Comment:
Additionally, I feel like most string substitution syntaxes put the
delimiters on the outside (`{foo(bar)}`) which makes it more obvious (and
possibly easier to parse?)
##########
c/driver_manager/adbc_driver_manager_test.cc:
##########
@@ -43,6 +43,7 @@ std::filesystem::path InternalAdbcUserConfigDir();
struct ParseDriverUriResult {
Review Comment:
At some point we're going to need to figure out how better to expose
internal functions for testing, e.g. I feel like env var substitution should be
something that we unit-test directly, and having to redeclare everything is not
a great way to do tests. (Also possibly break up the file, since it's getting
big, and have some way to assemble a single source file for distribution,
possibly one that we check in for convenience.)
##########
c/include/arrow-adbc/adbc_driver_manager.h:
##########
@@ -175,6 +175,171 @@ AdbcStatusCode
AdbcDriverManagerDatabaseSetAdditionalSearchPathList(
ADBC_EXPORT
const char* AdbcStatusCodeMessage(AdbcStatusCode code);
+/// \defgroup adbc-driver-manager-connection-profile Connection Profiles
+/// Similar to odbc.ini, the ADBC driver manager can support "connection
profiles"
+/// that specify a driver and options to use when connecting. This allows
users to
+/// specify connection information in a file or environment variable, and have
the
+/// driver manager load the appropriate driver and set options accordingly.
+///
+/// This allows creating reusable connection configurations for sharing and
distribution
+/// without needing to hardcode driver names and options in application code.
Profiles
+/// will be loaded during DatabaseInit before attempting to initialize the
driver. Any
+/// options specified by the profile will be applied but will not override
options
+/// that have already been set using DatabaseSetOption.
+///
+/// To facilitate customization, we define an interface for implementing a
Connection
+/// Profile object along with a provider function definition which can be set
into
+/// the driver manager to allow for customized profile loading.
+///
+/// A profile can be specified to the Driver Manager in one of two ways,
+/// which will invoke the profile provider during the call to DatabaseInit:
+///
+/// 1. The "profile" option can be set using DatabaseSetOption with the name
of the
+/// profile to load.
+/// 2. The "uri" being used can have the form "profile://<profile>"
+///
+/// @{
+
+/// \brief Abstract interface for connection profile providers
+struct ADBC_EXPORT AdbcConnectionProfile {
+ /// \brief Opaque implementation-defined state.
+ /// This field is NULL if the profile is uninitialized/freed (but
+ /// it need not have a value even if the profile is initialized).
+ void* private_data;
+
+ /// \brief Release the profile and perform any cleanup.
+ void (*release)(struct AdbcConnectionProfile* profile);
+
+ /// \brief Get the driver to use as specified by this profile.
+ ///
+ /// It is not required that a profile specify a driver. If the options
+ // can be reusable across drivers, then the profile does not need to specify
+ /// a driver (if this provides an empty string or nullptr then the driver
+ /// must be defined by other means, e.g. by the driver / uri options).
+ ///
+ /// \param[in] profile The profile to query.
+ /// \param[out] driver_name The name of the driver to use, or NULL if not
specified.
+ /// \param[out] error An optional location to return an error message
+ AdbcStatusCode (*GetDriverName)(struct AdbcConnectionProfile* profile,
+ const char** driver_name, struct AdbcError*
error);
Review Comment:
If we're allowing the profile to control everything else, why not let it
also control the init_func? I could imagine distributing a profile
implementation that bundles or loads drivers itself and doesn't want to let the
driver manager do the search. We already let profiles omit the driver/specify
it implicitly, so even if the bundled implementation won't ever use init_func,
we may as well allow the option for other implementations of profiles.
Conversely...if telling the user to just `init_func` is sufficient, then why
doesn't that also apply to profiles themselves?
##########
docs/source/format/connection_profiles.rst:
##########
@@ -0,0 +1,568 @@
+.. Licensed to the Apache Software Foundation (ASF) under one
+.. or more contributor license agreements. See the NOTICE file
+.. distributed with this work for additional information
+.. regarding copyright ownership. The ASF licenses this file
+.. to you under the Apache License, Version 2.0 (the
+.. "License"); you may not use this file except in compliance
+.. with the License. You may obtain a copy of the License at
+..
+.. http://www.apache.org/licenses/LICENSE-2.0
+..
+.. Unless required by applicable law or agreed to in writing,
+.. software distributed under the License is distributed on an
+.. "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+.. KIND, either express or implied. See the License for the
+.. specific language governing permissions and limitations
+.. under the License.
+
+==================================
+Driver Manager Connection Profiles
+==================================
+
+Overview
+========
+
+Similar to ODBC's ``odbc.ini``, the ADBC driver manager supports **connection
profiles**
+that specify a driver and connection options in a reusable configuration. This
allows users to:
+
+- Define connection information in files or environment variables
+- Share connection configurations across applications
+- Distribute standardized connection settings
+- Avoid hardcoding driver names and credentials in application code
+
+Profiles are loaded during ``AdbcDatabaseInit()`` before initializing the
driver. Options
+from the profile are applied automatically but do not override options already
set via ``AdbcDatabaseSetOption()``.
+
+Quick Start
+===========
+
+Using a Profile via URI
+-----------------------
+
+The simplest way to use a profile is through a URI:
+
+.. code-block:: c
+
+ AdbcDatabase database;
+ AdbcDatabaseNew(&database, &error);
+ AdbcDatabaseSetOption(&database, "uri", "profile://my_snowflake_prod",
&error);
+ AdbcDatabaseInit(&database, &error);
+
+Using a Profile via Option
+---------------------------
+
+Alternatively, specify the profile name directly:
+
+.. code-block:: c
+
+ AdbcDatabase database;
+ AdbcDatabaseNew(&database, &error);
+ AdbcDatabaseSetOption(&database, "profile", "my_snowflake_prod", &error);
+ AdbcDatabaseInit(&database, &error);
+
+Profile File Format
+===================
+
+Filesystem-based profiles use TOML format with the following structure:
+
+.. code-block:: toml
+
+ version = 1
+ driver = "snowflake"
+
+ [options]
+ # String options
+ adbc.snowflake.sql.account = "mycompany"
+ adbc.snowflake.sql.warehouse = "COMPUTE_WH"
+ adbc.snowflake.sql.database = "PRODUCTION"
+ adbc.snowflake.sql.schema = "PUBLIC"
+
+ # Integer options
+ adbc.snowflake.sql.client_session_keep_alive_heartbeat_frequency = 3600
+
+ # Double options
+ adbc.snowflake.sql.client_timeout = 30.5
+
+ # Boolean options (converted to "true" or "false" strings)
+ adbc.snowflake.sql.client_session_keep_alive = true
+
+version
+-------
+
+- **Required**: Yes
+- **Type**: Integer
+- **Supported values**: ``1``
+
+The ``version`` field specifies the profile format version. Currently, only
version 1 is supported.
+This will enable future changes while maintaining backward compatibility.
+
+driver
+------
+
+- **Required**: No
+- **Type**: String
+
+The ``driver`` field specifies which ADBC driver to load. This can be:
+
+- A driver name (e.g., ``"snowflake"``)
+- A path to a shared library (e.g.,
``"/usr/local/lib/libadbc_driver_snowflake.so"``)
+- A path to a driver manifest (e.g., ``"/etc/adbc/drivers/snowflake.toml"``)
+
+If omitted, the driver must be specified through other means (e.g., the
``driver`` option or ``uri`` parameter).
+The driver will be loaded identically to if it was specified via
``AdbcDatabaseSetOption("driver", "<driver>")``.
+For more detils, see :doc:`driver_manifests`.
+
+Options Section
+---------------
+
+The ``[options]`` section contains driver-specific configuration options.
Options can be of the following types:
+
+**String values**
+ Applied using ``AdbcDatabaseSetOption()``
+
+ .. code-block:: toml
+
+ adbc.snowflake.sql.account = "mycompany"
+ adbc.snowflake.sql.warehouse = "COMPUTE_WH"
+
+**Integer values**
+ Applied using ``AdbcDatabaseSetOptionInt()``
+
+ .. code-block:: toml
+
+ adbc.snowflake.sql.client_session_keep_alive_heartbeat_frequency = 3600
+
+**Double values**
+ Applied using ``AdbcDatabaseSetOptionDouble()``
+
+ .. code-block:: toml
+
+ adbc.snowflake.sql.client_timeout = 30.5
+
+**Boolean values**
+ Converted to strings ``"true"`` or ``"false"`` and applied using
``AdbcDatabaseSetOption()``
+
+ .. code-block:: toml
+
+ adbc.snowflake.sql.client_session_keep_alive = true
+
+Environment Variable Substitution
+----------------------------------
+
+Profile values can reference environment variables using the ``env_var()``
syntax:
+
+.. code-block:: toml
+
+ version = 1
+ driver = "adbc_driver_snowflake"
+
+ [options]
+ adbc.snowflake.sql.account = "env_var(SNOWFLAKE_ACCOUNT)"
Review Comment:
I think that's an argument for changing the syntax, at least :)
Even if we don't specify how to escape it now, we may have to down the
line...
##########
c/driver_manager/adbc_driver_manager.cc:
##########
@@ -1042,6 +1137,261 @@ struct ManagedLibrary {
#endif // defined(_WIN32)
};
+struct FilesystemProfile {
+ std::filesystem::path path;
+ std::string driver;
+ std::unordered_map<std::string, std::string> options;
+ std::unordered_map<std::string, int64_t> int_options;
+ std::unordered_map<std::string, double> double_options;
+
+ std::vector<const char*> options_keys;
+ std::vector<const char*> options_values;
+
+ std::vector<const char*> int_option_keys;
+ std::vector<int64_t> int_option_values;
+
+ std::vector<const char*> double_option_keys;
+ std::vector<double> double_option_values;
+
+ void PopulateConnectionProfile(struct AdbcConnectionProfile* out) {
+ options_keys.reserve(options.size());
+ options_values.reserve(options.size());
+ for (const auto& [key, value] : options) {
+ options_keys.push_back(key.c_str());
+ options_values.push_back(value.c_str());
+ }
+
+ int_option_keys.reserve(int_options.size());
+ int_option_values.reserve(int_options.size());
+ for (const auto& [key, value] : int_options) {
+ int_option_keys.push_back(key.c_str());
+ int_option_values.push_back(value);
+ }
+
+ double_option_keys.reserve(double_options.size());
+ double_option_values.reserve(double_options.size());
+ for (const auto& [key, value] : double_options) {
+ double_option_keys.push_back(key.c_str());
+ double_option_values.push_back(value);
+ }
+
+ out->private_data = new FilesystemProfile(std::move(*this));
+ out->release = [](AdbcConnectionProfile* profile) {
+ if (!profile || !profile->private_data) {
+ return;
+ }
+
+ delete static_cast<FilesystemProfile*>(profile->private_data);
+ profile->private_data = nullptr;
+ profile->release = nullptr;
+ };
+
+ out->GetDriverName = [](AdbcConnectionProfile* profile, const char** out,
+ struct AdbcError* error) -> AdbcStatusCode {
+ if (!profile || !profile->private_data) {
+ SetError(error, "Invalid connection profile");
+ return ADBC_STATUS_INVALID_ARGUMENT;
+ }
+
+ auto* fs_profile =
static_cast<FilesystemProfile*>(profile->private_data);
+ *out = fs_profile->driver.c_str();
+ return ADBC_STATUS_OK;
+ };
+
+ out->GetOptions = [](AdbcConnectionProfile* profile, const char*** keys,
+ const char*** values, size_t* num_options,
+ struct AdbcError* error) -> AdbcStatusCode {
+ if (!profile || !profile->private_data) {
+ SetError(error, "Invalid connection profile");
+ return ADBC_STATUS_INVALID_ARGUMENT;
+ }
+
+ if (!keys || !values || !num_options) {
+ SetError(error, "Output parameters cannot be null");
+ return ADBC_STATUS_INVALID_ARGUMENT;
+ }
+
+ auto* fs_profile =
static_cast<FilesystemProfile*>(profile->private_data);
+ *num_options = fs_profile->options.size();
+ *keys = fs_profile->options_keys.data();
+ *values = fs_profile->options_values.data();
+ return ADBC_STATUS_OK;
+ };
+
+ out->GetIntOptions = [](AdbcConnectionProfile* profile, const char*** keys,
+ const int64_t** values, size_t* num_options,
+ struct AdbcError* error) -> AdbcStatusCode {
+ if (!profile || !profile->private_data) {
+ SetError(error, "Invalid connection profile");
+ return ADBC_STATUS_INVALID_ARGUMENT;
+ }
+
+ if (!keys || !values || !num_options) {
+ SetError(error, "Output parameters cannot be null");
+ return ADBC_STATUS_INVALID_ARGUMENT;
+ }
+
+ auto* fs_profile =
static_cast<FilesystemProfile*>(profile->private_data);
+ *num_options = fs_profile->int_options.size();
+ *keys = fs_profile->int_option_keys.data();
+ *values = fs_profile->int_option_values.data();
+ return ADBC_STATUS_OK;
+ };
+
+ out->GetDoubleOptions = [](AdbcConnectionProfile* profile, const char***
keys,
+ const double** values, size_t* num_options,
+ struct AdbcError* error) -> AdbcStatusCode {
+ if (!profile || !profile->private_data) {
+ SetError(error, "Invalid connection profile");
+ return ADBC_STATUS_INVALID_ARGUMENT;
+ }
+
+ if (!keys || !values || !num_options) {
+ SetError(error, "Output parameters cannot be null");
+ return ADBC_STATUS_INVALID_ARGUMENT;
+ }
+
+ auto* fs_profile =
static_cast<FilesystemProfile*>(profile->private_data);
+ *num_options = fs_profile->double_options.size();
+ *keys = fs_profile->double_option_keys.data();
+ *values = fs_profile->double_option_values.data();
+ return ADBC_STATUS_OK;
+ };
+ }
+};
+
+struct ProfileVisitor {
+ FilesystemProfile& profile;
+ const std::filesystem::path& profile_path;
+ struct AdbcError* error;
+
+ bool VisitTable(const std::string& prefix, toml::table& table) {
+ for (const auto& [key, value] : table) {
+ if (auto* str = value.as_string()) {
+ profile.options[prefix + key.data()] = str->get();
+ } else if (auto* int_val = value.as_integer()) {
+ profile.int_options[prefix + key.data()] = int_val->get();
+ } else if (auto* double_val = value.as_floating_point()) {
+ profile.double_options[prefix + key.data()] = double_val->get();
+ } else if (auto* bool_val = value.as_boolean()) {
+ profile.options[prefix + key.data()] = bool_val->get() ? "true" :
"false";
+ } else if (value.is_table()) {
+ if (!VisitTable(prefix + key.data() + ".", *value.as_table())) {
+ return false;
+ }
+ } else {
+ std::string message = "Unsupported value type for key '" +
+ std::string(key.str()) + "' in profile '" +
+ profile_path.string() + "'";
+ SetError(error, std::move(message));
+ return false;
+ }
+ }
+ return !error->message;
+ }
+};
+
+AdbcStatusCode LoadProfileFile(const std::filesystem::path& profile_path,
+ FilesystemProfile& profile, struct AdbcError*
error) {
+ toml::table config;
+ try {
+ config = toml::parse_file(profile_path.native());
+ } catch (const toml::parse_error& err) {
+ std::string message = "Could not open profile. ";
+ message += err.what();
+ message += ". Profile: ";
+ message += profile_path.string();
+ SetError(error, std::move(message));
+ return ADBC_STATUS_INVALID_ARGUMENT;
+ }
+
+ profile.path = profile_path;
+ if (!config["version"].is_integer()) {
+ std::string message =
+ "Profile version is not an integer in profile '" +
profile_path.string() + "'";
+ SetError(error, std::move(message));
+ return ADBC_STATUS_INVALID_ARGUMENT;
+ }
+
+ const auto version = config["version"].value_or(int64_t(1));
+ switch (version) {
+ case 1:
+ break;
+ default: {
+ std::string message =
+ "Profile version '" + std::to_string(version) +
+ "' is not supported by this driver manager. Profile: " +
profile_path.string();
Review Comment:
Incidentally I think we should discuss moving to C++20 soon...then we can
use std::format
##########
c/driver_manager/adbc_driver_manager_test.cc:
##########
@@ -1460,4 +1465,418 @@ TEST_F(DriverManifest, ControlCodes) {
}
}
+class ConnectionProfiles : public ::testing::Test {
+ public:
+ void SetUp() override {
+ std::memset(&driver, 0, sizeof(driver));
+ std::memset(&error, 0, sizeof(error));
+
+ temp_dir =
+ std::filesystem::temp_directory_path() /
"adbc_driver_manager_profile_test";
+ std::filesystem::create_directories(temp_dir);
+
+ simple_profile = toml::table{
+ {"version", 1},
+ {"driver", "adbc_driver_sqlite"},
+ {"options",
+ toml::table{
+ {"uri", "file::memory:"},
+ }},
+ };
+ }
+
+ void TearDown() override {
+ if (error.release) {
+ error.release(&error);
+ }
+
+ if (driver.release) {
+ ASSERT_THAT(driver.release(&driver, &error), IsOkStatus(&error));
+ ASSERT_EQ(driver.private_data, nullptr);
+ ASSERT_EQ(driver.private_manager, nullptr);
+ }
+
+ driver_path.clear();
+ if (std::filesystem::exists(temp_dir)) {
+ std::filesystem::remove_all(temp_dir);
+ }
+ }
+
+ 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 = {};
+
+ std::filesystem::path temp_dir;
+ std::filesystem::path driver_path;
+ toml::table simple_profile;
+};
+
+TEST_F(ConnectionProfiles, SetProfileOption) {
+ auto filepath = temp_dir / "profile.toml";
+ toml::table profile = simple_profile;
+ std::ofstream test_manifest_file(filepath);
+ ASSERT_TRUE(test_manifest_file.is_open());
+ test_manifest_file << profile;
+ test_manifest_file.close();
+
+ adbc_validation::Handle<struct AdbcDatabase> database;
+
+ // absolute path to the profile
+ ASSERT_THAT(AdbcDatabaseNew(&database.value, &error), IsOkStatus(&error));
+ ASSERT_THAT(AdbcDatabaseSetOption(&database.value, "profile",
filepath.string().c_str(),
+ &error),
+ IsOkStatus(&error));
+ ASSERT_THAT(AdbcDatabaseInit(&database.value, &error), IsOkStatus(&error));
+ ASSERT_THAT(AdbcDatabaseRelease(&database.value, &error),
IsOkStatus(&error));
+
+ // inherit additional_search_path_list
+ ASSERT_THAT(AdbcDatabaseNew(&database.value, &error), IsOkStatus(&error));
+ ASSERT_THAT(AdbcDatabaseSetOption(&database.value, "profile", "profile",
&error),
+ IsOkStatus(&error));
+ ASSERT_THAT(AdbcDriverManagerDatabaseSetAdditionalSearchPathList(
+ &database.value, temp_dir.string().c_str(), &error),
+ IsOkStatus(&error));
+ ASSERT_THAT(AdbcDatabaseInit(&database.value, &error), IsOkStatus(&error));
+ ASSERT_THAT(AdbcDatabaseRelease(&database.value, &error),
IsOkStatus(&error));
+
+ // find profile by name using ADBC_PROFILE_PATH
+ SetConfigPath(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();
+}
+
+TEST_F(ConnectionProfiles, HierarchicalProfile) {
+ auto filepath = temp_dir / "dev" / "profile.toml";
+ std::filesystem::create_directories(filepath.parent_path());
+ toml::table profile = simple_profile;
+ std::ofstream test_manifest_file(filepath);
+ ASSERT_TRUE(test_manifest_file.is_open());
+ test_manifest_file << profile;
+ test_manifest_file.close();
+
+ adbc_validation::Handle<struct AdbcDatabase> database;
+
+ SetConfigPath(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();
+}
+
+TEST_F(ConnectionProfiles, UriProfileOption) {
+ auto filepath = temp_dir / "profile.toml";
+ toml::table profile = simple_profile;
+ std::ofstream test_manifest_file(filepath);
+ ASSERT_TRUE(test_manifest_file.is_open());
+ test_manifest_file << profile;
+ test_manifest_file.close();
+
+ adbc_validation::Handle<struct AdbcDatabase> database;
+
+ // absolute path to the profile
+ ASSERT_THAT(AdbcDatabaseNew(&database.value, &error), IsOkStatus(&error));
+ ASSERT_THAT(AdbcDatabaseSetOption(&database.value, "uri",
+ ("profile://" +
filepath.string()).c_str(), &error),
+ IsOkStatus(&error));
+ ASSERT_THAT(AdbcDatabaseInit(&database.value, &error), IsOkStatus(&error));
+ ASSERT_THAT(AdbcDatabaseRelease(&database.value, &error),
IsOkStatus(&error));
+
+ // find profile by name using ADBC_PROFILE_PATH
+ SetConfigPath(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();
+}
+
+TEST_F(ConnectionProfiles, DriverProfileOption) {
+ auto filepath = temp_dir / "profile.toml";
+ toml::table profile = simple_profile;
+ std::ofstream test_manifest_file(filepath);
+ ASSERT_TRUE(test_manifest_file.is_open());
+ test_manifest_file << profile;
+ test_manifest_file.close();
+
+ adbc_validation::Handle<struct AdbcDatabase> database;
+
+ // absolute path to the profile
+ ASSERT_THAT(AdbcDatabaseNew(&database.value, &error), IsOkStatus(&error));
+ ASSERT_THAT(AdbcDatabaseSetOption(&database.value, "driver",
+ ("profile://" +
filepath.string()).c_str(), &error),
+ IsOkStatus(&error));
+ ASSERT_THAT(AdbcDatabaseInit(&database.value, &error), IsOkStatus(&error));
+ ASSERT_THAT(AdbcDatabaseRelease(&database.value, &error),
IsOkStatus(&error));
+
+ // find profile by name using ADBC_PROFILE_PATH
+ SetConfigPath(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();
+}
+
+TEST_F(ConnectionProfiles, ExtraStringOption) {
+ auto filepath = temp_dir / "profile.toml";
+ toml::table profile = simple_profile;
+ profile["options"].as_table()->insert("foo", "bar");
+ std::ofstream test_manifest_file(filepath);
+ ASSERT_TRUE(test_manifest_file.is_open());
+ test_manifest_file << profile;
+ test_manifest_file.close();
+
+ adbc_validation::Handle<struct AdbcDatabase> database;
+
+ // find profile by name using ADBC_PROFILE_PATH
+ SetConfigPath(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();
+}
+
+TEST_F(ConnectionProfiles, ExtraIntOption) {
+ auto filepath = temp_dir / "profile.toml";
+ toml::table profile = simple_profile;
+ profile["options"].as_table()->insert("foo", int64_t(42));
+ std::ofstream test_manifest_file(filepath);
+ ASSERT_TRUE(test_manifest_file.is_open());
+ test_manifest_file << profile;
+ test_manifest_file.close();
+
+ adbc_validation::Handle<struct AdbcDatabase> database;
+
+ // find profile by name using ADBC_PROFILE_PATH
+ SetConfigPath(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();
+}
+
+TEST_F(ConnectionProfiles, ExtraDoubleOption) {
+ auto filepath = temp_dir / "profile.toml";
+ toml::table profile = simple_profile;
+ profile["options"].as_table()->insert("foo", 42.0);
+ std::ofstream test_manifest_file(filepath);
+ ASSERT_TRUE(test_manifest_file.is_open());
+ test_manifest_file << profile;
+ test_manifest_file.close();
+
+ adbc_validation::Handle<struct AdbcDatabase> database;
+
+ // find profile by name using ADBC_PROFILE_PATH
+ SetConfigPath(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();
+}
+
+TEST_F(ConnectionProfiles, DotSeparatedKey) {
+ auto filepath = temp_dir / "profile.toml";
+ toml::table profile = toml::parse(R"(
+ version = 1
+ driver = "adbc_driver_sqlite"
+ [options]
+ foo.bar.baz = "bar"
+ )");
+
+ std::ofstream test_manifest_file(filepath);
+ ASSERT_TRUE(test_manifest_file.is_open());
+ test_manifest_file << profile;
+ test_manifest_file.close();
+
+ adbc_validation::Handle<struct AdbcDatabase> database;
+
+ // find profile by name using ADBC_PROFILE_PATH
+ SetConfigPath(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.baz='bar'"));
+ UnsetConfigPath();
+}
+
+TEST_F(ConnectionProfiles, UseEnvVar) {
+ auto filepath = temp_dir / "profile.toml";
+ toml::table profile = toml::parse(R"|(
+ version = 1
+ driver = "adbc_driver_sqlite"
+ [options]
+ foo = "env_var(ADBC_PROFILE_PATH)"
+ )|");
+
+ std::ofstream test_manifest_file(filepath);
+ ASSERT_TRUE(test_manifest_file.is_open());
+ test_manifest_file << profile;
+ test_manifest_file.close();
+
+ adbc_validation::Handle<struct AdbcDatabase> database;
+
+ // find profile by name using ADBC_PROFILE_PATH
+ SetConfigPath(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='" +
+ temp_dir.string() + "'"));
+ UnsetConfigPath();
+}
+
+TEST_F(ConnectionProfiles, UseEnvVarInterpolation) {
+ auto filepath = temp_dir / "profile.toml";
+ toml::table profile = toml::parse(R"|(
+ version = 1
+ driver = "adbc_driver_sqlite"
+ [options]
+ foo = "super env_var(ADBC_PROFILE_PATH) duper"
+ )|");
+
+ std::ofstream test_manifest_file(filepath);
+ ASSERT_TRUE(test_manifest_file.is_open());
+ test_manifest_file << profile;
+ test_manifest_file.close();
+
+ adbc_validation::Handle<struct AdbcDatabase> database;
+
+ // find profile by name using ADBC_PROFILE_PATH
+ SetConfigPath(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='super " +
+ temp_dir.string() + "
duper'"));
+ UnsetConfigPath();
+}
+
+TEST_F(ConnectionProfiles, UseEnvVarInterpolationMultiple) {
Review Comment:
Do we test an invalid substitution? Also what about an env var that doesn't
exist?
##########
c/include/arrow-adbc/adbc_driver_manager.h:
##########
@@ -175,6 +175,171 @@ AdbcStatusCode
AdbcDriverManagerDatabaseSetAdditionalSearchPathList(
ADBC_EXPORT
const char* AdbcStatusCodeMessage(AdbcStatusCode code);
+/// \defgroup adbc-driver-manager-connection-profile Connection Profiles
+/// Similar to odbc.ini, the ADBC driver manager can support "connection
profiles"
+/// that specify a driver and options to use when connecting. This allows
users to
+/// specify connection information in a file or environment variable, and have
the
+/// driver manager load the appropriate driver and set options accordingly.
+///
+/// This allows creating reusable connection configurations for sharing and
distribution
+/// without needing to hardcode driver names and options in application code.
Profiles
+/// will be loaded during DatabaseInit before attempting to initialize the
driver. Any
+/// options specified by the profile will be applied but will not override
options
+/// that have already been set using DatabaseSetOption.
+///
+/// To facilitate customization, we define an interface for implementing a
Connection
+/// Profile object along with a provider function definition which can be set
into
+/// the driver manager to allow for customized profile loading.
+///
+/// A profile can be specified to the Driver Manager in one of two ways,
+/// which will invoke the profile provider during the call to DatabaseInit:
+///
+/// 1. The "profile" option can be set using DatabaseSetOption with the name
of the
+/// profile to load.
+/// 2. The "uri" being used can have the form "profile://<profile>"
+///
+/// @{
+
+/// \brief Abstract interface for connection profile providers
+struct ADBC_EXPORT AdbcConnectionProfile {
+ /// \brief Opaque implementation-defined state.
+ /// This field is NULL if the profile is uninitialized/freed (but
+ /// it need not have a value even if the profile is initialized).
+ void* private_data;
+
+ /// \brief Release the profile and perform any cleanup.
+ void (*release)(struct AdbcConnectionProfile* profile);
+
+ /// \brief Get the driver to use as specified by this profile.
+ ///
+ /// It is not required that a profile specify a driver. If the options
+ // can be reusable across drivers, then the profile does not need to specify
+ /// a driver (if this provides an empty string or nullptr then the driver
+ /// must be defined by other means, e.g. by the driver / uri options).
+ ///
+ /// \param[in] profile The profile to query.
+ /// \param[out] driver_name The name of the driver to use, or NULL if not
specified.
+ /// \param[out] error An optional location to return an error message
+ AdbcStatusCode (*GetDriverName)(struct AdbcConnectionProfile* profile,
+ const char** driver_name, struct AdbcError*
error);
+
+ /// \brief Get the string options specified by the profile
+ ///
+ /// The keys and values returned by this function are owned by the profile
+ /// object itself and do not need to be freed or managed by the caller.
+ /// They must not be accessed after calling release on the profile.
+ ///
+ /// The profile can also indicate that a value should be pulled from the
environment
+ /// by having a value in the form `env_var(ENV_VAR_NAME)`. If the driver
+ /// manager encounters a value of this form, it will replace it with the
actual value
+ /// of the environment variable `ENV_VAR_NAME` before setting the option.
This
+ /// is only valid for option *values* not *keys*.
+ ///
+ /// \param[in] profile The profile to query.
+ /// \param[out] keys The keys of the options specified by the profile.
+ /// \param[out] values The values of the options specified by the profile.
+ /// \param[out] num_options The number of options specified by the profile,
+ /// consumers must not access keys or values beyond this count.
+ /// \param[out] error An optional location to return an error message
+ AdbcStatusCode (*GetOptions)(struct AdbcConnectionProfile* profile, const
char*** keys,
+ const char*** values, size_t* num_options,
+ struct AdbcError* error);
+
+ /// \brief Get the integer options specified by the profile
+ ///
+ /// The keys and values returned by this function are owned by the profile
+ /// object itself and do not need to be freed or managed by the caller. They
must not be
+ /// accessed after calling release on the profile.
+ ///
+ /// Values returned by this function will be set using the
DatabaseSetOptionInt function
+ /// on the database object being initialized. If the driver does not support
the
+ /// DatabaseSetOptionInt function, then options should only be returned as
strings.
+ ///
+ /// \param[in] profile The profile to query.
+ /// \param[out] keys The keys of the options specified by the profile.
+ /// \param[out] values The values of the options specified by the profile.
+ /// \param[out] num_options The number of options specified by the profile,
+ /// consumers must not access keys or values beyond this count.
+ /// \param[out] error An optional location to return an error message
+ AdbcStatusCode (*GetIntOptions)(struct AdbcConnectionProfile* profile,
+ const char*** keys, const int64_t** values,
+ size_t* num_options, struct AdbcError*
error);
+
+ /// \brief Get the double options specified by the profile
+ ///
+ /// The keys and values returned by this function are owned by the profile
+ /// object itself and do not need to be freed or managed by the caller. They
must not be
+ /// accessed after calling release on the profile.
+ ///
+ /// Values returned by this function will be set using the
DatabaseSetOptionDouble
+ /// function on the database object being initialized. If the driver does
not support
+ /// the DatabaseSetOptionDouble function, then options should only be
returned as
+ /// strings.
+ ///
+ /// \param[in] profile The profile to query.
+ /// \param[out] keys The keys of the options specified by the profile.
+ /// \param[out] values The values of the options specified by the profile.
+ /// \param[out] num_options The number of options specified by the profile,
+ /// consumers must not access keys or values beyond this count.
+ /// \param[out] error An optional location to return an error message
+ AdbcStatusCode (*GetDoubleOptions)(struct AdbcConnectionProfile* profile,
+ const char*** keys, const double** values,
+ size_t* num_options, struct AdbcError*
error);
+};
+
+/// \brief Common definition for a connection profile provider
+///
+/// \param[in] profile_name The name of the profile to load. This is the value
of the
+/// "profile" option or the profile specified in the URI.
+/// \param[in] additional_search_path_list A list of additional paths to
search for
+/// profiles, delimited by the OS specific path list separator.
+/// \param[out] out The profile to return. The caller will take ownership of
the profile
+/// and is responsible for calling release on it when finished.
+/// \param[out] error An optional location to return an error message if
necessary.
+typedef AdbcStatusCode (*AdbcConnectionProfileProvider)(
+ const char* profile_name, const char* additional_search_path_list,
+ struct AdbcConnectionProfile* out, struct AdbcError* error);
+
+/// \brief Set a custom connection profile provider for the driver manager.
+///
+/// If no provider is set, the driver manager will use a default,
filesystem-based
+/// provider which will look for profiles in the following locations if not
given an
+/// absolute path to a file:
+///
+/// 1. The environment variable ADBC_PROFILE_PATH, which is a list of paths to
search for
+/// profiles.
+/// 2. The user-level configuration directory (e.g. ~/.config/adbc/profiles on
Linux).
+///
+/// The filesystem-based profile looks for a file named <profile_name>.toml if
there is
+/// no extension provided, attempting to parse the toml file for the profile
information.
+/// If the file is found and parsed successfully, the options specified in the
profile
+/// which have not already been set will be set as if by DatabaseSetOption
just before
+/// initialization as part of DatabaseInit.
+///
+/// For file-based profiles the expected format is as follows:
+/// ```toml
+/// version = 1
+/// driver = "driver_name"
+///
+/// [options]
+/// option1 = "value1"
+/// option2 = 42
+/// option3 = 3.14
Review Comment:
We don't do that for driver TOML files though :grimacing:
--
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]