From 7eec06dc8d1f0ad0b8088f5415484f513f7fd081 Mon Sep 17 00:00:00 2001 From: Polyglot AI <293096396+polyglotAI-bot@users.noreply.github.com> Date: Thu, 17 Sep 2026 08:37:00 +0000 Subject: [PATCH 1/3] Fix ADO.NET: apply CommandTimeout as max_execution_time ClickHouseCommand.CommandTimeout was a dead auto-property: BuildQueryOptions never populated QueryOptions.MaxExecutionTime, so no server-side limit reached ClickHouse and a command set to time out ran to completion. A non-zero CommandTimeout is now mapped onto max_execution_time for every execution path. Zero (the ADO.NET "no limit" value and the default) and a negative value leave the setting alone, so existing callers are unaffected. An explicit max_execution_time in the command's own CustomSettings is equally specific and keeps precedence. Fixes: https://github.com/ClickHouse/clickhouse-cs/issues/618 --- .../ADO/CommandTimeoutTests.cs | 112 ++++++++++++++++++ ClickHouse.Driver/ADO/ClickHouseCommand.cs | 21 +++- changelog.d/618-command-timeout.fixes.md | 4 + docs/overview.mdx | 15 +++ 4 files changed, 150 insertions(+), 2 deletions(-) create mode 100644 ClickHouse.Driver.Tests/ADO/CommandTimeoutTests.cs create mode 100644 changelog.d/618-command-timeout.fixes.md diff --git a/ClickHouse.Driver.Tests/ADO/CommandTimeoutTests.cs b/ClickHouse.Driver.Tests/ADO/CommandTimeoutTests.cs new file mode 100644 index 000000000..d04ceb0ba --- /dev/null +++ b/ClickHouse.Driver.Tests/ADO/CommandTimeoutTests.cs @@ -0,0 +1,112 @@ +using System; +using System.Threading.Tasks; +using ClickHouse.Driver.ADO; + +namespace ClickHouse.Driver.Tests.ADO; + +public class CommandTimeoutTests : AbstractConnectionTestFixture +{ + public enum ExecutionPath + { + Scalar, + NonQuery, + Reader, + RawResult, + } + + [Test] + public async Task ExecuteScalarAsync_WithCommandTimeout_AppliesMaxExecutionTimeSetting() + { + using var command = connection.CreateCommand("SELECT getSetting('max_execution_time')"); + command.CommandTimeout = 30; + + Assert.That(await command.ExecuteScalarAsync(), Is.EqualTo(30UL)); + } + + [TestCase(ExecutionPath.Scalar)] + [TestCase(ExecutionPath.NonQuery)] + [TestCase(ExecutionPath.Reader)] + [TestCase(ExecutionPath.RawResult)] + public void ExecuteAsync_WithCommandTimeout_LongQueryIsCancelledByServer(ExecutionPath path) + { + using var command = connection.CreateCommand("SELECT sleep(3)"); + command.CommandTimeout = 1; + + var ex = Assert.ThrowsAsync(async () => await ExecuteAsync(command, path)); + + // TIMEOUT_EXCEEDED = 159 + Assert.That(ex!.ErrorCode, Is.EqualTo(159)); + } + + [Test] + public async Task ExecuteScalarAsync_WithDefaultCommandTimeout_LeavesMaxExecutionTimeUnchanged() + { + var serverDefault = await client.ExecuteScalarAsync("SELECT getSetting('max_execution_time')"); + + using var command = connection.CreateCommand("SELECT getSetting('max_execution_time')"); + + Assert.That(await command.ExecuteScalarAsync(), Is.EqualTo(serverDefault)); + } + + [Test] + public async Task ExecuteScalarAsync_WithNegativeCommandTimeout_LeavesMaxExecutionTimeUnchanged() + { + var serverDefault = await client.ExecuteScalarAsync("SELECT getSetting('max_execution_time')"); + + using var command = connection.CreateCommand("SELECT getSetting('max_execution_time')"); + command.CommandTimeout = -1; + + Assert.That(await command.ExecuteScalarAsync(), Is.EqualTo(serverDefault)); + } + + [Test] + public async Task ExecuteScalarAsync_WithCommandTimeoutAndCommandCustomSetting_PrefersCustomSetting() + { + using var command = connection.CreateCommand("SELECT getSetting('max_execution_time')"); + command.CommandTimeout = 30; + command.CustomSettings.Add("max_execution_time", 45); + + Assert.That(await command.ExecuteScalarAsync(), Is.EqualTo(45UL)); + } + + [Test] + public async Task ExecuteScalarAsync_WithCommandTimeoutAndConnectionCustomSetting_PrefersCommandTimeout() + { + var builder = TestUtilities.GetConnectionStringBuilder(); + builder["set_max_execution_time"] = 45; + using var connectionWithSetting = new ClickHouseConnection(builder.ConnectionString); + + using var command = connectionWithSetting.CreateCommand("SELECT getSetting('max_execution_time')"); + command.CommandTimeout = 30; + + Assert.That(await command.ExecuteScalarAsync(), Is.EqualTo(30UL)); + } + + private static async Task ExecuteAsync(ClickHouseCommand command, ExecutionPath path) + { + switch (path) + { + case ExecutionPath.Scalar: + await command.ExecuteScalarAsync(); + break; + case ExecutionPath.NonQuery: + await command.ExecuteNonQueryAsync(); + break; + case ExecutionPath.Reader: + using (var reader = await command.ExecuteReaderAsync()) + { + while (await reader.ReadAsync()) + { + } + } + + break; + case ExecutionPath.RawResult: + using (await command.ExecuteRawResultAsync(default)) + { + } + + break; + } + } +} diff --git a/ClickHouse.Driver/ADO/ClickHouseCommand.cs b/ClickHouse.Driver/ADO/ClickHouseCommand.cs index 40e0a3914..9b4da54a1 100644 --- a/ClickHouse.Driver/ADO/ClickHouseCommand.cs +++ b/ClickHouse.Driver/ADO/ClickHouseCommand.cs @@ -45,7 +45,12 @@ public ClickHouseCommand(ClickHouseConnection connection) public override string CommandText { get; set; } /// - /// Gets or sets the command timeout in seconds. Not currently used by ClickHouse. + /// Gets or sets the command timeout in seconds. The value is sent to the server as the + /// max_execution_time setting, so the server cancels the query once it is exceeded. + /// Zero, the default, means no limit, as does a negative value. A max_execution_time + /// entry in this command's is equally specific and takes + /// precedence over this property; a connection-level one does not. The overall wait for a + /// response is bounded separately by . /// public override int CommandTimeout { get; set; } @@ -239,14 +244,26 @@ private async Task PostSqlQueryAsync(string sqlQuery, Cance private QueryOptions BuildQueryOptions() { + var settings = customSettings?.Count > 0 ? customSettings : null; + return new QueryOptions { QueryId = QueryId, BearerToken = BearerToken, Database = connection?.Database, Roles = roles?.Count > 0 ? roles : null, - CustomSettings = customSettings?.Count > 0 ? customSettings : null, + CustomSettings = settings, AcceptEncoding = AcceptEncoding, + MaxExecutionTime = GetMaxExecutionTime(settings), }; } + + // CommandTimeout carries the ADO.NET timeout contract, which ClickHouse enforces through the + // max_execution_time setting. Zero (the ADO.NET "no limit" value, and this property's default) + // leaves the setting alone, as does a negative value. An explicit max_execution_time in + // CustomSettings states the same thing in the server's own terms, so it wins. + private TimeSpan? GetMaxExecutionTime(IDictionary settings) => + CommandTimeout > 0 && settings?.ContainsKey("max_execution_time") != true + ? TimeSpan.FromSeconds(CommandTimeout) + : null; } diff --git a/changelog.d/618-command-timeout.fixes.md b/changelog.d/618-command-timeout.fixes.md new file mode 100644 index 000000000..1fbf67281 --- /dev/null +++ b/changelog.d/618-command-timeout.fixes.md @@ -0,0 +1,4 @@ +* Fixed `ClickHouseCommand.CommandTimeout` being ignored ([#618](https://github.com/ClickHouse/clickhouse-cs/issues/618)). + A non-zero value is now sent as the `max_execution_time` setting, so the server cancels a query + that exceeds it. Zero, the default, still means no limit, and an explicit `max_execution_time` in + the command's `CustomSettings` takes precedence. diff --git a/docs/overview.mdx b/docs/overview.mdx index 17537a1ad..d40dfa6d9 100644 --- a/docs/overview.mdx +++ b/docs/overview.mdx @@ -1366,6 +1366,21 @@ Command methods: - `ExecuteScalarAsync()` - Returns first column of first row - `ExecuteReaderAsync()` - Returns a `ClickHouseDataReader` for iterating results +`CommandTimeout` bounds execution server-side: the value in seconds is sent as the +`max_execution_time` setting, so ClickHouse stops the query once it is exceeded — with the default +`timeout_overflow_mode`, as a `ClickHouseServerException` with error code 159 (`TIMEOUT_EXCEEDED`). +Zero, the default, means no limit, as does a negative value. + +```csharp +await using var command = connection.CreateCommand("SELECT count() FROM huge_table"); +command.CommandTimeout = 30; // seconds +``` + +A `max_execution_time` entry in the command's own `CustomSettings` is equally specific and takes +precedence; a connection-level `max_execution_time` is less specific and does not. The overall wait +for a response is bounded separately by the connection string's `Timeout` (2 minutes by default), +which applies to the HTTP request as a whole. + --- ### Using ClickHouseDataReader {#ado-net-reader} From f2ba61c6dd65c6cd8d48b4147a879bd912f4fda4 Mon Sep 17 00:00:00 2001 From: Polyglot AI <293096396+polyglotAI-bot@users.noreply.github.com> Date: Thu, 17 Sep 2026 08:56:41 +0000 Subject: [PATCH 2/3] Narrow CommandTimeout docs to positive values MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review feedback on #619: the docs overstated the effect of the property. Only a positive CommandTimeout maps to max_execution_time, so zero and negative values add no command-level limit — they do not remove a max_execution_time inherited from the connection or the server profile. The property is also a server-side limit only and does not cancel the client-side wait. Adds a regression test pinning that CommandTimeout of 0 or -1 keeps a connection-level max_execution_time active. --- ClickHouse.Driver.Tests/ADO/CommandTimeoutTests.cs | 14 ++++++++++++++ ClickHouse.Driver/ADO/ClickHouseCommand.cs | 14 +++++++++----- changelog.d/618-command-timeout.fixes.md | 7 ++++--- docs/overview.mdx | 14 +++++++++----- 4 files changed, 36 insertions(+), 13 deletions(-) diff --git a/ClickHouse.Driver.Tests/ADO/CommandTimeoutTests.cs b/ClickHouse.Driver.Tests/ADO/CommandTimeoutTests.cs index d04ceb0ba..14dd210ad 100644 --- a/ClickHouse.Driver.Tests/ADO/CommandTimeoutTests.cs +++ b/ClickHouse.Driver.Tests/ADO/CommandTimeoutTests.cs @@ -59,6 +59,20 @@ public async Task ExecuteScalarAsync_WithNegativeCommandTimeout_LeavesMaxExecuti Assert.That(await command.ExecuteScalarAsync(), Is.EqualTo(serverDefault)); } + [TestCase(0)] + [TestCase(-1)] + public async Task ExecuteScalarAsync_WithoutPositiveCommandTimeout_KeepsConnectionCustomSetting(int commandTimeout) + { + var builder = TestUtilities.GetConnectionStringBuilder(); + builder["set_max_execution_time"] = 45; + using var connectionWithSetting = new ClickHouseConnection(builder.ConnectionString); + + using var command = connectionWithSetting.CreateCommand("SELECT getSetting('max_execution_time')"); + command.CommandTimeout = commandTimeout; + + Assert.That(await command.ExecuteScalarAsync(), Is.EqualTo(45UL)); + } + [Test] public async Task ExecuteScalarAsync_WithCommandTimeoutAndCommandCustomSetting_PrefersCustomSetting() { diff --git a/ClickHouse.Driver/ADO/ClickHouseCommand.cs b/ClickHouse.Driver/ADO/ClickHouseCommand.cs index 9b4da54a1..b240466bc 100644 --- a/ClickHouse.Driver/ADO/ClickHouseCommand.cs +++ b/ClickHouse.Driver/ADO/ClickHouseCommand.cs @@ -45,12 +45,16 @@ public ClickHouseCommand(ClickHouseConnection connection) public override string CommandText { get; set; } /// - /// Gets or sets the command timeout in seconds. The value is sent to the server as the + /// Gets or sets the command timeout in seconds. A positive value is sent to the server as the /// max_execution_time setting, so the server cancels the query once it is exceeded. - /// Zero, the default, means no limit, as does a negative value. A max_execution_time - /// entry in this command's is equally specific and takes - /// precedence over this property; a connection-level one does not. The overall wait for a - /// response is bounded separately by . + /// Zero, the default, and negative values add no command-level limit; they do not remove a + /// max_execution_time inherited from the connection or the server profile. A + /// max_execution_time entry in this command's is equally + /// specific and takes precedence over this property; a connection-level one does not. + /// This property is a server-side limit only: it does not cancel the client-side wait. The + /// HTTP request is bounded by , and results are + /// streamed, so use the overloads of the + /// asynchronous execute methods to abort a long read from the client side. /// public override int CommandTimeout { get; set; } diff --git a/changelog.d/618-command-timeout.fixes.md b/changelog.d/618-command-timeout.fixes.md index 1fbf67281..6e3271349 100644 --- a/changelog.d/618-command-timeout.fixes.md +++ b/changelog.d/618-command-timeout.fixes.md @@ -1,4 +1,5 @@ * Fixed `ClickHouseCommand.CommandTimeout` being ignored ([#618](https://github.com/ClickHouse/clickhouse-cs/issues/618)). - A non-zero value is now sent as the `max_execution_time` setting, so the server cancels a query - that exceeds it. Zero, the default, still means no limit, and an explicit `max_execution_time` in - the command's `CustomSettings` takes precedence. + A positive value is now sent as the `max_execution_time` setting, so the server cancels a query + that exceeds it. Zero, the default, and negative values add no command-level limit and leave an + inherited `max_execution_time` untouched, and an explicit `max_execution_time` in the command's + `CustomSettings` takes precedence. diff --git a/docs/overview.mdx b/docs/overview.mdx index d40dfa6d9..fc68de48e 100644 --- a/docs/overview.mdx +++ b/docs/overview.mdx @@ -1366,10 +1366,11 @@ Command methods: - `ExecuteScalarAsync()` - Returns first column of first row - `ExecuteReaderAsync()` - Returns a `ClickHouseDataReader` for iterating results -`CommandTimeout` bounds execution server-side: the value in seconds is sent as the +`CommandTimeout` bounds execution server-side: a positive value in seconds is sent as the `max_execution_time` setting, so ClickHouse stops the query once it is exceeded — with the default `timeout_overflow_mode`, as a `ClickHouseServerException` with error code 159 (`TIMEOUT_EXCEEDED`). -Zero, the default, means no limit, as does a negative value. +Zero, the default, and negative values add no command-level limit. They do not remove a limit: a +`max_execution_time` inherited from the connection or from the server profile still applies. ```csharp await using var command = connection.CreateCommand("SELECT count() FROM huge_table"); @@ -1377,9 +1378,12 @@ command.CommandTimeout = 30; // seconds ``` A `max_execution_time` entry in the command's own `CustomSettings` is equally specific and takes -precedence; a connection-level `max_execution_time` is less specific and does not. The overall wait -for a response is bounded separately by the connection string's `Timeout` (2 minutes by default), -which applies to the HTTP request as a whole. +precedence; a connection-level `max_execution_time` is less specific and does not. + +`CommandTimeout` is a server-side limit only — it does not cancel the client-side wait. The +connection string's `Timeout` (2 minutes by default) bounds the HTTP request, and results are +streamed, so use a `CancellationToken` on the asynchronous execute methods to abort a long read +from the client side. --- From 212e0372ec78080cb25102ed6f2df21965122894 Mon Sep 17 00:00:00 2001 From: Alex Soffronow Pagonidis <237136924+alex-clickhouse@users.noreply.github.com> Date: Tue, 29 Sep 2026 13:25:16 +0000 Subject: [PATCH 3/3] Shorten CommandTimeout comments and docs Make the XML doc, the code comment, the changelog fragment and the docs/http.mdx section shorter and more direct. Remove the advice to pass a CancellationToken to the execute methods to stop a long read. ExecuteDbDataReaderAsync disposes its linked token source before it returns the reader, so the token does not reach row reads. The docs now only say that the client does not enforce the limit. Co-Authored-By: Claude Opus 5.5 --- ClickHouse.Driver/ADO/ClickHouseCommand.cs | 20 ++++++-------------- changelog.d/618-command-timeout.fixes.md | 6 ++---- docs/http.mdx | 21 +++++++++------------ 3 files changed, 17 insertions(+), 30 deletions(-) diff --git a/ClickHouse.Driver/ADO/ClickHouseCommand.cs b/ClickHouse.Driver/ADO/ClickHouseCommand.cs index b240466bc..1ae862c21 100644 --- a/ClickHouse.Driver/ADO/ClickHouseCommand.cs +++ b/ClickHouse.Driver/ADO/ClickHouseCommand.cs @@ -45,16 +45,11 @@ public ClickHouseCommand(ClickHouseConnection connection) public override string CommandText { get; set; } /// - /// Gets or sets the command timeout in seconds. A positive value is sent to the server as the - /// max_execution_time setting, so the server cancels the query once it is exceeded. - /// Zero, the default, and negative values add no command-level limit; they do not remove a - /// max_execution_time inherited from the connection or the server profile. A - /// max_execution_time entry in this command's is equally - /// specific and takes precedence over this property; a connection-level one does not. - /// This property is a server-side limit only: it does not cancel the client-side wait. The - /// HTTP request is bounded by , and results are - /// streamed, so use the overloads of the - /// asynchronous execute methods to abort a long read from the client side. + /// Gets or sets the command timeout in seconds. A positive value is sent as the + /// max_execution_time setting, and the server stops a query that runs longer. + /// Zero (the default) and negative values send nothing, so a limit set on the connection or + /// in the server profile still applies. A max_execution_time in + /// overrides this property. The client does not enforce this limit. /// public override int CommandTimeout { get; set; } @@ -262,10 +257,7 @@ private QueryOptions BuildQueryOptions() }; } - // CommandTimeout carries the ADO.NET timeout contract, which ClickHouse enforces through the - // max_execution_time setting. Zero (the ADO.NET "no limit" value, and this property's default) - // leaves the setting alone, as does a negative value. An explicit max_execution_time in - // CustomSettings states the same thing in the server's own terms, so it wins. + // In ADO.NET, a CommandTimeout of 0 means "no limit". private TimeSpan? GetMaxExecutionTime(IDictionary settings) => CommandTimeout > 0 && settings?.ContainsKey("max_execution_time") != true ? TimeSpan.FromSeconds(CommandTimeout) diff --git a/changelog.d/618-command-timeout.fixes.md b/changelog.d/618-command-timeout.fixes.md index 6e3271349..a10de9317 100644 --- a/changelog.d/618-command-timeout.fixes.md +++ b/changelog.d/618-command-timeout.fixes.md @@ -1,5 +1,3 @@ * Fixed `ClickHouseCommand.CommandTimeout` being ignored ([#618](https://github.com/ClickHouse/clickhouse-cs/issues/618)). - A positive value is now sent as the `max_execution_time` setting, so the server cancels a query - that exceeds it. Zero, the default, and negative values add no command-level limit and leave an - inherited `max_execution_time` untouched, and an explicit `max_execution_time` in the command's - `CustomSettings` takes precedence. + A positive value is now sent as `max_execution_time`, unless the command's `CustomSettings` + sets it. Zero (the default) and negative values send nothing. diff --git a/docs/http.mdx b/docs/http.mdx index b3e6902eb..8ee7cae76 100644 --- a/docs/http.mdx +++ b/docs/http.mdx @@ -1366,24 +1366,21 @@ Command methods: - `ExecuteScalarAsync()` - Returns first column of first row - `ExecuteReaderAsync()` - Returns a `ClickHouseDataReader` for iterating results -`CommandTimeout` bounds execution server-side: a positive value in seconds is sent as the -`max_execution_time` setting, so ClickHouse stops the query once it is exceeded — with the default -`timeout_overflow_mode`, as a `ClickHouseServerException` with error code 159 (`TIMEOUT_EXCEEDED`). -Zero, the default, and negative values add no command-level limit. They do not remove a limit: a -`max_execution_time` inherited from the connection or from the server profile still applies. +`CommandTimeout` is a server-side limit in seconds. A positive value is sent as the +`max_execution_time` setting, and the server stops a query that runs longer. With the default +`timeout_overflow_mode`, the command throws a `ClickHouseServerException` with error code 159 +(`TIMEOUT_EXCEEDED`). ```csharp await using var command = connection.CreateCommand("SELECT count() FROM huge_table"); command.CommandTimeout = 30; // seconds ``` -A `max_execution_time` entry in the command's own `CustomSettings` is equally specific and takes -precedence; a connection-level `max_execution_time` is less specific and does not. - -`CommandTimeout` is a server-side limit only — it does not cancel the client-side wait. The -connection string's `Timeout` (2 minutes by default) bounds the HTTP request, and results are -streamed, so use a `CancellationToken` on the asynchronous execute methods to abort a long read -from the client side. +- Zero (the default) and negative values send nothing. A `max_execution_time` set on the + connection or in the server profile still applies. +- A `max_execution_time` in the command's `CustomSettings` overrides `CommandTimeout`. + `CommandTimeout` overrides a `max_execution_time` set on the connection. +- The client does not enforce `CommandTimeout`. ---