Conversation
alex-clickhouse
force-pushed
the
qbit
branch
from
5c12428 to
cf17550
Compare
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Pull request overview
This PR adds support for ClickHouse's QBit data type, which is a quantized vector type designed for efficient storage of embeddings with configurable precision. The implementation treats QBit as an array wrapper over Float32/Float64/BFloat16 element types, delegating to existing array serialization logic.
Key Changes:
- Added
QBitTypeclass that implements the wire protocol (array format) and type parsing - Registered QBit as a parameterized type with binary decoder support
- Added feature flag for version 25.10 with experimental setting enabled in tests
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| ClickHouse.Driver/Types/QBitType.cs | Core type implementation handling QBit parsing, reading, and writing |
| ClickHouse.Driver/Types/TypeConverter.cs | Registered QBitType as a parameterized type |
| ClickHouse.Driver/Types/BinaryTypeDecoder.cs | Added binary decoder for QBit type index |
| ClickHouse.Driver/Formats/HttpParameterFormatter.cs | Added HTTP parameter formatting for QBit arrays |
| ClickHouse.Driver/ADO/Feature.cs | Added QBit feature flag for version 25.10 |
| ClickHouse.Driver.Tests/Utilities/TestUtilities.cs | Added test data samples and experimental setting enablement |
| ClickHouse.Driver.Tests/BulkCopy/BulkCopyTests.cs | Added bulk copy tests for Float32, Float64, and BFloat16 variants |
| examples/Vector_001_QBitSimilaritySearch.cs | Added comprehensive example demonstrating similarity search use case |
| { | ||
| // QBit wire format is Array(UnderlyingType) | ||
| var length = reader.Read7BitEncodedInt(); | ||
| var data = Array.CreateInstance(ElementType.FrameworkType, Dimension); |
There was a problem hiding this comment.
The loop iterates length times but the array is created with size Dimension. If length != Dimension, this will either leave array elements uninitialized (if length < Dimension) or throw an IndexOutOfRangeException (if length > Dimension). The array should be created with size length, not Dimension.
| var data = Array.CreateInstance(ElementType.FrameworkType, Dimension); | |
| if (length != Dimension) | |
| { | |
| throw new InvalidOperationException( | |
| $"QBit element count mismatch. Expected {Dimension} elements but received {length} for type {this}."); | |
| } | |
| var data = Array.CreateInstance(ElementType.FrameworkType, length); |
| { | ||
| var elementType = FromByteCode(reader, typeSettings); | ||
| var dimension = reader.Read7BitEncodedInt(); | ||
| return new QBitType { ElementType = elementType, Dimension = dimension }; |
There was a problem hiding this comment.
The QBitType instance is missing initialization of the UnderlyingArrayType property, which is used in the Write method. This will cause a NullReferenceException when writing QBit values via binary protocol. Initialize it with new ArrayType { UnderlyingType = elementType }.
| return new QBitType { ElementType = elementType, Dimension = dimension }; | |
| return new QBitType | |
| { | |
| ElementType = elementType, | |
| Dimension = dimension, | |
| UnderlyingArrayType = new ArrayType { UnderlyingType = elementType } | |
| }; |
No description provided.