ClickHouse / ClickHouse/adbc_clickhouse
Rename repo and crate to `adbc_driver_clickhouse`
- Dominant language
- Rust
- Stars
- 25
- Forks
- 8
- Avg merge
- 2d 37m
- Merged PRs (30d)
- 4
Description
When I first set up this repo, I didn't realize that there was an expected naming convention for ADBC drivers: https://github.com/ClickHouse/adbc_clickhouse/pull/66
As of writing, this is not really documented anywhere:
* https://arrow.apache.org/adbc/24/driver/authoring.html
* https://arrow.apache.org/adbc/24/format/driver_manifests.html
The only discussion I found regarding naming conventions in my initial research was in the docs for `adbc_ffi::export_driver!()`, which doesn't prescribe a naming convention but just explains how the entrypoint symbol lookup works: https://docs.rs/adbc_ffi/0.23.0/adbc_ffi/macro.export_driver.html
I chose `adbc_clickhouse` (and `AdbcClickhouseInit`) because it felt more concise than `adbc_driver_clickhouse` (and `AdbcDriverClickhouseInit`) while still seeming to work for the rules laid out by `export_driver!()`. It loaded just fine with the Rust `adbc_driver_manager` crate.
The example dummy driver for Rust appears to support this choice:
* https://github.com/apache/arrow-adbc/blob/f1d6412b809784a882ad1c971018e4401c91aecd/rust/driver/dummy/Cargo.toml#L19
* https://github.com/apache/arrow-adbc/blob/f1d6412b809784a882ad1c971018e4401c91aecd/rust/driver/dummy/src/lib.rs#L941
However, it sounds like some driver managers _specifically_ look for `AdbcDriver*Init` and break if you try to load a driver that doesn't follow those conventions. Unfortunately, #66 is all the context I have here.
To work around this, the driver binary ends up getting renamed during the release process to `libadbc_driver_clickhouse.so`: https://github.com/adbc-drivers/clickhouse/blob/main/src/ci/scripts/build.sh#L46
And an additional `AdbcDriverClickhouseInit` entrypoint gets added for compatibility with old driver managers that don't recognize the `entrypoint` manifest key: https://github.com/adbc-drivers/clickhouse/blob/41ef9369dae4c594edc926cdf6858140a949cdb3/src/ci/scripts/pre-build.sh#L33-L39
I'm not really comfortable with this situation because it means the driver installed through `dbc` has a different filename and entrypoint than if the driver is built manually, which could confuse and frustrate users trying to contribute to the driver or test the latest changes on `main`.
# Steps to Resolve
* Release `adbc_clickhouse 0.1.1` (already approved, don't want to block it on this: #74)
* Update README to mention the driver is being renamed to `adbc_driver_clickhouse` and that `adbc_clickhouse` will no longer receive updates
* Release `adbc_clickhouse 0.1.2` to update the README on crates.io
* Release `adbc_driver_clickhouse 0.2.0` with `AdbcDriverClickhouseInit` as the main entrypoint, keeping `AdbcClickhouseInit` for backwards-compatibility.
At this point, the extra build steps can be deleted from `adbc-drivers/clickhouse`.
Contributor guide
No contributing guide indexed for this repository
Research direction
Start by reviewing the README and the naming and compatibility logic in src/ci/scripts/build.sh and src/ci/scripts/pre-build.sh, along with the AdbcClickhouseInit and AdbcDriverClickhouseInit entrypoints. Follow the listed release sequence and verify that the renamed crate, README updates, main entrypoint, and backwards-compatible entrypoint work without the extra build steps.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- databases
- Issue type
- Refactor
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 42/100