You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "kentkwu (via GitHub)" <gi...@apache.org> on 2026/07/17 03:01:27 UTC
[PR] feat(javascript/driver-manager): make driver optional when uri or profile is provided [arrow-adbc]
kentkwu opened a new pull request, #4537:
URL: https://github.com/apache/arrow-adbc/pull/4537
### Summary
Allows creating `AdbcDatabase` without an explicit `driver` option when `databaseOptions.uri` contains a URI that identifies the driver, or when `databaseOptions.profile` specifies a connection profile
**Before**
```ts
// before: driver was required
new AdbcDatabase({ driver: 'sqlite', databaseOptions: { uri: ':memory:' } })
```
**After**
```ts
// now: driver can be omitted when uri carries the scheme
new AdbcDatabase({ databaseOptions: { uri: 'sqlite::memory:' } })
// now: driver can be omitted with a bare profile name
new AdbcDatabase({ databaseOptions: { profile: 'my_profile' } })
```
Specifying more than one profile resolution path results in an error
```ts
// will error
new AdbcDatabase({ databaseOptions: { profile: 'p1', uri: 'profile://p2' } })
```
**Test Plan**
```
npm build
npm test
```
Closes #4533
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(javascript): make driver optional when uri or profile is provided [arrow-adbc]
Posted by "kentkwu (via GitHub)" <gi...@apache.org>.
kentkwu commented on PR #4537:
URL: https://github.com/apache/arrow-adbc/pull/4537#issuecomment-5007411835
> The Rust API makes this driver-mismatch condition less likely because normal usage involves choosing either `ManagedDriver::load_from_name` or `ManagedDatabase::from_uri`. @kentkwu do you think it's still worth opening an issue to look into this on the Rust side?
Yeah it's worth raising. The Rust API requires the caller to know which entrypoint to use beforehand which differs from the other driver manager implementations.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(javascript): make driver optional when uri or profile is provided [arrow-adbc]
Posted by "ianmcook (via GitHub)" <gi...@apache.org>.
ianmcook commented on PR #4537:
URL: https://github.com/apache/arrow-adbc/pull/4537#issuecomment-5007995239
> Yeah it's worth raising. The Rust API requires the caller to know which entrypoint to use beforehand which differs from the other driver manager implementations.
Could you please open an issue? Thanks!
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(javascript): make driver optional when uri or profile is provided [arrow-adbc]
Posted by "ianmcook (via GitHub)" <gi...@apache.org>.
ianmcook commented on PR #4537:
URL: https://github.com/apache/arrow-adbc/pull/4537#issuecomment-5007062299
The Rust API makes this driver-mismatch error less likely because normal usage involves choosing either `ManagedDriver::load_from_name` or `ManagedDatabase::from_uri`. @kentkwu do you think it's still worth opening an issue to look into this on the Rust side?
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(javascript): make driver optional when uri or profile is provided [arrow-adbc]
Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on code in PR #4537:
URL: https://github.com/apache/arrow-adbc/pull/4537#discussion_r3600344982
##########
javascript/src/client.rs:
##########
@@ -87,34 +87,78 @@ impl AdbcDatabaseCore {
.profile_search_paths
.map(|paths| paths.into_iter().map(PathBuf::from).collect());
- let database_opts = opts.database_options.map(map_database_options);
-
- let database = if opts.driver.contains(':') {
- let provider = adbc_driver_manager::profile::FilesystemProfileProvider::new_with_search_paths(
- profile_search_paths,
- );
- // URI-style ("sqlite:file::memory:") or profile URI ("profile://my_profile")
- ManagedDatabase::from_uri_with_profile_provider(
- &opts.driver,
- entrypoint.as_deref(),
- version,
- load_flags,
- manifest_search_paths,
- provider,
- database_opts.into_iter().flatten(),
- )?
- } else {
- // Short name ("sqlite") or path ("/usr/lib/libadbc_driver_sqlite.so")
- let mut driver = ManagedDriver::load_from_name(
- &opts.driver,
- entrypoint.as_deref(),
- version,
- load_flags,
- manifest_search_paths,
- )?;
- match database_opts {
- Some(db_opts) => driver.new_database_with_opts(db_opts)?,
- None => driver.new_database()?,
+ let database = match opts.driver {
+ Some(ref driver) if driver.contains(':') => {
Review Comment:
I think in C++ we look for `://` (this does catch out `sqlite:` without the recent changes to also accept `sqlite://`). Wouldn't this catch Windows-style paths?
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(javascript): make driver optional when uri or profile is provided [arrow-adbc]
Posted by "ianmcook (via GitHub)" <gi...@apache.org>.
ianmcook commented on PR #4537:
URL: https://github.com/apache/arrow-adbc/pull/4537#issuecomment-4999627587
These should both work but error:
```js
const db = new AdbcDatabase({
driver: 'sqlite',
databaseOptions: { profile: './profile.toml' },
});
```
```js
const db = new AdbcDatabase({
driver: 'sqlite',
databaseOptions: { uri: 'profile://./profile.toml' },
});
```
In other driver managers, code like this succeeds as long as the driver passed as an argument doesn't disagree with the driver set in the connection profile.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(javascript): make driver optional when uri or profile is provided [arrow-adbc]
Posted by "kentkwu (via GitHub)" <gi...@apache.org>.
kentkwu commented on code in PR #4537:
URL: https://github.com/apache/arrow-adbc/pull/4537#discussion_r3604268203
##########
javascript/src/client.rs:
##########
@@ -87,34 +87,78 @@ impl AdbcDatabaseCore {
.profile_search_paths
.map(|paths| paths.into_iter().map(PathBuf::from).collect());
- let database_opts = opts.database_options.map(map_database_options);
-
- let database = if opts.driver.contains(':') {
- let provider = adbc_driver_manager::profile::FilesystemProfileProvider::new_with_search_paths(
- profile_search_paths,
- );
- // URI-style ("sqlite:file::memory:") or profile URI ("profile://my_profile")
- ManagedDatabase::from_uri_with_profile_provider(
- &opts.driver,
- entrypoint.as_deref(),
- version,
- load_flags,
- manifest_search_paths,
- provider,
- database_opts.into_iter().flatten(),
- )?
- } else {
- // Short name ("sqlite") or path ("/usr/lib/libadbc_driver_sqlite.so")
- let mut driver = ManagedDriver::load_from_name(
- &opts.driver,
- entrypoint.as_deref(),
- version,
- load_flags,
- manifest_search_paths,
- )?;
- match database_opts {
- Some(db_opts) => driver.new_database_with_opts(db_opts)?,
- None => driver.new_database()?,
+ let database = match opts.driver {
+ Some(ref driver) if driver.contains(':') => {
Review Comment:
It does catch Windows-style paths. However it's then directly passed into the Rust driver manager which is responsible for resolving it.
This part here is basically just compensating for Rust not having a single entrypoint.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(javascript): make driver optional when uri or profile is provided [arrow-adbc]
Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm merged PR #4537:
URL: https://github.com/apache/arrow-adbc/pull/4537
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org