diff --git a/AGENTS.md b/AGENTS.md index bb64ae649..ea4b1b36c 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -22,7 +22,7 @@ ClickHouse.Driver.sln │ ├── Types/ # 60+ ClickHouse type implementations + TypeConverter.cs │ ├── Copy/ # Binary serialization (used internally by ClickHouseClient) │ ├── Http/ # HTTP layer & connection pooling -│ └── PublicAPI/ # Public API surface tracking (analyzer-enforced) +│ └── PublicAPI/ # Public API surface tracking (hand-maintained) ├── ClickHouse.Driver.Tests/ # NUnit tests (multi-framework) ├── ClickHouse.Driver.IntegrationTests/ # Integration tests (net10.0) └── ClickHouse.Driver.Benchmark/ # BenchmarkDotNet performance tests @@ -35,7 +35,9 @@ Prefer using LSP to grep when navigating the codebase. - **Type system**: `Types/TypeConverter.cs` (14KB, complex), `Types/Grammar/` (type parsing) - **ADO.NET layer**: `ADO/ClickHouseConnection.cs`, `ADO/ClickHouseCommand.cs`, `ADO/Readers/` - **Feature detection**: `Utility/ClickHouseFeatureMap.cs` (version-based capabilities) -- **Public API**: `PublicAPI/*.txt` (Roslyn analyzer enforces shipped signatures) +- **Public API**: `PublicAPI/*.txt` (hand-maintained record of shipped signatures; the + `Microsoft.CodeAnalysis.PublicApiAnalyzers` package is *not* referenced, so nothing checks these + files at build time — keep them in sync yourself) - **Config**: `.editorconfig` (file-scoped namespaces, StyleCop suppressions) ### API Architecture @@ -241,7 +243,7 @@ If the value is null/`DBNull` and no explicit type or hint is provided, resoluti - **Connection state**: Clear logging of connection lifecycle events ### Public API Surface -- **Breaking changes**: Must update `PublicAPI/*.txt` files (analyzer enforces) +- **Breaking changes**: Must update `PublicAPI/*.txt` files (by hand — no analyzer enforces this) - **ADO.NET compliance**: Follow ADO.NET patterns and interfaces correctly - **Dispose patterns**: Proper `IDisposable` implementation, no resource leaks diff --git a/ClickHouse.Driver.Benchmark/AdoReadPathBenchmark.cs b/ClickHouse.Driver.Benchmark/AdoReadPathBenchmark.cs new file mode 100644 index 000000000..2535b1df8 --- /dev/null +++ b/ClickHouse.Driver.Benchmark/AdoReadPathBenchmark.cs @@ -0,0 +1,135 @@ +using System; +using System.Threading.Tasks; +using BenchmarkDotNet.Attributes; +using BenchmarkDotNet.Engines; +using ClickHouse.Driver.ADO; +using ClickHouse.Driver.ADO.Readers; +using ClickHouse.Driver.Utility; + +namespace ClickHouse.Driver.Benchmark; + +/// +/// The four ways a caller drives , over one realistic wide row. +/// +/// measures single-column reads with and without a converter, which +/// isolates per-accessor cost but hides the thing typed column slots actually changed: how much a row costs +/// to decode before anyone looks at it, and how that scales with the fraction of columns read. The +/// old object[] row buffer boxed every cell during Read(), so all five variants below allocated +/// identically — even , which reads nothing. +/// +/// +/// — the floor: decode cost with no accessor calls at all. +/// — the linq2db path. Its compiled mapper inlines +/// GetInt64/GetDouble/GetString/GetDateTime/GetGuid per column per row. +/// — hand-written GetFieldValue<T> code. +/// — the Dapper path. Its emitted IL calls the this[int] indexer, +/// i.e. GetValue, so it still boxes and pays one fixed slot allocation per returned column; this +/// variant is the "must not regress" control. +/// — reads 2 of 10 columns, the case the old eager boxing +/// punished hardest. +/// +/// +[Config(typeof(ComparisonConfig))] +[MemoryDiagnoser(true)] +public class AdoReadPathBenchmark +{ + private readonly Consumer consumer = new(); + private ClickHouseConnection connection; + + // The short cases expose the fixed per-reader slot cost that a 200k-row allocation total rounds away. + [Params(1, 10, 200000)] + public int Count { get; set; } + + // Ten columns, eight of them value types — the shape that used to box eight times per row. + private string Sql => $@" +SELECT toInt64(number) AS c0, + toInt64(number * 2) AS c1, + toInt64(number * 3) AS c2, + toInt64(number * 5) AS c3, + toFloat64(number) * 0.5 AS c4, + toFloat64(number) * 1.5 AS c5, + concat('s', toString(number % 8)) AS c6, + concat('t', toString(number % 4)) AS c7, + toDateTime(1700000000 + (number % 65536), 'UTC') AS c8, + toUUID(concat('00000000-0000-0000-0000-', leftPad(toString(number % 1000), 12, '0'))) AS c9 +FROM system.numbers LIMIT {Count}"; + + [GlobalSetup] + public void Setup() + { + var connectionString = Environment.GetEnvironmentVariable("CLICKHOUSE_CONNECTION") ?? "Host=localhost"; + connection = new ClickHouseConnection(new ClickHouseClientSettings(connectionString)); + } + + [GlobalCleanup] + public void Cleanup() => connection?.Dispose(); + + [Benchmark(Baseline = true)] + public async Task UntypedAccessor() + { + using var reader = await connection.ExecuteReaderAsync(Sql); + while (reader.Read()) + { + for (var i = 0; i < 10; i++) + consumer.Consume(reader.GetValue(i)); + } + } + + [Benchmark] + public async Task Scan() + { + using var reader = await connection.ExecuteReaderAsync(Sql); + while (reader.Read()) + { + } + } + + [Benchmark] + public async Task TypedAccessors() + { + using var reader = (ClickHouseDataReader)await connection.ExecuteReaderAsync(Sql); + while (reader.Read()) + { + consumer.Consume(reader.GetInt64(0)); + consumer.Consume(reader.GetInt64(1)); + consumer.Consume(reader.GetInt64(2)); + consumer.Consume(reader.GetInt64(3)); + consumer.Consume(reader.GetDouble(4)); + consumer.Consume(reader.GetDouble(5)); + consumer.Consume(reader.GetString(6)); + consumer.Consume(reader.GetString(7)); + consumer.Consume(reader.GetDateTime(8)); + consumer.Consume(reader.GetGuid(9)); + } + } + + [Benchmark] + public async Task GenericAccessor() + { + using var reader = (ClickHouseDataReader)await connection.ExecuteReaderAsync(Sql); + while (reader.Read()) + { + consumer.Consume(reader.GetFieldValue(0)); + consumer.Consume(reader.GetFieldValue(1)); + consumer.Consume(reader.GetFieldValue(2)); + consumer.Consume(reader.GetFieldValue(3)); + consumer.Consume(reader.GetFieldValue(4)); + consumer.Consume(reader.GetFieldValue(5)); + consumer.Consume(reader.GetFieldValue(6)); + consumer.Consume(reader.GetFieldValue(7)); + consumer.Consume(reader.GetFieldValue(8)); + consumer.Consume(reader.GetFieldValue(9)); + } + } + + [Benchmark] + public async Task TypedAccessorsProjected() + { + using var reader = (ClickHouseDataReader)await connection.ExecuteReaderAsync(Sql); + while (reader.Read()) + { + consumer.Consume(reader.GetInt64(0)); + consumer.Consume(reader.GetString(6)); + } + } +} diff --git a/ClickHouse.Driver.Tests/ADO/BoxFreeReaderAccessorTests.cs b/ClickHouse.Driver.Tests/ADO/BoxFreeReaderAccessorTests.cs new file mode 100644 index 000000000..517e0e1d2 --- /dev/null +++ b/ClickHouse.Driver.Tests/ADO/BoxFreeReaderAccessorTests.cs @@ -0,0 +1,516 @@ +using System; +using System.Collections.Generic; +using System.Threading.Tasks; +using ClickHouse.Driver.ADO; +using ClickHouse.Driver.ADO.Readers; +using ClickHouse.Driver.Types; +using ClickHouse.Driver.Utility; + +namespace ClickHouse.Driver.Tests.ADO; + +/// +/// Pins the accessor semantics of the typed-column-slot reader against a live server. +/// +/// proves a slot decodes the same bytes to the same value as the boxed reader. +/// What it cannot see is the layer above: which slot each accessor reaches for, and — more importantly — that the +/// cases the fast path deliberately declines still fail exactly as they used to. Widening, reading a NULL as a +/// non-nullable target, and T = U? all fall through to the boxed cast, and their +/// s are part of the ADO.NET contract callers rely on. +/// +[TestFixture] +public class BoxFreeReaderAccessorTests : AbstractConnectionTestFixture +{ + private async Task ReadOneAsync(string selectList) + { + var reader = (ClickHouseDataReader)await connection.ExecuteReaderAsync($"SELECT {selectList}"); + Assert.That(reader.Read(), Is.True); + return reader; + } + + // ---- GetFieldValue: the typed slot must produce exactly what the boxed cast produced ---- + + [TestCase("toInt8(-8)", typeof(sbyte), (sbyte)-8)] + [TestCase("toInt16(-16)", typeof(short), (short)-16)] + [TestCase("toInt32(-32)", typeof(int), -32)] + [TestCase("toInt64(-64)", typeof(long), -64L)] + [TestCase("toUInt8(8)", typeof(byte), (byte)8)] + [TestCase("toUInt16(16)", typeof(ushort), (ushort)16)] + [TestCase("toUInt32(32)", typeof(uint), 32u)] + [TestCase("toUInt64(64)", typeof(ulong), 64ul)] + [TestCase("toFloat32(1.5)", typeof(float), 1.5f)] + [TestCase("toFloat64(2.5)", typeof(double), 2.5d)] + [TestCase("true", typeof(bool), true)] + [TestCase("'abc'", typeof(string), "abc")] + public async Task GetFieldValue_ExactTargetType_ReturnsValue(string expression, Type target, object expected) + { + using var reader = await ReadOneAsync($"{expression} AS c"); + Assert.That(GetFieldValueDynamic(reader, target), Is.EqualTo(expected)); + } + + // Invariant in T, as the boxed unbox-any was: no widening, ever. + [TestCase("toInt32(1)", typeof(long))] + [TestCase("toFloat32(1)", typeof(double))] + [TestCase("toUInt8(1)", typeof(int))] + [TestCase("toInt64(1)", typeof(string))] + public async Task GetFieldValue_WideningTarget_ThrowsInvalidCast(string expression, Type target) + { + using var reader = await ReadOneAsync($"{expression} AS c"); + Assert.Throws(() => GetFieldValueDynamic(reader, target)); + } + + // A Nullable target over a non-nullable column has no typed slot (the slot holds U, not U?), but the + // boxed fallback lifts it, so it keeps working. + [Test] + public async Task GetFieldValue_NullableTargetOverNonNullableColumn_LiftsThroughBoxedFallback() + { + using var reader = await ReadOneAsync("toInt64(9) AS c"); + Assert.That(reader.GetFieldValue(0), Is.EqualTo(9L)); + } + + [Test] + public async Task GetFieldValue_NullableColumnWithValue_ReadsUnderlyingType() + { + using var reader = await ReadOneAsync("toInt64OrNull('7') AS c"); + Assert.Multiple(() => + { + Assert.That(reader.IsDBNull(0), Is.False); + Assert.That(reader.GetFieldValue(0), Is.EqualTo(7L)); + // T = U? is deliberately left to the boxed fallback (a generic-interface dispatch that would + // serve both costs 3.5-5.5x the sealed-class check). It must still work. + Assert.That(reader.GetFieldValue(0), Is.EqualTo(7L)); + }); + } + + // A NULL cell has no value to hand back as a non-nullable T, so the boxed DBNull cast throws — the + // pre-slot behaviour, preserved by letting the null case fall through to GetBoxed(). + [Test] + public async Task GetFieldValue_NullCellAsNonNullableTarget_ThrowsInvalidCast() + { + using var reader = await ReadOneAsync("CAST(NULL AS Nullable(Int64)) AS c"); + Assert.That(reader.IsDBNull(0), Is.True); + Assert.Throws(() => reader.GetFieldValue(0)); + } + + // Also pre-slot behaviour, and the surprising one: DBNull does not unbox to Nullable either. + [Test] + public async Task GetFieldValue_NullCellAsNullableTarget_ThrowsInvalidCast() + { + using var reader = await ReadOneAsync("CAST(NULL AS Nullable(Int64)) AS c"); + Assert.Throws(() => reader.GetFieldValue(0)); + } + + [Test] + public async Task GetFieldValue_ObjectTarget_ReturnsTheBoxedValueAndDBNullForNull() + { + using var reader = await ReadOneAsync("toInt64(5) AS a, CAST(NULL AS Nullable(Int64)) AS b"); + Assert.Multiple(() => + { + Assert.That(reader.GetFieldValue(0), Is.EqualTo(5L)); + Assert.That(reader.GetFieldValue(1), Is.EqualTo(DBNull.Value)); + }); + } + + // A composite has no typed slot; GetFieldValue must keep working through the boxed fallback. + [Test] + public async Task GetFieldValue_CompositeColumn_ReadsThroughBoxedFallback() + { + using var reader = await ReadOneAsync("array(toInt32(1), toInt32(2), toInt32(3)) AS c"); + Assert.That(reader.GetFieldValue(0), Is.EqualTo(new[] { 1, 2, 3 })); + } + + // Pins the user-visible contract for an AggregateFunction column: the reader opens, FieldCount answers, + // and the failure arrives on the row with the message that tells you to use xMerge(). + // + // Deliberately not the guard on ColumnSlotFactory's bail-out ordering, though it reads like one. Slots are + // built on the first Read(), so an ordering regression would throw the same exception with the same + // message from this same call; only the unit test + // (ColumnSlotTests.Create_AggregateFunctionColumn_FallsBackToBoxedWithoutEvaluatingFrameworkType) can + // distinguish them. + [Test] + public async Task Read_AggregateFunctionColumn_OpensTheReaderAndFailsOnlyOnTheValue() + { + using var reader = (ClickHouseDataReader)await connection.ExecuteReaderAsync( + "SELECT quantileState(0.5)(number) AS c FROM numbers(10)"); + + Assert.That(reader.FieldCount, Is.EqualTo(1)); + var ex = Assert.Throws(() => reader.Read()); + Assert.That(ex.Message, Does.Contain("Merge()")); + } + + // ---- No current row ---- + // + // Slots hold typed storage, so without this guard a non-nullable value column would read back as a + // perfectly plausible 0 / false / Guid.Empty before the first Read() — data-shaped, and indistinguishable + // from a real value. (The old object[] buffer started all-null, so GetValue returned null and a typed + // accessor threw NullReferenceException.) Every value accessor now reports the mistake instead. + + private static IEnumerable ValueAccessors() + { + yield return Accessor("GetValue", r => r.GetValue(0)); + yield return Accessor("Indexer", r => r[0]); + yield return Accessor("IndexerByName", r => r["a"]); + yield return Accessor("GetValues", r => r.GetValues(new object[3])); + yield return Accessor("GetFieldValue", r => r.GetFieldValue(0)); + yield return Accessor("IsDBNull", r => r.IsDBNull(0)); + yield return Accessor("GetInt64", r => r.GetInt64(0)); + yield return Accessor("GetString", r => r.GetString(1)); + yield return Accessor("GetBoolean", r => r.GetBoolean(0)); + yield return Accessor("GetDecimal", r => r.GetDecimal(2)); + yield return Accessor("GetDateTime", r => r.GetDateTime(0)); + + static TestCaseData Accessor(string name, Func read) + => new TestCaseData(read).SetArgDisplayNames(name); + } + + private const string ThreeColumns = "toInt64(1) AS a, 's' AS b, toDecimal64(1.5, 2) AS c"; + + [TestCaseSource(nameof(ValueAccessors))] + public async Task ValueAccessor_BeforeFirstRead_ThrowsInvalidOperation(Func read) + { + using var reader = (ClickHouseDataReader)await connection.ExecuteReaderAsync($"SELECT {ThreeColumns}"); + + var ex = Assert.Throws(() => read(reader)); + Assert.That(ex.Message, Does.Contain("Read()")); + } + + [TestCaseSource(nameof(ValueAccessors))] + public async Task ValueAccessor_AfterReadReturnsFalse_ThrowsInvalidOperation(Func read) + { + using var reader = (ClickHouseDataReader)await connection.ExecuteReaderAsync($"SELECT {ThreeColumns}"); + Assert.That(reader.Read(), Is.True); + Assert.That(reader.Read(), Is.False); + + Assert.Throws(() => read(reader)); + } + + // Column metadata does not depend on a row and must stay reachable — this is what a caller inspecting the + // shape of an empty result set needs, and what DataTable.Load asks for before its first Read(). + [Test] + public async Task ColumnMetadata_WithNoCurrentRow_IsStillAvailable() + { + using var reader = (ClickHouseDataReader)await connection.ExecuteReaderAsync( + $"SELECT {ThreeColumns} FROM system.numbers WHERE 0"); + + Assert.Multiple(() => + { + Assert.That(reader.FieldCount, Is.EqualTo(3)); + Assert.That(reader.GetName(0), Is.EqualTo("a")); + Assert.That(reader.GetOrdinal("b"), Is.EqualTo(1)); + Assert.That(reader.GetFieldType(0), Is.EqualTo(typeof(long))); + Assert.That(reader.GetDataTypeName(0), Is.EqualTo("Int64")); + Assert.That(reader.GetSchemaTable().Rows, Has.Count.EqualTo(3)); + }); + } + + // ---- IsDBNull now answers from the slot's presence flag rather than inspecting a boxed value ---- + + [Test] + public async Task IsDBNull_AcrossColumnKinds_MatchesNullability() + { + using var reader = await ReadOneAsync( + "toInt64(1) AS nonNullableValue, " + + "'s' AS nonNullableRef, " + + "toInt64OrNull('1') AS nullableWithValue, " + + "CAST(NULL AS Nullable(Int64)) AS nullableValue, " + + "CAST(NULL AS Nullable(String)) AS nullableRef, " + + "array(1) AS composite, " + + "CAST(NULL AS Nullable(UUID)) AS nullableUuid"); + + Assert.Multiple(() => + { + Assert.That(reader.IsDBNull(0), Is.False); + Assert.That(reader.IsDBNull(1), Is.False); + Assert.That(reader.IsDBNull(2), Is.False); + Assert.That(reader.IsDBNull(3), Is.True); + Assert.That(reader.IsDBNull(4), Is.True); + Assert.That(reader.IsDBNull(5), Is.False); + Assert.That(reader.IsDBNull(6), Is.True); + }); + } + + // A slot is reused across rows, so presence has to be re-decoded every row rather than latched. + [Test] + public async Task IsDBNull_AlternatingNullsAcrossRows_TracksEachRow() + { + using var reader = (ClickHouseDataReader)await connection.ExecuteReaderAsync( + "SELECT if(number % 2 = 0, NULL, toInt64(number)) AS c FROM system.numbers LIMIT 6"); + + for (var row = 0; row < 6; row++) + { + Assert.That(reader.Read(), Is.True); + Assert.That(reader.IsDBNull(0), Is.EqualTo(row % 2 == 0), $"row {row}"); + if (row % 2 != 0) + Assert.That(reader.GetFieldValue(0), Is.EqualTo((long)row), $"row {row}"); + } + } + + // ---- GetValues / GetValue keep the boxed contract the BCL data-binding stack depends on ---- + + [Test] + public async Task GetValues_MixedRow_ProducesTheSameBoxedValuesAsGetValue() + { + using var reader = await ReadOneAsync( + "toInt64(1) AS a, 'b' AS b, CAST(NULL AS Nullable(Int64)) AS c, array(1, 2) AS d"); + + var values = new object[reader.FieldCount]; + Assert.That(reader.GetValues(values), Is.EqualTo(4)); + Assert.Multiple(() => + { + Assert.That(values[0], Is.EqualTo(1L)); + Assert.That(values[1], Is.EqualTo("b")); + Assert.That(values[2], Is.EqualTo(DBNull.Value)); + Assert.That(values[3], Is.EqualTo(new[] { 1, 2 })); + for (var i = 0; i < values.Length; i++) + Assert.That(values[i], Is.EqualTo(reader.GetValue(i)), $"column {i}"); + }); + } + + // GetValues writes as many cells as the caller's array has room for, and no more. + [Test] + public async Task GetValues_ShorterDestinationArray_FillsOnlyWhatFits() + { + using var reader = await ReadOneAsync("toInt64(1) AS a, toInt64(2) AS b, toInt64(3) AS c"); + + var values = new object[2]; + Assert.That(reader.GetValues(values), Is.EqualTo(2)); + Assert.That(values, Is.EqualTo(new object[] { 1L, 2L })); + } + + // Boxing is now per call rather than once per row, so the same cell yields two distinct boxes. They must + // still compare equal — that is the comparison ADO.NET consumers and DataTable actually use. + [Test] + public async Task GetValue_CalledTwiceOnAValueTypeCell_ReturnsEqualValues() + { + using var reader = await ReadOneAsync("toInt64(42) AS c"); + Assert.That(reader.GetValue(0), Is.EqualTo(reader.GetValue(0))); + } + + // Values handed out must not be invalidated by advancing the reader: GetValue returns a fresh box, and a + // reference-typed cell hands out the reference the caller then owns. DataReaderTests exercises the same + // shape via LINQ over IDataRecord; this states the guarantee directly. + [Test] + public async Task GetValue_RetainedAcrossRead_KeepsTheOriginalRowsValue() + { + using var reader = (ClickHouseDataReader)await connection.ExecuteReaderAsync( + "SELECT toInt64(number) AS n, toString(number) AS s FROM system.numbers LIMIT 2"); + + Assert.That(reader.Read(), Is.True); + var firstNumber = reader.GetValue(0); + var firstString = reader.GetValue(1); + + Assert.That(reader.Read(), Is.True); + Assert.Multiple(() => + { + Assert.That(firstNumber, Is.EqualTo(0L)); + Assert.That(firstString, Is.EqualTo("0")); + Assert.That(reader.GetValue(0), Is.EqualTo(1L)); + }); + } + + // ---- Typed accessors: what linq2db's compiled mapper actually calls, one per column per row ---- + + [Test] + public async Task TypedAccessors_ExactColumnTypes_ReturnValues() + { + using var reader = await ReadOneAsync( + "toInt8(-8) AS a, toInt16(-16) AS b, toInt32(-32) AS c, toInt64(-64) AS d, " + + "toUInt8(8) AS e, toUInt16(16) AS f, toUInt32(32) AS g, toUInt64(64) AS h, " + + "toFloat32(1.5) AS i, toFloat64(2.5) AS j, true AS k, 'abc' AS l, " + + "toUUID('11223344-5566-7788-99aa-bbccddeeff00') AS m, " + + "toIPv6('2001:db8::1') AS n, toInt256(-256) AS o, " + + "toDateTime('2025-01-15 12:00:00', 'UTC') AS p"); + + Assert.Multiple(() => + { + Assert.That(reader.GetSByte(0), Is.EqualTo((sbyte)-8)); + Assert.That(reader.GetInt16(1), Is.EqualTo((short)-16)); + Assert.That(reader.GetInt32(2), Is.EqualTo(-32)); + Assert.That(reader.GetInt64(3), Is.EqualTo(-64L)); + Assert.That(reader.GetByte(4), Is.EqualTo((byte)8)); + Assert.That(reader.GetUInt16(5), Is.EqualTo((ushort)16)); + Assert.That(reader.GetUInt32(6), Is.EqualTo(32u)); + Assert.That(reader.GetUInt64(7), Is.EqualTo(64ul)); + Assert.That(reader.GetFloat(8), Is.EqualTo(1.5f)); + Assert.That(reader.GetDouble(9), Is.EqualTo(2.5d)); + Assert.That(reader.GetBoolean(10), Is.True); + Assert.That(reader.GetString(11), Is.EqualTo("abc")); + Assert.That(reader.GetGuid(12), Is.EqualTo(Guid.Parse("11223344-5566-7788-99aa-bbccddeeff00"))); + Assert.That(reader.GetIPAddress(13), Is.EqualTo(System.Net.IPAddress.Parse("2001:db8::1"))); + Assert.That(reader.GetBigInteger(14), Is.EqualTo(new System.Numerics.BigInteger(-256))); + Assert.That(reader.GetDateTime(15), Is.EqualTo(new DateTime(2025, 1, 15, 12, 0, 0, DateTimeKind.Utc))); + Assert.That(reader.GetDateTimeOffset(15), Is.EqualTo(new DateTimeOffset(2025, 1, 15, 12, 0, 0, TimeSpan.Zero))); + }); + } + + // A Nullable(T) slot holds the underlying T, so the typed accessors reach it without a box. + [Test] + public async Task TypedAccessors_NullableColumnsWithValues_ReturnUnderlyingValues() + { + using var reader = await ReadOneAsync( + "toInt64OrNull('7') AS a, toFloat64OrNull('1.5') AS b, " + + "CAST('s' AS Nullable(String)) AS c, CAST(true AS Nullable(Bool)) AS d, " + + "CAST(false AS Nullable(Bool)) AS e"); + + Assert.Multiple(() => + { + Assert.That(reader.GetInt64(0), Is.EqualTo(7L)); + Assert.That(reader.GetDouble(1), Is.EqualTo(1.5d)); + Assert.That(reader.GetString(2), Is.EqualTo("s")); + + // Both polarities, so the assertion cannot be satisfied by a branch that hard-codes one. + Assert.That(reader.GetBoolean(3), Is.True); + Assert.That(reader.GetBoolean(4), Is.False); + }); + } + + // Pre-slot behaviour: the typed accessors were all `(T)GetValue(ordinal)`, so a NULL threw. + [Test] + public async Task TypedAccessors_NullCell_ThrowInvalidCast() + { + using var reader = await ReadOneAsync( + "CAST(NULL AS Nullable(Int64)) AS a, CAST(NULL AS Nullable(Float64)) AS b, " + + "CAST(NULL AS Nullable(UUID)) AS c, CAST(NULL AS Nullable(Bool)) AS d"); + + Assert.Multiple(() => + { + Assert.Throws(() => reader.GetInt64(0)); + Assert.Throws(() => reader.GetDouble(1)); + Assert.Throws(() => reader.GetGuid(2)); + Assert.Throws(() => reader.GetBoolean(3)); + }); + } + + // GetString coerces rather than casts, and DBNull.Value.ToString() is "". Surprising, but pre-existing + // and load-bearing for anyone relying on it — which is why the fast path only covers non-nullable String. + [Test] + public async Task GetString_NullCell_ReturnsEmptyStringNotNull() + { + using var reader = await ReadOneAsync("CAST(NULL AS Nullable(String)) AS c"); + Assert.That(reader.GetString(0), Is.EqualTo(string.Empty)); + } + + // GetBoolean is the one accessor that widens; only an exact Bool column short-circuits, so the + // Convert.ToBoolean coercion has to survive for everything else. + [TestCase("toUInt8(1)", true)] + [TestCase("toUInt8(0)", false)] + [TestCase("toInt32(5)", true)] + [TestCase("toFloat64(0)", false)] + public async Task GetBoolean_NonBoolColumn_StillCoerces(string expression, bool expected) + { + using var reader = await ReadOneAsync($"{expression} AS c"); + Assert.That(reader.GetBoolean(0), Is.EqualTo(expected)); + } + + // GetString coerces too, and must keep doing so for columns that are not String at all. + [Test] + public async Task GetString_NonStringColumn_StillCoerces() + { + using var reader = await ReadOneAsync("toInt64(42) AS c"); + Assert.That(reader.GetString(0), Is.EqualTo("42")); + } + + // A Decimal column resolves to a decimal or a ClickHouseDecimal slot depending on UseCustomDecimals; + // GetDecimal has to reach the same decimal either way. Nullable() doubles that again — it is a different + // slot kind, so a different branch — and a NULL cell has to keep throwing off the boxed fallback. + [TestCase(false)] + [TestCase(true)] + public async Task GetDecimal_UnderEitherDecimalRepresentation_ReturnsTheSameValue(bool useCustomDecimals) + { + var settings = TestUtilities.GetTestClickHouseClientSettings(); + settings = new ClickHouseClientSettings(settings) { UseCustomDecimals = useCustomDecimals }; + using var client = new ClickHouseClient(settings); + + using var reader = await client.ExecuteReaderAsync( + "SELECT toDecimal64(12.34, 2) AS c, toDecimal64OrNull('56.78', 2) AS n, " + + "CAST(NULL AS Nullable(Decimal64(2))) AS z"); + Assert.That(reader.Read(), Is.True); + + Assert.Multiple(() => + { + Assert.That(reader.GetDecimal(0), Is.EqualTo(12.34m)); + Assert.That(reader.GetDecimal(1), Is.EqualTo(56.78m)); + Assert.Throws(() => reader.GetDecimal(2)); + }); + } + + // ---- Converter routing ---- + + // Which overload a read takes follows how it read, not whether a converter is configured: a value pulled + // unboxed out of its slot converts through ConvertValue, and only a genuinely boxed read uses + // ConvertValue(object, ...). This pins all three cases against a converter that doubles in the object + // overload alone, so the choice is directly observable. + [Test] + public async Task WithConverter_TypedReadsUseTheGenericOverloadAndBoxedReadsUseTheObjectOverload() + { + var settings = TestUtilities.GetTestClickHouseClientSettings(); + settings = new ClickHouseClientSettings(settings) { ReadValueConverter = new ObjectOnlyDoublingConverter() }; + using var client = new ClickHouseClient(settings); + + using var reader = await client.ExecuteReaderAsync("SELECT toInt64(21) AS c"); + Assert.That(reader.Read(), Is.True); + + Assert.Multiple(() => + { + Assert.That(reader.GetInt64(0), Is.EqualTo(21L), "GetInt64 reads its slot unboxed, so it converts through ConvertValue"); + Assert.That(reader.GetFieldValue(0), Is.EqualTo(21L), "GetFieldValue converts through ConvertValue"); + Assert.That(reader.GetValue(0), Is.EqualTo(42L), "GetValue is a boxed read, so it stays on ConvertValue(object, ...)"); + }); + } + + // A converter does not take GetDecimal off its slot path: a ClickHouseDecimal column is narrowed to decimal + // first, so the converter sees the decimal the accessor returns rather than the column's own + // representation. A NULL cell falls through to the boxed path and throws there. + [Test] + public async Task GetDecimal_WithConverter_ReadsTheSlotAndConvertsOnTheGenericOverload() + { + var settings = TestUtilities.GetTestClickHouseClientSettings(); + settings = new ClickHouseClientSettings(settings) { ReadValueConverter = new GenericOnlyDecimalDoublingConverter() }; + using var client = new ClickHouseClient(settings); + + using var reader = await client.ExecuteReaderAsync( + "SELECT toDecimal64(12.34, 2) AS c, CAST(NULL AS Nullable(Decimal64(2))) AS z"); + Assert.That(reader.Read(), Is.True); + + Assert.Multiple(() => + { + Assert.That(reader.GetDecimal(0), Is.EqualTo(24.68m)); + Assert.Throws(() => reader.GetDecimal(1)); + }); + } + + // Doubles longs in the object overload only, so which overload an accessor picks is directly observable. + private sealed class ObjectOnlyDoublingConverter : IReadValueConverter + { + public object ConvertValue(object value, string columnName, string clickHouseType) + => value is long l ? l * 2 : value; + + public T ConvertValue(T value, string columnName, string clickHouseType) => value; + } + + // The mirror image, for the accessors whose slot value is a decimal. + private sealed class GenericOnlyDecimalDoublingConverter : IReadValueConverter + { + public object ConvertValue(object value, string columnName, string clickHouseType) => value; + + public T ConvertValue(T value, string columnName, string clickHouseType) + => value is decimal d ? (T)(object)(d * 2) : value; + } + + // Reflection is the only way to parametrize over T. Unwraps TargetInvocationException so the assertions + // above see the exception the caller would actually get. + private static object GetFieldValueDynamic(ClickHouseDataReader reader, Type target) + { + var method = typeof(ClickHouseDataReader) + .GetMethod(nameof(ClickHouseDataReader.GetFieldValue), [typeof(int)]) + .MakeGenericMethod(target); + try + { + return method.Invoke(reader, [0]); + } + catch (System.Reflection.TargetInvocationException ex) when (ex.InnerException != null) + { + System.Runtime.ExceptionServices.ExceptionDispatchInfo.Capture(ex.InnerException).Throw(); + throw; // unreachable + } + } +} diff --git a/ClickHouse.Driver.Tests/ADO/BoxFreeReaderAllocationTests.cs b/ClickHouse.Driver.Tests/ADO/BoxFreeReaderAllocationTests.cs new file mode 100644 index 000000000..1dca8db16 --- /dev/null +++ b/ClickHouse.Driver.Tests/ADO/BoxFreeReaderAllocationTests.cs @@ -0,0 +1,284 @@ +using System; +using System.IO; +using System.Net; +using System.Net.Http; +using System.Reflection; +using System.Threading.Tasks; +using ClickHouse.Driver.ADO.Readers; +using ClickHouse.Driver.Formats; +using ClickHouse.Driver.Numerics; +using ClickHouse.Driver.Types; + +namespace ClickHouse.Driver.Tests.ADO; + +/// +/// The regression guard for the typed-column-slot reader. Every other test would still pass if boxing crept +/// back in — the values would be identical, only the garbage would return — so this is the one that fails. +/// +/// Deliberately server-free and synchronous: the payload is a pre-built RowBinaryWithNamesAndTypes +/// buffer served from a , so Read() and the accessors run entirely on +/// the test thread and is exact rather than a sample of +/// whatever else the process is doing. +/// +[TestFixture] +[NonParallelizable] +public class BoxFreeReaderAllocationTests +{ + private const int Rows = 5000; + private const int Columns = 4; + + // Four value-type columns, all with typed readers, none of whose decode allocates. That isolates the + // measurement to boxing: anything left is the thing under test. + private static readonly string[] ColumnTypes = ["Int64", "Float64", "Int32", "UInt64"]; + + private static byte[] BuildPayload() => BuildPayload(ColumnTypes, TypeSettings.Default, WriteNumericRow); + + private static void WriteNumericRow(ExtendedBinaryWriter writer, ClickHouseType[] types, int row) + { + types[0].Write(writer, (long)row); + types[1].Write(writer, (double)row); + types[2].Write(writer, row); + types[3].Write(writer, (ulong)row); + } + + private static byte[] BuildPayload(string[] columnTypes, TypeSettings settings, Action writeRow, int rows = Rows) + { + var types = Array.ConvertAll(columnTypes, t => TypeConverter.ParseClickHouseType(t, settings)); + + using var stream = new MemoryStream(); + using var writer = new ExtendedBinaryWriter(stream); + + writer.Write7BitEncodedInt(columnTypes.Length); + for (var i = 0; i < columnTypes.Length; i++) + writer.Write($"c{i}"); + foreach (var name in columnTypes) + writer.Write(name); + + for (var row = 0; row < rows; row++) + writeRow(writer, types, row); + + writer.Flush(); + return stream.ToArray(); + } + + private static Task CreateReaderAsync(byte[] payload) => CreateReaderAsync(payload, TypeSettings.Default); + + private static Task CreateReaderAsync(byte[] payload, TypeSettings settings) + => ClickHouseDataReader.FromHttpResponseAsync( + new HttpResponseMessage(HttpStatusCode.OK) { Content = new ByteArrayContent(payload) }, + settings); + + // Returns bytes allocated while draining the reader. Sums the values into `checksum` so nothing the + // accessors produce can be optimized away as dead. + private static long Measure(ClickHouseDataReader reader, Func readRow, out double checksum) + { + var sum = 0d; + var before = GC.GetAllocatedBytesForCurrentThread(); + var rows = 0; + while (reader.Read()) + { + sum += readRow(reader); + rows++; + } + var allocated = GC.GetAllocatedBytesForCurrentThread() - before; + + Assert.That(rows, Is.EqualTo(Rows), "the payload did not decode to the expected row count"); + checksum = sum; + return allocated; + } + + private static double Scan(ClickHouseDataReader reader) => 0d; + + private static double ReadTyped(ClickHouseDataReader reader) + => reader.GetInt64(0) + reader.GetDouble(1) + reader.GetInt32(2) + reader.GetUInt64(3); + + private static double ReadFieldValue(ClickHouseDataReader reader) + => reader.GetFieldValue(0) + reader.GetFieldValue(1) + + reader.GetFieldValue(2) + reader.GetFieldValue(3); + + private static double ReadBoxed(ClickHouseDataReader reader) + => (long)reader.GetValue(0) + (double)reader.GetValue(1) + + (int)reader.GetValue(2) + (ulong)reader.GetValue(3); + + [Test] + public async Task Read_ValueTypeColumns_AllocatesNothingPerRow() + { + var payload = BuildPayload(); + + // First pass per shape pays for JIT and generic instantiation. Neither is per-row, and neither should + // be attributed to the measurement. + foreach (var warmup in new[] { Scan, ReadTyped, ReadFieldValue, ReadBoxed }) + { + using var reader = await CreateReaderAsync(payload); + Measure(reader, warmup, out _); + } + + long scan, typed, fieldValue, boxed; + using (var reader = await CreateReaderAsync(payload)) + scan = Measure(reader, Scan, out _); + using (var reader = await CreateReaderAsync(payload)) + typed = Measure(reader, ReadTyped, out _); + using (var reader = await CreateReaderAsync(payload)) + fieldValue = Measure(reader, ReadFieldValue, out _); + using (var reader = await CreateReaderAsync(payload)) + boxed = Measure(reader, ReadBoxed, out _); + + TestContext.Out.WriteLine( + $"scan={PerRow(scan)} typed={PerRow(typed)} fieldValue={PerRow(fieldValue)} boxed={PerRow(boxed)} (bytes/row)"); + + // One box per value-type cell, and a box is 24 bytes on 64-bit. Requiring even half of that is a wide + // margin against allocation the reader does for other reasons, while still failing outright if any of + // the three de-boxed paths starts boxing again. + const long BoxedFloor = Rows * Columns * 12; + + Assert.Multiple(() => + { + // The headline: Read() used to box every cell of every row whether or not anyone looked at it. + Assert.That(scan, Is.LessThan(Rows), $"Read() must not allocate per row, saw {PerRow(scan)} B/row"); + Assert.That(typed, Is.LessThan(Rows), $"typed accessors must not box, saw {PerRow(typed)} B/row"); + Assert.That(fieldValue, Is.LessThan(Rows), $"GetFieldValue must not box, saw {PerRow(fieldValue)} B/row"); + + // And the control: the untyped path still boxes, so the comparison above is measuring something. + Assert.That(boxed, Is.GreaterThan(BoxedFloor), + $"GetValue is expected to still box; saw only {PerRow(boxed)} B/row, so this test is not measuring boxing"); + }); + } + + // Bool and Decimal(10,2), read through the two accessors that coerce rather than cast. + private static readonly string[] CoercedColumnTypes = ["Bool", "Decimal(10,2)"]; + private static readonly string[] NullableCoercedColumnTypes = ["Nullable(Bool)", "Nullable(Decimal(10,2))"]; + + // Every cell is populated: a NULL costs nothing on either path (DBNull.Value is a singleton), so it is + // the present value that can be left boxing. + private static void WriteCoercedRow(ExtendedBinaryWriter writer, ClickHouseType[] types, int row) + { + types[0].Write(writer, row % 2 == 0); + types[1].Write(writer, row / 100m); + } + + private static double ReadCoerced(ClickHouseDataReader reader) + => (reader.GetBoolean(0) ? 1d : 0d) + (double)reader.GetDecimal(1); + + // The control has to do the same *work* as ReadCoerced, not merely touch the same cells, or it measures + // the decimal conversion rather than the box. This is the pre-slot body of both accessors verbatim, so + // the difference between the two is exactly the two boxes. + private static double ReadCoercedBoxed(ClickHouseDataReader reader) + { + var flag = Convert.ToBoolean(reader.GetValue(0), System.Globalization.CultureInfo.InvariantCulture); + var raw = reader.GetValue(1); + var value = raw is ClickHouseDecimal chd + ? chd.ToDecimal(System.Globalization.CultureInfo.InvariantCulture) + : (decimal)raw; + return (flag ? 1d : 0d) + (double)value; + } + + /// + /// and coerce + /// rather than cast, so neither can use the shared GetSlotValue<T> body and both match their + /// slot kinds by hand — which is how they came to recognise only the non-nullable ValueSlot<T> + /// and box every populated Nullable(Bool)/Nullable(Decimal) cell, on column types the + /// feature claims to cover. + /// + /// + /// Measured against the identical non-nullable shape rather than against zero, because zero is not the + /// right answer here: under useBigDecimal the ClickHouseDecimal-to-decimal conversion + /// allocates two byte arrays per call (~72 B/row) whatever the reader does, and that cost is common to + /// both sides. The difference isolates exactly the property under test — a Nullable column must + /// cost no more than its non-nullable twin. Both decimal representations are covered because + /// useBigDecimal picks between two different slots reached by two different branches. + /// + [TestCase(true)] + [TestCase(false)] + public async Task GetBooleanAndGetDecimal_PopulatedNullableCells_AllocateNoMoreThanNonNullable(bool useBigDecimal) + { + var settings = TypeSettings.Default with { useBigDecimal = useBigDecimal }; + var plain = BuildPayload(CoercedColumnTypes, settings, WriteCoercedRow); + var nullable = BuildPayload(NullableCoercedColumnTypes, settings, WriteCoercedRow); + + foreach (var payload in new[] { plain, nullable, nullable }) + { + using var warmup = await CreateReaderAsync(payload, settings); + Measure(warmup, ReadCoerced, out _); + using var warmupBoxed = await CreateReaderAsync(payload, settings); + Measure(warmupBoxed, ReadCoercedBoxed, out _); + } + + long plainAllocated, nullableAllocated, nullableBoxed; + using (var reader = await CreateReaderAsync(plain, settings)) + plainAllocated = Measure(reader, ReadCoerced, out _); + using (var reader = await CreateReaderAsync(nullable, settings)) + nullableAllocated = Measure(reader, ReadCoerced, out _); + using (var reader = await CreateReaderAsync(nullable, settings)) + nullableBoxed = Measure(reader, ReadCoercedBoxed, out _); + + TestContext.Out.WriteLine( + $"useBigDecimal={useBigDecimal} plain={PerRow(plainAllocated)} nullable={PerRow(nullableAllocated)} " + + $"nullableBoxed={PerRow(nullableBoxed)} (bytes/row)"); + + Assert.Multiple(() => + { + // One byte per row of slack, which is far below the 24-byte box either accessor would take. + Assert.That(nullableAllocated, Is.LessThanOrEqualTo(plainAllocated + Rows), + $"a populated Nullable cell must cost no more than a non-nullable one; saw " + + $"{PerRow(nullableAllocated)} B/row against {PerRow(plainAllocated)} B/row"); + + // The control: the same two cells through GetValue do box, so the comparison above is measuring + // something rather than two equally-zero numbers. + Assert.That(nullableBoxed, Is.GreaterThan(nullableAllocated + (Rows * 12)), + $"GetValue is expected to box both cells; saw only {PerRow(nullableBoxed)} B/row, so this " + + $"test is not measuring boxing"); + }); + } + + /// + /// The column slots are the reader's only per-column allocation, and QueryAsync<T>'s box-free + /// POCO path never reads one — it materializes straight from the stream. Building them in the constructor + /// would put one permanently dead object per column on the driver's primary read API, and on every empty + /// or metadata-only reader besides. + /// + /// + /// Reached by reflection because the effect under test is an absence. No public surface reports whether + /// the storage exists, and measuring it in bytes would mean resolving a few hundred bytes against the row + /// materialization it is supposed to be dwarfed by. + /// + [Test] + public async Task Read_BeforeFirstRow_HasNotBuiltColumnSlots() + { + var slotsField = SlotsField(); + + using var reader = await CreateReaderAsync(BuildPayload()); + Assert.That(slotsField.GetValue(reader), Is.Null, "column slots must not be built before the first Read()"); + + Assert.That(reader.Read(), Is.True); + Assert.That(slotsField.GetValue(reader), Is.Not.Null, "the first Read() must build the column slots"); + } + + [Test] + public async Task Read_EmptyResult_BuildsNoColumnSlotsAndKeepsMetadata() + { + var slotsField = SlotsField(); + var payload = BuildPayload(ColumnTypes, TypeSettings.Default, WriteNumericRow, rows: 0); + + using var reader = await CreateReaderAsync(payload); + Assert.That(reader.Read(), Is.False); + + Assert.Multiple(() => + { + Assert.That(slotsField.GetValue(reader), Is.Null, "an empty result must not build column slots"); + + // Metadata stays available with no row, which is what makes skipping the storage safe. + Assert.That(reader.FieldCount, Is.EqualTo(Columns)); + Assert.That(reader.GetName(0), Is.EqualTo("c0")); + Assert.That(reader.GetFieldType(0), Is.EqualTo(typeof(long))); + }); + } + + private static FieldInfo SlotsField() + { + var field = typeof(ClickHouseDataReader).GetField("slots", BindingFlags.Instance | BindingFlags.NonPublic); + Assert.That(field, Is.Not.Null, "the reader's column-slot storage was renamed; update these tests"); + return field; + } + + private static string PerRow(long allocated) => (allocated / (double)Rows).ToString("F1", System.Globalization.CultureInfo.InvariantCulture); +} diff --git a/ClickHouse.Driver.Tests/ADO/ColumnSlotTests.cs b/ClickHouse.Driver.Tests/ADO/ColumnSlotTests.cs new file mode 100644 index 000000000..40100bbb1 --- /dev/null +++ b/ClickHouse.Driver.Tests/ADO/ColumnSlotTests.cs @@ -0,0 +1,360 @@ +using System; +using System.Collections; +using System.Collections.Generic; +using System.IO; +using System.Linq; +using System.Net; +using System.Numerics; +using System.Reflection; +using ClickHouse.Driver.ADO.Readers; +using ClickHouse.Driver.Formats; +using ClickHouse.Driver.Numerics; +using ClickHouse.Driver.Types; + +namespace ClickHouse.Driver.Tests.ADO; + +/// +/// Server-free tests for the typed column slots that replaced ClickHouseDataReader's shared +/// object[] row buffer. +/// +/// The design rests on one invariant: a slot must be observationally identical to the boxed path it +/// replaced — consuming exactly the bytes +/// would, and returning +/// exactly the value it would (same CLR type, for a NULL). +/// asserts that differentially against the real +/// boxed reader. The rest pins what parity cannot see: which slot kind a column resolves to (a silent +/// demotion to keeps values correct and only loses the allocation win), and +/// , which has no boxed counterpart to compare against. +/// +[TestFixture] +public class ColumnSlotTests +{ + // Written after the value under test; a slot that consumes the wrong number of bytes decodes garbage here + // rather than silently corrupting the next column of a real row. + private const long Sentinel = 0x1234_5678_09AB_CDEFL; + + private const string Enum8Def = "Enum8('a' = -5, 'b' = 7)"; + + private static ClickHouseType Parse(string typeName, TypeSettings? settings = null) + => TypeConverter.ParseClickHouseType(typeName, settings ?? TypeSettings.Default); + + private static byte[] Write(ClickHouseType type, object value) + { + using var stream = new MemoryStream(); + using var writer = new ExtendedBinaryWriter(stream); + type.Write(writer, value); + new Int64Type().Write(writer, Sentinel); + writer.Flush(); + return stream.ToArray(); + } + + private static (object Value, long Trailer) ReadBoxed(ClickHouseType type, byte[] payload) + { + using var stream = new MemoryStream(payload); + using var reader = new ExtendedBinaryReader(stream); + return (type.Read(reader), reader.ReadInt64()); + } + + private static (object Value, long Trailer, ColumnSlot Slot) ReadSlot(ClickHouseType type, byte[] payload) + { + var slot = ColumnSlotFactory.Create(type); + using var stream = new MemoryStream(payload); + using var reader = new ExtendedBinaryReader(stream); + slot.Read(reader); + return (slot.GetBoxed(), reader.ReadInt64(), slot); + } + + // ---- Differential parity against the boxed reader ---- + + // (type, value) pairs covering every ITypedReader shape a slot can be built over, both bare and under + // Nullable, plus the composites that must fall back. The value is written by the type's own Write, so the + // payload is exactly what the server would send. + private static IEnumerable ParityCases() + { + var settingsByName = new Dictionary + { + ["ReadAsBytes"] = TypeSettings.Default with { readStringsAsByteArrays = true }, + ["BigDecimal"] = TypeSettings.Default with { useBigDecimal = true }, + }; + + foreach (var (typeName, value) in Values()) + { + yield return Case(typeName, value, TypeSettings.Default, string.Empty); + + // Nullable(X) in both states. Composites are covered bare; Nullable(Array(..)) etc. have no typed + // slot anyway and the bare case already pins that. + if (!typeName.StartsWith("Array(", StringComparison.Ordinal) && + !typeName.StartsWith("Tuple(", StringComparison.Ordinal) && + !typeName.StartsWith("Map(", StringComparison.Ordinal)) + { + yield return Case($"Nullable({typeName})", value, TypeSettings.Default, "Present"); + yield return Case($"Nullable({typeName})", DBNull.Value, TypeSettings.Default, "Null"); + } + } + + // The two settings that change FrameworkType, and so which typed reader a slot binds to. + yield return Case("String", "abc", settingsByName["ReadAsBytes"], "ReadAsBytes"); + yield return Case("FixedString(5)", "abcde", settingsByName["ReadAsBytes"], "ReadAsBytes"); + yield return Case("Nullable(String)", "abc", settingsByName["ReadAsBytes"], "ReadAsBytes"); + yield return Case("Nullable(String)", DBNull.Value, settingsByName["ReadAsBytes"], "ReadAsBytesNull"); + yield return Case("Decimal(10, 2)", 12.34m, settingsByName["BigDecimal"], "BigDecimal"); + yield return Case("Nullable(Decimal(10, 2))", DBNull.Value, settingsByName["BigDecimal"], "BigDecimalNull"); + + // Wire-transparent wrappers must behave exactly like the type they wrap. + yield return Case("LowCardinality(String)", "abc", TypeSettings.Default, string.Empty); + yield return Case("LowCardinality(Nullable(String))", "abc", TypeSettings.Default, "Present"); + yield return Case("LowCardinality(Nullable(String))", DBNull.Value, TypeSettings.Default, "Null"); + yield return Case("SimpleAggregateFunction(sum, UInt64)", 42ul, TypeSettings.Default, string.Empty); + yield return Case("SimpleAggregateFunction(any, Nullable(Int32))", DBNull.Value, TypeSettings.Default, "Null"); + + // Passed as a closure rather than as arguments because TypeSettings is internal, and an internal + // parameter type on a public test method does not compile. + static TestCaseData Case(string typeName, object value, TypeSettings settings, string suffix) + => new TestCaseData((Action)(() => AssertParity(typeName, value, settings))) + .SetName($"Parity_{TestUtilities.SanitizeTableName(typeName)}{suffix}_MatchesBoxedRead"); + + static IEnumerable<(string TypeName, object Value)> Values() + { + yield return ("Int8", (sbyte)-8); + yield return ("Int16", (short)-16); + yield return ("Int32", -32); + yield return ("Int64", -64L); + yield return ("UInt8", (byte)8); + yield return ("UInt16", (ushort)16); + yield return ("UInt32", 32u); + yield return ("UInt64", 64ul); + yield return ("Int128", (BigInteger)(-128)); + yield return ("UInt128", (BigInteger)128); + yield return ("Int256", (BigInteger)(-256)); + yield return ("UInt256", (BigInteger)256); + yield return ("Float32", 1.5f); + yield return ("Float64", 2.5d); + yield return ("BFloat16", 1.25f); + yield return ("Bool", true); + yield return ("String", "abc"); + yield return ("FixedString(5)", "abcde"); + yield return ("UUID", Guid.Parse("11223344-5566-7788-99aa-bbccddeeff00")); + yield return ("IPv4", IPAddress.Parse("10.0.0.1")); + yield return ("IPv6", IPAddress.Parse("2001:db8::1")); + yield return ("Date", new DateTime(2021, 3, 4, 0, 0, 0, DateTimeKind.Utc)); + yield return ("Date32", new DateTime(2021, 3, 4, 0, 0, 0, DateTimeKind.Utc)); + yield return ("DateTime('UTC')", new DateTime(2021, 3, 4, 5, 6, 7, DateTimeKind.Utc)); + yield return ("DateTime64(3, 'UTC')", new DateTime(2021, 3, 4, 5, 6, 7, 123, DateTimeKind.Utc)); + yield return ("Time", TimeSpan.FromSeconds(3661)); + yield return ("Time64(3)", TimeSpan.FromMilliseconds(3661123)); + yield return ("Decimal(10, 2)", 12.34m); + yield return ("Decimal(30, 4)", 1234.5678m); + yield return (Enum8Def, "b"); + + // No typed reader: these must fall back and still round-trip byte-for-byte. + yield return ("Array(Int32)", new[] { 1, 2, 3 }); + yield return ("Tuple(Int32, String)", Tuple.Create(1, "a")); + yield return ("Map(String, Int32)", new Dictionary { ["a"] = 1 }); + } + } + + [TestCaseSource(nameof(ParityCases))] + public void Parity_SlotMatchesBoxedRead_InValueAndByteCount(Action assertion) => assertion(); + + private static void AssertParity(string typeName, object value, TypeSettings settings) + { + var type = Parse(typeName, settings); + var payload = Write(type, value); + + var boxed = ReadBoxed(type, payload); + var slot = ReadSlot(type, payload); + + Assert.Multiple(() => + { + Assert.That(slot.Trailer, Is.EqualTo(Sentinel), + $"{typeName}: the slot consumed the wrong number of bytes"); + Assert.That(boxed.Trailer, Is.EqualTo(Sentinel), + $"{typeName}: the boxed reader consumed the wrong number of bytes (bad test payload)"); + Assert.That(slot.Value, Is.EqualTo(boxed.Value), + $"{typeName}: slot value differs from the boxed read"); + Assert.That(slot.Value?.GetType(), Is.EqualTo(boxed.Value?.GetType()), + $"{typeName}: slot boxed the value as a different CLR type than the boxed read"); + Assert.That(slot.Slot.IsNull, Is.EqualTo(boxed.Value is null or DBNull), + $"{typeName}: IsNull disagrees with the boxed read's null representation"); + }); + } + + // ---- Slot selection: a silent demotion to BoxedSlot costs the allocation win but changes no value ---- + + [TestCase("Int32", typeof(ValueSlot))] + [TestCase("UInt64", typeof(ValueSlot))] + [TestCase("Float64", typeof(ValueSlot))] + [TestCase("Bool", typeof(ValueSlot))] + [TestCase("String", typeof(ValueSlot))] + [TestCase("FixedString(3)", typeof(ValueSlot))] + [TestCase("UUID", typeof(ValueSlot))] + [TestCase("IPv6", typeof(ValueSlot))] + [TestCase("Date", typeof(ValueSlot))] + [TestCase("DateTime64(3, 'UTC')", typeof(ValueSlot))] + [TestCase("Time64(3)", typeof(ValueSlot))] + [TestCase("Decimal(10, 2)", typeof(ValueSlot))] // TypeSettings.Default is useBigDecimal + [TestCase("Int256", typeof(ValueSlot))] + [TestCase(Enum8Def, typeof(ValueSlot))] + [TestCase("LowCardinality(String)", typeof(ValueSlot))] + [TestCase("SimpleAggregateFunction(sum, UInt64)", typeof(ValueSlot))] + [TestCase("Nullable(Int32)", typeof(NullableSlot))] + [TestCase("Nullable(String)", typeof(NullableSlot))] + [TestCase("Nullable(UUID)", typeof(NullableSlot))] + [TestCase("LowCardinality(Nullable(String))", typeof(NullableSlot))] + [TestCase("SimpleAggregateFunction(any, Nullable(Int32))", typeof(NullableSlot))] + // No ITypedReader for the column's own FrameworkType: composites, polymorphic and geo types. + [TestCase("Array(Int32)", typeof(BoxedSlot))] + [TestCase("Tuple(Int32, String)", typeof(BoxedSlot))] + [TestCase("Map(String, Int32)", typeof(BoxedSlot))] + [TestCase("Nullable(Array(Int32))", typeof(BoxedSlot))] + [TestCase("Variant(Int64, String)", typeof(BoxedSlot))] + [TestCase("Point", typeof(BoxedSlot))] + [TestCase("Nothing", typeof(BoxedSlot))] + public void Create_Column_ResolvesToExpectedSlotKind(string typeName, Type expected) + => Assert.That(ColumnSlotFactory.Create(Parse(typeName)), Is.TypeOf(expected)); + + // FrameworkType is instance state, not class state, so the factory has to read it off the resolved + // instance. Caching by ClickHouseType class alone would bind every String column to one representation. + [TestCase(false, typeof(ValueSlot))] + [TestCase(true, typeof(ValueSlot))] + public void Create_StringColumn_BindsToTheRepresentationTheSettingsSelect(bool asByteArray, Type expected) + { + var settings = TypeSettings.Default with { readStringsAsByteArrays = asByteArray }; + Assert.That(ColumnSlotFactory.Create(Parse("String", settings)), Is.TypeOf(expected)); + } + + [TestCase(false, typeof(ValueSlot))] + [TestCase(true, typeof(ValueSlot))] + public void Create_DecimalColumn_BindsToTheRepresentationTheSettingsSelect(bool useBigDecimal, Type expected) + { + var settings = TypeSettings.Default with { useBigDecimal = useBigDecimal }; + Assert.That(ColumnSlotFactory.Create(Parse("Decimal(10, 2)", settings)), Is.TypeOf(expected)); + } + + // Object(...) is wire-transparent too, but the grammar never yields an ObjectType instance — + // ObjectType.Parse returns a SimpleAggregateFunctionType — so the only way to reach that unwrap branch is + // to construct one. Worth pinning: if Parse is ever corrected, this is already covered. + [Test] + public void Create_ObjectWrappedColumn_ResolvesToWrappedTypedSlot() + { + var type = new ObjectType { UnderlyingType = new Int64Type() }; + Assert.That(ColumnSlotFactory.Create(type), Is.TypeOf>()); + } + + // The factory dispatches through a hand-written table of ValueSlot/NullableSlot constructors rather + // than MakeGenericMethod, so that NativeAOT and trimming can see every instantiation the reader needs. + // The cost of giving up runtime generic construction is that the table does not maintain itself: adding + // an ITypedReader for a new T and forgetting the entry would silently demote that column to the boxed + // path — values still correct, allocation quietly back. Nothing else would catch it, so this does. + [Test] + public void Binders_CoverEveryTypedReadTarget() + { + var declared = typeof(ClickHouseType).Assembly + .GetTypes() + .Where(t => typeof(ClickHouseType).IsAssignableFrom(t)) + .SelectMany(t => t.GetInterfaces()) + .Where(i => i.IsGenericType && i.GetGenericTypeDefinition() == typeof(ITypedReader<>)) + .Select(i => i.GetGenericArguments()[0]) + .Distinct() + .ToArray(); + + Assert.That(declared, Is.Not.Empty, "found no ITypedReader implementations at all — check the query"); + + var bound = (IDictionary)typeof(ColumnSlotFactory) + .GetField("Binders", BindingFlags.NonPublic | BindingFlags.Static) + .GetValue(null); + + Assert.That(declared.Where(t => !bound.Contains(t)), Is.Empty, + "every CLR type some ClickHouseType can read box-free needs a ColumnSlotFactory.Binders entry"); + } + + // AggregateFunctionType throws from FrameworkType (and from Read and ToString) so that you learn you need + // xMerge() when you read the value. Slots are built for every column of the row on the reader's first + // Read(), so the factory must reach its no-typed-reader bail-out without ever evaluating FrameworkType — + // otherwise building the slots would throw before a single column had been decoded. Guards the ordering + // in TryCreateTyped, which is otherwise easy to "tidy up" into a regression. + // + // This is the only test that can guard it. End to end the two orderings are indistinguishable: both raise + // AggregateFunctionException, with the same message, from the same Read() call. + [Test] + public void Create_AggregateFunctionColumn_FallsBackToBoxedWithoutEvaluatingFrameworkType() + { + var type = Parse("AggregateFunction(quantile(0.5), UInt64)"); + Assert.That(() => ColumnSlotFactory.Create(type), Throws.Nothing); + Assert.That(ColumnSlotFactory.Create(type), Is.TypeOf()); + } + + // The factory's pre-filter finds the ITypedReader<> interfaces a class implements; the binding rule is + // narrower than that — the reader has to be for the column's own FrameworkType, or the slot's GetBoxed() + // would hand back a different CLR type than the boxed Read did. No shipped type is shaped this way, so + // only a purpose-built one can prove the factory declines rather than mis-binding. + [Test] + public void Create_TypeWhoseTypedReaderIsNotItsFrameworkType_FallsBackToBoxed() + => Assert.That(ColumnSlotFactory.Create(new MismatchedRepresentationType()), Is.TypeOf()); + + private sealed class MismatchedRepresentationType : ClickHouseType, ITypedReader + { + public override Type FrameworkType => typeof(string); + + public override object Read(ExtendedBinaryReader reader) => reader.ReadString(); + + public int ReadValue(ExtendedBinaryReader reader) => reader.ReadInt32(); + + public override void Write(ExtendedBinaryWriter writer, object value) => throw new NotSupportedException(); + + public override string ToString() => nameof(MismatchedRepresentationType); + } + + // ---- IsNull: no boxed counterpart, so parity cannot cover it ---- + + [Test] + public void NullableSlot_AfterNullThenValue_TracksPresencePerRow() + { + var type = Parse("Nullable(Int32)"); + var slot = ColumnSlotFactory.Create(type); + + using var stream = new MemoryStream(Concat(Write(type, DBNull.Value), Write(type, 7))); + using var reader = new ExtendedBinaryReader(stream); + + slot.Read(reader); + Assert.That(slot.IsNull, Is.True); + Assert.That(slot.GetBoxed(), Is.EqualTo(DBNull.Value)); + Assert.That(reader.ReadInt64(), Is.EqualTo(Sentinel)); + + slot.Read(reader); + Assert.That(slot.IsNull, Is.False); + Assert.That(slot.GetBoxed(), Is.EqualTo(7)); + Assert.That(reader.ReadInt64(), Is.EqualTo(Sentinel)); + } + + // A slot is reused across rows, so a null must clear the previous row's value rather than leaving it + // reachable — otherwise a single null cell pins the last string/byte[] for the life of the reader. + [Test] + public void NullableSlot_ValueThenNull_ClearsPreviousRowsReference() + { + var type = Parse("Nullable(String)"); + var slot = (NullableSlot)ColumnSlotFactory.Create(type); + + using var stream = new MemoryStream(Concat(Write(type, "kept-alive"), Write(type, DBNull.Value))); + using var reader = new ExtendedBinaryReader(stream); + + slot.Read(reader); + Assert.That(slot.Value, Is.EqualTo("kept-alive")); + reader.ReadInt64(); + + slot.Read(reader); + Assert.That(slot.Value, Is.Null, "a null cell must release the previous row's value"); + } + + // A non-nullable column has no null marker on the wire, so nothing it decodes is ever null. + [TestCase("Int32", 5)] + [TestCase("String", "")] + public void ValueSlot_AfterRead_IsNeverNull(string typeName, object value) + { + var type = Parse(typeName); + var (_, _, slot) = ReadSlot(type, Write(type, value)); + Assert.That(slot.IsNull, Is.False); + } + + private static byte[] Concat(byte[] first, byte[] second) => first.Concat(second).ToArray(); +} diff --git a/ClickHouse.Driver.Tests/Copy/PocoReadFastPathParityTests.cs b/ClickHouse.Driver.Tests/Copy/PocoReadFastPathParityTests.cs index 7f2f7b16b..1a676c62f 100644 --- a/ClickHouse.Driver.Tests/Copy/PocoReadFastPathParityTests.cs +++ b/ClickHouse.Driver.Tests/Copy/PocoReadFastPathParityTests.cs @@ -356,11 +356,35 @@ public void TryBuildReadBody_MismatchedTargetType_ReturnsNull(string typeName, T public void TryBuildReadBody_CompositeColumn_ReturnsNull(string typeName, Type target) => AssertNoFastPath(typeName, target); - // SimpleAggregateFunction is wire-transparent like LowCardinality, but the factory does not unwrap it, - // so it currently falls back to the boxed path. Pins today's behaviour; see the note in the PR. + // SimpleAggregateFunction is wire-transparent like LowCardinality, so it must reach the same typed + // reader the bare column would. Nesting checks the unwrap loop rather than a single-level special case. + [TestCase("SimpleAggregateFunction(sum, UInt64)", typeof(ulong))] + [TestCase("SimpleAggregateFunction(any, LowCardinality(String))", typeof(string))] + [TestCase("LowCardinality(SimpleAggregateFunction(any, String))", typeof(byte[]))] + public void TryBuildReadBody_WireTransparentWrapper_ResolvesToWrappedTypedRead(string typeName, Type target) + { + var body = TryBuild(Parse(typeName), target); + Assert.That(body, Is.Not.Null, $"expected a fast path for {typeName} -> {target}"); + Assert.That(body.Type, Is.EqualTo(target)); + } + + // The wrapper is transparent, not permissive: unwrapping must not widen the target-type match either. [Test] - public void TryBuildReadBody_SimpleAggregateFunctionColumn_ReturnsNull() - => AssertNoFastPath("SimpleAggregateFunction(sum, UInt64)", typeof(ulong)); + public void TryBuildReadBody_WireTransparentWrapperWithMismatchedTarget_ReturnsNull() + => AssertNoFastPath("SimpleAggregateFunction(sum, UInt64)", typeof(long)); + + // SimpleAggregateFunction(any, Nullable(T)) still has to go through the null-marker read. + [Test] + public void TryBuildReadBody_WireTransparentWrapperOverNullable_ReadsNullMarker() + { + var type = Parse("SimpleAggregateFunction(any, Nullable(Int32))"); + Assert.Multiple(() => + { + Assert.That(TryBuild(type, typeof(int?))?.Type, Is.EqualTo(typeof(int?))); + // A non-nullable target on a nullable column cannot represent the null and must decline. + Assert.That(TryBuild(type, typeof(int)), Is.Null); + }); + } private static void AssertNoFastPath(string typeName, Type targetClrType) { diff --git a/ClickHouse.Driver.Tests/Copy/PocoReadFastPathTests.cs b/ClickHouse.Driver.Tests/Copy/PocoReadFastPathTests.cs index 56f750cf4..bacdafcc7 100644 --- a/ClickHouse.Driver.Tests/Copy/PocoReadFastPathTests.cs +++ b/ClickHouse.Driver.Tests/Copy/PocoReadFastPathTests.cs @@ -390,6 +390,52 @@ public async Task QueryAsync_LowCardinalityColumn_ReadsUnderlyingBoxFree() Assert.That(rows[7].Name, Is.EqualTo("n7")); } + public class AggregatePoco + { + public string Name { get; set; } + public ulong Total { get; set; } + } + + [Test] + public async Task QueryAsync_SimpleAggregateFunctionColumn_ReadsUnderlyingBoxFree() + { + client.RegisterPocoType(); + var table = CreateTableName(); + await client.ExecuteNonQueryAsync( + $"CREATE TABLE {table} (Name String, Total SimpleAggregateFunction(sum, UInt64)) " + + "ENGINE AggregatingMergeTree ORDER BY Name"); + await client.ExecuteNonQueryAsync($"INSERT INTO {table} VALUES ('a', 3), ('a', 4), ('b', 10)"); + + // FINAL, so the two 'a' parts are merged whether or not the background merge has run yet, while the + // column stays declared — and therefore encoded on the wire — as SimpleAggregateFunction(sum, UInt64). + // This is the query that actually exercises the wrapper: sum(Total) below projects to a plain UInt64 + // and would pass with no wrapper support at all. + var rawSql = $"SELECT Name, Total FROM {table} FINAL ORDER BY Name"; + + using (var reader = (ClickHouseDataReader)await client.ExecuteReaderAsync(rawSql)) + { + Assert.That(reader.GetDataTypeName(1), Does.StartWith("SimpleAggregateFunction("), + "the wrapper must survive to the wire, or this test is not covering it"); + Assert.That(reader.TryGetRowMaterializer(out _, out _), Is.True, + "SimpleAggregateFunction is wire-transparent and must reach the wrapped typed reader"); + } + + var raw = new List(); + await foreach (var row in client.QueryAsync(rawSql)) + raw.Add(row); + + Assert.That(raw.Select(r => (r.Name, r.Total)), Is.EqualTo(new[] { ("a", 7ul), ("b", 10ul) }), + "the wrapped column must materialize through the fast path"); + + // And separately the aggregate result, whose Total is an ordinary UInt64. + var aggregated = new List(); + await foreach (var row in client.QueryAsync( + $"SELECT Name, sum(Total) AS Total FROM {table} GROUP BY Name ORDER BY Name")) + aggregated.Add(row); + + Assert.That(aggregated.Select(r => (r.Name, r.Total)), Is.EqualTo(new[] { ("a", 7ul), ("b", 10ul) })); + } + public class NullablePropPoco { public long? Id { get; set; } diff --git a/ClickHouse.Driver.Tests/Utilities/TestUtilities.cs b/ClickHouse.Driver.Tests/Utilities/TestUtilities.cs index 8a3b19097..4243014a1 100644 --- a/ClickHouse.Driver.Tests/Utilities/TestUtilities.cs +++ b/ClickHouse.Driver.Tests/Utilities/TestUtilities.cs @@ -375,7 +375,12 @@ public static object[] GetEnsureSingleRow(this DbDataReader reader) var data = reader.GetFieldValues(); - ClassicAssert.IsFalse(reader.Read(), "Unexpected extra row: " + string.Join(",", reader.GetFieldValues())); + // Read the extra row's values only once there is one. The argument to IsFalse is evaluated eagerly, + // so inlining GetFieldValues() there read the row *after* Read() had returned false — which the + // reader now rejects — and allocated a joined string of every column on every successful call just + // to discard it. + if (reader.Read()) + Assert.Fail("Unexpected extra row: " + string.Join(",", reader.GetFieldValues())); return data; } diff --git a/ClickHouse.Driver/ADO/Readers/ClickHouseDataReader.cs b/ClickHouse.Driver/ADO/Readers/ClickHouseDataReader.cs index 0f9411430..2095d2a04 100644 --- a/ClickHouse.Driver/ADO/Readers/ClickHouseDataReader.cs +++ b/ClickHouse.Driver/ADO/Readers/ClickHouseDataReader.cs @@ -36,6 +36,19 @@ public class ClickHouseDataReader : DbDataReader, IEnumerator, IEnu private readonly string[] columnTypeNames; // Raw server-sent type strings, exactly as declared private readonly PocoTypeRegistry pocoRegistry; private readonly Dictionary bindingPlanCache = new(); + + // Per-column typed storage for the current row; replaces the old shared object[] buffer. Built on the first + // Read() and mutated in place by every one after it. + // + // Not built in the constructor: QueryAsync's box-free POCO path materializes straight from the stream and + // never touches a slot, so eager construction would allocate one permanently dead object per column on the + // primary read API. Empty result sets and metadata-only readers likewise pay nothing. + // + // Built once and never nulled again, so `hasCurrentRow` implies non-null — the whole safety argument, which + // every value accessor establishes by going through Slot(). GetValues is the one exception: it indexes this + // array directly and carries its own copy of the guard, so "simplifying" that guard away yields a + // NullReferenceException instead of the intended InvalidOperationException. + private ColumnSlot[] slots; private bool hasCurrentRow; private ClickHouseDataReader(HttpResponseMessage httpResponse, ExtendedBinaryReader reader, PooledReadBufferStream pooledReadBuffer, string[] names, ClickHouseType[] types, string[] rawTypeNames, PocoTypeRegistry pocoRegistry, ExceptionTagAwareStream exceptionTagStream = null, IReadValueConverter readValueConverter = null, Stream decompressor = null) @@ -52,7 +65,6 @@ private ClickHouseDataReader(HttpResponseMessage httpResponse, ExtendedBinaryRea this.pocoRegistry = pocoRegistry; RawTypes = types; FieldNames = names; - CurrentRow = new object[FieldNames.Length]; columnTypeNames = rawTypeNames; } @@ -149,36 +161,89 @@ internal ClickHouseType GetEffectiveClickHouseType(int ordinal) public override int RecordsAffected { get; } - protected object[] CurrentRow { get; set; } - protected string[] FieldNames { get; set; } private protected ClickHouseType[] RawTypes { get; set; } - public override bool GetBoolean(int ordinal) => Convert.ToBoolean(GetValue(ordinal), CultureInfo.InvariantCulture); + /// + /// Shared body for the strict typed accessors, which cast rather than coerce: the value must already be + /// the requested . This is the path compiled ORM mappers drive (linq2db inlines + /// GetInt64/GetDouble/GetDateTime/… per column per row), so it is where the boxing + /// elimination is worth the most. + /// + /// + /// An does not force the accessor onto the boxed path. The value comes + /// out of the slot unboxed and converts through ConvertValue<T>, the overload + /// uses for a typed read. Only genuinely boxed reads — + /// , , and the coercing fall-throughs below — use + /// ConvertValue(object, …). + /// + private T GetTypedValue(int ordinal) + { + var value = GetSlotValue(ordinal); + return readValueConverter == null ? value : ConvertTyped(ordinal, value); + } + + // Applies the converter on the typed overload to a value already read out of its slot. + private T ConvertTyped(int ordinal, T value) + => readValueConverter.ConvertValue(value, FieldNames[ordinal], columnTypeNames[ordinal]); + + // Unlike its neighbours this one coerces rather than casts, so only an exact Bool column takes the fast path; + // anything else keeps Convert.ToBoolean's widening and its exception messages. A NULL cell falls through too, + // so Convert.ToBoolean(DBNull.Value) still throws exactly as it did. + public override bool GetBoolean(int ordinal) + { + var slot = Slot(ordinal); + if (slot is ValueSlot boolSlot) + return readValueConverter == null ? boolSlot.Value : ConvertTyped(ordinal, boolSlot.Value); + if (slot is NullableSlot nullableSlot && nullableSlot.HasValue) + return readValueConverter == null ? nullableSlot.Value : ConvertTyped(ordinal, nullableSlot.Value); + + return Convert.ToBoolean(GetValue(ordinal), CultureInfo.InvariantCulture); + } - public override byte GetByte(int ordinal) => (byte)GetValue(ordinal); + public override byte GetByte(int ordinal) => GetTypedValue(ordinal); public override long GetBytes(int ordinal, long dataOffset, byte[] buffer, int bufferOffset, int length) => throw new NotImplementedException(); + // No ClickHouse type reads as char, so there is no slot to hit — left on the boxed cast. public override char GetChar(int ordinal) => (char)GetValue(ordinal); public override long GetChars(int ordinal, long dataOffset, char[] buffer, int bufferOffset, int length) => throw new NotImplementedException(); public override string GetDataTypeName(int ordinal) => GetClickHouseType(ordinal).ToString(); - public override DateTime GetDateTime(int ordinal) => (DateTime)GetValue(ordinal); + public override DateTime GetDateTime(int ordinal) => GetTypedValue(ordinal); + // Box-free by construction once GetDateTime is: CoerceToDateTimeOffset has a DateTime overload. public virtual DateTimeOffset GetDateTimeOffset(int ordinal) => GetEffectiveClickHouseType(ordinal) is AbstractDateTimeType adt ? adt.CoerceToDateTimeOffset(GetDateTime(ordinal)) : throw new InvalidCastException(); public override decimal GetDecimal(int ordinal) { + // Which of the two representations a Decimal column resolves to is the UseBigDecimal setting's doing; + // both reach the same decimal without a box, nullable or not. A NULL cell falls through to the boxed + // path below, where casting DBNull.Value throws exactly as it did. + var slot = Slot(ordinal); + decimal? unboxed = slot switch + { + ValueSlot decimalSlot => decimalSlot.Value, + NullableSlot n when n.HasValue => n.Value, + ValueSlot bigDecimalSlot => bigDecimalSlot.Value.ToDecimal(CultureInfo.InvariantCulture), + NullableSlot nb when nb.HasValue => nb.Value.ToDecimal(CultureInfo.InvariantCulture), + _ => null, + }; + + // A ClickHouseDecimal column is narrowed first, so the converter sees the decimal this accessor + // returns rather than the column's own representation. + if (unboxed.HasValue) + return readValueConverter == null ? unboxed.Value : ConvertTyped(ordinal, unboxed.Value); + var value = GetValue(ordinal); return value is ClickHouseDecimal clickHouseDecimal ? clickHouseDecimal.ToDecimal(CultureInfo.InvariantCulture) : (decimal)value; } - public override double GetDouble(int ordinal) => (double)GetValue(ordinal); + public override double GetDouble(int ordinal) => GetTypedValue(ordinal); public override Type GetFieldType(int ordinal) { @@ -186,15 +251,15 @@ public override Type GetFieldType(int ordinal) return rawType is NullableType nt ? nt.UnderlyingType.FrameworkType : rawType.FrameworkType; } - public override float GetFloat(int ordinal) => (float)GetValue(ordinal); + public override float GetFloat(int ordinal) => GetTypedValue(ordinal); - public override Guid GetGuid(int ordinal) => (Guid)GetValue(ordinal); + public override Guid GetGuid(int ordinal) => GetTypedValue(ordinal); - public override short GetInt16(int ordinal) => (short)GetValue(ordinal); + public override short GetInt16(int ordinal) => GetTypedValue(ordinal); - public override int GetInt32(int ordinal) => (int)GetValue(ordinal); + public override int GetInt32(int ordinal) => GetTypedValue(ordinal); - public override long GetInt64(int ordinal) => (long)GetValue(ordinal); + public override long GetInt64(int ordinal) => GetTypedValue(ordinal); public override string GetName(int ordinal) => FieldNames[ordinal]; @@ -209,44 +274,87 @@ public override int GetOrdinal(string name) return index; } - public override string GetString(int ordinal) => GetValue(ordinal)?.ToString(); + // Deliberately narrower than the other accessors: only a non-nullable String column short-circuits. Every other + // shape keeps ToString()'s coercion, including the quirk that a NULL cell yields "" rather than null, because + // DBNull.Value.ToString() is the empty string. + public override string GetString(int ordinal) + => Slot(ordinal) is ValueSlot stringSlot + ? (readValueConverter == null ? stringSlot.Value : ConvertTyped(ordinal, stringSlot.Value)) + : GetValue(ordinal)?.ToString(); + /// + /// The one boxing entry point on the read path. Boxes lazily, per call, so a query that projects ten columns + /// and reads two pays for two — where the old object[] buffer boxed all ten during + /// regardless. + /// + /// + /// Consequence of boxing per call rather than once per row: two GetValue(i) calls on the same value-type + /// cell return two distinct boxes. They still compare equal by — the + /// ADO.NET-relevant comparison — but no longer by . + /// public override object GetValue(int ordinal) - => readValueConverter == null - ? CurrentRow[ordinal] - : readValueConverter.ConvertValue(CurrentRow[ordinal], FieldNames[ordinal], columnTypeNames[ordinal]); + { + var value = Slot(ordinal).GetBoxed(); + return readValueConverter == null + ? value + : readValueConverter.ConvertValue(value, FieldNames[ordinal], columnTypeNames[ordinal]); + } public override int GetValues(object[] values) { - if (CurrentRow == null) - { - throw new InvalidOperationException(); - } + if (!hasCurrentRow) + ThrowNoCurrentRow(); - var count = Math.Min(CurrentRow.Length, values.Length); + var count = Math.Min(slots.Length, values.Length); if (readValueConverter != null) { for (var i = 0; i < count; i++) - values[i] = readValueConverter.ConvertValue(CurrentRow[i], FieldNames[i], columnTypeNames[i]); + values[i] = readValueConverter.ConvertValue(slots[i].GetBoxed(), FieldNames[i], columnTypeNames[i]); } else { - Array.Copy(CurrentRow, values, count); + for (var i = 0; i < count; i++) + values[i] = slots[i].GetBoxed(); } return count; } public override bool IsDBNull(int ordinal) + // Asks the slot directly rather than going through GetValue: a configured IReadValueConverter must not run + // during a null check (it could throw, do expensive work, or change the result's nullness), and a null + // check has no business materializing a box. + => Slot(ordinal).IsNull; + + /// + /// The single gate every value accessor passes through. Column metadata (, + /// , , , …) is available + /// without a current row and is deliberately not gated. + /// + /// + /// Slots hold typed storage, so without this a non-nullable value column would read back before the first + /// as a perfectly plausible 0/false/Guid.Empty — a value + /// indistinguishable from real data. Worth a branch to report the mistake instead, as SqlClient and the + /// rest of ADO.NET do. + /// + private ColumnSlot Slot(int ordinal) { - // Read CurrentRow directly rather than going through GetValue so a configured - // IReadValueConverter does not run during a null check — it could throw, do - // expensive work, or change the nullness of the result. - var value = CurrentRow[ordinal]; - return value is DBNull || value is null; + if (!hasCurrentRow) + ThrowNoCurrentRow(); + + return slots[ordinal]; } + // Separate and non-inlined so the check above stays small enough for the JIT to inline into the hot + // accessors. + [MethodImpl(MethodImplOptions.NoInlining)] + private static void ThrowNoCurrentRow() + => throw new InvalidOperationException( + "The reader has no current row. Call Read() and check that it returned true before reading " + + "column values. Column metadata (FieldCount, GetName, GetFieldType, GetSchemaTable) is " + + "available without a current row."); + public override bool NextResult() => false; public override void Close() => Dispose(); @@ -293,12 +401,42 @@ public override T GetFieldValue(int ordinal) } } - var value = (T)CurrentRow[ordinal]; + var value = GetSlotValue(ordinal); if (readValueConverter != null) return readValueConverter.ConvertValue(value, FieldNames[ordinal], columnTypeNames[ordinal]); return value; } + /// + /// Extracts the current row's column value as , without boxing where the slot + /// already holds exactly that type. + /// + /// + /// Both checks are sealed-class type tests, so for a value-typed each is a plain + /// isinst against a known method table. A generic IValueGetter<T> implemented twice would let + /// one slot serve both long and long?, but it goes through the shared-generics dictionary and + /// measured several times slower — so T = U? is deliberately left to the boxed fallback. + /// + /// That fallback is the pre-slot expression verbatim, which preserves the exact-type strictness callers + /// depend on: GetFieldValue<long> over an Int32 column throws rather than widening, and + /// reading a NULL as a non-nullable throws the runtime's own "cannot cast DBNull" + /// as before. + /// + private T GetSlotValue(int ordinal) + { + var slot = Slot(ordinal); + + if (slot is ValueSlot valueSlot) + return valueSlot.Value; + + // A Nullable(T) column holds the underlying T, so this also serves GetFieldValue over + // Nullable(Int64) — and when the cell is null it falls through to the box, which throws. + if (slot is NullableSlot nullableSlot && nullableSlot.HasValue) + return nullableSlot.Value; + + return (T)slot.GetBoxed(); + } + /// /// Per- cached predicate driving 's /// dispatch. The .NET runtime instantiates this generic exactly once per closed @@ -320,25 +458,25 @@ private static class FieldValueDispatcher public override Task NextResultAsync(CancellationToken cancellationToken) => Task.FromResult(false); // Custom extension - public ushort GetUInt16(int ordinal) => (ushort)GetValue(ordinal); + public ushort GetUInt16(int ordinal) => GetTypedValue(ordinal); // Custom extension - public uint GetUInt32(int ordinal) => (uint)GetValue(ordinal); + public uint GetUInt32(int ordinal) => GetTypedValue(ordinal); // Custom extension - public ulong GetUInt64(int ordinal) => (ulong)GetValue(ordinal); + public ulong GetUInt64(int ordinal) => GetTypedValue(ordinal); // Custom extension - public IPAddress GetIPAddress(int ordinal) => (IPAddress)GetValue(ordinal); + public IPAddress GetIPAddress(int ordinal) => GetTypedValue(ordinal); - // Custom extension + // Custom extension. Tuple columns have no typed slot, so this stays on the boxed cast. public ITuple GetTuple(int ordinal) => (ITuple)GetValue(ordinal); // Custom extension - public sbyte GetSByte(int ordinal) => (sbyte)GetValue(ordinal); + public sbyte GetSByte(int ordinal) => GetTypedValue(ordinal); // Custom extension - public BigInteger GetBigInteger(int ordinal) => (BigInteger)GetValue(ordinal); + public BigInteger GetBigInteger(int ordinal) => GetTypedValue(ordinal); /// /// Materializes the current row into a new instance of . @@ -436,8 +574,8 @@ internal bool TryGetRowMaterializer(out RowColumnReader[] materializers, o } /// - /// Reads the next row straight from the stream via the fast-path delegates, bypassing the shared - /// object[] row buffer and its per-value boxing. Returns false at end of stream, and mirrors + /// Reads the next row straight from the stream via the fast-path delegates, bypassing the reader's column + /// slots, which this path never allocates. Returns false at end of stream, and mirrors /// 's mid-stream server-exception handling. The delegates consume every wire column in /// order, so the stream stays aligned even for columns the POCO does not map. /// @@ -515,11 +653,8 @@ private void ValidateBinding(Type pocoType, PocoPropertyInfo propInfo, int colum public override bool Read() { - var count = RawTypes.Length; - var data = CurrentRow; - // Clear before the per-column loop so a mid-row throw cannot leave a stale - // CurrentRow visible to MapTo if the caller catches and continues. + // row visible to MapTo if the caller catches and continues. hasCurrentRow = false; try { @@ -528,10 +663,13 @@ public override bool Read() if (reader.PeekChar() == -1) return false; // End of stream reached - for (var i = 0; i < count; i++) + // Built on the first row rather than in the ctor: the POCO fast path materializes straight + // from the stream and never touches a slot, so eager construction would allocate a + // permanently dead object per column on the primary read API. An empty result never gets here. + var columns = slots ??= CreateSlots(); + for (var i = 0; i < columns.Length; i++) { - var rawType = RawTypes[i]; - data[i] = rawType.Read(reader); + columns[i].Read(reader); } hasCurrentRow = true; return true; @@ -550,6 +688,17 @@ public override bool Read() } } + // Runs at most once per reader, so it is kept out of Read() to leave that method small enough for the + // JIT to treat the slot loop as the hot path it is. + [MethodImpl(MethodImplOptions.NoInlining)] + private ColumnSlot[] CreateSlots() + { + var created = new ColumnSlot[RawTypes.Length]; + for (var i = 0; i < created.Length; i++) + created[i] = ColumnSlotFactory.Create(RawTypes[i]); + return created; + } + #pragma warning disable CA2215 // Dispose methods should call base class dispose protected override void Dispose(bool disposing) { diff --git a/ClickHouse.Driver/ADO/Readers/ColumnSlot.cs b/ClickHouse.Driver/ADO/Readers/ColumnSlot.cs new file mode 100644 index 000000000..f055d4d10 --- /dev/null +++ b/ClickHouse.Driver/ADO/Readers/ColumnSlot.cs @@ -0,0 +1,114 @@ +using System; +using ClickHouse.Driver.Formats; +using ClickHouse.Driver.Types; + +namespace ClickHouse.Driver.ADO.Readers; + +/// +/// Per-column storage for the reader's current row, replacing the shared object[] row buffer that boxed +/// every value-type cell of every row whether the caller asked for it or not. One instance per wire column, +/// allocated once per and overwritten in place on every +/// ; the box now happens only in . +/// +/// Every slot must be observationally identical to the boxed path it replaces: has to +/// consume exactly the bytes would, and +/// has to return exactly what it would have returned — same CLR type, and +/// for a SQL NULL. +/// +internal abstract class ColumnSlot +{ + /// Decodes this column's bytes from the row stream into typed storage. + public abstract void Read(ExtendedBinaryReader reader); + + /// + /// Boxes the current value on demand for the untyped path. Returns for a SQL + /// NULL, matching . + /// + public abstract object GetBoxed(); + + /// Null check that neither materializes nor boxes the value. + public abstract bool IsNull { get; } +} + +/// +/// A non-nullable column whose type can decode straight into , which is always the +/// column's — so the stored value is exactly what the boxed +/// produces. +/// +internal sealed class ValueSlot : ColumnSlot +{ + // typeof(T) is a JIT-time constant per closed generic, so this folds to a constant load; for a value-typed + // T the IsNull check below then folds to `false` and never reaches its boxing conversion. + private static readonly bool CanBeNull = !typeof(T).IsValueType; + + private readonly ITypedReader typedReader; + + public T Value; + + public ValueSlot(ITypedReader typedReader) => this.typedReader = typedReader; + + public override void Read(ExtendedBinaryReader reader) => Value = typedReader.ReadValue(reader); + + public override object GetBoxed() => Value; + + // A non-nullable column has no wire null marker, so this can only be null if a reference-typed reader hands + // one back. None currently do; the check keeps IsNull agreeing with GetBoxed if one ever did. + public override bool IsNull => CanBeNull && (object)Value is null; +} + +/// +/// A Nullable(T) column: the decoded value plus a presence flag, so a non-null cell no longer boxes +/// (a NULL never allocated — the boxed path returned the singleton). +/// +internal sealed class NullableSlot : ColumnSlot +{ + private readonly ITypedReader typedReader; + + public T Value; + + public bool HasValue; + + public NullableSlot(ITypedReader typedReader) => this.typedReader = typedReader; + + public override void Read(ExtendedBinaryReader reader) + { + // Byte-identical to NullableType.Read: marker > 0 means NULL, and the underlying type then wrote + // nothing, so nothing more is consumed. + if (reader.ReadByte() > 0) + { + HasValue = false; + Value = default; // don't pin the previous row's value (a string/byte[] would stay reachable) + } + else + { + Value = typedReader.ReadValue(reader); + HasValue = true; + } + } + + // Boxes the *underlying* value, never a Nullable — NullableType.Read did the same, and + // GetFieldValue on a Nullable(Int64) column depends on finding a boxed long here. + public override object GetBoxed() => HasValue ? (object)Value : DBNull.Value; + + public override bool IsNull => !HasValue; +} + +/// +/// Fallback for any column with no for its own +/// — composites, the polymorphic types, geo, and so on. Byte-for-byte +/// and value-for-value the pre-slot behaviour. +/// +internal sealed class BoxedSlot : ColumnSlot +{ + private readonly ClickHouseType type; + + public object Value; + + public BoxedSlot(ClickHouseType type) => this.type = type; + + public override void Read(ExtendedBinaryReader reader) => Value = type.Read(reader); + + public override object GetBoxed() => Value; + + public override bool IsNull => Value is null || Value is DBNull; +} diff --git a/ClickHouse.Driver/ADO/Readers/ColumnSlotFactory.cs b/ClickHouse.Driver/ADO/Readers/ColumnSlotFactory.cs new file mode 100644 index 000000000..a774af13b --- /dev/null +++ b/ClickHouse.Driver/ADO/Readers/ColumnSlotFactory.cs @@ -0,0 +1,101 @@ +using System; +using System.Collections.Generic; +using System.Net; +using System.Numerics; +using ClickHouse.Driver.Numerics; +using ClickHouse.Driver.Types; + +namespace ClickHouse.Driver.ADO.Readers; + +/// +/// Builds the for a resolved column type. +/// +/// A column gets a typed slot iff its type implements ITypedReader<FrameworkType> — iff it can +/// decode straight into the very CLR type its boxed Read would have returned, so that slot and boxed path +/// always produce the same value from the same bytes. Anything else gets a . +/// +/// Slots are built from the resolved type instance, never from a cached shape, so settings that +/// change FrameworkType (ReadStringsAsByteArrays, UseCustomDecimals) are handled without the +/// cache-key hazards a per-query-shape cache would have. +/// +internal static class ColumnSlotFactory +{ + /// + /// CLR type → the slot constructor for it. Every entry is a static generic instantiation the compiler emits + /// rather than a runtime MakeGenericMethod — which NativeAOT cannot satisfy over value types — so every + /// ValueSlot<T>/NullableSlot<T> the reader needs is visible to AOT and trimming. + /// + /// + /// A few entries are unreachable today (, , native + /// Int128/UInt128): those are alternative representations offered alongside a type's + /// FrameworkType, never as it. They are listed anyway so the table is exactly "every + /// ITypedReader<T> target" and ColumnSlotTests.Binders_CoverEveryTypedReadTarget can check + /// it mechanically. Over-inclusion is inert; a missing entry silently demotes a column to the boxed path. + /// + private static readonly Dictionary> Binders = new() + { + [typeof(sbyte)] = Bind, + [typeof(short)] = Bind, + [typeof(int)] = Bind, + [typeof(long)] = Bind, + [typeof(byte)] = Bind, + [typeof(ushort)] = Bind, + [typeof(uint)] = Bind, + [typeof(ulong)] = Bind, + [typeof(BigInteger)] = Bind, + [typeof(float)] = Bind, + [typeof(double)] = Bind, + [typeof(decimal)] = Bind, + [typeof(ClickHouseDecimal)] = Bind, + [typeof(bool)] = Bind, + [typeof(string)] = Bind, + [typeof(byte[])] = Bind, + [typeof(Guid)] = Bind, + [typeof(IPAddress)] = Bind, + [typeof(DateTime)] = Bind, + [typeof(DateTimeOffset)] = Bind, + [typeof(DateOnly)] = Bind, + [typeof(TimeSpan)] = Bind, +#if NET8_0_OR_GREATER + [typeof(Int128)] = Bind, + [typeof(UInt128)] = Bind, +#endif + }; + + /// + /// Returns the slot for . Never null: falls back to a + /// over the original (still-wrapped) type, so the wire read is unchanged for anything unsupported. + /// + public static ColumnSlot Create(ClickHouseType type) + { + var unwrapped = TransparentWrapper.Unwrap(type); + + // Nullable is not wire-transparent — it prefixes a marker byte — so it selects the slot kind rather + // than being unwrapped away. Its underlying may itself be wrapped, e.g. Nullable(LowCardinality(T)). + if (unwrapped is NullableType nullableType) + return TryCreateTyped(TransparentWrapper.Unwrap(nullableType.UnderlyingType), nullable: true) ?? new BoxedSlot(type); + + return TryCreateTyped(unwrapped, nullable: false) ?? new BoxedSlot(type); + } + + private static ColumnSlot TryCreateTyped(ClickHouseType type, bool nullable) + { + // Order matters, and not only for speed: FrameworkType is not safe to evaluate on every column type. + // AggregateFunctionType throws from it (deliberately — you are meant to learn you need xMerge()), and the + // composite types build a fresh Type object per call. Slot construction has to succeed for every column + // of every reader, so it may only touch the FrameworkType of types that advertise a typed reader. + // Guarded by ColumnSlotTests.Create_AggregateFunctionColumn_FallsBackToBoxedWithoutEvaluatingFrameworkType, + // which has to be a unit test — end to end both orderings throw the same exception from the same call. + if (type is not ITypedReader) + return null; + + return Binders.TryGetValue(type.FrameworkType, out var bind) ? bind(type, nullable) : null; + } + + // Binds only when the type's typed reader is for its *own* FrameworkType. A type offering extra + // representations (a DateTime column also readable as DateTimeOffset) must not get a slot bound to one the + // boxed path would not have produced, or GetValue would start handing back a different CLR type. + private static ColumnSlot Bind(ClickHouseType type, bool nullable) + => type is not ITypedReader typedReader ? null + : nullable ? new NullableSlot(typedReader) : new ValueSlot(typedReader); +} diff --git a/ClickHouse.Driver/ClickHouseClient.cs b/ClickHouse.Driver/ClickHouseClient.cs index 72e807a34..3ada0106a 100644 --- a/ClickHouse.Driver/ClickHouseClient.cs +++ b/ClickHouse.Driver/ClickHouseClient.cs @@ -445,7 +445,7 @@ public async IAsyncEnumerable QueryAsync( // the underlying HTTP stream is buffered, so per-row reads do not perform real I/O. if (reader.TryGetRowMaterializer(out var materializers, out var constructor)) { - // Bypasses the shared object[] row buffer and the boxing/unboxing MapTo setter. + // Bypasses the reader's column slots and the boxing/unboxing MapTo setter. while (reader.TryMaterializeNextRow(materializers, constructor, out var row)) { cancellationToken.ThrowIfCancellationRequested(); diff --git a/ClickHouse.Driver/Poco/PocoReadExpressionFactory.cs b/ClickHouse.Driver/Poco/PocoReadExpressionFactory.cs index 5addf4600..e4e2f5afa 100644 --- a/ClickHouse.Driver/Poco/PocoReadExpressionFactory.cs +++ b/ClickHouse.Driver/Poco/PocoReadExpressionFactory.cs @@ -10,16 +10,16 @@ namespace ClickHouse.Driver.Poco; /// /// Builds box-free read expressions for the POCO read (materialization) fast path. /// -/// The default path boxes every value-type column through -/// into the reader's shared object[] row -/// buffer, then unboxes it in a compiled Action<T,object> setter (MapTo<T>). Where a -/// type can decode straight into the target CLR type, this compiles a fused read-and-assign delegate -/// instead, dropping both the box and the unbox. +/// The fallback path decodes each column into the reader's per-column slot, boxes it on the way out through +/// GetValue, then unboxes it in a compiled Action<T,object> setter (MapTo<T>). +/// Where a type can decode straight into the target CLR type, this compiles a fused read-and-assign delegate +/// instead, dropping both the box and the unbox and bypassing the slots. /// /// Dispatch is driven by : a column type takes the fast path for a property CLR /// type iff it implements ITypedReader<thatType>, which also lets one column offer several -/// representations. and are transparent. Anything -/// with no typed reader returns null, and the caller falls back to the boxed path for that column. +/// representations. and the wire-transparent wrappers (see +/// ) are handled transparently. Anything with no typed reader returns +/// null, and the caller falls back to the boxed path for that column. /// internal static class PocoReadExpressionFactory { @@ -42,10 +42,10 @@ internal static class PocoReadExpressionFactory /// The bound property's CLR type; the returned expression has this type. public static Expression TryBuildReadBody(ClickHouseType type, Expression reader, Type targetClrType) { - // LowCardinality is transparent on the RowBinary wire (LowCardinalityType.Read delegates to the - // underlying), so read the underlying value directly. - if (type is LowCardinalityType lowCardinality) - return TryBuildReadBody(lowCardinality.UnderlyingType, reader, targetClrType); + // LowCardinality / SimpleAggregateFunction / Object are transparent on the RowBinary wire (their + // Read delegates to the wrapped type), so read the wrapped value directly. Without this a wrapped + // column would silently fall back to the boxed path despite decoding identically to a bare one. + type = TransparentWrapper.Unwrap(type); if (type is NullableType nullableType) return TryBuildNullableRead(nullableType, reader, targetClrType); diff --git a/ClickHouse.Driver/Types/ITypedReader.cs b/ClickHouse.Driver/Types/ITypedReader.cs index 424dfb357..6b88910b1 100644 --- a/ClickHouse.Driver/Types/ITypedReader.cs +++ b/ClickHouse.Driver/Types/ITypedReader.cs @@ -3,18 +3,26 @@ namespace ClickHouse.Driver.Types; /// -/// Implemented by a that can deserialize a value of the CLR type -/// without boxing, for the POCO read (materialization) fast path. +/// Implemented by a that can deserialize without +/// boxing, for the box-free read paths (POCO materialization and the reader's column slots). /// -/// A type may implement this for more than one when it can produce several CLR -/// representations of the same column (e.g. a DateTime column as , -/// or ). Such implementations are explicit, -/// since they differ only by return type. The boxed -/// reads the same bytes and returns the canonical -/// representation, so every is byte-identical to it by construction. +/// A type may implement this for several when one column has more than one CLR +/// representation — a DateTime column as // +/// , a String column as /byte[]. Such implementations +/// are explicit, since they differ only by return type. Every must consume exactly +/// the bytes would. /// -/// The exact CLR type this type can read without boxing (e.g. ). -internal interface ITypedReader +/// The exact CLR type read without boxing. +internal interface ITypedReader : ITypedReader { T ReadValue(ExtendedBinaryReader reader); } + +/// +/// Non-generic base, so "can this type read box-free at all?" is a plain type test rather than an +/// interface-list walk. asks that for every +/// column, and must ask it before touching . +/// +internal interface ITypedReader +{ +} diff --git a/ClickHouse.Driver/Types/TransparentWrapper.cs b/ClickHouse.Driver/Types/TransparentWrapper.cs new file mode 100644 index 000000000..a82c568fc --- /dev/null +++ b/ClickHouse.Driver/Types/TransparentWrapper.cs @@ -0,0 +1,34 @@ +namespace ClickHouse.Driver.Types; + +/// +/// Column types that are pass-through on the RowBinary wire: their Read/Write delegate straight +/// to the wrapped type and they report its . Any fast path +/// dispatching on the concrete column type has to look through these, or a wrapped column silently falls back +/// to the boxed path despite decoding identically to a bare one. +/// +internal static class TransparentWrapper +{ + /// + /// Returns the innermost non-wrapper type, or itself if it is not a wrapper. + /// is deliberately not unwrapped: it prefixes a null marker byte and + /// so is not wire-transparent. + /// + public static ClickHouseType Unwrap(ClickHouseType type) + { + while (true) + { + var inner = type switch + { + LowCardinalityType lowCardinality => lowCardinality.UnderlyingType, + SimpleAggregateFunctionType simpleAggregate => simpleAggregate.UnderlyingType, + ObjectType obj => obj.UnderlyingType, + _ => null, + }; + + if (inner is null) + return type; + + type = inner; + } + } +} diff --git a/changelog.d/449-poco-read-boxfree.improvements.md b/changelog.d/449-poco-read-boxfree.improvements.md index b90df4389..ff074bae8 100644 --- a/changelog.d/449-poco-read-boxfree.improvements.md +++ b/changelog.d/449-poco-read-boxfree.improvements.md @@ -1 +1 @@ -* Reduced allocations in POCO reads (`client.QueryAsync(...)`): scalar, `String` and `FixedString` columns are now materialized straight into the target property instead of through a boxed `object[]` row buffer, removing one box and one unbox per value-type property per row. A 500k-row read of a 3-column POCO (`Int64`, `String`, `Float64`) allocates about 38% less. Composite columns (`Array`, `Tuple`, `Map`, `Nested`, `Variant`, `Dynamic`, `JSON`) fall back per-column to the previous path. Values and their CLR types are unchanged, as are `MapTo` and the ADO accessors. +* Reduced allocations in POCO reads (`client.QueryAsync(...)`): scalar, `String` and `FixedString` columns are now materialized straight into the target property instead of through a boxed `object[]` row buffer, removing one box and one unbox per value-type property per row. A 500k-row read of a 3-column POCO (`Int64`, `String`, `Float64`) allocates about 38% less. Composite columns (`Array`, `Tuple`, `Map`, `Nested`, `Variant`, `Dynamic`, `JSON`) fall back per-column to the previous path. Values and their CLR types are unchanged, as is `MapTo`; the ADO accessors get their own box-free path (see below). diff --git a/changelog.d/499-ado-read-with-no-current-row.breaking.md b/changelog.d/499-ado-read-with-no-current-row.breaking.md new file mode 100644 index 000000000..b3e8ec1d9 --- /dev/null +++ b/changelog.d/499-ado-read-with-no-current-row.breaking.md @@ -0,0 +1 @@ +* **Reading a column value from `ClickHouseDataReader` with no current row now throws `InvalidOperationException`** — that is, before the first `Read()` or after `Read()` has returned `false`; this covers `GetValue`, the indexers, `GetValues`, `GetFieldValue`, `IsDBNull` and the typed accessors. Before the first row the previous result depended on the accessor (`GetValue` returned `null`, `IsDBNull` returned `true`, and a typed accessor threw `NullReferenceException`), while after a non-empty result ended accessors continued exposing the last row; column metadata (`FieldCount`, `GetName`, `GetOrdinal`, `GetFieldType`, `GetDataTypeName`, `GetSchemaTable`) remains available without a row. diff --git a/changelog.d/499-ado-typed-column-slots.improvements.md b/changelog.d/499-ado-typed-column-slots.improvements.md new file mode 100644 index 000000000..2cba1db2e --- /dev/null +++ b/changelog.d/499-ado-typed-column-slots.improvements.md @@ -0,0 +1,2 @@ +* Reduced per-row allocations for `ClickHouseDataReader` typed accessors (`GetInt64`, `GetDouble`, `GetDateTime`, `GetGuid`, …), `GetFieldValue`, `IsDBNull` and ORMs such as linq2db by decoding scalar columns into reusable typed slots instead of eagerly boxing every value-type cell. Allocations drop by roughly two thirds on a typical multi-column typed read and to zero per row for all-numeric typed reads; composite columns (`Array`, `Tuple`, `Map`, `Variant`, `Dynamic`, `JSON`, geo), values, CLR types, exact-type strictness and the `DBNull.Value` representation of NULL are unchanged. + - `GetValue`, `GetValues` and the indexers — including Dapper's default materialization path — still box each requested value and pay a small fixed slot allocation per returned column. Because they box per call, two `GetValue(i)` calls on the same value-type cell now return two distinct boxes — still equal by `Equals`, but no longer by `ReferenceEquals`. diff --git a/changelog.d/499-simpleaggregatefunction-poco-read.improvements.md b/changelog.d/499-simpleaggregatefunction-poco-read.improvements.md new file mode 100644 index 000000000..c30e68c86 --- /dev/null +++ b/changelog.d/499-simpleaggregatefunction-poco-read.improvements.md @@ -0,0 +1 @@ +* `SimpleAggregateFunction(f, T)` columns now take the box-free fast path in `QueryAsync`, as `LowCardinality(T)` already did. Previously they fell back to the slower boxed read. diff --git a/changelog.d/499-typed-accessor-converter.improvements.md b/changelog.d/499-typed-accessor-converter.improvements.md new file mode 100644 index 000000000..dda066f31 --- /dev/null +++ b/changelog.d/499-typed-accessor-converter.improvements.md @@ -0,0 +1 @@ +* With an `IReadValueConverter` configured, `ClickHouseDataReader`'s typed accessors (`GetInt64`, `GetDouble`, `GetDecimal`, `GetString`, …) keep their reduced allocations: each reads its typed slot and converts through `ConvertValue`, the overload `GetFieldValue` uses. These accessors previously routed through `GetValue` and so through the boxed `ConvertValue`, so a converter whose two overloads return different results for the same cell now gets the typed result from them. `GetValue`, `GetValues` and the coercing fall-throughs still use the boxed overload.