From ade760354d72f69df814f29300faf6a17778cfee Mon Sep 17 00:00:00 2001 From: Alex Soffronow-Pagonidis Date: Tue, 21 Jul 2026 11:15:49 +0200 Subject: [PATCH 1/5] Add FixedString(N) support for the TCP client MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit FixedString(N) is N contiguous bytes per row with no length prefix, so it is a fixed-width type: the codec reports FixedRowByteSize = N and the insert splitter prices it in O(1). Rows are read in one bulk transfer into a pooled blob at a fixed stride and surfaced as a per-row byte[] via a bespoke FixedStringColumn (zero-copy GetBytes, GetString(encoding), lazy materialized view) — mirroring StringColumn but without an offsets array. byte[] is the honest surface for a byte-oriented type, so embedded NULs and non-UTF-8 bytes round-trip intact. On write, a value is emitted verbatim and right-padded with zero bytes to N; over-length and null rows are rejected (nulls only reach the wire via Nullable, which substitutes the empty-array placeholder). Nullable( FixedString(N)) composes through the reference-nullable shape. Co-Authored-By: Claude Opus 4.8 --- .../Types/FixedStringColumnCodecTests.cs | 180 ++++++++++++++++++ .../Utilities/InsertRoundTripCase.cs | 34 ++++ .../Types/Codecs/FixedStringColumnCodec.cs | 120 ++++++++++++ .../Types/ColumnCodecRegistry.cs | 3 + .../Types/FixedStringColumn.cs | 133 +++++++++++++ 5 files changed, 470 insertions(+) create mode 100644 ClickHouse.Driver.Tcp.Tests/Types/FixedStringColumnCodecTests.cs create mode 100644 ClickHouse.Driver.Tcp/Types/Codecs/FixedStringColumnCodec.cs create mode 100644 ClickHouse.Driver.Tcp/Types/FixedStringColumn.cs diff --git a/ClickHouse.Driver.Tcp.Tests/Types/FixedStringColumnCodecTests.cs b/ClickHouse.Driver.Tcp.Tests/Types/FixedStringColumnCodecTests.cs new file mode 100644 index 000000000..1a414e1a9 --- /dev/null +++ b/ClickHouse.Driver.Tcp.Tests/Types/FixedStringColumnCodecTests.cs @@ -0,0 +1,180 @@ +using System; +using System.IO; +using System.Text; +using System.Threading; +using System.Threading.Tasks; +using ClickHouse.Driver.Tcp.Protocol; +using ClickHouse.Driver.Tcp.Types; + +namespace ClickHouse.Driver.Tcp.Tests.Types; + +[TestFixture] +public class FixedStringColumnCodecTests +{ + private static readonly CancellationToken None = CancellationToken.None; + + [Test] + public void Create_MissingOrNonIntegerOrNonPositiveLength_ThrowsFormat() + { + Assert.Multiple(() => + { + Assert.Throws(() => Resolve("FixedString")); + Assert.Throws(() => Resolve("FixedString(x)")); + Assert.Throws(() => Resolve("FixedString(0)")); + Assert.Throws(() => Resolve("FixedString(-4)")); + Assert.Throws(() => Resolve("FixedString(4, 5)")); + }); + } + + [Test] + public void FixedRowByteSize_IsTheDeclaredWidth() + { + Assert.That(Codec(16).FixedRowByteSize, Is.EqualTo(16)); + } + + [Test] + public async Task WriteColumn_ExactWidthValue_WritesBytesVerbatim() + { + byte[] value = { 0xDE, 0xAD, 0xBE, 0xEF }; + byte[] bytes = await WriteAsync(w => Codec(4).WriteColumn(w, new ArrayColumn("c", "FixedString(4)", new[] { value }))); + + CollectionAssert.AreEqual(value, bytes); + } + + [Test] + public async Task WriteColumn_ShortAndEmptyValues_RightPadsWithZeros() + { + var values = new[] { new byte[] { 1, 2, 3 }, Array.Empty() }; + byte[] bytes = await WriteAsync(w => Codec(6).WriteColumn(w, new ArrayColumn("c", "FixedString(6)", values))); + + CollectionAssert.AreEqual(new byte[] { 1, 2, 3, 0, 0, 0, 0, 0, 0, 0, 0, 0 }, bytes); + } + + [Test] + public async Task WriteColumn_WidthLargerThanZeroRun_PadsAcrossMultipleChunks() + { + // The write path pads with a 64-byte stack zero-run in a loop; a width well past 64 exercises the + // multi-chunk path. A three-byte value into FixedString(200) must emit exactly 200 bytes: the value then + // 197 zeros. + const int width = 200; + byte[] value = { 1, 2, 3 }; + byte[] bytes = await WriteAsync(w => Codec(width).WriteColumn(w, new ArrayColumn("c", $"FixedString({width})", new[] { value }))); + + byte[] expected = new byte[width]; + value.CopyTo(expected, 0); + CollectionAssert.AreEqual(expected, bytes); + } + + [Test] + public async Task WriteColumn_ValueLongerThanWidth_ThrowsArgument() + { + var column = new ArrayColumn("c", "FixedString(2)", new[] { new byte[] { 1, 2, 3 } }); + var ex = await CaptureAsync(w => Codec(2).WriteColumn(w, column)); + + Assert.That(ex, Is.TypeOf()); + } + + [Test] + public async Task WriteColumn_NullRow_ThrowsArgument() + { + var column = new ArrayColumn("c", "FixedString(4)", new byte[][] { null }); + var ex = await CaptureAsync(w => Codec(4).WriteColumn(w, column)); + + Assert.That(ex, Is.TypeOf()); + } + + [Test] + public async Task RoundTrip_MultipleRowsWithEmbeddedNulAndNonUtf8_PreservedAtFixedStride() + { + var values = new[] + { + new byte[] { 0, 0, 0, 0 }, + new byte[] { (byte)'A', 0x00, (byte)'B', 0xFF }, + new byte[] { 0xFF, 0xFE, 0xFD, 0xFC }, + }; + + byte[] bytes = await WriteAsync(w => Codec(4).WriteColumn(w, new ArrayColumn("c", "FixedString(4)", values))); + using var reader = ReaderOver(bytes); + using var column = (FixedStringColumn)await Codec(4).ReadColumnAsync(reader, "c", "FixedString(4)", values.Length, None); + + Assert.Multiple(() => + { + CollectionAssert.AreEqual(values[1], column.GetBytes(1).ToArray()); + Assert.That(column.GetString(1, Encoding.Latin1), Is.EqualTo("A\0Bÿ")); + CollectionAssert.AreEqual(values, column.Values.ToArray()); + }); + } + + [Test] + public async Task ReadColumn_ZeroRows_ReturnsEmptyColumn() + { + using var reader = ReaderOver(Array.Empty()); + using var column = (IColumn)await Codec(4).ReadColumnAsync(reader, "c", "FixedString(4)", 0, None); + + Assert.That(column.RowCount, Is.EqualTo(0)); + } + + [Test] + public async Task ReadColumn_IndexOrGetBytesBeyondRowCount_Throws() + { + // The read path rents the blob from the pool, so it is typically larger than rowCount * N. Access beyond + // RowCount must still fail fast rather than return a stale pooled slot — both before and after the cache + // is materialized by touching Values. + var values = new[] { new byte[] { 1, 2 }, new byte[] { 3, 4 } }; + byte[] bytes = await WriteAsync(w => Codec(2).WriteColumn(w, new ArrayColumn("c", "FixedString(2)", values))); + using var reader = ReaderOver(bytes); + using var column = (FixedStringColumn)await Codec(2).ReadColumnAsync(reader, "c", "FixedString(2)", values.Length, None); + + Assert.Multiple(() => + { + Assert.Throws(() => _ = column.GetBytes(values.Length).Length); + Assert.Throws(() => _ = column[values.Length]); + _ = column.Values.Length; // materialize the cache, then re-check the indexer + Assert.Throws(() => _ = column[values.Length]); + }); + } + + [Test] + public void CanWrite_AcceptsByteArrayColumn_RejectsOthers() + { + Assert.Multiple(() => + { + Assert.That(Codec(4).CanWrite(new ArrayColumn("c", "FixedString(4)", new[] { new byte[4] })), Is.True); + Assert.That(Codec(4).CanWrite(new ArrayColumn("c", "String", new[] { "x" })), Is.False); + }); + } + + private static IColumnCodec Codec(int size) => ColumnCodecRegistry.Default.Resolve($"FixedString({size})", ResolveContext.ForWrite); + + private static void Resolve(string type) => ColumnCodecRegistry.Default.Resolve(type, ResolveContext.ForWrite); + + private static async Task WriteAsync(Action write) + { + using var ms = new MemoryStream(); + using (var writer = new ClickHouseBinaryWriter(ms)) + { + write(writer); + await writer.FlushAsync(None); + } + + return ms.ToArray(); + } + + private static async Task CaptureAsync(Action write) + { + using var ms = new MemoryStream(); + using var writer = new ClickHouseBinaryWriter(ms); + try + { + write(writer); + await writer.FlushAsync(None); + return null; + } + catch (Exception ex) + { + return ex; + } + } + + private static ClickHouseBinaryReader ReaderOver(byte[] bytes) => new(new MemoryStream(bytes)); +} diff --git a/ClickHouse.Driver.Tcp.Tests/Utilities/InsertRoundTripCase.cs b/ClickHouse.Driver.Tcp.Tests/Utilities/InsertRoundTripCase.cs index e7e3c21b9..df0f6b088 100644 --- a/ClickHouse.Driver.Tcp.Tests/Utilities/InsertRoundTripCase.cs +++ b/ClickHouse.Driver.Tcp.Tests/Utilities/InsertRoundTripCase.cs @@ -86,6 +86,19 @@ public static IEnumerable Cases() yield return Strings("String", string.Empty, "hello", "héllo✓", "a\0b", new string('x', 500)); + // FixedString(N): N contiguous bytes per row, surfaced as a per-row byte[] of exactly N bytes. The bytes + // are byte-oriented, so embedded NULs and non-UTF-8 bytes ride along unchanged. + yield return FixedStrings(4, new byte[] { 0, 0, 0, 0 }, new byte[] { 1, 2, 3, 4 }, new byte[] { 0xFF, 0x00, 0xFF, 0x00 }); + + // A value shorter than N is right-padded to N zero bytes by the server, so the read-back differs from the + // inserted bytes; the empty value becomes an all-zero row and a full-width value is unchanged. + yield return new InsertRoundTripCase( + "FixedString(6) [padding]", + "FixedString(6)", + name => new ArrayColumn(name, "FixedString(6)", new[] { Array.Empty(), new byte[] { 1, 2, 3 }, new byte[] { 1, 2, 3, 4, 5, 6 } }), + name => new ArrayColumn(name, "FixedString(6)", new[] { new byte[6], new byte[] { 1, 2, 3, 0, 0, 0 }, new byte[] { 1, 2, 3, 4, 5, 6 } }), + settings: null); + yield return Dates("Date", new DateOnly(1970, 1, 1), new DateOnly(2024, 1, 15), new DateOnly(2149, 6, 6)); yield return Dates("Date32", new DateOnly(1900, 1, 1), new DateOnly(1970, 1, 1), new DateOnly(2024, 1, 15), new DateOnly(2299, 12, 31)); @@ -252,6 +265,12 @@ public static IEnumerable Cases() yield return NullableStrings("hello", null, "world", string.Empty); yield return NullableStrings(null, null); // every row null + // Nullable(FixedString(N)): byte[] is reference-typed, so a null row surfaces as null; present rows are + // exactly N bytes. A null row must not reach the FixedString codec (the nullable write substitutes the + // N-zero-byte placeholder instead), so the all-null case proves the placeholder-only values stream. + yield return NullableFixedStrings(4, new byte[] { 1, 2, 3, 4 }, null, new byte[] { 0xFF, 0xFF, 0xFF, 0xFF }); + yield return NullableFixedStrings(4, null, null); // every row null + // IPv4/IPv6 are reference-typed (IPAddress) but fixed-width; a null row must not reach the IP codec (it // dereferences the address), so the nullable write substitutes a placeholder instead. yield return NullableIps("IPv4", "127.0.0.1", null, "255.255.255.255"); @@ -281,6 +300,7 @@ public static IEnumerable Cases() yield return Arrays("Float64", new[] { 0d, -1.5e100, double.MaxValue }); yield return Arrays("Bool", new[] { true, false, true }, Array.Empty()); yield return Arrays("String", new[] { "a", "bb" }, Array.Empty(), new[] { string.Empty, "héllo✓" }); + yield return Arrays("FixedString(4)", new[] { new byte[] { 1, 2, 3, 4 }, new byte[] { 0xFF, 0, 0xFF, 0 } }, Array.Empty()); yield return Arrays("Date", new[] { new DateOnly(1970, 1, 1), new DateOnly(2149, 6, 6) }, Array.Empty()); yield return Arrays("Date32", new[] { new DateOnly(1900, 1, 1), new DateOnly(2299, 12, 31) }); @@ -673,6 +693,20 @@ private static InsertRoundTripCase Primitive(string clickHouseType, T[] value private static InsertRoundTripCase Strings(string clickHouseType, params string[] values) => Same($"{clickHouseType} [{values.Length} rows]", clickHouseType, name => new ArrayColumn(name, clickHouseType, values)); + // FixedString(N) inserts and reads back a per-row byte[]; values must be exactly N bytes for the read-back to + // equal the inserted column (a shorter value is server-padded — see the dedicated padding case). + private static InsertRoundTripCase FixedStrings(int size, params byte[][] values) + { + string type = $"FixedString({size})"; + return Same($"{type} [{values.Length} rows]", type, name => new ArrayColumn(name, type, values)); + } + + private static InsertRoundTripCase NullableFixedStrings(int size, params byte[][] values) + { + string type = $"Nullable(FixedString({size}))"; + return Same($"{type} [{values.Length} rows]", type, name => new ArrayColumn(name, type, values)); + } + // BFloat16 widens to float; values are chosen to be exactly representable so the narrow-on-write is lossless. private static InsertRoundTripCase BFloat16s(string clickHouseType, IReadOnlyDictionary settings, params float[] values) => Same($"{clickHouseType} [{values.Length} rows]", clickHouseType, name => new ArrayColumn(name, clickHouseType, values), settings); diff --git a/ClickHouse.Driver.Tcp/Types/Codecs/FixedStringColumnCodec.cs b/ClickHouse.Driver.Tcp/Types/Codecs/FixedStringColumnCodec.cs new file mode 100644 index 000000000..8f963a111 --- /dev/null +++ b/ClickHouse.Driver.Tcp/Types/Codecs/FixedStringColumnCodec.cs @@ -0,0 +1,120 @@ +using System; +using System.Buffers; +using System.Globalization; +using System.Threading; +using System.Threading.Tasks; +using ClickHouse.Driver.Tcp.Protocol; + +namespace ClickHouse.Driver.Tcp.Types.Codecs; + +/// +/// A codec for the ClickHouse FixedString(N) column: every row is exactly N bytes with no +/// length prefix, so the column body is num_rows * N contiguous bytes. The rows are read in one bulk +/// transfer into a pooled blob and surfaced as a (each row a +/// array). On write, a row's bytes are emitted verbatim and right-padded with zeros to N; a value longer +/// than N is rejected, matching the server, which stores over-length values as an error rather than +/// truncating. +/// +internal sealed class FixedStringColumnCodec : IColumnCodec +{ + private readonly int size; + + private FixedStringColumnCodec(int size, string typeName) + { + this.size = size; + TypeName = typeName; + } + + /// + public string TypeName { get; } + + /// + public Type ElementType => typeof(byte[]); + + /// + /// The placeholder for a null row is the empty byte array; the write path pads it to N zero bytes, + /// so the values stream stays aligned at a Nullable(FixedString(N)) null position. + /// + public object NullPlaceholder => Array.Empty(); + + /// + public int? FixedRowByteSize => size; + + /// Builds a FixedString(N) codec from its type node's single integer length argument. + /// The parsed FixedString type node. + /// The codec. + /// The type does not have exactly one positive integer length argument. + public static FixedStringColumnCodec Create(TypeNode node) + { + if (node.Arguments.Count != 1) + { + throw new FormatException($"FixedString type '{node}' must have exactly one length argument."); + } + + string token = node.Arguments[0].Name.Trim(); + if (!int.TryParse(token, NumberStyles.None, CultureInfo.InvariantCulture, out int size) || size <= 0) + { + throw new FormatException($"FixedString type '{node}' has an invalid length '{token}'; expected a positive integer."); + } + + return new FixedStringColumnCodec(size, node.ToString()); + } + + /// + public async ValueTask ReadColumnAsync(ClickHouseBinaryReader reader, string columnName, string columnType, int rowCount, CancellationToken cancellationToken) + { + if (rowCount == 0) + { + return new FixedStringColumn(columnName, columnType, size, Array.Empty(), rowCount: 0, pooled: false); + } + + int byteCount = checked(rowCount * size); + byte[] blob = ArrayPool.Shared.Rent(byteCount); + try + { + await reader.ReadBytesAsync(blob.AsMemory(0, byteCount), cancellationToken).ConfigureAwait(false); + } + catch + { + // The column never took ownership of the rent, so return it rather than leak it on a read failure. + ArrayPool.Shared.Return(blob); + throw; + } + + return new FixedStringColumn(columnName, columnType, size, blob, rowCount, pooled: true); + } + + /// + public bool CanWrite(IColumn column) => column is IColumn; + + /// + public void WriteColumn(ClickHouseBinaryWriter writer, IColumn column, int start, int length) + { + // A reusable zero run for the right-padding; the common case (value exactly N bytes) writes none of it. + Span zeros = stackalloc byte[64]; + zeros.Clear(); + + foreach (byte[] value in ((IColumn)column).Values.Slice(start, length)) + { + if (value is null) + { + throw new ArgumentException($"A {TypeName} column cannot hold a null row; wrap the type in Nullable to write nulls.", nameof(column)); + } + + if (value.Length > size) + { + throw new ArgumentException($"A {TypeName} value is {value.Length} bytes, longer than the fixed width of {size}.", nameof(column)); + } + + writer.WriteBytes(value); + + int pad = size - value.Length; + while (pad > 0) + { + int chunk = Math.Min(pad, zeros.Length); + writer.WriteBytes(zeros.Slice(0, chunk)); + pad -= chunk; + } + } + } +} diff --git a/ClickHouse.Driver.Tcp/Types/ColumnCodecRegistry.cs b/ClickHouse.Driver.Tcp/Types/ColumnCodecRegistry.cs index e8a76f948..d4107d577 100644 --- a/ClickHouse.Driver.Tcp/Types/ColumnCodecRegistry.cs +++ b/ClickHouse.Driver.Tcp/Types/ColumnCodecRegistry.cs @@ -92,6 +92,9 @@ private static ColumnCodecRegistry CreateDefault() AddConstant(new FixedWidthColumnCodec("Bool")); AddConstant(StringColumnCodec.Instance); + // FixedString(N): N contiguous bytes per row, the length parsed from the type argument. + AddFactory("FixedString", static (TypeNode node, in ResolveContext _, ColumnCodecRegistry _) => FixedStringColumnCodec.Create(node)); + // Dates and times. AddConstant(DateColumnCodec.Instance); AddConstant(Date32ColumnCodec.Instance); diff --git a/ClickHouse.Driver.Tcp/Types/FixedStringColumn.cs b/ClickHouse.Driver.Tcp/Types/FixedStringColumn.cs new file mode 100644 index 000000000..31d86c1ed --- /dev/null +++ b/ClickHouse.Driver.Tcp/Types/FixedStringColumn.cs @@ -0,0 +1,133 @@ +using System; +using System.Buffers; +using System.Text; + +namespace ClickHouse.Driver.Tcp.Types; + +/// +/// A FixedString(N) column: every row is exactly N bytes on the wire (no length prefix), so the +/// rows are kept back-to-back in one pooled blob at a fixed stride and a row's bytes are the slice +/// [row * N, (row + 1) * N). Like String, FixedString is byte-oriented (not necessarily +/// UTF-8, and commonly holds fixed binary such as hashes), so the bytes are retained verbatim and a caller +/// chooses how to read each row: the raw bytes (, zero-copy), a string under an explicit +/// encoding (), or — the default view — a +/// per-row array. Values shorter than N are right-padded with zero bytes by the +/// server, so a decoded row always has exactly N bytes, trailing zeros included. +/// +/// +/// The blob is rented from and returned on ; like every column, +/// the bytes and any span returned by are borrowed for the block's lifetime. Copy out +/// ( or GetBytes(row).ToArray()) to retain. +/// +/// +internal sealed class FixedStringColumn : IColumn +{ + private readonly int size; + private readonly int rowCount; + private readonly bool pooled; + private byte[] blob; + private byte[][] cache; + + /// Initializes a column over a raw-bytes blob laid out at a fixed stride. + /// The column name. + /// The ClickHouse type string (e.g. FixedString(16)). + /// The fixed per-row byte width N. + /// The concatenated row bytes (may be longer than used); row i is [i * N, (i + 1) * N). + /// The number of rows. + /// Whether was rented and should be returned on dispose. + public FixedStringColumn(string name, string typeName, int size, byte[] blob, int rowCount, bool pooled) + { + Name = name; + TypeName = typeName; + this.size = size; + this.blob = blob ?? throw new ArgumentNullException(nameof(blob)); + this.rowCount = rowCount; + this.pooled = pooled; + } + + /// + public string Name { get; } + + /// + public string TypeName { get; } + + /// + public int RowCount => rowCount; + + /// + /// The rows as per-row arrays, materialized once and cached. Prefer + /// to avoid allocating one array per row when the bytes can be read in place. + /// + public ReadOnlySpan Values + { + get + { + if (cache is null) + { + // Rent rather than allocate: this is a convenience view consumers copy out of, so it only needs + // to live until Dispose returns it to the pool. Single-consumer per connection, so the lazy fill + // needs no synchronization. The rented buffer may be longer than rowCount; Values slices to it. + byte[][] decoded = ArrayPool.Shared.Rent(rowCount); + for (int i = 0; i < rowCount; i++) + { + decoded[i] = GetBytes(i).ToArray(); + } + + cache = decoded; + } + + return cache.AsSpan(0, rowCount); + } + } + + /// + // The cache is rented and may be longer than rowCount, so slice before indexing to keep an out-of-range row + // failing fast rather than returning a stale slot; the uncached path is bounded by GetBytes. + public byte[] this[int row] => cache is not null ? cache.AsSpan(0, rowCount)[row] : GetBytes(row).ToArray(); + + /// + public object GetValue(int row) => this[row]; + + /// Returns the raw bytes of a row as a zero-copy slice of the blob (borrowed), always N bytes. + /// The zero-based row index. + /// The row's bytes. + public ReadOnlySpan GetBytes(int row) + { + // Bound the row against rowCount, not the blob: the blob is rented and may be longer, so slicing it + // directly would let an out-of-range row read a stale pooled region instead of failing fast. + if ((uint)row >= (uint)rowCount) + { + throw new IndexOutOfRangeException(); + } + + return blob.AsSpan(row * size, size); + } + + /// Decodes a row's bytes to a string under the given encoding. + /// The zero-based row index. + /// The encoding to decode with. + /// The decoded string. + public string GetString(int row, Encoding encoding) + { + ArgumentNullException.ThrowIfNull(encoding); + return encoding.GetString(GetBytes(row)); + } + + /// + public void Dispose() + { + if (pooled && blob.Length != 0) + { + ArrayPool.Shared.Return(blob); + } + + blob = Array.Empty(); + + if (cache is not null) + { + // The elements are byte[] references, so clear on return to avoid the pool pinning decoded rows. + ArrayPool.Shared.Return(cache, clearArray: true); + cache = null; + } + } +} From 02682a2040576c496996108ce6d841ead4c3c9f5 Mon Sep 17 00:00:00 2001 From: Alex Soffronow-Pagonidis Date: Fri, 24 Jul 2026 20:20:52 +0200 Subject: [PATCH 2/5] Ergonomic FixedString write; drop byte measurement FixedString writes each row's fixed-width value straight from the ergonomic source via the value-writer path, with no byte measurement. Co-Authored-By: Claude Opus 4.8 --- .../Types/FixedStringColumnCodecTests.cs | 6 -- .../Types/Codecs/FixedStringColumnCodec.cs | 68 ++++++++++++------- 2 files changed, 44 insertions(+), 30 deletions(-) diff --git a/ClickHouse.Driver.Tcp.Tests/Types/FixedStringColumnCodecTests.cs b/ClickHouse.Driver.Tcp.Tests/Types/FixedStringColumnCodecTests.cs index 1a414e1a9..48126badb 100644 --- a/ClickHouse.Driver.Tcp.Tests/Types/FixedStringColumnCodecTests.cs +++ b/ClickHouse.Driver.Tcp.Tests/Types/FixedStringColumnCodecTests.cs @@ -26,12 +26,6 @@ public void Create_MissingOrNonIntegerOrNonPositiveLength_ThrowsFormat() }); } - [Test] - public void FixedRowByteSize_IsTheDeclaredWidth() - { - Assert.That(Codec(16).FixedRowByteSize, Is.EqualTo(16)); - } - [Test] public async Task WriteColumn_ExactWidthValue_WritesBytesVerbatim() { diff --git a/ClickHouse.Driver.Tcp/Types/Codecs/FixedStringColumnCodec.cs b/ClickHouse.Driver.Tcp/Types/Codecs/FixedStringColumnCodec.cs index 8f963a111..f644bfee1 100644 --- a/ClickHouse.Driver.Tcp/Types/Codecs/FixedStringColumnCodec.cs +++ b/ClickHouse.Driver.Tcp/Types/Codecs/FixedStringColumnCodec.cs @@ -15,7 +15,7 @@ namespace ClickHouse.Driver.Tcp.Types.Codecs; /// than N is rejected, matching the server, which stores over-length values as an error rather than /// truncating. /// -internal sealed class FixedStringColumnCodec : IColumnCodec +internal sealed class FixedStringColumnCodec : IColumnCodec, ISpanWritableCodec { private readonly int size; @@ -37,9 +37,6 @@ private FixedStringColumnCodec(int size, string typeName) /// public object NullPlaceholder => Array.Empty(); - /// - public int? FixedRowByteSize => size; - /// Builds a FixedString(N) codec from its type node's single integer length argument. /// The parsed FixedString type node. /// The codec. @@ -88,33 +85,56 @@ public async ValueTask ReadColumnAsync(ClickHouseBinaryReader reader, s public bool CanWrite(IColumn column) => column is IColumn; /// + // Read per element through the indexer so a scattered write-path view (a substitute for a nullable value, a + // Tuple field) writes with no materialized copy; a dense FixedStringColumn materializes each row's bytes just + // the same. public void WriteColumn(ClickHouseBinaryWriter writer, IColumn column, int start, int length) { // A reusable zero run for the right-padding; the common case (value exactly N bytes) writes none of it. Span zeros = stackalloc byte[64]; zeros.Clear(); - foreach (byte[] value in ((IColumn)column).Values.Slice(start, length)) + var typed = (IColumn)column; + for (int i = 0; i < length; i++) + { + WriteRow(writer, typed[start + i], zeros); + } + } + + /// + // Each row is its own fixed-width byte run, so a run of values is written in order. + public void WriteValues(ClickHouseBinaryWriter writer, ReadOnlySpan values) + { + Span zeros = stackalloc byte[64]; + zeros.Clear(); + + foreach (byte[] value in values) + { + WriteRow(writer, value, zeros); + } + } + + // Emits one row's bytes verbatim, right-padded with zeros to the fixed width. + private void WriteRow(ClickHouseBinaryWriter writer, byte[] value, ReadOnlySpan zeros) + { + if (value is null) + { + throw new ArgumentException($"A {TypeName} column cannot hold a null row; wrap the type in Nullable to write nulls.", nameof(value)); + } + + if (value.Length > size) + { + throw new ArgumentException($"A {TypeName} value is {value.Length} bytes, longer than the fixed width of {size}.", nameof(value)); + } + + writer.WriteBytes(value); + + int pad = size - value.Length; + while (pad > 0) { - if (value is null) - { - throw new ArgumentException($"A {TypeName} column cannot hold a null row; wrap the type in Nullable to write nulls.", nameof(column)); - } - - if (value.Length > size) - { - throw new ArgumentException($"A {TypeName} value is {value.Length} bytes, longer than the fixed width of {size}.", nameof(column)); - } - - writer.WriteBytes(value); - - int pad = size - value.Length; - while (pad > 0) - { - int chunk = Math.Min(pad, zeros.Length); - writer.WriteBytes(zeros.Slice(0, chunk)); - pad -= chunk; - } + int chunk = Math.Min(pad, zeros.Length); + writer.WriteBytes(zeros.Slice(0, chunk)); + pad -= chunk; } } } From 3f8b0769fe3695c45d558939d8a5d8e90e1054cd Mon Sep 17 00:00:00 2001 From: Alex Soffronow-Pagonidis Date: Fri, 24 Jul 2026 23:24:41 +0200 Subject: [PATCH 3/5] Address Copilot review: zero-copy FixedString write from dense column MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR 452 feedback: WriteColumn read every row through the IColumn indexer, which for a dense FixedStringColumn allocates a byte[] per row (GetBytes(row).ToArray()) — the hot path when re-inserting a value read straight back. Special-case FixedStringColumn to write directly from its zero-copy GetBytes span via a new span-based WriteRow overload; scattered views still fall back to the indexer. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../Types/Codecs/FixedStringColumnCodec.cs | 28 +++++++++++++++---- 1 file changed, 23 insertions(+), 5 deletions(-) diff --git a/ClickHouse.Driver.Tcp/Types/Codecs/FixedStringColumnCodec.cs b/ClickHouse.Driver.Tcp/Types/Codecs/FixedStringColumnCodec.cs index f644bfee1..6dd0e47ed 100644 --- a/ClickHouse.Driver.Tcp/Types/Codecs/FixedStringColumnCodec.cs +++ b/ClickHouse.Driver.Tcp/Types/Codecs/FixedStringColumnCodec.cs @@ -85,15 +85,26 @@ public async ValueTask ReadColumnAsync(ClickHouseBinaryReader reader, s public bool CanWrite(IColumn column) => column is IColumn; /// - // Read per element through the indexer so a scattered write-path view (a substitute for a nullable value, a - // Tuple field) writes with no materialized copy; a dense FixedStringColumn materializes each row's bytes just - // the same. public void WriteColumn(ClickHouseBinaryWriter writer, IColumn column, int start, int length) { // A reusable zero run for the right-padding; the common case (value exactly N bytes) writes none of it. Span zeros = stackalloc byte[64]; zeros.Clear(); + // A dense FixedStringColumn exposes each row's bytes as a zero-copy slice of its blob, so write straight + // from that and skip the per-row byte[] the IColumn indexer would materialize — the hot path when + // re-inserting a value read straight back. A scattered write-path view (a nullable substitute, a Tuple + // field) has no such slice, so it falls back to reading each row through the indexer. + if (column is FixedStringColumn dense) + { + for (int i = 0; i < length; i++) + { + WriteRow(writer, dense.GetBytes(start + i), zeros); + } + + return; + } + var typed = (IColumn)column; for (int i = 0; i < length; i++) { @@ -114,7 +125,8 @@ public void WriteValues(ClickHouseBinaryWriter writer, ReadOnlySpan valu } } - // Emits one row's bytes verbatim, right-padded with zeros to the fixed width. + // Emits one row's bytes verbatim, right-padded with zeros to the fixed width; rejects a null row (a + // FixedString row is never null — Nullable carries that) before delegating to the span path. private void WriteRow(ClickHouseBinaryWriter writer, byte[] value, ReadOnlySpan zeros) { if (value is null) @@ -122,9 +134,15 @@ private void WriteRow(ClickHouseBinaryWriter writer, byte[] value, ReadOnlySpan< throw new ArgumentException($"A {TypeName} column cannot hold a null row; wrap the type in Nullable to write nulls.", nameof(value)); } + WriteRow(writer, (ReadOnlySpan)value, zeros); + } + + // Emits one row's bytes verbatim, right-padded with zeros to the fixed width. + private void WriteRow(ClickHouseBinaryWriter writer, ReadOnlySpan value, ReadOnlySpan zeros) + { if (value.Length > size) { - throw new ArgumentException($"A {TypeName} value is {value.Length} bytes, longer than the fixed width of {size}.", nameof(value)); + throw new ArgumentException($"A {TypeName} value is {value.Length} bytes, longer than the fixed width of {size}.", "value"); } writer.WriteBytes(value); From c2ae14d5df1723b56e8cc864234c7ba1d0a1563d Mon Sep 17 00:00:00 2001 From: Alex Soffronow-Pagonidis Date: Sat, 25 Jul 2026 08:37:42 +0200 Subject: [PATCH 4/5] Address Copilot review: use nameof(value) in FixedString span WriteRow PR 452 (round 2): the span-based WriteRow overload used a "value" string literal for the paramName; use nameof(value) to match the byte[] overload and stay correct across renames. Co-Authored-By: Claude Opus 4.8 (1M context) --- ClickHouse.Driver.Tcp/Types/Codecs/FixedStringColumnCodec.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ClickHouse.Driver.Tcp/Types/Codecs/FixedStringColumnCodec.cs b/ClickHouse.Driver.Tcp/Types/Codecs/FixedStringColumnCodec.cs index 6dd0e47ed..33ff2c138 100644 --- a/ClickHouse.Driver.Tcp/Types/Codecs/FixedStringColumnCodec.cs +++ b/ClickHouse.Driver.Tcp/Types/Codecs/FixedStringColumnCodec.cs @@ -142,7 +142,7 @@ private void WriteRow(ClickHouseBinaryWriter writer, ReadOnlySpan value, R { if (value.Length > size) { - throw new ArgumentException($"A {TypeName} value is {value.Length} bytes, longer than the fixed width of {size}.", "value"); + throw new ArgumentException($"A {TypeName} value is {value.Length} bytes, longer than the fixed width of {size}.", nameof(value)); } writer.WriteBytes(value); From 4e74673a5b368fd58e5e4d4bb92c0c5fdad5ed44 Mon Sep 17 00:00:00 2001 From: Alex Soffronow-Pagonidis Date: Wed, 5 Aug 2026 11:07:36 +0200 Subject: [PATCH 5/5] Require exactly N bytes on FixedString write; blit dense columns MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The write path used to right-pad a short byte[] to N zero bytes. Drop that: a wrong-width value now throws. Padding silently rewrites the caller's data and hides whatever produced the wrong width, and the HTTP path already rejects it — FixedStringType.WriteByteArray requires a byte[] to be exactly N bytes (as do its ReadOnlyMemory and Stream overloads), padding only for string. So the same package was accepting over TCP what it refused over HTTP. Padding belongs with the deferred string-write overload, not with byte[]. Separately, write a dense FixedStringColumn as one contiguous blit instead of walking it row by row. Its rows already sit at the wire stride, so rows [start, start + length) are a single slice of the blob — the hot path when re-inserting a value read straight back. This is independent of the padding change: no other write source is contiguous, since user data arrives as jagged byte[][]. The blit is guarded on the column's width matching the codec's. That guard fixes a latent bug rather than merely enabling the fast path: the old dense branch had no width check at all, and CanWrite only tests the CLR element type, so inserting a FixedString(2) read-back into a FixedString(4) target silently zero-padded every row to 4 bytes. Also: - NullPlaceholder is now N zero bytes rather than the empty array it relied on the pad loop to widen. Built lazily: a codec is resolved per column per block, so allocating it eagerly would charge every read of a wide FixedString for a buffer only the Nullable write path touches. - Both rejection messages carry the offending position — its row in a column, its index within the row's array under Array(FixedString(N)) — since rejecting is now the only signal a caller gets. - FixedStringColumn gains Size and a GetBytes(start, length) range accessor, bounded against RowCount so an over-long range cannot blit a stale region of the pooled blob. Tests: the three width rejections collapse into one parametrized case; the FixedString(6) padding round-trip is gone (the behavior it pinned no longer exists) and FixedString(200) replaces it, keeping the >64-byte width coverage at the layer that owns per-type values and giving the blit a stride wider than one row. New unit tests cover the sub-range blit, the mismatched-width fallback, range bounds past RowCount, and the placeholder — all shapes no server round-trip can reach. Adds a Tuple(FixedString(4), String) case for the one entrance that reaches the strict per-value branch through a field projection. Co-Authored-By: Claude --- .../Types/FixedStringColumnCodecTests.cs | 99 +++++++++++++------ .../Utilities/InsertRoundTripCase.cs | 30 +++--- .../Types/Codecs/FixedStringColumnCodec.cs | 92 +++++++++-------- .../Types/FixedStringColumn.cs | 45 +++++++-- 4 files changed, 171 insertions(+), 95 deletions(-) diff --git a/ClickHouse.Driver.Tcp.Tests/Types/FixedStringColumnCodecTests.cs b/ClickHouse.Driver.Tcp.Tests/Types/FixedStringColumnCodecTests.cs index 48126badb..1415d067b 100644 --- a/ClickHouse.Driver.Tcp.Tests/Types/FixedStringColumnCodecTests.cs +++ b/ClickHouse.Driver.Tcp.Tests/Types/FixedStringColumnCodecTests.cs @@ -35,37 +35,19 @@ public async Task WriteColumn_ExactWidthValue_WritesBytesVerbatim() CollectionAssert.AreEqual(value, bytes); } - [Test] - public async Task WriteColumn_ShortAndEmptyValues_RightPadsWithZeros() - { - var values = new[] { new byte[] { 1, 2, 3 }, Array.Empty() }; - byte[] bytes = await WriteAsync(w => Codec(6).WriteColumn(w, new ArrayColumn("c", "FixedString(6)", values))); - - CollectionAssert.AreEqual(new byte[] { 1, 2, 3, 0, 0, 0, 0, 0, 0, 0, 0, 0 }, bytes); - } - - [Test] - public async Task WriteColumn_WidthLargerThanZeroRun_PadsAcrossMultipleChunks() - { - // The write path pads with a 64-byte stack zero-run in a loop; a width well past 64 exercises the - // multi-chunk path. A three-byte value into FixedString(200) must emit exactly 200 bytes: the value then - // 197 zeros. - const int width = 200; - byte[] value = { 1, 2, 3 }; - byte[] bytes = await WriteAsync(w => Codec(width).WriteColumn(w, new ArrayColumn("c", $"FixedString({width})", new[] { value }))); - - byte[] expected = new byte[width]; - value.CopyTo(expected, 0); - CollectionAssert.AreEqual(expected, bytes); - } - - [Test] - public async Task WriteColumn_ValueLongerThanWidth_ThrowsArgument() + // A value of any width other than N is rejected rather than padded or truncated: padding a short value would + // silently rewrite the caller's data and hide whatever produced the wrong width. This matches the HTTP path's + // FixedStringType, which requires a byte[] to be exactly N bytes. + [TestCase(0, TestName = "WriteColumn_EmptyValue_ThrowsArgument")] + [TestCase(3, TestName = "WriteColumn_ValueShorterThanWidth_ThrowsArgument")] + [TestCase(7, TestName = "WriteColumn_ValueLongerThanWidth_ThrowsArgument")] + public async Task WriteColumn_ValueWidthOtherThanN_ThrowsArgument(int valueLength) { - var column = new ArrayColumn("c", "FixedString(2)", new[] { new byte[] { 1, 2, 3 } }); - var ex = await CaptureAsync(w => Codec(2).WriteColumn(w, column)); + var column = new ArrayColumn("c", "FixedString(6)", new[] { new byte[valueLength] }); + var ex = await CaptureAsync(w => Codec(6).WriteColumn(w, column)); Assert.That(ex, Is.TypeOf()); + Assert.That(ex.Message, Does.Contain("exactly 6 bytes")); } [Test] @@ -128,6 +110,57 @@ public async Task ReadColumn_IndexOrGetBytesBeyondRowCount_Throws() }); } + [Test] + public async Task WriteColumn_DenseColumnSubRange_BlitsOnlyThatRangeOfTheBlob() + { + // The dense read-back holds its rows at the wire stride, so the codec blits the range in one copy instead + // of walking it. The insert path splits a large column into per-block ranges, so a partial range must emit + // exactly its own rows — a stride slip would show up as neighbouring rows' bytes. + using var dense = await DenseAsync(2, new byte[] { 1, 1 }, new byte[] { 2, 2 }, new byte[] { 3, 3 }); + byte[] bytes = await WriteAsync(w => Codec(2).WriteColumn(w, dense, start: 1, length: 2)); + + CollectionAssert.AreEqual(new byte[] { 2, 2, 3, 3 }, bytes); + } + + [Test] + public async Task WriteColumn_DenseColumnOfDifferentWidth_ThrowsArgument() + { + // A FixedString(2) read-back is not a valid body for a FixedString(4) column: blitting its blob would emit + // half the bytes the header promises and corrupt the block. The width guard must send it down the per-row + // path, which rejects each row on width — a shape no server round-trip can produce, hence the unit test. + using var dense = await DenseAsync(2, new byte[] { 1, 2 }, new byte[] { 3, 4 }); + var ex = await CaptureAsync(w => Codec(4).WriteColumn(w, dense)); + + Assert.That(ex, Is.TypeOf()); + Assert.That(ex.Message, Does.Contain("exactly 4 bytes")); + } + + [Test] + public async Task GetBytes_RangeBeyondRowCount_ThrowsArgumentOutOfRange() + { + // The blob is rented and typically longer than rowCount * N, so an over-long range must fail fast rather + // than blit a stale pooled region into the block. + using var dense = await DenseAsync(2, new byte[] { 1, 2 }, new byte[] { 3, 4 }); + + Assert.Multiple(() => + { + Assert.Throws(() => _ = dense.GetBytes(0, 3).Length); + Assert.Throws(() => _ = dense.GetBytes(1, 2).Length); + Assert.Throws(() => _ = dense.GetBytes(-1, 1).Length); + Assert.Throws(() => _ = dense.GetBytes(0, -1).Length); + CollectionAssert.AreEqual(new byte[] { 1, 2, 3, 4 }, dense.GetBytes(0, 2).ToArray()); + Assert.That(dense.GetBytes(2, 0).Length, Is.EqualTo(0)); + }); + } + + [Test] + public void NullPlaceholder_IsWidthZeroBytes() + { + // Nullable substitutes this at a null position, and the values stream must still advance a full row there, + // so the placeholder has to be exactly N bytes now that a short value is rejected. + CollectionAssert.AreEqual(new byte[6], (byte[])Codec(6).NullPlaceholder); + } + [Test] public void CanWrite_AcceptsByteArrayColumn_RejectsOthers() { @@ -140,6 +173,16 @@ public void CanWrite_AcceptsByteArrayColumn_RejectsOthers() private static IColumnCodec Codec(int size) => ColumnCodecRegistry.Default.Resolve($"FixedString({size})", ResolveContext.ForWrite); + // Builds the dense, blob-backed column the read path produces — the shape the codec's bulk-blit write covers — + // by writing the values and reading them straight back. + private static async Task DenseAsync(int size, params byte[][] values) + { + string type = $"FixedString({size})"; + byte[] bytes = await WriteAsync(w => Codec(size).WriteColumn(w, new ArrayColumn("c", type, values))); + using var reader = ReaderOver(bytes); + return (FixedStringColumn)await Codec(size).ReadColumnAsync(reader, "c", type, values.Length, None); + } + private static void Resolve(string type) => ColumnCodecRegistry.Default.Resolve(type, ResolveContext.ForWrite); private static async Task WriteAsync(Action write) diff --git a/ClickHouse.Driver.Tcp.Tests/Utilities/InsertRoundTripCase.cs b/ClickHouse.Driver.Tcp.Tests/Utilities/InsertRoundTripCase.cs index df0f6b088..85dfbe24d 100644 --- a/ClickHouse.Driver.Tcp.Tests/Utilities/InsertRoundTripCase.cs +++ b/ClickHouse.Driver.Tcp.Tests/Utilities/InsertRoundTripCase.cs @@ -87,17 +87,10 @@ public static IEnumerable Cases() yield return Strings("String", string.Empty, "hello", "héllo✓", "a\0b", new string('x', 500)); // FixedString(N): N contiguous bytes per row, surfaced as a per-row byte[] of exactly N bytes. The bytes - // are byte-oriented, so embedded NULs and non-UTF-8 bytes ride along unchanged. + // are byte-oriented, so embedded NULs and non-UTF-8 bytes ride along unchanged. A wider N crosses the + // stride past a single row so a mis-strided blit could not pass unnoticed. yield return FixedStrings(4, new byte[] { 0, 0, 0, 0 }, new byte[] { 1, 2, 3, 4 }, new byte[] { 0xFF, 0x00, 0xFF, 0x00 }); - - // A value shorter than N is right-padded to N zero bytes by the server, so the read-back differs from the - // inserted bytes; the empty value becomes an all-zero row and a full-width value is unchanged. - yield return new InsertRoundTripCase( - "FixedString(6) [padding]", - "FixedString(6)", - name => new ArrayColumn(name, "FixedString(6)", new[] { Array.Empty(), new byte[] { 1, 2, 3 }, new byte[] { 1, 2, 3, 4, 5, 6 } }), - name => new ArrayColumn(name, "FixedString(6)", new[] { new byte[6], new byte[] { 1, 2, 3, 0, 0, 0 }, new byte[] { 1, 2, 3, 4, 5, 6 } }), - settings: null); + yield return FixedStrings(200, Enumerable.Range(0, 200).Select(i => (byte)i).ToArray(), new byte[200]); yield return Dates("Date", new DateOnly(1970, 1, 1), new DateOnly(2024, 1, 15), new DateOnly(2149, 6, 6)); yield return Dates("Date32", new DateOnly(1900, 1, 1), new DateOnly(1970, 1, 1), new DateOnly(2024, 1, 15), new DateOnly(2299, 12, 31)); @@ -366,6 +359,18 @@ public static IEnumerable Cases() "Tuple(Int32)", name => new TupleColumn(name, "Tuple(Int32)", new[] { new ValueTuple(1), new ValueTuple(int.MinValue), new ValueTuple(int.MaxValue) })); + // FixedString(N) as a tuple element: the write path reaches the FixedString codec through a + // TupleFieldColumn projection rather than a dense blob, so it takes the strict per-value branch instead of + // the bulk blit — the one entrance the bare, Nullable and Array cases all miss. + yield return Same( + "Tuple(FixedString(4), String)", + "Tuple(FixedString(4), String)", + name => new TupleColumn(name, "Tuple(FixedString(4), String)", new (byte[], string)[] + { + (new byte[] { 1, 2, 3, 4 }, "a"), + (new byte[] { 0xFF, 0x00, 0xFF, 0x00 }, string.Empty), + })); + // Arity 3 was the one arity between 1 and 7 with no case at all. yield return Same( "Tuple(Int32, String, Float64) [arity 3]", @@ -693,8 +698,9 @@ private static InsertRoundTripCase Primitive(string clickHouseType, T[] value private static InsertRoundTripCase Strings(string clickHouseType, params string[] values) => Same($"{clickHouseType} [{values.Length} rows]", clickHouseType, name => new ArrayColumn(name, clickHouseType, values)); - // FixedString(N) inserts and reads back a per-row byte[]; values must be exactly N bytes for the read-back to - // equal the inserted column (a shorter value is server-padded — see the dedicated padding case). + // FixedString(N) inserts and reads back a per-row byte[]. Every value must be exactly N bytes: the write path + // rejects any other width rather than padding or truncating, so a wrong-width case belongs in the codec's unit + // tests (it never reaches the server), not here. private static InsertRoundTripCase FixedStrings(int size, params byte[][] values) { string type = $"FixedString({size})"; diff --git a/ClickHouse.Driver.Tcp/Types/Codecs/FixedStringColumnCodec.cs b/ClickHouse.Driver.Tcp/Types/Codecs/FixedStringColumnCodec.cs index 33ff2c138..a537f357c 100644 --- a/ClickHouse.Driver.Tcp/Types/Codecs/FixedStringColumnCodec.cs +++ b/ClickHouse.Driver.Tcp/Types/Codecs/FixedStringColumnCodec.cs @@ -11,14 +11,23 @@ namespace ClickHouse.Driver.Tcp.Types.Codecs; /// A codec for the ClickHouse FixedString(N) column: every row is exactly N bytes with no /// length prefix, so the column body is num_rows * N contiguous bytes. The rows are read in one bulk /// transfer into a pooled blob and surfaced as a (each row a -/// array). On write, a row's bytes are emitted verbatim and right-padded with zeros to N; a value longer -/// than N is rejected, matching the server, which stores over-length values as an error rather than -/// truncating. +/// array). On write, a row's bytes are emitted verbatim and must be exactly N: a longer value is +/// rejected, matching the server, which errors on an over-length value rather than truncating, and a shorter +/// one is rejected too rather than zero-padded — padding would silently rewrite the caller's data, hiding the +/// bug that produced a wrong-width value. This matches the HTTP path, where +/// ClickHouse.Driver.Types.FixedStringType likewise requires a array to be exactly +/// N bytes. /// internal sealed class FixedStringColumnCodec : IColumnCodec, ISpanWritableCodec { private readonly int size; + // N zero bytes, shared: the write path only ever reads it, and it is the exact width a null position must + // advance the values stream by. Built on first use, not in the constructor: a codec is resolved per column per + // block, so a pure read of a wide FixedString would otherwise allocate an N-byte buffer per block that only the + // Nullable write path ever touches. Single-consumer per connection, so the lazy fill needs no synchronization. + private byte[] nullPlaceholder; + private FixedStringColumnCodec(int size, string typeName) { this.size = size; @@ -32,10 +41,10 @@ private FixedStringColumnCodec(int size, string typeName) public Type ElementType => typeof(byte[]); /// - /// The placeholder for a null row is the empty byte array; the write path pads it to N zero bytes, - /// so the values stream stays aligned at a Nullable(FixedString(N)) null position. + /// The placeholder for a null row is N zero bytes, so the values stream stays aligned at a + /// Nullable(FixedString(N)) null position — the width every row occupies. /// - public object NullPlaceholder => Array.Empty(); + public object NullPlaceholder => nullPlaceholder ??= new byte[size]; /// Builds a FixedString(N) codec from its type node's single integer length argument. /// The parsed FixedString type node. @@ -87,72 +96,61 @@ public async ValueTask ReadColumnAsync(ClickHouseBinaryReader reader, s /// public void WriteColumn(ClickHouseBinaryWriter writer, IColumn column, int start, int length) { - // A reusable zero run for the right-padding; the common case (value exactly N bytes) writes none of it. - Span zeros = stackalloc byte[64]; - zeros.Clear(); - - // A dense FixedStringColumn exposes each row's bytes as a zero-copy slice of its blob, so write straight - // from that and skip the per-row byte[] the IColumn indexer would materialize — the hot path when - // re-inserting a value read straight back. A scattered write-path view (a nullable substitute, a Tuple - // field) has no such slice, so it falls back to reading each row through the indexer. - if (column is FixedStringColumn dense) + // A dense FixedStringColumn of this width already holds its rows back-to-back at the stride the wire uses, + // so the whole range is one contiguous blit — no per-row byte[] materialized through the IColumn + // indexer, and no per-row width check, since every row is N bytes by construction. This is the hot path + // when re-inserting a value read straight back. A dense column of a *different* width is not a valid body + // for this type, so it falls through to the per-row path, which rejects each row with the width message + // rather than silently blitting a mis-strided run. A scattered write-path view (a nullable substitute, a + // Tuple field) has no contiguous run either, so it reads each row through the indexer. + if (column is FixedStringColumn dense && dense.Size == size) { - for (int i = 0; i < length; i++) - { - WriteRow(writer, dense.GetBytes(start + i), zeros); - } - + writer.WriteBytes(dense.GetBytes(start, length)); return; } var typed = (IColumn)column; for (int i = 0; i < length; i++) { - WriteRow(writer, typed[start + i], zeros); + WriteValue(writer, typed[start + i], start + i, "row"); } } /// - // Each row is its own fixed-width byte run, so a run of values is written in order. + // Each value is its own fixed-width byte run, so a run of values is written in order. These are the jagged + // per-element arrays of one Array(FixedString(N)) row, so there is no contiguous run to blit here. public void WriteValues(ClickHouseBinaryWriter writer, ReadOnlySpan values) { - Span zeros = stackalloc byte[64]; - zeros.Clear(); - - foreach (byte[] value in values) + for (int i = 0; i < values.Length; i++) { - WriteRow(writer, value, zeros); + WriteValue(writer, values[i], i, "element"); } } - // Emits one row's bytes verbatim, right-padded with zeros to the fixed width; rejects a null row (a - // FixedString row is never null — Nullable carries that) before delegating to the span path. - private void WriteRow(ClickHouseBinaryWriter writer, byte[] value, ReadOnlySpan zeros) + // Emits one value's bytes verbatim. It must be exactly N bytes — a shorter one is rejected rather than + // zero-padded, so a wrong-width value surfaces as an error instead of silently reaching the server rewritten. + // A null is rejected too: a FixedString row is never null, Nullable carries that and substitutes the + // placeholder at a null position so this never sees one. + // + // Rejecting is now the only signal a caller gets, so both messages carry the offending position: its row in a + // column, or its index within the row's array when the values come from an Array(FixedString(N)). The noun is + // a literal at each call site, so naming it costs nothing on the path that does not throw. + private void WriteValue(ClickHouseBinaryWriter writer, byte[] value, int position, string positionNoun) { if (value is null) { - throw new ArgumentException($"A {TypeName} column cannot hold a null row; wrap the type in Nullable to write nulls.", nameof(value)); + throw new ArgumentException( + $"A {TypeName} column cannot hold a null value (at {positionNoun} {position}); wrap the type in Nullable to write nulls.", + nameof(value)); } - WriteRow(writer, (ReadOnlySpan)value, zeros); - } - - // Emits one row's bytes verbatim, right-padded with zeros to the fixed width. - private void WriteRow(ClickHouseBinaryWriter writer, ReadOnlySpan value, ReadOnlySpan zeros) - { - if (value.Length > size) + if (value.Length != size) { - throw new ArgumentException($"A {TypeName} value is {value.Length} bytes, longer than the fixed width of {size}.", nameof(value)); + throw new ArgumentException( + $"A {TypeName} value at {positionNoun} {position} is {value.Length} bytes; every value must be exactly {size} bytes. Resize it to {size} bytes before writing it — the write path will not pad or truncate, since doing so would silently alter the data.", + nameof(value)); } writer.WriteBytes(value); - - int pad = size - value.Length; - while (pad > 0) - { - int chunk = Math.Min(pad, zeros.Length); - writer.WriteBytes(zeros.Slice(0, chunk)); - pad -= chunk; - } } } diff --git a/ClickHouse.Driver.Tcp/Types/FixedStringColumn.cs b/ClickHouse.Driver.Tcp/Types/FixedStringColumn.cs index 31d86c1ed..39b94476f 100644 --- a/ClickHouse.Driver.Tcp/Types/FixedStringColumn.cs +++ b/ClickHouse.Driver.Tcp/Types/FixedStringColumn.cs @@ -9,15 +9,15 @@ namespace ClickHouse.Driver.Tcp.Types; /// rows are kept back-to-back in one pooled blob at a fixed stride and a row's bytes are the slice /// [row * N, (row + 1) * N). Like String, FixedString is byte-oriented (not necessarily /// UTF-8, and commonly holds fixed binary such as hashes), so the bytes are retained verbatim and a caller -/// chooses how to read each row: the raw bytes (, zero-copy), a string under an explicit -/// encoding (), or — the default view — a -/// per-row array. Values shorter than N are right-padded with zero bytes by the -/// server, so a decoded row always has exactly N bytes, trailing zeros included. +/// chooses how to read each row: the raw bytes (, zero-copy), a string under an +/// explicit encoding (), or — the default view — +/// a per-row array. A decoded row always has exactly N bytes, any trailing zeros a +/// shorter stored value was padded with included. /// /// /// The blob is rented from and returned on ; like every column, -/// the bytes and any span returned by are borrowed for the block's lifetime. Copy out -/// ( or GetBytes(row).ToArray()) to retain. +/// the bytes and any span returned by are borrowed for the block's lifetime. Copy +/// out ( or GetBytes(row).ToArray()) to retain. /// /// internal sealed class FixedStringColumn : IColumn @@ -54,9 +54,12 @@ public FixedStringColumn(string name, string typeName, int size, byte[] blob, in /// public int RowCount => rowCount; + /// The fixed per-row byte width N — the stride the rows sit at in the blob. + public int Size => size; + /// - /// The rows as per-row arrays, materialized once and cached. Prefer - /// to avoid allocating one array per row when the bytes can be read in place. + /// The rows as per-row arrays, materialized once and cached. Prefer + /// to avoid allocating one array per row when the bytes can be read in place. /// public ReadOnlySpan Values { @@ -103,6 +106,32 @@ public ReadOnlySpan GetBytes(int row) return blob.AsSpan(row * size, size); } + /// + /// Returns the bytes of the row range [start, start + length) as one zero-copy slice of the blob + /// (borrowed), exactly length * N bytes. The rows sit back-to-back at the same stride the wire uses, so + /// a codec can blit a whole range in one copy rather than walking it row by row. + /// + /// The zero-based first row of the range. + /// The number of rows in the range. + /// The range's bytes. + /// The range lies outside the column's rows. + public ReadOnlySpan GetBytes(int start, int length) + { + // Bound the range against rowCount, not the blob: the blob is rented and may be longer, so slicing it + // directly would let an over-long range read a stale pooled region instead of failing fast. The products + // cannot overflow — the read path sized the blob with a checked rowCount * size, and this range fits in it. + if (start < 0 || length < 0 || start + (long)length > rowCount) + { + // Blame whichever argument is itself out of range; a pair that is only jointly too long is the range's + // fault, so anchor that on start. + throw new ArgumentOutOfRangeException( + length < 0 ? nameof(length) : nameof(start), + $"Rows [{start}, {start + (long)length}) lie outside the {rowCount} row(s) of column '{Name}'."); + } + + return blob.AsSpan(start * size, length * size); + } + /// Decodes a row's bytes to a string under the given encoding. /// The zero-based row index. /// The encoding to decode with.