From 9193ba53ab13ff6c4d03550fffb4ce0a6f79b9e1 Mon Sep 17 00:00:00 2001 From: Alex Soffronow-Pagonidis Date: Thu, 16 Jul 2026 09:37:38 +0200 Subject: [PATCH 1/3] Preserve code-only driver settings on DbConnection/DbDataSource paths Settings that exist only in code and have no connection-string representation (SkipServerCertificateValidation, BearerToken, HttpClient, HttpClientFactory, LoggerFactory, CustomHeaders, ApplicationInfo, EnableDebugMode, and the parameter/read converters) were silently dropped whenever the provider round-tripped a connection through its connection string. The driver's ClickHouseConnection.ConnectionString setter rebuilds Settings purely from the string, so any code-only setting is lost. Two sites in ClickHouseRelationalConnection did this: - EnsureJoinUseNulls (Open/OpenAsync): now injects join_use_nulls via a cloned Settings object for a ClickHouseConnection instead of rewriting ConnectionString. The string rewrite remains only as a fallback for a non-ClickHouse DbConnection. The user opt-out (set_join_use_nulls=0) is still honored. - CreateMasterConnection (CREATE/DROP DATABASE): now clones the driver settings and only swaps Database="default", handing back a context-owned ClickHouseConnection, instead of reconstructing from the connection string. ApplicationInfo (the lib User-Agent tag) is preserved via the copy constructor. Fixes #50. Co-Authored-By: Claude Opus 4.8 --- .../ClickHouseRelationalConnection.cs | 85 ++++++++++--- .../ConnectionSettingsTests.cs | 117 ++++++++++++++++++ 2 files changed, 187 insertions(+), 15 deletions(-) diff --git a/src/EFCore.ClickHouse/Storage/Internal/ClickHouseRelationalConnection.cs b/src/EFCore.ClickHouse/Storage/Internal/ClickHouseRelationalConnection.cs index 33f7084..e75afe5 100644 --- a/src/EFCore.ClickHouse/Storage/Internal/ClickHouseRelationalConnection.cs +++ b/src/EFCore.ClickHouse/Storage/Internal/ClickHouseRelationalConnection.cs @@ -52,32 +52,55 @@ protected override DbConnection CreateDbConnection() /// /// The ClickHouse.Driver HTTP protocol is stateless by default (UseSession=False): /// a standalone SET join_use_nulls = 1 statement does not persist to subsequent - /// queries. Instead the driver applies set_* connection-string parameters as URL - /// parameters on every query, so we must mutate the connection string itself. + /// queries. Instead the driver applies set_* parameters (the driver's + /// CustomSettings) as URL parameters on every query, so we must record the setting on + /// the connection before it is opened. /// /// Applies on all paths. For the connection-string path the setting is already baked in by /// ; this override covers /// the and paths where EF hands us /// connection objects we didn't construct. We only mutate when the connection is Closed /// and the user has not explicitly configured join_use_nulls. + /// + /// For a we inject the setting by copying and reassigning + /// its rather than rewriting + /// . Assigning the connection string rebuilds the + /// driver's settings purely from the string, silently discarding code-only settings such as + /// SkipServerCertificateValidation or a custom HttpClient. Only the string + /// fallback (for a non-ClickHouse ) mutates the connection string. /// public override bool Open(bool errorsExpected = false) { - EnsureJoinUseNullsInConnectionString(); + EnsureJoinUseNulls(); return base.Open(errorsExpected); } public override Task OpenAsync(CancellationToken cancellationToken, bool errorsExpected = false) { - EnsureJoinUseNullsInConnectionString(); + EnsureJoinUseNulls(); return base.OpenAsync(cancellationToken, errorsExpected); } - private void EnsureJoinUseNullsInConnectionString() + private void EnsureJoinUseNulls() { if (_joinNullSemanticsDisabled || DbConnection.State != ConnectionState.Closed) return; + // Prefer mutating the strongly-typed settings so that code-only settings (which have no + // connection-string equivalent) are preserved. Rewriting ConnectionString would rebuild + // the driver's settings from the string alone and drop them. + if (DbConnection is ClickHouseConnection clickHouseConnection) + { + var settings = clickHouseConnection.Settings; + if (settings.CustomSettings.ContainsKey(JoinUseNullsSetting)) + return; + + var updatedSettings = new ClickHouseClientSettings(settings); + updatedSettings.CustomSettings[JoinUseNullsSetting] = "1"; + clickHouseConnection.Settings = updatedSettings; + return; + } + var cs = DbConnection.ConnectionString; if (string.IsNullOrEmpty(cs) || cs.Contains("join_use_nulls", StringComparison.OrdinalIgnoreCase)) @@ -88,26 +111,58 @@ private void EnsureJoinUseNullsInConnectionString() DbConnection.ConnectionString = ClickHouseDataSourceManager.EnsureDefaultSettings(cs); } + private const string JoinUseNullsSetting = "join_use_nulls"; + public IClickHouseRelationalConnection CreateMasterConnection() { - var connectionStringBuilder = new ClickHouseConnectionStringBuilder( - _dataSource?.ConnectionString ?? ConnectionString) - { - Database = "default" - }; + var optionsBuilder = new DbContextOptionsBuilder(); - var masterConnectionString = connectionStringBuilder.ConnectionString; + if (TryGetClickHouseClientSettings(out var settings)) + { + // Clone the driver settings and only swap the database. Round-tripping through a + // connection string here would drop code-only settings that have no connection-string + // representation (SkipServerCertificateValidation, BearerToken, HttpClient, + // HttpClientFactory, LoggerFactory, CustomHeaders, ApplicationInfo, EnableDebugMode, + // and the parameter/read converters), breaking database create/drop against servers + // that depend on them (e.g. a self-signed certificate). The connection is owned by the + // context so it is disposed with the master connection. + var masterSettings = new ClickHouseClientSettings(settings) { Database = "default" }; + optionsBuilder.UseClickHouse(new ClickHouseConnection(masterSettings), contextOwnsConnection: true); + } + else + { + var masterConnectionString = new ClickHouseConnectionStringBuilder(ConnectionString) + { + Database = "default" + }.ConnectionString; - var contextOptions = new DbContextOptionsBuilder() - .UseClickHouse(masterConnectionString) - .Options; + optionsBuilder.UseClickHouse(masterConnectionString); + } return new ClickHouseRelationalConnection( - Dependencies with { ContextOptions = contextOptions }, + Dependencies with { ContextOptions = optionsBuilder.Options }, dataSource: null, _joinNullSemanticsDisabled); } + private bool TryGetClickHouseClientSettings(out ClickHouseClientSettings settings) + { + if (_dataSource is ClickHouseDataSource clickHouseDataSource) + { + settings = clickHouseDataSource.Settings; + return true; + } + + if (DbConnection is ClickHouseConnection clickHouseConnection) + { + settings = clickHouseConnection.Settings; + return true; + } + + settings = null!; + return false; + } + public override IDbContextTransaction BeginTransaction(IsolationLevel isolationLevel) => new ClickHouseTransaction(); diff --git a/test/EFCore.ClickHouse.Tests/ConnectionSettingsTests.cs b/test/EFCore.ClickHouse.Tests/ConnectionSettingsTests.cs index c1430b9..fddc83d 100644 --- a/test/EFCore.ClickHouse.Tests/ConnectionSettingsTests.cs +++ b/test/EFCore.ClickHouse.Tests/ConnectionSettingsTests.cs @@ -1,5 +1,7 @@ using ClickHouse.Driver.ADO; +using ClickHouse.EntityFrameworkCore.Storage.Internal; using Microsoft.EntityFrameworkCore; +using Microsoft.EntityFrameworkCore.Infrastructure; using Xunit; namespace EFCore.ClickHouse.Tests; @@ -96,6 +98,121 @@ public async Task UseClickHouse_DbConnection_RespectsUserOptOut() Assert.False(await GetJoinUseNullsAsync(ctx)); } + [Fact] + public async Task UseClickHouse_DbConnection_PreservesCodeOnlySettings() + { + // Settings that only exist in code (no connection-string equivalent) must survive the + // join_use_nulls injection performed on Open. Previously + // the provider rewrote ConnectionString, which rebuilt the driver's settings from the + // string alone and silently dropped SkipServerCertificateValidation. + var settings = new ClickHouseClientSettings(_fixture.ConnectionString) + { + SkipServerCertificateValidation = true + }; + + await using var connection = new ClickHouseConnection(settings); + await using var ctx = new MinimalContext(o => o.UseClickHouse(connection)); + + Assert.True(await GetJoinUseNullsAsync(ctx)); + Assert.True(connection.Settings.SkipServerCertificateValidation); + } + + [Fact] + public async Task UseClickHouse_DbDataSource_PreservesCodeOnlySettings() + { + var settings = new ClickHouseClientSettings(_fixture.ConnectionString) + { + SkipServerCertificateValidation = true + }; + + await using var dataSource = new ClickHouseDataSource(settings); + await using var ctx = new MinimalContext(o => o.UseClickHouse(dataSource)); + + Assert.True(await GetJoinUseNullsAsync(ctx)); + + var connection = (ClickHouseConnection)ctx.Database.GetDbConnection(); + Assert.True(connection.Settings.SkipServerCertificateValidation); + } + + [Fact] + public void CreateMasterConnection_DbDataSource_PreservesCodeOnlySettings() + { + // The master connection (used for CREATE/DROP DATABASE) must not lose code-only settings + // by round-tripping through a connection string. Exercises several settings that have no + // connection-string representation, not just SkipServerCertificateValidation. + var settings = new ClickHouseClientSettings(_fixture.ConnectionString) + { + SkipServerCertificateValidation = true, + BearerToken = "token-abc", + CustomHeaders = new Dictionary { ["X-Custom"] = "value" } + }; + + using var dataSource = new ClickHouseDataSource(settings); + using var ctx = new MinimalContext(o => o.UseClickHouse(dataSource)); + + var master = ctx.Database.GetService().CreateMasterConnection(); + try + { + var masterConnection = (ClickHouseConnection)master.DbConnection; + Assert.Equal("default", masterConnection.Settings.Database); + Assert.True(masterConnection.Settings.SkipServerCertificateValidation); + Assert.Equal("token-abc", masterConnection.Settings.BearerToken); + Assert.Equal("value", masterConnection.Settings.CustomHeaders["X-Custom"]); + } + finally + { + master.Dispose(); + } + } + + [Fact] + public void CreateMasterConnection_PreservesUserJoinUseNullsOptOut() + { + // If the user opted out via set_join_use_nulls=0, the master connection must carry that + // through so the injection on Open sees it and does not override the choice. + var optOutConnectionString = _fixture.ConnectionString + + (_fixture.ConnectionString.EndsWith(';') ? "" : ";") + + "set_join_use_nulls=0"; + + using var connection = new ClickHouseConnection(optOutConnectionString); + using var ctx = new MinimalContext(o => o.UseClickHouse(connection)); + + var master = ctx.Database.GetService().CreateMasterConnection(); + try + { + var masterConnection = (ClickHouseConnection)master.DbConnection; + Assert.Equal("0", masterConnection.Settings.CustomSettings["join_use_nulls"].ToString()); + } + finally + { + master.Dispose(); + } + } + + [Fact] + public void CreateMasterConnection_DbConnection_PreservesCodeOnlySettings() + { + var settings = new ClickHouseClientSettings(_fixture.ConnectionString) + { + SkipServerCertificateValidation = true + }; + + using var connection = new ClickHouseConnection(settings); + using var ctx = new MinimalContext(o => o.UseClickHouse(connection)); + + var master = ctx.Database.GetService().CreateMasterConnection(); + try + { + var masterConnection = (ClickHouseConnection)master.DbConnection; + Assert.Equal("default", masterConnection.Settings.Database); + Assert.True(masterConnection.Settings.SkipServerCertificateValidation); + } + finally + { + master.Dispose(); + } + } + private static async Task GetJoinUseNullsAsync(DbContext ctx) { var connection = ctx.Database.GetDbConnection(); From 9ee68edcf9bf846eef12fdebb03d2010151533c6 Mon Sep 17 00:00:00 2001 From: Alex Soffronow-Pagonidis Date: Thu, 16 Jul 2026 13:21:51 +0200 Subject: [PATCH 2/3] test: cover non-ClickHouse DbConnection fallback paths Adds a minimal fake DbConnection to exercise the defensive fallback branches in EnsureJoinUseNulls, CreateMasterConnection, and TryGetClickHouseClientSettings that only run when the underlying connection is not a ClickHouseConnection, bringing patch coverage to 100%. Co-Authored-By: Claude Opus 4.8 --- .../ConnectionSettingsTests.cs | 81 +++++++++++++++++++ 1 file changed, 81 insertions(+) diff --git a/test/EFCore.ClickHouse.Tests/ConnectionSettingsTests.cs b/test/EFCore.ClickHouse.Tests/ConnectionSettingsTests.cs index fddc83d..f7098f3 100644 --- a/test/EFCore.ClickHouse.Tests/ConnectionSettingsTests.cs +++ b/test/EFCore.ClickHouse.Tests/ConnectionSettingsTests.cs @@ -213,6 +213,52 @@ public void CreateMasterConnection_DbConnection_PreservesCodeOnlySettings() } } + [Fact] + public void EnsureJoinUseNulls_NonClickHouseConnection_FallsBackToConnectionString() + { + // Defensive fallback: for a DbConnection that is not a ClickHouseConnection we cannot + // mutate driver settings, so join_use_nulls is injected by rewriting the connection string. + using var connection = new FakeDbConnection("Host=localhost;Port=8123"); + using var ctx = new MinimalContext(o => o.UseClickHouse(connection)); + + ctx.Database.OpenConnection(); + + Assert.Contains("set_join_use_nulls=1", connection.ConnectionString, StringComparison.OrdinalIgnoreCase); + } + + [Fact] + public void EnsureJoinUseNulls_NonClickHouseConnection_LeavesExistingSettingUntouched() + { + // If the connection string already configures join_use_nulls, the fallback must not append. + const string configured = "Host=localhost;Port=8123;set_join_use_nulls=0"; + using var connection = new FakeDbConnection(configured); + using var ctx = new MinimalContext(o => o.UseClickHouse(connection)); + + ctx.Database.OpenConnection(); + + Assert.Equal(configured, connection.ConnectionString); + } + + [Fact] + public void CreateMasterConnection_NonClickHouseConnection_FallsBackToConnectionString() + { + // When neither a ClickHouseDataSource nor a ClickHouseConnection is available, the master + // connection is built from the connection string. + using var connection = new FakeDbConnection("Host=localhost;Port=8123;Database=app"); + using var ctx = new MinimalContext(o => o.UseClickHouse(connection)); + + var master = ctx.Database.GetService().CreateMasterConnection(); + try + { + var masterConnection = (ClickHouseConnection)master.DbConnection; + Assert.Equal("default", masterConnection.Settings.Database); + } + finally + { + master.Dispose(); + } + } + private static async Task GetJoinUseNullsAsync(DbContext ctx) { var connection = ctx.Database.GetDbConnection(); @@ -235,6 +281,41 @@ private static async Task GetJoinUseNullsAsync(DbContext ctx) } } +/// +/// Minimal non-ClickHouse used to exercise the +/// provider's defensive fallback paths that only run when the underlying connection is not a +/// . +/// +internal sealed class FakeDbConnection : System.Data.Common.DbConnection +{ + private System.Data.ConnectionState _state = System.Data.ConnectionState.Closed; + + public FakeDbConnection(string connectionString) => ConnectionString = connectionString; + + [System.Diagnostics.CodeAnalysis.AllowNull] + public override string ConnectionString { get; set; } + + public override string Database => "app"; + + public override string DataSource => "fake"; + + public override string ServerVersion => "0.0"; + + public override System.Data.ConnectionState State => _state; + + public override void Open() => _state = System.Data.ConnectionState.Open; + + public override void Close() => _state = System.Data.ConnectionState.Closed; + + public override void ChangeDatabase(string databaseName) { } + + protected override System.Data.Common.DbTransaction BeginDbTransaction(System.Data.IsolationLevel isolationLevel) + => throw new NotSupportedException(); + + protected override System.Data.Common.DbCommand CreateDbCommand() + => throw new NotSupportedException(); +} + public class ConnectionSettingsFixture : IAsyncLifetime { public string ConnectionString { get; private set; } = string.Empty; From 45edc2d5e78ba3b3a1d3c6e63e3619dea018a572 Mon Sep 17 00:00:00 2001 From: Alex Soffronow-Pagonidis Date: Thu, 16 Jul 2026 13:26:18 +0200 Subject: [PATCH 3/3] Address review: master fallback reads data source connection string - CreateMasterConnection fallback now uses `_dataSource?.ConnectionString ?? ConnectionString` so a non-ClickHouse DbDataSource (which sets no connection string on the EF options) builds a valid master connection string instead of an empty one. - Corrected the EnsureJoinUseNulls XML doc to reflect that the setting is recorded on ClickHouseConnection.Settings, with the connection-string path used only as a fallback. - Added a regression test covering the non-ClickHouse DbDataSource path. Co-Authored-By: Claude Opus 4.8 --- .../ClickHouseRelationalConnection.cs | 13 ++++--- .../ConnectionSettingsTests.cs | 36 +++++++++++++++++++ 2 files changed, 44 insertions(+), 5 deletions(-) diff --git a/src/EFCore.ClickHouse/Storage/Internal/ClickHouseRelationalConnection.cs b/src/EFCore.ClickHouse/Storage/Internal/ClickHouseRelationalConnection.cs index e75afe5..7441b51 100644 --- a/src/EFCore.ClickHouse/Storage/Internal/ClickHouseRelationalConnection.cs +++ b/src/EFCore.ClickHouse/Storage/Internal/ClickHouseRelationalConnection.cs @@ -45,10 +45,12 @@ protected override DbConnection CreateDbConnection() protected override bool SupportsAmbientTransactions => false; /// - /// Ensures join_use_nulls=1 is present in the underlying connection's connection - /// string before it's opened. Required because ClickHouse's default (0) makes LEFT JOIN - /// return column defaults rather than NULL, which breaks EF Core's null-based navigation - /// detection. + /// Ensures join_use_nulls=1 is applied to the underlying connection before it is + /// opened. Required because ClickHouse's default (0) makes LEFT JOIN return column defaults + /// rather than NULL, which breaks EF Core's null-based navigation detection. For a + /// the setting is recorded on its + /// ; the connection-string fallback is used only + /// for a non-ClickHouse . /// /// The ClickHouse.Driver HTTP protocol is stateless by default (UseSession=False): /// a standalone SET join_use_nulls = 1 statement does not persist to subsequent @@ -131,7 +133,8 @@ public IClickHouseRelationalConnection CreateMasterConnection() } else { - var masterConnectionString = new ClickHouseConnectionStringBuilder(ConnectionString) + var masterConnectionString = new ClickHouseConnectionStringBuilder( + _dataSource?.ConnectionString ?? ConnectionString) { Database = "default" }.ConnectionString; diff --git a/test/EFCore.ClickHouse.Tests/ConnectionSettingsTests.cs b/test/EFCore.ClickHouse.Tests/ConnectionSettingsTests.cs index f7098f3..8dc9629 100644 --- a/test/EFCore.ClickHouse.Tests/ConnectionSettingsTests.cs +++ b/test/EFCore.ClickHouse.Tests/ConnectionSettingsTests.cs @@ -259,6 +259,26 @@ public void CreateMasterConnection_NonClickHouseConnection_FallsBackToConnection } } + [Fact] + public void CreateMasterConnection_NonClickHouseDataSource_UsesDataSourceConnectionString() + { + // A non-ClickHouse DbDataSource does not set a connection string on the EF options, so the + // fallback must read it from the data source itself rather than the (empty) options value. + using var dataSource = new FakeDbDataSource("Host=localhost;Port=8123;Database=app"); + using var ctx = new MinimalContext(o => o.UseClickHouse(dataSource)); + + var master = ctx.Database.GetService().CreateMasterConnection(); + try + { + var masterConnection = (ClickHouseConnection)master.DbConnection; + Assert.Equal("default", masterConnection.Settings.Database); + } + finally + { + master.Dispose(); + } + } + private static async Task GetJoinUseNullsAsync(DbContext ctx) { var connection = ctx.Database.GetDbConnection(); @@ -316,6 +336,22 @@ protected override System.Data.Common.DbCommand CreateDbCommand() => throw new NotSupportedException(); } +/// +/// Minimal non-ClickHouse used to verify that +/// CreateMasterConnection falls back to the data source's own connection string. +/// +internal sealed class FakeDbDataSource : System.Data.Common.DbDataSource +{ + private readonly string _connectionString; + + public FakeDbDataSource(string connectionString) => _connectionString = connectionString; + + public override string ConnectionString => _connectionString; + + protected override System.Data.Common.DbConnection CreateDbConnection() + => new FakeDbConnection(_connectionString); +} + public class ConnectionSettingsFixture : IAsyncLifetime { public string ConnectionString { get; private set; } = string.Empty;