Fix GetDateTimeOffset: return the stored instant for DST-ambiguous timestamps

CHANGELOG.md

@@ -27,6 +27,7 @@ Unreleased
 * `ClickHouseClient.MemoryStreamManager` is now `[Obsolete]`. Since binary inserts stream directly into the request body (see above), this property is no longer used and has no effect; it will be removed in a future version.
 
 **Bug Fixes:**
+* Fixed `ClickHouseDataReader.GetDateTimeOffset()` returning an instant one hour off for timestamps in the later half of a DST fall-back hour in a timezone-aware `DateTime`/`DateTime64` column. The accessor re-interpreted the decoded wall-clock `DateTime` in the column timezone, and a wall clock inside a fall-back hour occurs at two different offsets, so the lenient resolution always picked the earlier one. The offset of the stored instant is now captured while the row is decoded, so the returned `DateTimeOffset` preserves the instant exactly (issue #515).
 * Fixed `{name:Type}` parameter type hints being mis-detected in queries containing `//` comments, nested block comments, backtick/double-quoted identifiers, backslash escapes or `$tag$` heredocs. A bare `#` no longer starts a comment (only `# ` and `#!` do) (issue #508).
 * Fixed `{name:Type}` parameter type hints being dropped, or a hint being invented for a parameter that does not exist, when the query contains another `{` that is not a type hint — for example a `SETTINGS` map value such as `additional_table_filters = {'t': 'a > 0'}`. A dropped hint fell back to CLR-type inference, losing precision (issue #510).
 * Fixed JSON typed paths whose names start with `max_dynamic_paths` or `max_dynamic_types` being mistaken for JSON settings and decoded as dynamic values.

ClickHouse.Driver.Tests/Types/TimezoneHandlingTests.cs

@@ -1349,3 +1349,174 @@ public void Parse_WithValidFixedUtcOffset_ResolvesToFixedOffsetZone(string typeS
         Assert.That(type.TimeZone.MaxOffset, Is.EqualTo(Offset.FromSeconds(expectedOffsetSeconds)));
     }
 }
+
+/// <summary>
+/// Regression tests for GitHub issue #515: GetDateTimeOffset returned the wrong instant for
+/// timestamps falling in the later half of a DST fall-back hour, because it re-interpreted the
+/// wall-clock DateTime in the column timezone instead of using the offset of the stored instant.
+/// </summary>
+[TestFixture]
+public class ReadDateTimeOffsetAmbiguousDstTests : AbstractConnectionTestFixture
+{
+    // Both instants render as local 01:30:00 in America/New_York on the 2025 fall-back day:
+    //   1762061400 = 2025-11-02 05:30:00Z, the first occurrence  (EDT, -04:00)
+    //   1762065000 = 2025-11-02 06:30:00Z, the second occurrence (EST, -05:00)
+    private const long FirstOccurrence = 1762061400;
+    private const long SecondOccurrence = 1762065000;
+
+    private static readonly DateTime AmbiguousWallClock = new(2025, 11, 2, 1, 30, 0);
+
+    // (sql, expected UTC instant, expected offset)
+    private static IEnumerable<TestCaseData> AmbiguousDstCases()
+    {
+        // The second occurrence is the reported bug: the lenient resolver picked -04:00,
+        // shifting the returned instant an hour earlier than the stored one.
+        yield return new TestCaseData(
+            $"SELECT toTimeZone(toDateTime({SecondOccurrence}, 'UTC'), 'America/New_York')",
+            SecondOccurrence, TimeSpan.FromHours(-5))
+            .SetName("ReadDateTimeOffset_DateTime_SecondOccurrence");
+        yield return new TestCaseData(
+            $"SELECT toTimeZone(toDateTime64({SecondOccurrence}, 3, 'UTC'), 'America/New_York')",
+            SecondOccurrence, TimeSpan.FromHours(-5))
+            .SetName("ReadDateTimeOffset_DateTime64_SecondOccurrence");
+        yield return new TestCaseData(
+            $"SELECT CAST(toTimeZone(toDateTime({SecondOccurrence}, 'UTC'), 'America/New_York') AS Nullable(DateTime('America/New_York')))",
+            SecondOccurrence, TimeSpan.FromHours(-5))
+            .SetName("ReadDateTimeOffset_NullableDateTime_SecondOccurrence");
+
+        // Contrast cases: the first occurrence of the same wall clock already agreed with the
+        // stored instant and must keep its -04:00 offset.
+        yield return new TestCaseData(
+            $"SELECT toTimeZone(toDateTime({FirstOccurrence}, 'UTC'), 'America/New_York')",
+            FirstOccurrence, TimeSpan.FromHours(-4))
+            .SetName("ReadDateTimeOffset_DateTime_FirstOccurrence");
+        yield return new TestCaseData(
+            $"SELECT toTimeZone(toDateTime64({FirstOccurrence}, 3, 'UTC'), 'America/New_York')",
+            FirstOccurrence, TimeSpan.FromHours(-4))
+            .SetName("ReadDateTimeOffset_DateTime64_FirstOccurrence");
+    }
+
+    /// <summary>
+    /// GetDateTimeOffset must return the instant the server stored, with the offset that instant
+    /// actually had in the column timezone — including for the ambiguous second occurrence of a
+    /// fall-back hour, where the wall clock alone cannot distinguish the two offsets.
+    /// </summary>
+    [TestCaseSource(nameof(AmbiguousDstCases))]
+    public async Task ReadDateTimeOffset_FromAmbiguousDstInstant_PreservesInstantAndOffset(
+        string sql, long expectedUnixSeconds, TimeSpan expectedOffset)
+    {
+        using var reader = (ClickHouseDataReader)await connection.ExecuteReaderAsync(sql);
+        Assert.That(reader.Read(), Is.True);
+
+        var dto = reader.GetDateTimeOffset(0);
+
+        Assert.Multiple(() =>
+        {
+            Assert.That(dto.ToUnixTimeSeconds(), Is.EqualTo(expectedUnixSeconds), "stored instant");
+            Assert.That(dto.Offset, Is.EqualTo(expectedOffset), "offset of the stored instant");
+            Assert.That(dto.DateTime, Is.EqualTo(AmbiguousWallClock), "wall-clock value");
+        });
+    }
+
+    /// <summary>
+    /// Contrast case: a non-ambiguous instant in the same DST-observing zone keeps the offset and
+    /// wall clock it had before the fix, proving the change is limited to instant preservation.
+    /// </summary>
+    [Test]
+    public async Task ReadDateTimeOffset_FromUnambiguousInstant_IsUnchanged()
+    {
+        // 2025-07-15 16:30:00Z is 12:30 EDT (-04:00), well clear of any transition.
+        using var reader = (ClickHouseDataReader)await connection.ExecuteReaderAsync(
+            "SELECT toTimeZone(toDateTime('2025-07-15 16:30:00', 'UTC'), 'America/New_York')");
+        Assert.That(reader.Read(), Is.True);
+
+        var dto = reader.GetDateTimeOffset(0);
+
+        Assert.Multiple(() =>
+        {
+            Assert.That(dto.Offset, Is.EqualTo(TimeSpan.FromHours(-4)), "EDT offset");
+            Assert.That(dto.DateTime, Is.EqualTo(new DateTime(2025, 7, 15, 12, 30, 0)), "wall-clock value");
+            Assert.That(dto.UtcDateTime, Is.EqualTo(new DateTime(2025, 7, 15, 16, 30, 0, DateTimeKind.Utc)), "stored instant");
+        });
+    }
+
+    /// <summary>
+    /// A row mixing date/time columns with other types must stay byte-aligned while the instants are
+    /// captured, and every column must keep decoding to the same value it did before.
+    /// </summary>
+    [Test]
+    public async Task ReadDateTimeOffset_FromMixedColumnRow_KeepsAllColumnsAligned()
+    {
+        using var reader = (ClickHouseDataReader)await connection.ExecuteReaderAsync($@"
+            SELECT 'before' AS s1,
+                   toTimeZone(toDateTime({SecondOccurrence}, 'UTC'), 'America/New_York') AS dt,
+                   CAST(NULL AS Nullable(DateTime('America/New_York'))) AS dt_null,
+                   toDate('2025-11-02') AS d,
+                   toTimeZone(toDateTime64({SecondOccurrence}, 3, 'UTC'), 'America/New_York') AS dt64,
+                   toInt32(42) AS n,
+                   'after' AS s2");
+        Assert.That(reader.Read(), Is.True);
+
+        Assert.Multiple(() =>
+        {
+            Assert.That(reader.GetString(0), Is.EqualTo("before"));
+            Assert.That(reader.GetDateTimeOffset(1).ToUnixTimeSeconds(), Is.EqualTo(SecondOccurrence));
+            Assert.That(reader.IsDBNull(2), Is.True, "NULL date/time column stays null");
+            Assert.That(reader.GetDateTime(3), Is.EqualTo(new DateTime(2025, 11, 2)));
+            Assert.That(reader.GetDateTimeOffset(3).Offset, Is.EqualTo(TimeSpan.Zero), "Date has no instant");
+            Assert.That(reader.GetDateTimeOffset(4).ToUnixTimeSeconds(), Is.EqualTo(SecondOccurrence));
+            Assert.That(reader.GetInt32(5), Is.EqualTo(42));
+            Assert.That(reader.GetString(6), Is.EqualTo("after"));
+        });
+    }
+
+    /// <summary>
+    /// The captured instant must belong to the current row: reading a second row has to replace the
+    /// offset of the first, never leave it visible.
+    /// </summary>
+    [Test]
+    public async Task ReadDateTimeOffset_AcrossRows_UsesCurrentRowInstant()
+    {
+        using var reader = (ClickHouseDataReader)await connection.ExecuteReaderAsync($@"
+            SELECT toTimeZone(toDateTime(ts, 'UTC'), 'America/New_York') AS dt
+            FROM values('ts UInt32', ({SecondOccurrence}), ({FirstOccurrence}), (1752597000))
+            ORDER BY ts DESC");
+
+        var offsets = new List<DateTimeOffset>();
+        while (reader.Read())
+            offsets.Add(reader.GetDateTimeOffset(0));
+
+        Assert.Multiple(() =>
+        {
+            Assert.That(offsets[0].ToUnixTimeSeconds(), Is.EqualTo(SecondOccurrence));
+            Assert.That(offsets[0].Offset, Is.EqualTo(TimeSpan.FromHours(-5)));
+            Assert.That(offsets[1].ToUnixTimeSeconds(), Is.EqualTo(FirstOccurrence));
+            Assert.That(offsets[1].Offset, Is.EqualTo(TimeSpan.FromHours(-4)));
+            Assert.That(offsets[2].ToUnixTimeSeconds(), Is.EqualTo(1752597000));
+            Assert.That(offsets[2].Offset, Is.EqualTo(TimeSpan.FromHours(-4)));
+        });
+    }
+
+    /// <summary>
+    /// Contrast case: a zone whose offset is zero at the read instant keeps returning a Kind=Utc
+    /// wall clock with a zero offset (the London fall-back hour ends in GMT, not BST).
+    /// </summary>
+    [Test]
+    public async Task ReadDateTimeOffset_FromZoneWithZeroOffsetAtInstant_IsUnchanged()
+    {
+        // 2025-10-26 01:30:00Z is the second occurrence of local 01:30 in Europe/London, at +00:00.
+        using var reader = (ClickHouseDataReader)await connection.ExecuteReaderAsync(
+            "SELECT toTimeZone(toDateTime('2025-10-26 01:30:00', 'UTC'), 'Europe/London')");
+        Assert.That(reader.Read(), Is.True);
+
+        var dateTime = reader.GetDateTime(0);
+        var dto = reader.GetDateTimeOffset(0);
+
+        Assert.Multiple(() =>
+        {
+            Assert.That(dateTime.Kind, Is.EqualTo(DateTimeKind.Utc), "zero offset yields Kind=Utc");
+            Assert.That(dto.Offset, Is.EqualTo(TimeSpan.Zero), "GMT offset");
+            Assert.That(dto.UtcDateTime, Is.EqualTo(new DateTime(2025, 10, 26, 1, 30, 0, DateTimeKind.Utc)));
+        });
+    }
+}

ClickHouse.Driver/ADO/Readers/ClickHouseDataReader.cs

@@ -17,6 +17,7 @@
 using ClickHouse.Driver.Poco;
 using ClickHouse.Driver.Types;
 using ClickHouse.Driver.Utility;
+using NodaTime;
 
 namespace ClickHouse.Driver.ADO.Readers;
 
@@ -34,6 +35,14 @@ public class ClickHouseDataReader : DbDataReader, IEnumerator<IDataReader>, IEnu
     private readonly string[] columnTypeNames; // Raw server-sent type strings; null when no converter
     private readonly PocoTypeRegistry pocoRegistry;
     private readonly Dictionary<Type, object> bindingPlanCache = new();
+
+    // Per-ordinal type used to capture the instant a value decoded from, non-null only for date/time
+    // columns (see BuildDateTimeColumns); null when the result set has none of them.
+    private readonly ClickHouseType[] dateTimeColumns;
+
+    // Instants captured for the current row, indexed by ordinal; null when dateTimeColumns is null.
+    private readonly Instant?[] rowInstants;
+
     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)
@@ -51,10 +60,37 @@ private ClickHouseDataReader(HttpResponseMessage httpResponse, ExtendedBinaryRea
         FieldNames = names;
         CurrentRow = new object[FieldNames.Length];
 
+        dateTimeColumns = BuildDateTimeColumns(types);
+        if (dateTimeColumns != null)
+            rowInstants = new Instant?[types.Length];
+
         if (readValueConverter != null)
             columnTypeNames = rawTypeNames;
     }
 
+    /// <summary>
+    /// Returns, per ordinal, the type to decode date/time columns through so the instant they were stored
+    /// as is captured alongside the value, or <see langword="null"/> when the result set contains no
+    /// date/time column. These are exactly the columns <see cref="GetDateTimeOffset"/> accepts: a
+    /// <see cref="AbstractDateTimeType"/>, optionally wrapped in <see cref="NullableType"/>.
+    /// </summary>
+    private static ClickHouseType[] BuildDateTimeColumns(ClickHouseType[] types)
+    {
+        ClickHouseType[] result = null;
+        for (var i = 0; i < types.Length; i++)
+        {
+            var type = types[i];
+            var effectiveType = type is NullableType nt ? nt.UnderlyingType : type;
+            if (effectiveType is AbstractDateTimeType)
+            {
+                result ??= new ClickHouseType[types.Length];
+                result[i] = type;
+            }
+        }
+
+        return result;
+    }
+
     internal static Task<ClickHouseDataReader> FromHttpResponseAsync(HttpResponseMessage httpResponse, TypeSettings settings)
         => FromHttpResponseAsync(httpResponse, settings, pocoRegistry: null);
 
@@ -154,8 +190,31 @@ internal ClickHouseType GetEffectiveClickHouseType(int ordinal)
 
     public override DateTime GetDateTime(int ordinal) => (DateTime)GetValue(ordinal);
 
-    public virtual DateTimeOffset GetDateTimeOffset(int ordinal) => GetEffectiveClickHouseType(ordinal) is AbstractDateTimeType adt ?
-        adt.CoerceToDateTimeOffset(GetDateTime(ordinal)) : throw new InvalidCastException();
+    public virtual DateTimeOffset GetDateTimeOffset(int ordinal)
+    {
+        if (GetEffectiveClickHouseType(ordinal) is not AbstractDateTimeType adt)
+            throw new InvalidCastException();
+
+        // Prefer the instant captured while the row was decoded. Coercing the wall-clock DateTime back
+        // into the column timezone cannot recover it: during a DST fall-back hour the same wall clock
+        // occurs at two offsets, and the lenient resolution used for a wall clock always picks the
+        // earlier one, returning a different instant for the second occurrence.
+        if (rowInstants?[ordinal] is Instant instant)
+        {
+            // Without a converter the decoded value is the value the caller sees, so the captured
+            // instant describes it. A converter may replace the value, so it is only trusted while the
+            // value the caller sees still matches the one the instant was captured for.
+            if (readValueConverter == null)
+                return adt.ToDateTimeOffset(instant);
+
+            var convertedDateTime = GetDateTime(ordinal);
+            return adt.ToDateTime(instant) == convertedDateTime
+                ? adt.ToDateTimeOffset(instant)
+                : adt.CoerceToDateTimeOffset(convertedDateTime);
+        }
+
+        return adt.CoerceToDateTimeOffset(GetDateTime(ordinal));
+    }
 
     public override decimal GetDecimal(int ordinal)
     {
@@ -461,11 +520,29 @@ public override bool Read()
             if (reader.PeekChar() == -1)
                 return false; // End of stream reached
 
-            for (var i = 0; i < count; i++)
+            var instants = rowInstants;
+            if (instants == null)
             {
-                var rawType = RawTypes[i];
-                data[i] = rawType.Read(reader);
+                for (var i = 0; i < count; i++)
+                {
+                    var rawType = RawTypes[i];
+                    data[i] = rawType.Read(reader);
+                }
+            }
+            else
+            {
+                // Date/time columns decode through ReadWithInstant so the instant they were stored as is
+                // captured for GetDateTimeOffset; every other column reads exactly as before.
+                for (var i = 0; i < count; i++)
+                {
+                    var dateTimeType = dateTimeColumns[i];
+                    if (dateTimeType == null)
+                        data[i] = RawTypes[i].Read(reader);
+                    else
+                        data[i] = dateTimeType.ReadWithInstant(reader, out instants[i]);
+                }
             }
+
             hasCurrentRow = true;
             return true;
         }

ClickHouse.Driver/Types/AbstractDateTimeType.cs

@@ -123,7 +123,7 @@ public DateTimeOffset CoerceToDateTimeOffset(DateTime value)
 
     public override string ToString() => TimeZone == null ? $"{Name}" : $"{Name}('{TimeZone.Id}')";
 
-    private DateTimeOffset ToDateTimeOffset(Instant instant) => instant.InZone(TimeZoneOrUtc).ToDateTimeOffset();
+    internal DateTimeOffset ToDateTimeOffset(Instant instant) => instant.InZone(TimeZoneOrUtc).ToDateTimeOffset();
 
     public DateTime ToDateTime(Instant instant)
     {

ClickHouse.Driver/Types/ClickHouseType.cs

@@ -1,5 +1,6 @@
 using System;
 using ClickHouse.Driver.Formats;
+using NodaTime;
 
 namespace ClickHouse.Driver.Types;
 
@@ -9,6 +10,18 @@ internal abstract class ClickHouseType
 
     public abstract object Read(ExtendedBinaryReader reader);
 
+    /// <summary>
+    /// Reads a value exactly like <see cref="Read(ExtendedBinaryReader)"/>, additionally reporting in
+    /// <paramref name="instant"/> the absolute instant it decoded, for types that encode one. The default
+    /// reports <see langword="null"/>, meaning the value carries no instant, and callers fall back to
+    /// their own handling.
+    /// </summary>
+    internal virtual object ReadWithInstant(ExtendedBinaryReader reader, out Instant? instant)
+    {
+        instant = null;
+        return Read(reader);
+    }
+
     public abstract void Write(ExtendedBinaryWriter writer, object value);
 
     public abstract override string ToString();

ClickHouse.Driver/Types/DateTime64Type.cs

@@ -15,11 +15,13 @@ internal class DateTime64Type : AbstractDateTimeType
 
     public override string ToString() => TimeZone == null ? $"DateTime64({Scale})" : $"DateTime64({Scale}, '{TimeZone.Id}')";
 
-    public DateTime FromClickHouseTicks(long clickHouseTicks)
+    public DateTime FromClickHouseTicks(long clickHouseTicks) => ToDateTime(ToInstant(clickHouseTicks));
+
+    public Instant ToInstant(long clickHouseTicks)
     {
         // Convert ClickHouse variable precision ticks into "standard" .NET 100ns ones
         var ticks = MathUtils.ShiftDecimalPlaces(clickHouseTicks, 7 - Scale);
-        return ToDateTime(Instant.FromUnixTimeTicks(ticks));
+        return Instant.FromUnixTimeTicks(ticks);
     }
 
     public long ToClickHouseTicks(Instant instant) => MathUtils.ShiftDecimalPlaces(instant.ToUnixTimeTicks(), Scale - 7);
@@ -44,6 +46,13 @@ public override ParameterizedType Parse(SyntaxTreeNode node, Func<SyntaxTreeNode
 
     public override object Read(ExtendedBinaryReader reader) => FromClickHouseTicks(reader.ReadInt64());
 
+    internal override object ReadWithInstant(ExtendedBinaryReader reader, out Instant? instant)
+    {
+        var decoded = ToInstant(reader.ReadInt64());
+        instant = decoded;
+        return ToDateTime(decoded);
+    }
+
     // No range check: any coerced instant is representable, so 'original' is unused.
     protected override void WriteChecked<T>(ExtendedBinaryWriter writer, DateTimeOffset dto, T original)
         => writer.Write(ToClickHouseTicks(Instant.FromDateTimeOffset(dto)));