feat: implement connection option getters by pedro-filardi · Pull Request #65 · ClickHouse/adbc_clickhouse

pedro-filardi · 2026-07-23T10:55:01Z

Summary

Connection::get_option_string() returned NotImplemented for every key. ADBC driver managers read these back before they will use a connection, so erroring on all of them can leave the driver unusable through one.

Implemented Connection::get_option_string() for the variants that have a well-defined answer for ClickHouse:
- OptionConnection::AutoCommit → "true". ClickHouse has no transactions, so statements always commit as they run. rollback() already reports this; answering here too lets managers check without provoking an error.
- OptionConnection::CurrentCatalog → "". ClickHouse has no catalog level above the database. The database maps onto ADBC's db_schema (cf. get_table_schema()), which leaves the catalog empty rather than unsupported.
- OptionConnection::CurrentSchema → SELECT currentDatabase().
get_option_bytes() defers to get_option_string(), and get_option_int()/get_option_double() report InvalidArguments, mirroring the ClickhouseDatabase impl added in feat: support clickhouse:// URLs #62.
ReadOnly and IsolationLevel keep returning NotImplemented, since set_option() does not accept them either.

Addresses #8 — readback of custom (Other) options is not covered here, as it would need the connection to retain its product_info; happy to fold that in if you'd prefer the issue closed outright.

The upstream blocker noted on #8, ClickHouse/clickhouse-rs#358, is now closed and released in clickhouse 0.15.1.

Note on scope

This is necessary but not sufficient for a driver manager to attach a ClickHouse database — get_objects() (#10) is required as well. I verified the pair is sufficient by building this branch as a cdylib and pairing it with a temporary pass-through driver supplying only get_objects(); attaching then succeeded and a 1,000,000-row INSERT ... SELECT streamed through bulk ingest in 0.26s.

With this branch alone, attaching fails on get_objects() instead of CurrentCatalog, which is the intended progression.

Checklist

Delete items not relevant to your PR:

Unit and integration tests covering the common scenarios were added
A human-readable description of the changes was provided so that we can include it in CHANGELOG later

CLAassistant · 2026-07-23T10:55:10Z

All committers have signed the CLA.

`ClickhouseConnection::get_option_string()` returned `NotImplemented` for every key. ADBC driver managers read these back before they will use a connection, so erroring on all of them can leave the driver unusable through one. Implements the variants that have a well-defined answer for ClickHouse: * `AutoCommit` -> `"true"`. ClickHouse has no transactions, so statements always commit as they run. `rollback()` already reports this; answering here too lets managers check without provoking an error. * `CurrentCatalog` -> `""`. ClickHouse has no catalog level above the database. The database maps onto ADBC's `db_schema` (cf. `get_table_schema()`), which leaves the catalog empty rather than unsupported. * `CurrentSchema` -> `SELECT currentDatabase()`. `ReadOnly` and `IsolationLevel` keep returning `NotImplemented`, as `set_option()` does not accept them either, and readback of custom (`Other`) options is left for a follow-up. The remaining getters mirror `ClickhouseDatabase`: `get_option_bytes()` defers to `get_option_string()`, and the numeric getters report `InvalidArguments`. The upstream blocker noted on ClickHouse#8, ClickHouse/clickhouse-rs#358, is now closed and released in `clickhouse` 0.15.1. Note that `get_objects()` (ClickHouse#10) is also required before a driver manager can attach a ClickHouse database; this change is necessary but not sufficient on its own.

Copilot

Pull request overview

Implements missing Connection::get_option_* readbacks needed by ADBC driver managers to successfully validate and use a ClickHouse connection, aligning connection option behavior with the existing ClickhouseDatabase option handling.

Changes:

Implemented ClickhouseConnection::get_option_string() for AutoCommit, CurrentCatalog, and CurrentSchema.
Made get_option_bytes() delegate to get_option_string(), and updated get_option_int()/get_option_double() to return InvalidArguments.
Added integration tests covering the connection option getters and updated the changelog.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File	Description
tests/it/main.rs	Adds an integration test validating the new connection option getter behaviors and statuses.
src/lib.rs	Implements connection option getters (string/bytes/int/double) for well-defined ClickHouse behaviors.
CHANGELOG.md	Documents the new option getter support under Unreleased and links the tracking issue.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

koletzilla · 2026-07-29T10:33:06Z

+            OptionConnection::CurrentSchema => self
+                .tokio
+                .block_on(async {
+                    self.client
+                        .query("SELECT currentDatabase()")
+                        .fetch_one::<String>()
+                        .await
+                })
+                .map_err(|e| {
+                    Error::with_message_and_status(
+                        format!("could not read the current database: {e}"),
+                        Status::Internal,
+                    )
+                }),


I have some concerns with this approach to get the currentDatabase.

It would require one roundtrip to the database to get it.

It's already static in the ADBC connector: as we use HTTP here, we can't just call a SET to change it and then depend on CH to know which is the current one. Even if we do it with CH sessions (as we carry a session_id), this may change after the default session timeout of 60 seconds.

To manage the defaultSchema I think we should:

Allow the default Schema to be configured as a configuration paramenter

Use that value in each call as the database URL parameter / X-ClickHouse-Database header

the return that value back here.

If the user wants to change that value (not sure if something like set_option(CurrentSchema) may happen) then the change needs to be applied at the ADBC CH driver level, not with a SET.

Btw this is specially relevant for me as in DBT we need to be able to define a different default database to make operations in and right now that is a limitation in this ADBC. I can tackle it in a different PR to leave this one more compact if you prefer @pedro-filardi .

CC @abonander . What do you think?

Makes sense to land the setter first and rebase this on top of it, go ahead
in a separate PR if you like.

One question either way: what should CurrentSchema return when nothing is
configured? Do we fall back to SELECT currentDatabase() cached once per connection?

Added #67 to track the need to be able to set the default database for the connection

what should CurrentSchema return when nothing is configured? Do we fall back to SELECT currentDatabase() cached once per connection?

Good question. I think we must just return default which is the name for the default one in CH

If so, we are not covering cases where default_database is configured by the admin, either at user or server level. Maybe it's good enough for a first iteration, but it should at least be a known limitation

I think we can get away with returning whatever is configured, or an empty string otherwise. If the admin has set a default_database on the server-side, then the empty string is essentially an alias for that database.

I'm worried about doing a fallible blocking call in get_option_string() as it's rather surprising for a simple getter to execute a query on the database. I'm not sure if that's how it's intended for this API to behave.

@pedro-filardi we added a way to set the database in #72 so this can just read that back.

pedro-filardi requested review from abonander and koletzilla as code owners July 23, 2026 10:55

pedro-filardi force-pushed the feat/connection-option-getters branch from 798f86d to 2b75b70 Compare July 23, 2026 10:58

mshustov requested a review from Copilot July 26, 2026 13:30

Copilot started reviewing on behalf of mshustov July 26, 2026 13:31 View session

Copilot AI reviewed Jul 26, 2026

View reviewed changes

Comment thread src/lib.rs

Potential fix for pull request finding

b2c4929

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

koletzilla reviewed Jul 29, 2026

View reviewed changes

abonander requested changes Aug 31, 2026

View reviewed changes

Navigation Menu

Uh oh!

feat: implement connection option getters - #65

feat: implement connection option getters#65
pedro-filardi wants to merge 2 commits into
ClickHouse:mainfrom
pedro-filardi:feat/connection-option-getters

pedro-filardi commented Jul 23, 2026 •

edited

Loading

Uh oh!

CLAassistant commented Jul 23, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

koletzilla Jul 29, 2026

Uh oh!

pedro-filardi Jul 29, 2026

Uh oh!

koletzilla Jul 30, 2026 •

edited

Loading

Uh oh!

pedro-filardi Jul 30, 2026

Uh oh!

abonander Jul 30, 2026

Uh oh!

abonander Aug 31, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Uh oh!

Conversation

pedro-filardi commented Jul 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Note on scope

Checklist

Uh oh!

CLAassistant commented Jul 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

koletzilla Jul 29, 2026

Choose a reason for hiding this comment

Uh oh!

pedro-filardi Jul 29, 2026

Choose a reason for hiding this comment

Uh oh!

koletzilla Jul 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pedro-filardi Jul 30, 2026

Choose a reason for hiding this comment

Uh oh!

abonander Jul 30, 2026

Choose a reason for hiding this comment

Uh oh!

abonander Aug 31, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

pedro-filardi commented Jul 23, 2026 •

edited

Loading

CLAassistant commented Jul 23, 2026 •

edited

Loading

koletzilla Jul 30, 2026 •

edited

Loading