Fix ADO.NET: apply CommandTimeout as max_execution_time

[polyglotAI-bot]

Description

Fixes #618.

ClickHouseCommand.CommandTimeout (the DbCommand.CommandTimeout override) was a dead
auto-property. BuildQueryOptions() populated QueryId, BearerToken, Database, Roles,
CustomSettings and AcceptEncoding, but never QueryOptions.MaxExecutionTime — the one option
that carries a timeout to the server. Nothing else read the property either, so a command set to
time out after N seconds ran to completion: the server kept working and the caller kept waiting.
ADO.NET consumers (Dapper, EF Core, linq2db) configure timeouts through exactly this property, so
the ADO.NET surface could not bound execution at all while the lower-level IClickHouseClient API
could.

BuildQueryOptions() now maps a non-zero CommandTimeout onto MaxExecutionTime, which the
existing URI builder emits as max_execution_time. Zero — the ADO.NET "no limit" value and this
property's default — leaves the setting alone, as does a negative value, so callers that never set
the property see no change. A max_execution_time entry in the command's own CustomSettings
states the same intent in the server's own terms and is equally specific, so it keeps precedence;
CommandTimeout is command-scoped and therefore outranks a connection-level max_execution_time.

Scope note: this bounds execution server-side only. The overall wait for a response stays bounded by
the connection-level Timeout (HttpClient.Timeout, 2 minutes by default). A per-command
client-side abort would change the lifetime of a streaming ClickHouseDataReader, which is a
separate behavioral decision and is deliberately not part of this fix.

Changes

• ClickHouse.Driver/ADO/ClickHouseCommand.cs: BuildQueryOptions() sets MaxExecutionTime from
  CommandTimeout when it is positive and the command's CustomSettings does not already specify
  max_execution_time; doc comment updated to describe the behavior and precedence.
• docs/overview.mdx: documented CommandTimeout in the ADO.NET command section.
• changelog.d/618-command-timeout.fixes.md: changelog fragment.
• ClickHouse.Driver.Tests/ADO/CommandTimeoutTests.cs: new tests.

Test

CommandTimeoutTests runs against a real server:

• ExecuteAsync_WithCommandTimeout_LongQueryIsCancelledByServer — parametrized over all four
  execution paths that reach BuildQueryOptions (ExecuteScalarAsync, ExecuteNonQueryAsync,
  ExecuteReaderAsync, ExecuteRawResultAsync): SELECT sleep(3) with CommandTimeout = 1 must
  fail with error 159 (TIMEOUT_EXCEEDED). On main every case returns a result instead.
• ExecuteScalarAsync_WithCommandTimeout_AppliesMaxExecutionTimeSetting — getSetting('max_execution_time')
  reports 30 for CommandTimeout = 30; on main it reports the server default.
• Default (0) and negative values leave max_execution_time at the server default.
• Precedence: a command-level CustomSettings["max_execution_time"] wins over CommandTimeout, and
  CommandTimeout wins over a connection-level set_max_execution_time.

dotnet test ClickHouse.Driver.Tests --framework net10.0, filtered to the ADO, QueryOptions,
UriBuilder and ORM suites: 1792 passed, 0 failed. dotnet run scripts/changelog.cs -- --check is OK.

Pre-PR validation gate

• Deterministic repro confirmed (new tests fail on main, pass on this branch)
• Root cause documented above
• Fix targets the root cause (the single site that builds the command's QueryOptions)
• Test fails without fix, passes with fix
• No existing tests broken, none weakened
• Convention compliance verified per AGENTS.md (NUnit, real-server tests, [TestCase]
  parametrization, changelog fragment, docs updated)
• No public API surface change — CommandTimeout is already in PublicAPI.Shipped.txt

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[Copilot]

🟡 Changes recommended

Review assessments request clarification of timeout semantics and documentation before approval.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

This PR maps positive ClickHouseCommand.CommandTimeout values to ClickHouse’s server-side max_execution_time.

Changes:

• Adds timeout mapping with command-level setting precedence.
• Adds real-server tests for execution paths, defaults, negatives, and precedence.
• Updates documentation and changelog.

File summaries

File	Summary
docs/overview.mdx	Documents CommandTimeout; nit (2 votes) to clarify inherited settings for zero/negative values.
ClickHouse.Driver/ADO/ClickHouseCommand.cs	Applies command timeouts; nits (3, 1 votes) to clarify positive-value behavior and streaming response timeout semantics.
ClickHouse.Driver.Tests/ADO/CommandTimeoutTests.cs	Adds coverage for timeout behavior and precedence.
changelog.d/618-command-timeout.fixes.md	Records the fix; nit (2 votes) to change “non-zero” to “positive” or document negative values.

Review details

Suppressed comments (2)

ClickHouse.Driver/ADO/ClickHouseCommand.cs:53

• The XML docs repeat an inaccurate client-timeout guarantee: this command path uses ResponseHeadersRead, so HttpClient.Timeout does not necessarily cover later streaming-body consumption. Clarify that it bounds the response-header wait and that CommandTimeout remains server-side only for a streaming reader.

    /// precedence over this property; a connection-level one does not. The overall wait for a
    /// response is bounded separately by <see cref="ClickHouseClientSettings.Timeout"/>.

docs/overview.mdx:1382

• These statements overpromise the effective timeout. The implementation only supplies MaxExecutionTime for positive command values, so zero/negative leaves a connection-level max_execution_time active; additionally PostSqlQueryAsync uses ResponseHeadersRead, so the connection Timeout does not bound consuming a streaming reader or raw result after headers arrive. Please document positive-timeout precedence and the initial-response limit, or change the implementation if a whole-operation client deadline is intended.

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.

• Files reviewed: 4/4 changed files
• Comments generated: 3
• Review effort level: Lite

---

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.