feat(c/driver_manager): add connection profile interface by zeroshade · Pull Request #3876 · apache/arrow-adbc

zeroshade · 2026-01-09T20:03:36Z

An attempt to implement the ideas as suggested in #3765 to provide the ability to specify and load connection profiles to define options for use by the driver manager.

This creates an AdbcConnectionProfile struct and defines an AdbcConnectionProfileProvider function pointer typedef to allow for customized management of profiles. This also implements a default file-based profile provider as described in #3765 (comment) which will be used if no custom provider has been set.

This allows easy expansion in the future for non-file-based connection profile providers while still implementing the easier case of using file-based profiles, including hierarchical specification for now. See the documentation comments added to adbc_driver_manager.h for the full description of the semantics and explanation.

zeroshade · 2026-01-09T20:04:58Z

CC @davidhcoe

if there's anyone I missed please feel free to tag others that might be interested in looking this over and reviewing it. Once everyone is onboard with the design and ideas I'll implement the handling of these profiles for the other language bindings.

paleolimbot

Cool!

All just questions from me to take or leave 🙂

paleolimbot · 2026-01-09T22:06:21Z

c/include/arrow-adbc/adbc_driver_manager.h

+/// version = 1
+/// driver = "driver_name"
+///
+/// [options]
+/// option1 = "value1"
+/// option2 = 42
+/// option3 = 3.14


Do you want to wrap this in a top level [profile] item (like pyproject.toml's [project] or Cargo.toml's [workspace])?

Suggested change

/// version = 1

/// driver = "driver_name"

///

/// [options]

/// option1 = "value1"

/// option2 = 42

/// option3 = 3.14

/// [adbc.profile]

/// version = 1

/// driver = "driver_name"

///

/// [options]

/// option1 = "value1"

/// option2 = 42

/// option3 = 3.14

Personally I was trying to avoid having to do that. Is there any benefit to doing so?

Just seemed like a common idiom in .toml configs, perhaps to allow for future expansion (e.g., if you ever want >1 profile to live in one .toml)

i think after a lot of discussion between myself, @lidavidm and @ianmcook we came to the decision that we don't want more than 1 profile to live in one .toml file haha 😄

Having multiple connection profiles defined in a single TOML file could potentially be nice, but it complicates the profile search procedure, and I worry that it could confuse users.

I think my point was just that you have some future flexibility if you nest the configuration and if you put adbc and profile in there somewhere there's a better chance somebody who runs across this file without context will know what it is.

We don't do that for driver TOML files though 😬

This one is very easy to change if it ever comes up 🙂

c/include/arrow-adbc/adbc_driver_manager.h

paleolimbot · 2026-01-09T22:19:53Z

c/include/arrow-adbc/adbc_driver_manager.h

+  AdbcStatusCode (*GetDriverName)(struct AdbcConnectionProfile* profile,
+                                  const char** driver_name, struct AdbcError* error);


Suggested change

AdbcStatusCode (*GetDriverName)(struct AdbcConnectionProfile* profile,

const char** driver_name, struct AdbcError* error);

AdbcStatusCode (*GetDriverName)(struct AdbcConnectionProfile* profile,

const char** driver_name, AdbcDriverInitFunc* init_func, struct AdbcError* error);

Is there any value to allowing a profile to optionally specify the init function directly? (R could maybe use this to find R package versions of drivers)

The way the code is set up doesn't allow for this. If there is an init func set then the profile handling is currently entirely skipped and it's up to the init_func to handle everything. Given the way we envision profiles working, I don't think that it makes sense for the profile to set or specify the init function.

I think you'd just set args->init_func instead of args->driver (understood if you'd prefer not to)

I personally prefer not, but I'm open to it if others think it's worthwhile.

I think you do want this while you're here and I think it is fairly easy to do. Providing an init function is a more predictable/secure way to precisely specify which driver will actually get loaded. There are now quite a lot of ways that a driver can be resolved based on name that change based on working directory/environment and if I were embedding ADBC in an application I would personally want the ability to be absolutely sure about what driver/version was loaded. (Alternatively, as an application I could invent my own way to manage Profiles and opt out of the driver manager).

We can, I just didn't based on the way I implemented it. If both of you think it's a good way to go, I don't have a good argument against it so I can add it to the profile object. Sure.

It would be nice to have other people chime in as well ^_^

Agreed, please poke people! 😄

Mostly the way this is set up with an embedded callback makes it very difficult to change later, so if if it's easy to do and there's a plausible scenario where it might be needed I think it is worth it. There are a few types of drivers that can never be loaded by name/entrypoint and so I think it is plausible this could come up (e.g., giant statically linked piles of Rust crates, some R packages).

updated with your suggestion here to allow the profile to provide an init_func

go/adbc/drivermgr/adbc_driver_manager.cc

davidhcoe · 2026-01-09T23:26:37Z

@CurtHagenlocher will be interested in this as well.

docs/source/format/connection_profiles.rst

lidavidm · 2026-01-12T02:26:32Z

docs/source/format/connection_profiles.rst

+   driver = "adbc_driver_snowflake"
+
+   [options]
+   adbc.snowflake.sql.account = "env_var(SNOWFLAKE_ACCOUNT)"


Hmm. Why not use option = { env_var = "NAME" }?

That would prevent us from using environment variables to define substrings, which we will need for the drivers that expect their connection arguments to be in a URI.

uri = "scheme://host:port?user=user&pw=env_var(PASSWORD)

Should we consider using ${} instead of env_var()?

That is more concise and makes escaping literals easier (e.g. with \$ or $$).

I think ${} might be a little more familiar, unless you expect to eventually have other things besides env_var.

I believe the expectation was that we would eventually have more special functions in addition to env_var which is why this was the original suggestion. For eventual consistency

You will end up with the lambda calculus in these configuration strings :)

I think we also still need to specify how to escape this (in the very unlikely instance that it's needed...)

I was really trying to avoid inventing a way to escape this.... I can't even think of what a good syntax would be to escape this unless we change it to something like $env_var or provide some actual delimiter for it.

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...

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/include/arrow-adbc/adbc_driver_manager.h

zeroshade · 2026-01-14T19:00:44Z

@CurtHagenlocher @davidhcoe @lidavidm @ianmcook any further comments?

docs/source/format/connection_profiles.rst

zeroshade · 2026-01-19T20:52:52Z

I've updated the comments and the rst document based on everyone's comments. Please re-review

davidhcoe · 2026-01-21T00:20:04Z

fyi @jadewang-db and @eric-wang-1990

c/include/arrow-adbc/adbc_driver_manager.h

lidavidm · 2026-01-22T08:34:08Z

docs/source/format/connection_profiles.rst

+   driver = "adbc_driver_snowflake"
+
+   [options]
+   adbc.snowflake.sql.account = "env_var(SNOWFLAKE_ACCOUNT)"


I think we also still need to specify how to escape this (in the very unlikely instance that it's needed...)

c/driver_manager/adbc_driver_manager.cc

paleolimbot · 2026-02-06T15:48:40Z

c/driver_manager/adbc_driver_manager.cc

+#ifdef _WIN32
+  const wchar_t* profiles_dir = L"Profiles";
+#elif defined(__APPLE__)


I forget whether the Windows configuration directory from InternalAdbcUserConfigDir() uses the "Roaming" or local version but it might be worth checking to make sure the Profiles are stored in the right one (i.e., should Profiles follow a user around when logging in to the same account on multiple computers, or should they be local to one computer?).

Currently we use LocalAppData I believe, I don't think it necessarily makes sense for the profiles to follow the user around.

I suppose if anybody notices they can open an issue 🙂

paleolimbot · 2026-02-06T16:03:07Z

c/include/arrow-adbc/adbc_driver_manager.h

+  AdbcStatusCode (*GetDriverName)(struct AdbcConnectionProfile* profile,
+                                  const char** driver_name, struct AdbcError* error);


I think you do want this while you're here and I think it is fairly easy to do. Providing an init function is a more predictable/secure way to precisely specify which driver will actually get loaded. There are now quite a lot of ways that a driver can be resolved based on name that change based on working directory/environment and if I were embedding ADBC in an application I would personally want the ability to be absolutely sure about what driver/version was loaded. (Alternatively, as an application I could invent my own way to manage Profiles and opt out of the driver manager).

paleolimbot · 2026-02-06T16:11:26Z

c/include/arrow-adbc/adbc_driver_manager.h

+/// version = 1
+/// driver = "driver_name"
+///
+/// [options]
+/// option1 = "value1"
+/// option2 = 42
+/// option3 = 3.14


I think my point was just that you have some future flexibility if you nest the configuration and if you put adbc and profile in there somewhere there's a better chance somebody who runs across this file without context will know what it is.

paleolimbot · 2026-02-06T16:28:11Z

c/driver_manager/adbc_driver_manager.cc

+#ifdef _WIN32
+    auto local_env_var = Utf8Decode(std::string(env_var_name));
+    DWORD required_size = GetEnvironmentVariableW(local_env_var.c_str(), NULL, 0);
+    if (required_size == 0) {
+      out = "";
+      return ADBC_STATUS_OK;
+    }


I see why this is important (to avoid putting secrets in .toml files), but there is now quite a bit of environment variable state that is required to do certain things (like prepend the driver search path with a directory of preferred drivers if I remember the order correctly), which means the chance of an application concurrently setting and the driver manager fetching an environment variable is non-trivial in the presence of something like a connection pool. Not in this PR, but it may be worth documenting how to create connections in a thread-safe way and/or ensuring there is a thread safe alternative to accomplish some of these things.

I can agree with this, let's follow up in a separate PR for this though as you suggest.

c/include/arrow-adbc/adbc_driver_manager.h

lidavidm · 2026-02-11T01:52:25Z

c/include/arrow-adbc/adbc_driver_manager.h

+  AdbcStatusCode (*GetDriverName)(struct AdbcConnectionProfile* profile,
+                                  const char** driver_name, struct AdbcError* error);


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?

lidavidm · 2026-02-11T02:02:05Z

docs/source/format/connection_profiles.rst

+   driver = "adbc_driver_snowflake"
+
+   [options]
+   adbc.snowflake.sql.account = "env_var(SNOWFLAKE_ACCOUNT)"


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...

lidavidm · 2026-02-11T02:02:55Z

docs/source/format/connection_profiles.rst

+   driver = "adbc_driver_snowflake"
+
+   [options]
+   adbc.snowflake.sql.account = "env_var(SNOWFLAKE_ACCOUNT)"


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?)

lidavidm · 2026-02-11T02:08:04Z

c/include/arrow-adbc/adbc_driver_manager.h

+/// version = 1
+/// driver = "driver_name"
+///
+/// [options]
+/// option1 = "value1"
+/// option2 = 42
+/// option3 = 3.14


We don't do that for driver TOML files though 😬

lidavidm · 2026-02-11T02:13:27Z

c/driver_manager/adbc_driver_manager.cc

+      std::string message =
+          "Profile version '" + std::to_string(version) +
+          "' is not supported by this driver manager. Profile: " + profile_path.string();


Incidentally I think we should discuss moving to C++20 soon...then we can use std::format

lidavidm · 2026-02-11T02:14:39Z

c/driver_manager/adbc_driver_manager.cc

-    return ParseDriverUriResult{d, str};
+    if (d == "profile" && str.size() > pos + 2) {
+      // found a profile URI "profile://"
+      return ParseDriverUriResult{"", std::nullopt, str.substr(pos + 3)};


not necessarily something we have to do in this PR, but I think this should become a std::variant (driver+URI or profile)

c/driver_manager/adbc_driver_manager_test.cc

lidavidm · 2026-02-11T02:18:35Z

c/driver_manager/adbc_driver_manager_test.cc

@@ -43,6 +43,7 @@ std::filesystem::path InternalAdbcUserConfigDir();
 struct ParseDriverUriResult {


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.)

I agree, though I don't think it's something we need to do in this PR.

c/include/arrow-adbc/adbc_driver_manager.h

c/driver_manager/adbc_driver_manager.cc

lidavidm · 2026-02-12T05:57:56Z

c/driver_manager/adbc_driver_manager.cc

+    auto diff = pos_match - pos_last_match;
+    auto start_match = end_of_last_match;
+    std::advance(start_match, diff);
+    if (pos_match > 0 && value[pos_match - 1] == '\\') {


Hmm, this syntax is a little funky. So just \{ isn't an escape, for instance, only {{...}}? I guess that's reasonable enough...it's not consistent with jinja2/etc. but also I don't know if we want to imply a full templating language either

I'm open to any suggestions. I explicitly don't want to imply a full templating language. Currently we won't match a single { only {{, as far as I know, most templating languages don't actually provide a way to escape the {{ you just need to use an expression that outputs the {{ like {{ print "{{" }} etc. I also can't think of a reason offhand why someone would really need to escape it. So I'm fine removing this escaping since the syntax is funky. Up to you

Honestly, I'd be OK not supporting escapes for now, and if/when someone asks for it then we can work out the syntax

Just, I thought it was difficult for the env_var(FOO) to be escaped 🙂

Co-authored-by: David Li <li.davidm96@gmail.com>

paleolimbot

Thank you!

paleolimbot · 2026-02-12T22:21:18Z

c/include/arrow-adbc/adbc_driver_manager.h

+/// version = 1
+/// driver = "driver_name"
+///
+/// [options]
+/// option1 = "value1"
+/// option2 = 42
+/// option3 = 3.14


This one is very easy to change if it ever comes up 🙂

zeroshade requested a review from lidavidm as a code owner January 9, 2026 20:03

zeroshade requested review from CurtHagenlocher, felipecrv, ianmcook, kou and paleolimbot January 9, 2026 20:03

github-actions bot modified the milestone: ADBC Libraries 23 Jan 9, 2026

paleolimbot reviewed Jan 9, 2026

View reviewed changes

go/adbc/drivermgr/adbc_driver_manager.cc Outdated Show resolved Hide resolved

lidavidm reviewed Jan 12, 2026

View reviewed changes

ianmcook reviewed Jan 15, 2026

View reviewed changes

docs/source/format/connection_profiles.rst Show resolved Hide resolved

lidavidm reviewed Jan 22, 2026

View reviewed changes

lidavidm reviewed Jan 26, 2026

View reviewed changes

zeroshade added 11 commits January 26, 2026 15:46

feat(c/driver_manager): add connection profile interface

9f282da

pre-commit linting

bf9d9fd

typos

d7fae5a

darn you windows

a9c1fb4

update test cases for ParseDriverURI

e279a27

darn you windows

14895c5

fix heap issue

f75ed95

some test fixes

f64f3b8

more test fix

fc8ab25

one more test leak

c54fe84

windows sucks

83a1820

zeroshade and others added 9 commits January 26, 2026 15:46

extra complexity to make clang happy?

9528c67

add docs

fda0197

yet another attempt

e421803

fix remaining issues, i hope

2bd4330

this might fix the issue?

792e223

Disallow profile as a driver manifest basename

fe2e94c

Reserve the profile option key for use by driver managers

9d9adfc

update from comments

bc074d3

updates from comments

3cebf5a

zeroshade force-pushed the profile-settings-configuration branch from 23fed89 to 3cebf5a Compare January 26, 2026 20:46

zeroshade requested review from ianmcook, lidavidm and paleolimbot February 5, 2026 23:49

paleolimbot reviewed Feb 6, 2026

View reviewed changes

lidavidm approved these changes Feb 11, 2026

View reviewed changes

zeroshade added 5 commits February 11, 2026 17:54

updates from comments

9896615

pre-commit

c3f34ff

better option for init_func

6166eaf

pre-commit lint

549fcb3

update to {{ }} syntax

f124d1a

zeroshade requested a review from paleolimbot February 12, 2026 00:59

zeroshade added 2 commits February 11, 2026 20:10

missed closing quotes

b0a32ef

vscode hates me

0a42959

lidavidm approved these changes Feb 12, 2026

View reviewed changes

zeroshade and others added 4 commits February 12, 2026 10:50

Update c/include/arrow-adbc/adbc_driver_manager.h

33c9d16

Co-authored-by: David Li <li.davidm96@gmail.com>

sync

a8fa1ce

make regex onst static

2581b5c

static const not const static

48d8071

paleolimbot reviewed Feb 12, 2026

View reviewed changes

paleolimbot approved these changes Feb 12, 2026

View reviewed changes

		AdbcStatusCode (GetDriverName)(struct AdbcConnectionProfile profile,
		const char** driver_name, struct AdbcError* error);

		@@ -43,6 +43,7 @@ std::filesystem::path InternalAdbcUserConfigDir();
		struct ParseDriverUriResult {

Conversation

zeroshade commented Jan 9, 2026

Uh oh!

zeroshade commented Jan 9, 2026

Uh oh!

paleolimbot left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

davidhcoe commented Jan 9, 2026

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

zeroshade commented Jan 14, 2026

Uh oh!

Uh oh!

zeroshade commented Jan 19, 2026

Uh oh!

davidhcoe commented Jan 21, 2026

Uh oh!

Uh oh!

Choose a reason for hiding this comment