Conversation
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Pull request overview
This PR extends ClickHouseClient’s binary insert pipeline to accept strongly-typed POCOs (IEnumerable<T>) via a new InsertBinaryAsync<T> overload, using upfront type registration to precompute property→column mappings and compiled getters, with optional attribute-based column naming/type hints to reduce or eliminate schema probing.
Changes:
- Add POCO binary insert API (
InsertBinaryAsync<T>) + registration (RegisterBinaryInsertType<T>) with[ClickHouseColumn]/[ClickHouseNotMapped]attributes. - Introduce POCO-specific batching/serialization components mirroring the existing object[] bulk insert path.
- Add examples, benchmarks, and extensive NUnit coverage for POCO insert behavior (mapping, exclusion, defaults, schema-probe skipping).
Reviewed changes
Copilot reviewed 21 out of 21 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| RELEASENOTES.md | Document POCO binary insert feature and attribute-based mapping/schema probe skip. |
| CHANGELOG.md | Mirror release notes entry for the new POCO insert feature. |
| examples/README.md | Add the new POCO insert example to the index. |
| examples/Program.cs | Wire the new example into the example runner. |
| examples/Insert/Insert_009_PocoInsert.cs | New end-to-end example demonstrating POCO insert + attributes + explicit types. |
| ClickHouse.Driver/InsertOptions.cs | Add internal helper to apply column type maps derived from POCO registration. |
| ClickHouse.Driver/IClickHouseClient.cs | Add new public POCO insert + registration APIs on the interface. |
| ClickHouse.Driver/ClickHouseClient.cs | Implement POCO insert pipeline; factor shared insert setup into PrepareInsertAsync(). |
| ClickHouse.Driver/Attributes/ClickHouseColumnAttribute.cs | New attribute for column name/type mapping. |
| ClickHouse.Driver/Attributes/ClickHouseNotMappedAttribute.cs | New attribute to exclude properties from mapping. |
| ClickHouse.Driver/Copy/BinaryInsertTypeRegistry.cs | New registry building mappings + compiled getters for POCO inserts. |
| ClickHouse.Driver/Copy/BinaryInsertTypeMapping.cs | New mapping model for registered POCOs (properties/getters/optional ColumnTypes). |
| ClickHouse.Driver/Copy/BinaryInsertPropertyInfo.cs | New cached property metadata model. |
| ClickHouse.Driver/Copy/PocoBatch.cs | New batch container for pooled POCO rows. |
| ClickHouse.Driver/Copy/Serializer/IPocoRowSerializer.cs | New row-serializer abstraction for POCO rows. |
| ClickHouse.Driver/Copy/Serializer/PocoRowBinarySerializer.cs | RowBinary POCO row serializer. |
| ClickHouse.Driver/Copy/Serializer/PocoRowBinaryWithDefaultsSerializer.cs | RowBinaryWithDefaults POCO row serializer with DBDefault handling. |
| ClickHouse.Driver/Copy/Serializer/PocoBatchSerializer.cs | Batch serializer for POCO rows (gzip + query header + binary rows). |
| ClickHouse.Driver.Tests/Copy/InsertBinaryPocoTests.cs | New integration tests validating mapping, exclusions, batching, schema-probe behavior. |
| ClickHouse.Driver.Tests/Copy/BinaryInsertTypeRegistryTests.cs | New unit tests for registry validation/duplicate columns/no mappable properties. |
| ClickHouse.Driver.Benchmark/PocoInsertBenchmark.cs | New benchmark comparing object[] vs POCO insert performance. |
| namespace ClickHouse.Driver.Tests.Copy; | ||
|
|
||
| [TestFixture] | ||
| public class BinaryInsertTypeRegistryTests |
There was a problem hiding this comment.
I think we should add a few more edge cases
- two columns with the same name
- column with reserved name
There was a problem hiding this comment.
two columns with the same name
Already covered by PocoWithDuplicateColumnNames.
| /// <param name="stream">The output stream (typically a recyclable memory stream).</param> | ||
| public void Serialize<T>(PocoBatch<T> batch, Func<T, object>[] getters, Stream stream) | ||
| { | ||
| using var gzipStream = new BufferedStream(new GZipStream(stream, CompressionLevel.Fastest, true), 256 * 1024); |
There was a problem hiding this comment.
i tihnk we should have some of the params of GZipStream configerable
There was a problem hiding this comment.
Sounds good but let's leave that for another PR. I added a story for it.
f1874d0 to
14c229a
Compare
Right now the binary insert requires users to provide
object[][]. This PR lets them use POCOs instead.InsertBinaryAsync<T>()method for inserting.[ClickHouseColumn(Name, Type)]for setting column name/type,[ClickHouseNotMapped]for exclusion. If all properties are covered, we skip the schema probe.RegisterBinaryInstertType<T>().This saves the serialization info and compiles property accessors intoFunc<T, object>delegates to avoid reflection when we're reading the properties in the serialization loop.PrepareInsertAsync().New benchmark added to compare object[][] and POCO serialization. Current results:
Related docs PR: ClickHouse/clickhouse-docs#5919