feat(python/adbc_driver_manager): simplify connect #3537
Conversation
|
IMO, if we want to add this to the Rust driver manager, the behavior should not be magic and instead we should add |
amoeba
left a comment
amoeba
left a comment
There was a problem hiding this comment.
This looks good but I ran into one issue. If you do something like,
dbapi.connect("sqlite://games.sqlite")The correct driver is detected but, since the full URI string is passed unmodified as the "uri", and I assume the ADBC SQLite driver passes that through, you get,
adbc_driver_manager.OperationalError: IO: failed to open 'sqlite://games.sqlite': failed to allocate memory
Ultimately, sqlite itself doesn't handle such URIs so I don't think any change is needed but I thought I'd point it out in case you had any good ideas. Other drivers, such as postgresql, handle the unmodified URIs just fine.
| For example, "adbc_driver_sqlite" will first attempt to load | ||
| adbc_driver_sqlite.toml from the various search paths. Then, it | ||
| will try to load libadbc_driver_sqlite.so on Linux, | ||
| libadbc_driver_sqlite.dylib on MacOS, or adbc_driver_sqlite.dll on |
There was a problem hiding this comment.
| libadbc_driver_sqlite.dylib on MacOS, or adbc_driver_sqlite.dll on | |
| libadbc_driver_sqlite.dylib on macOS, or adbc_driver_sqlite.dll on |
| libadbc_driver_sqlite.dylib on MacOS, or adbc_driver_sqlite.dll on | ||
| Windows. See :doc:`/format/driver_manifests`. | ||
| - A full path to a shared library to load. |
There was a problem hiding this comment.
| - A full path to a shared library to load. | |
| - A relative or absolute path to a shared library to load. |
Is this right?
Well, this was discussed. Not everything uses URIs, and not every URI is self-identifying (sqlite uses |
|
Right, it just took me a sec to remember. I don't think any change is needed to the PR in its current form, then. |
| - Only a URI, in which case the URI scheme will be assumed to be the | ||
| driver name and will be loaded as above. This will happen when | ||
| "://" is detected in the driver name. (It is not assumed that the | ||
| URI is actually a valid URI.) |
There was a problem hiding this comment.
Maybe mention here that this only works if the driver natively supports URIs with a scheme that happens to be the same as the driver name (since the driver manager does not strip off the scheme before passing it to the driver).
ianmcook
left a comment
ianmcook
left a comment
There was a problem hiding this comment.
Tested and works as described. Thanks! Just one minor comment to address a source of potential confusion for users.
3 participants
Closes #3517.