Fix ADO.NET: apply CommandTimeout as max_execution_time

ClickHouse.Driver.Tests/ADO/CommandTimeoutTests.cs

@@ -0,0 +1,126 @@
+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));
+    }
+
+    [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()
+    {
+        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,11 @@ 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. A positive value is sent as the
+    /// <c>max_execution_time</c> 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 <c>max_execution_time</c> in
+    /// <see cref="CustomSettings"/> overrides this property. The client does not enforce this limit.
     /// </summary>
     public override int CommandTimeout { get; set; }
 
@@ -239,14 +243,23 @@ 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),
         };
     }
+
+    // In ADO.NET, a CommandTimeout of 0 means "no limit".
+    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,3 @@
+* Fixed `ClickHouseCommand.CommandTimeout` being ignored ([#618](https://github.com/ClickHouse/clickhouse-cs/issues/618)).
+  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.

docs/http.mdx

@@ -1366,6 +1366,22 @@ Command methods:
 - `ExecuteScalarAsync()` - Returns first column of first row
 - `ExecuteReaderAsync()` - Returns a `ClickHouseDataReader` for iterating results
 
+`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
+```
+
+- 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`.
+
 ---
 
 ### Using ClickHouseDataReader {#ado-net-reader}