Skip to content

Commit e3f2605

Browse files
Refactor client structure (#219)
* Refactor phase 1: ClickHouseClient skeleton (#205) * ClickHouseClient skeleton * null checks * remove parameter * No insert parallelism by default * fix UseSession override * Add Timeout to QueryOptions * Adjust xml comment * Adjust InsertRawStreamAsync for backwards compatibility * Refactor phase 2: Connection and Command (#207) * Move ClickHouseConnection logic to Client * update example * fix logging tests * Update client reference when command has its connection changed * fix role override * Isolate database changes to connection * datasource adjustments around disposal * remove unused usings * Don't dispose response in PostStreamAsync * ExecuteRawResultAsync implementation * Add AddParameter convenience method (#210) * Refactor phase 3: move binary insert functionality from BulkCopy to ClickHouseClient (#209) * move binary insert functionality from BulkCopy to ClickHouseClient * Magic numbers to consts * fix batch insert issues * tests and adjustments (#212) * Update examples following refactoring (#211) * examples updates * FINAL performance link * Test cleanup and claude/readme/releasenotes update (#217) * disable obsolete warnings in tests using BulkCopy * CLAUDE.md update * examples readme update * release notes update * fix windows and macos tests? * win and macos test workflow fixes * test workflow fix #15 * different approach * cleanup unnecessary stuff? * add tests for client constructor * fix resource cleanup issues * remove client reference from command, interact with connection instead (prevent stale client reference) * Add GetClient() to DataSource
1 parent d2e142b commit e3f2605

File tree

83 files changed

+4037
-2491
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

83 files changed

+4037
-2491
lines changed

.github/workflows/tests-macos.yml

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,8 @@ jobs:
2525
- name: Setup ClickHouse
2626
run: |
2727
curl https://clickhouse.com/ | sh
28-
./clickhouse server --daemon
28+
sudo ./clickhouse install --noninteractive
29+
sudo clickhouse start
2930
3031
- uses: actions/cache@v5
3132
name: Cache NuGet

.github/workflows/tests-windows.yml

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,8 @@ jobs:
2626
shell: wsl-bash -u root {0}
2727
run: |
2828
curl https://clickhouse.com/ | sh
29-
./clickhouse server --daemon
29+
./clickhouse install --noninteractive
30+
clickhouse start
3031
3132
- uses: actions/cache@v5
3233
name: Cache NuGet

CLAUDE.md

Lines changed: 58 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,9 @@
33
## Repository Overview
44

55
### Project Context
6-
- **ClickHouse.Driver** is the official ADO.NET client for ClickHouse database
6+
- **ClickHouse.Driver** is the official .NET client for ClickHouse database
7+
- **Primary API**: `ClickHouseClient` - thread-safe, singleton-friendly, recommended for most use cases
8+
- **ADO.NET API**: `ClickHouseConnection`/`ClickHouseCommand` - for ORM compatibility (Dapper, EF Core, linq2db)
79
- **Critical priorities**: Stability, correctness, performance, and comprehensive testing
810
- **Tech stack**: C#/.NET targeting `net6.0`, `net8.0`, `net9.0`, `net10.0`
911
- **Tests run on**: `net6.0`, `net8.0`, `net9.0`, `net10.0`; Integration tests: `net10.0`; Benchmarks: `net10.0`
@@ -12,25 +14,52 @@
1214
```
1315
ClickHouse.Driver.sln
1416
├── ClickHouse.Driver/ # Main library (NuGet package)
15-
│ ├── ADO/ # Core ADO.NET (Connection, Command, DataReader, Parameters)
17+
│ ├── Utility/ # ClickHouseClient (primary API), schema, feature detection
18+
│ ├── ADO/ # ADO.NET layer (Connection, Command, DataReader, Parameters)
1619
│ ├── Types/ # 60+ ClickHouse type implementations + TypeConverter.cs
17-
│ ├── Copy/ # Bulk copy & binary serialization
20+
│ ├── Copy/ # Binary serialization (used internally by ClickHouseClient)
1821
│ ├── Http/ # HTTP layer & connection pooling
19-
│ ├── Utility/ # Schema, feature detection, extensions
2022
│ └── PublicAPI/ # Public API surface tracking (analyzer-enforced)
2123
├── ClickHouse.Driver.Tests/ # NUnit tests (multi-framework)
2224
├── ClickHouse.Driver.IntegrationTests/ # Integration tests (net10.0)
2325
└── ClickHouse.Driver.Benchmark/ # BenchmarkDotNet performance tests
2426
```
2527

2628
### Key Files
29+
- **Primary API**: `ClickHouseClient.cs` - main entry point for most applications
2730
- **Type system**: `Types/TypeConverter.cs` (14KB, complex), `Types/Grammar/` (type parsing)
28-
- **Core ADO**: `ADO/ClickHouseConnection.cs`, `ADO/ClickHouseCommand.cs`, `ADO/Readers/`
29-
- **Protocol**: Binary serialization in `Copy/Serializer/`, HTTP formatting in `Formats/`
31+
- **ADO.NET layer**: `ADO/ClickHouseConnection.cs`, `ADO/ClickHouseCommand.cs`, `ADO/Readers/`
3032
- **Feature detection**: `Utility/ClickHouseFeatureMap.cs` (version-based capabilities)
3133
- **Public API**: `PublicAPI/*.txt` (Roslyn analyzer enforces shipped signatures)
3234
- **Config**: `.editorconfig` (file-scoped namespaces, StyleCop suppressions)
3335

36+
### API Architecture
37+
38+
**ClickHouseClient** (recommended):
39+
```csharp
40+
using var client = new ClickHouseClient("Host=localhost");
41+
await client.ExecuteNonQueryAsync("CREATE TABLE ...");
42+
await client.InsertBinaryAsync(tableName, columns, rows); // High-performance bulk insert
43+
using var reader = await client.ExecuteReaderAsync("SELECT ...");
44+
var scalar = await client.ExecuteScalarAsync("SELECT count() ...");
45+
```
46+
47+
**ClickHouseConnection** (for ORMs):
48+
```csharp
49+
// Use ClickHouseDataSource for proper connection lifetime management with ORMs
50+
var dataSource = new ClickHouseDataSource("Host=localhost");
51+
services.AddSingleton(dataSource);
52+
53+
// Dapper, EF Core, linq2db work with DbConnection
54+
using var connection = dataSource.CreateConnection();
55+
var users = connection.Query<User>("SELECT * FROM users");
56+
```
57+
58+
**Key differences**:
59+
- `ClickHouseClient`: Thread-safe, can be singleton, has `InsertBinaryAsync` for bulk inserts
60+
- `ClickHouseConnection`: ADO.NET `DbConnection`, required for ORM compatibility
61+
- `ClickHouseBulkCopy`: **Deprecated** - use `ClickHouseClient.InsertBinaryAsync` instead
62+
3463
---
3564

3665
## Development Guidelines
@@ -68,9 +97,31 @@ ClickHouse.Driver.sln
6897
- **Analyzers**: Respect `.editorconfig`, StyleCop suppressions, nullable contexts
6998

7099
### Configuration & Settings
71-
- **Configuration**: happens through connection string and ClickHouseClientSettings
100+
- **Client configuration**: Connection string or `ClickHouseClientSettings` for client-level settings
101+
- **Per-query options**: `QueryOptions` for query-specific settings (QueryId, CustomSettings, Roles, BearerToken)
102+
- **Parameters**: Use `ClickHouseParameterCollection` with `ClickHouseDbParameter` for parameterized queries
72103
- **Feature flags**: Consider adding optional behavior behind connection string settings
73104

105+
```csharp
106+
// Client-level settings
107+
var settings = new ClickHouseClientSettings("Host=localhost");
108+
settings.CustomSettings.Add("max_threads", 4);
109+
using var client = new ClickHouseClient(settings);
110+
111+
// Per-query options
112+
var options = new QueryOptions
113+
{
114+
QueryId = "my-query-id",
115+
CustomSettings = new Dictionary<string, object> { ["max_execution_time"] = 30 },
116+
};
117+
await client.ExecuteReaderAsync("SELECT ...", options: options);
118+
119+
// Parameters
120+
var parameters = new ClickHouseParameterCollection();
121+
parameters.Add("id", 42UL);
122+
await client.ExecuteReaderAsync("SELECT * FROM t WHERE id = {id:UInt64}", parameters);
123+
```
124+
74125
### Observability & Diagnostics
75126
- **Error messages**: Must be clear, actionable, include context (connection string, query, server version)
76127
- **OpenTelemetry**: Changes to diagnostic paths should maintain telemetry integration

ClickHouse.Driver.Tests/ADO/ConnectionTests.cs

Lines changed: 69 additions & 81 deletions
Original file line numberDiff line numberDiff line change
@@ -166,7 +166,7 @@ public async Task ClientShouldSetQueryId()
166166
public async Task ClientShouldSetUserAgent()
167167
{
168168
var headers = new HttpRequestMessage().Headers;
169-
connection.AddDefaultHttpHeaders(headers);
169+
client.AddDefaultHttpHeaders(headers);
170170
// Build assembly version defaults to 1.0.0
171171
Assert.That(headers.UserAgent.ToString().Contains("ClickHouse.Driver/1.0.0"), Is.True);
172172
}
@@ -335,22 +335,6 @@ public void ChangeDatabaseShouldChangeDatabase()
335335
Assert.That(conn.Database, Is.EqualTo("default"));
336336
}
337337

338-
[Test]
339-
public void ShouldExcludePasswordFromRedactedConnectionString()
340-
{
341-
const string MOCK = "verysecurepassword";
342-
var settings = new ClickHouseClientSettings()
343-
{
344-
Password = MOCK,
345-
};
346-
using var conn = new ClickHouseConnection(settings);
347-
Assert.Multiple(() =>
348-
{
349-
Assert.That(conn.ConnectionString, Contains.Substring($"Password={MOCK}"));
350-
Assert.That(conn.RedactedConnectionString, Is.Not.Contains($"Password={MOCK}"));
351-
});
352-
}
353-
354338
[Test]
355339
[TestCase("https")]
356340
[TestCase("http")]
@@ -386,8 +370,8 @@ public async Task ShouldUseQueryIdForRawStream()
386370
{
387371
var queryId = Guid.NewGuid().ToString();
388372
var httpResponseMessage = await connection.PostStreamAsync("SELECT version()", (_, _) => Task.CompletedTask, false, CancellationToken.None, queryId);
389-
390-
Assert.That(ClickHouseConnection.ExtractQueryId(httpResponseMessage), Is.EqualTo(queryId));
373+
var queryResult = new QueryResult(httpResponseMessage);
374+
Assert.That(queryResult.QueryId, Is.EqualTo(queryId));
391375
}
392376

393377
private static string[] GetColumnNames(DataTable table) => table.Columns.Cast<DataColumn>().Select(dc => dc.ColumnName).ToArray();
@@ -665,97 +649,101 @@ public async Task Constructor_WithSettingsWithPath_ShouldApplyPath()
665649
}
666650

667651
[Test]
668-
public void InsertRawStreamAsync_WithNullTable_ShouldThrowArgumentException()
652+
public void Dispose_WithOwnedClient_ShouldDisposeClient()
669653
{
670-
using var stream = new MemoryStream(Encoding.UTF8.GetBytes("1,2,3"));
671-
var ex = Assert.ThrowsAsync<ArgumentException>(async () =>
672-
await connection.InsertRawStreamAsync(table: null, stream: stream, format: "CSV"));
673-
Assert.That(ex.ParamName, Is.EqualTo("table"));
674-
}
654+
// Arrange - connection created from connection string owns its client
655+
var conn = new ClickHouseConnection("Host=localhost");
656+
var client = conn.ClickHouseClient;
675657

676-
[Test]
677-
public void InsertRawStreamAsync_WithEmptyTable_ShouldThrowArgumentException()
678-
{
679-
using var stream = new MemoryStream(Encoding.UTF8.GetBytes("1,2,3"));
680-
var ex = Assert.ThrowsAsync<ArgumentException>(async () =>
681-
await connection.InsertRawStreamAsync(table: "", stream: stream, format: "CSV"));
682-
Assert.That(ex.ParamName, Is.EqualTo("table"));
683-
}
658+
// Act
659+
conn.Dispose();
684660

685-
[Test]
686-
public void InsertRawStreamAsync_WithNullStream_ShouldThrowArgumentNullException()
687-
{
688-
var ex = Assert.ThrowsAsync<ArgumentNullException>(async () =>
689-
await connection.InsertRawStreamAsync(table: "test", stream: null, format: "CSV"));
690-
Assert.That(ex.ParamName, Is.EqualTo("stream"));
661+
// Assert - client should be disposed (calling Dispose again should be safe but we can't easily verify)
662+
// We verify indirectly by checking that a new connection with the same client would fail
663+
// after the original connection disposed it
664+
Assert.DoesNotThrow(() => client.Dispose()); // Dispose should be idempotent
691665
}
692666

693667
[Test]
694-
public void InsertRawStreamAsync_WithNullFormat_ShouldThrowArgumentException()
668+
public void Dispose_WithSharedClient_ShouldNotDisposeClient()
695669
{
696-
using var stream = new MemoryStream(Encoding.UTF8.GetBytes("1,2,3"));
697-
var ex = Assert.ThrowsAsync<ArgumentException>(async () =>
698-
await connection.InsertRawStreamAsync(table: "test", stream: stream, format: null));
699-
Assert.That(ex.ParamName, Is.EqualTo("format"));
700-
}
670+
// Arrange - create a shared client
671+
using var sharedClient = new ClickHouseClient("Host=localhost");
701672

702-
[Test]
703-
public void InsertRawStreamAsync_WithEmptyFormat_ShouldThrowArgumentException()
704-
{
705-
using var stream = new MemoryStream(Encoding.UTF8.GetBytes("1,2,3"));
706-
var ex = Assert.ThrowsAsync<ArgumentException>(async () =>
707-
await connection.InsertRawStreamAsync(table: "test", stream: stream, format: ""));
708-
Assert.That(ex.ParamName, Is.EqualTo("format"));
673+
// Create two connections sharing the same client
674+
var conn1 = new ClickHouseConnection(sharedClient);
675+
var conn2 = new ClickHouseConnection(sharedClient);
676+
677+
// Act - dispose both connections
678+
conn1.Dispose();
679+
conn2.Dispose();
680+
681+
// Assert - shared client should still be usable (not disposed)
682+
// Ping will fail since there's no server, but it shouldn't throw ObjectDisposedException
683+
Assert.DoesNotThrowAsync(async () => await sharedClient.PingAsync());
709684
}
710685

711686
[Test]
712-
public async Task PingAsync_ReturnsTrue_WhenServerResponds()
687+
public void ApplySettings_WithSharedClient_ShouldNotDisposeOriginalClient()
713688
{
714-
var trackingHandler = new TrackingHandler(request =>
715-
{
716-
return new HttpResponseMessage(HttpStatusCode.OK) { Content = new StringContent("Ok.\n") };
717-
});
718-
using var httpClient = new HttpClient(trackingHandler);
719-
var settings = new ClickHouseClientSettings { Host = "localhost", HttpClient = httpClient };
720-
using var conn = new ClickHouseConnection(settings);
721-
await conn.OpenAsync();
689+
// Arrange - create a shared client
690+
using var sharedClient = new ClickHouseClient("Host=localhost");
691+
var conn = new ClickHouseConnection(sharedClient);
722692

723-
var result = await conn.PingAsync();
693+
// Act - change connection string (which calls ApplySettings internally)
694+
conn.ConnectionString = "Host=otherhost";
724695

725-
Assert.That(result, Is.True);
726-
Assert.That(trackingHandler.Requests.Last().RequestUri.PathAndQuery, Is.EqualTo("/ping"));
696+
// Assert - shared client should still be usable (not disposed)
697+
Assert.DoesNotThrowAsync(async () => await sharedClient.PingAsync());
727698
}
728699

729700
[Test]
730-
public async Task PingAsync_ReturnsFalse_WhenServerReturnsError()
701+
public void ApplySettings_AfterChange_ShouldOwnNewClient()
731702
{
732-
var trackingHandler = new TrackingHandler(request =>
733-
{
734-
return new HttpResponseMessage(HttpStatusCode.InternalServerError);
735-
});
736-
using var httpClient = new HttpClient(trackingHandler);
737-
var settings = new ClickHouseClientSettings { Host = "localhost", HttpClient = httpClient };
738-
using var conn = new ClickHouseConnection(settings);
739-
await conn.OpenAsync();
703+
// Arrange - start with shared client
704+
using var sharedClient = new ClickHouseClient("Host=localhost");
705+
var conn = new ClickHouseConnection(sharedClient);
706+
707+
// Act - change connection string creates a new owned client
708+
conn.ConnectionString = "Host=otherhost";
709+
var newClient = conn.ClickHouseClient;
740710

741-
var result = await conn.PingAsync();
711+
// Assert - new client should be different from shared client
712+
Assert.That(newClient, Is.Not.SameAs(sharedClient));
742713

743-
Assert.That(result, Is.False);
714+
// Dispose connection - should dispose the new client (not the shared one)
715+
conn.Dispose();
716+
Assert.DoesNotThrowAsync(async () => await sharedClient.PingAsync());
744717
}
745718

746719
[Test]
747-
public void PingAsync_ThrowsInvalidOperationException_WhenConnectionNotOpen()
720+
public void ChangeDatabase_ShouldBePerConnection()
748721
{
749-
using var conn = new ClickHouseConnection("Host=localhost");
722+
// Arrange - create shared client with default database
723+
using var sharedClient = new ClickHouseClient("Host=localhost;Database=default");
724+
using var conn1 = new ClickHouseConnection(sharedClient);
725+
using var conn2 = new ClickHouseConnection(sharedClient);
750726

751-
Assert.ThrowsAsync<InvalidOperationException>(() => conn.PingAsync());
727+
// Act - change database on conn1 only
728+
conn1.ChangeDatabase("other_db");
729+
730+
// Assert - conn1 should have new database, conn2 should still have default
731+
Assert.That(conn1.Database, Is.EqualTo("other_db"));
732+
Assert.That(conn2.Database, Is.EqualTo("default"));
752733
}
753734

754735
[Test]
755-
public async Task PingAsync_WithRealServer_ReturnsTrue()
736+
public void ChangeDatabase_ShouldNotAffectClientSettings()
756737
{
757-
var result = await connection.PingAsync();
738+
// Arrange
739+
using var sharedClient = new ClickHouseClient("Host=localhost;Database=default");
740+
using var conn = new ClickHouseConnection(sharedClient);
741+
742+
// Act
743+
conn.ChangeDatabase("other_db");
758744

759-
Assert.That(result, Is.True);
745+
// Assert - client settings should be unchanged
746+
Assert.That(conn.Database, Is.EqualTo("other_db"));
747+
Assert.That(sharedClient.Settings.Database, Is.EqualTo("default"));
760748
}
761749
}

ClickHouse.Driver.Tests/ADO/SessionConnectionTest.cs

Lines changed: 0 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -113,26 +113,4 @@ public async Task Session_WithCustomHttpClient_ShouldWork()
113113
await connection.ExecuteStatementAsync("CREATE TEMPORARY TABLE test_temp_table (value UInt8)");
114114
await connection.ExecuteScalarAsync("SELECT COUNT(*) from test_temp_table");
115115
}
116-
117-
[Test]
118-
public async Task Session_ConcurrentRequests_AreSerialized()
119-
{
120-
var sessionId = "TEST-" + Guid.NewGuid();
121-
var marker = Guid.NewGuid().ToString("N");
122-
123-
using var connection = (ClickHouseConnection)CreateConnection(useSession: true, sessionId);
124-
125-
var stopwatch = System.Diagnostics.Stopwatch.StartNew();
126-
127-
// Two 300ms sleep queries with markers we can find in query_log
128-
var task1 = connection.ExecuteScalarAsync($"SELECT sleep(0.3), 'marker1_{marker}'");
129-
var task2 = connection.ExecuteScalarAsync($"SELECT sleep(0.3), 'marker2_{marker}'");
130-
131-
await Task.WhenAll(task1, task2);
132-
stopwatch.Stop();
133-
134-
// Quick sanity check: should take >550ms if serialized
135-
Assert.That(stopwatch.ElapsedMilliseconds, Is.GreaterThan(550),
136-
$"Requests should be serialized. Expected >550ms but took {stopwatch.ElapsedMilliseconds}ms");
137-
}
138116
}

ClickHouse.Driver.Tests/AbstractConnectionTestFixture.cs

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,13 +9,13 @@ namespace ClickHouse.Driver.Tests;
99
public abstract class AbstractConnectionTestFixture : IDisposable
1010
{
1111
protected readonly ClickHouseConnection connection;
12+
protected readonly ClickHouseClient client;
1213

1314
protected AbstractConnectionTestFixture()
1415
{
15-
connection = TestUtilities.GetTestClickHouseConnection();
16-
using var command = connection.CreateCommand();
17-
command.CommandText = "CREATE DATABASE IF NOT EXISTS test;";
18-
command.ExecuteScalar();
16+
client = TestUtilities.GetTestClickHouseClient();
17+
connection = client.CreateConnection();
18+
client.ExecuteNonQueryAsync("CREATE DATABASE IF NOT EXISTS test;").GetAwaiter().GetResult();
1919
}
2020

2121
protected static string SanitizeTableName(string input)
@@ -31,5 +31,9 @@ protected static string SanitizeTableName(string input)
3131
}
3232

3333
[OneTimeTearDown]
34-
public void Dispose() => connection?.Dispose();
34+
public void Dispose()
35+
{
36+
connection?.Dispose();
37+
client?.Dispose();
38+
}
3539
}

ClickHouse.Driver.Tests/ActivitySourceHelperTests.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ public void ShouldCreateActivity()
3434
};
3535
ActivitySource.AddActivityListener(listener);
3636

37-
using var activity = connection.StartActivity("TestActivity");
37+
using var activity = client.StartActivity("TestActivity");
3838
ClassicAssert.NotNull(activity);
3939
}
4040

0 commit comments

Comments
 (0)