BinaryImport#332
Open
SoftStoneDevelop wants to merge 22 commits into
Open
Conversation
…tchData write data into batches
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds a new, packet-oriented binary insert API to ClickHouseClient, allowing callers to prepare an insert plan once, manually build multiple gzip-compressed RowBinary batches, and send them asynchronously with unique query IDs.
Changes:
- Added
StartInsertAsync(...)to prepare insert metadata and return aBinaryImporthelper. - Introduced
ClickHouseClient.BinaryImportwithStartNewBatch()andSendBatchAsync(...)for user-managed batch creation/sending. - Broadened
InsertPlanvisibility tointernalto support the new helper type.
Contributor
Author
|
Of course, there's still some work to be done (InsertOptions not for BinaryImport completly) to ensure it's ready for merging into 'main,' but I think it's clear what I mean and what benefits the user will receive. If you're generally on board, I can refine the pull request. |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
Reviewed by Cursor Bugbot for commit d458d03. Configure here.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

Implementation of sending data packets manual.
Usage:
Thus, creating packages is the prerogative of the library user, not the library itself. How they choose to do so is entirely up to them. PR for example, to show what I mean.
No attributes needed, no object[] array needed. Where the developer gets the data to write to
Writeis his problem and solution.Based on the benchmark results, 'BinaryImport' is faster and consumes less memory for any collection size.
Benchmark result:
#328
Note
Medium Risk
Adds a new streaming/batched insert path and refactors ClickHouse binary type serialization to use generic
Write<T>/CanWrite<T>, which could affect all binary inserts if any type coercion or null-handling behavior changes.Overview
Adds a new
ClickHouseClient.StartInsertAsyncAPI that returns aBinaryImporthelper, letting callers manually build and send multiple gzipped RowBinary batches viaBatchData.WriteData/SendBatchAsyncwith per-batch query IDs.Refactors the binary type system to use generic
Write<T>(andCanWrite<T>where applicable) across manyClickHouseTypeimplementations, with minor coercion tweaks (e.g., numeric/UUID/IP parsing) and corresponding test updates for null cases. Adds a benchmark comparingBinaryImportto existingobject[]and POCO binary insert paths, plus a new test covering inserts with a subset of table columns.Reviewed by Cursor Bugbot for commit b459199. Bugbot is set up for automated code reviews on this repo. Configure here.