Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds cancellation-token overloads to ClickHouseRawResult read/copy APIs so callers can cancel ReadAsStringAsync, ReadAsByteArrayAsync, stream reads, decompressed-stream reads, and CopyToAsync, aligning the raw-result surface with typical .NET async cancellation patterns.
Changes:
- Added
CancellationTokenoverloads forReadAsStreamAsync,ReadDecompressedStreamAsync,ReadAsByteArrayAsync,ReadAsStringAsync, andCopyToAsynconClickHouseRawResult. - Expanded ADO test coverage to validate the new overloads and canceled-token behavior for buffering/copying operations.
- Added a changelog fragment for issue #546.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| ClickHouse.Driver/ADO/Readers/ClickHouseRawResult.cs | Adds cancellation-token overloads for raw result reading/copying APIs. |
| ClickHouse.Driver.Tests/ADO/RawResultReaderAsyncTests.cs | Adds/renames tests to cover new cancellation overloads and cancellation behavior. |
| ClickHouse.Driver.Tests/ADO/ClickHouseRawResultDecompressionTests.cs | Adds cancellation-related tests for decompressed stream reads. |
| changelog.d/546-raw-result-reader-cancellation-api.improvements.md | Changelog fragment documenting the user-facing improvement. |
Suppressed comments (1)
ClickHouse.Driver/ADO/Readers/ClickHouseRawResult.cs:130
ReadDecompressedStreamAsync(CancellationToken)currently relies onHttpContent.ReadAsStreamAsync(cancellationToken)for cancellation. If that underlying overload ignores cancellation, this method won’t honor an already-canceled token (and the added test that expectsOperationCanceledExceptionbecomes unreliable). Add an explicit pre-check before acquiring/wrapping the content stream.
if (decompressedStream != null)
return decompressedStream;
var rawStream = await response.Content.ReadAsStreamAsync(cancellationToken).ConfigureAwait(false);
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
There are 2 total unresolved issues (including 1 from previous review).
Reviewed by Cursor Bugbot for commit 46a60eb. Configure here.
b08c2ba to
b3223f8
Compare

Summary
Fixes #546
Implement ClickHouseRawResult cancellation APIs
Warning: A test for passing a cancelled token to ReadAsStreamAsync was not added, as it is ignored in .NET. - Link
Checklist
Delete items not relevant to your PR: