diff --git a/ClickHouse.Driver.Tests/ADO/CommandTimeoutTests.cs b/ClickHouse.Driver.Tests/ADO/CommandTimeoutTests.cs new file mode 100644 index 000000000..14dd210ad --- /dev/null +++ b/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(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; + } + } +} diff --git a/ClickHouse.Driver/ADO/ClickHouseCommand.cs b/ClickHouse.Driver/ADO/ClickHouseCommand.cs index 40e0a3914..1ae862c21 100644 --- a/ClickHouse.Driver/ADO/ClickHouseCommand.cs +++ b/ClickHouse.Driver/ADO/ClickHouseCommand.cs @@ -45,7 +45,11 @@ 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. 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; } @@ -239,14 +243,23 @@ 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), }; } + + // 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) + : 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..a10de9317 --- /dev/null +++ b/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. diff --git a/docs/http.mdx b/docs/http.mdx index 72b0d8b6d..8ee7cae76 100644 --- a/docs/http.mdx +++ b/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}