ClickHouse / ClickHouse/adbc_clickhouse

Rename repo and crate to `adbc_driver_clickhouse`

Open
#75 0 comments 0 reactions 0 assignees View on GitHub
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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.