Fix ADO.NET: apply CommandTimeout as max_execution_time

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<ClickHouseServerException>(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;
+        }
+    }
+}

ClickHouse.Driver/ADO/ClickHouseCommand.cs

@@ -45,7 +45,12 @@ public ClickHouseCommand(ClickHouseConnection connection)
     public override string CommandText { get; set; }
 
     /// <summary>
-    /// 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
+    /// <c>max_execution_time</c> setting, so the server cancels the query once it is exceeded.
+    /// Zero, the default, means no limit, as does a negative value. A <c>max_execution_time</c>
+    /// entry in this command's <see cref="CustomSettings"/> 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 <see cref="ClickHouseClientSettings.Timeout"/>.
     /// </summary>
     public override int CommandTimeout { get; set; }
 
@@ -239,14 +244,26 @@ private async Task<HttpResponseMessage> 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<string, object> settings) =>
+        CommandTimeout > 0 && settings?.ContainsKey("max_execution_time") != true
+            ? TimeSpan.FromSeconds(CommandTimeout)
+            : null;
 }

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.

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}