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

ClickHouse.Driver.Tests/Types/TimezoneHandlingTests.cs

@@ -1349,3 +1349,199 @@ 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)));
+        });
+    }
+
+    // (type string, whether the type decodes an absolute instant)
+    private static IEnumerable<TestCaseData> InstantReportingTypes()
+    {
+        yield return new TestCaseData("DateTime('America/New_York')", true).SetName("ReportsInstant_DateTime");
+        yield return new TestCaseData("DateTime64(3, 'America/New_York')", true).SetName("ReportsInstant_DateTime64");
+        yield return new TestCaseData("Nullable(DateTime('America/New_York'))", true).SetName("ReportsInstant_NullableDateTime");
+        yield return new TestCaseData("Date", false).SetName("ReportsInstant_Date");
+        yield return new TestCaseData("Date32", false).SetName("ReportsInstant_Date32");
+        yield return new TestCaseData("Nullable(Date)", false).SetName("ReportsInstant_NullableDate");
+        yield return new TestCaseData("Int32", false).SetName("ReportsInstant_Int32");
+    }
+
+    /// <summary>
+    /// Instants are captured only for the types that decode one, so a result set of Date/Date32 columns —
+    /// which encode a day number, not an instant — keeps the plain decode path instead of taking the
+    /// instant-capturing one to be reported nothing.
+    /// </summary>
+    [TestCaseSource(nameof(InstantReportingTypes))]
+    public void Create_ForResultSetOfType_ReturnsReaderOnlyWhenAnInstantIsDecoded(string typeString, bool expected)
+    {
+        var type = TypeConverter.ParseClickHouseType(typeString, TypeSettings.Default);
+
+        Assert.That(RowInstantReader.Create([type]), expected ? Is.Not.Null : Is.Null);
+    }
+}

ClickHouse.Driver/ADO/Readers/ClickHouseDataReader.cs

@@ -36,6 +36,11 @@ 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();
+
+    // Decodes rows while capturing the instants date/time values were stored as, so GetDateTimeOffset can
+    // report the stored instant; null when no column of the result set carries one.
+    private readonly RowInstantReader rowInstantReader;
+
     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)
@@ -54,6 +59,8 @@ private ClickHouseDataReader(HttpResponseMessage httpResponse, ExtendedBinaryRea
         FieldNames = names;
         CurrentRow = new object[FieldNames.Length];
 
+        rowInstantReader = RowInstantReader.Create(types);
+
         if (readValueConverter != null)
             columnTypeNames = rawTypeNames;
     }
@@ -171,8 +178,22 @@ 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. A read-value converter
+        // can replace the decoded value, so pass what the caller sees when one is in play.
+        var visibleValue = readValueConverter == null ? (DateTime?)null : GetDateTime(ordinal);
+        if (rowInstantReader != null && rowInstantReader.TryGetDateTimeOffset(ordinal, adt, visibleValue, out var dto))
+            return dto;
+
+        return adt.CoerceToDateTimeOffset(visibleValue ?? GetDateTime(ordinal));
+    }
 
     public override decimal GetDecimal(int ordinal)
     {
@@ -478,11 +499,20 @@ public override bool Read()
             if (reader.PeekChar() == -1)
                 return false; // End of stream reached
 
-            for (var i = 0; i < count; i++)
+            if (rowInstantReader == 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
+            {
+                // Same decode, plus the instant of every column that carries one, for GetDateTimeOffset.
+                rowInstantReader.ReadRow(reader, RawTypes, data);
+            }
+
             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/DateTime64Type.cs

@@ -7,19 +7,21 @@
 
 namespace ClickHouse.Driver.Types;
 
-internal class DateTime64Type : AbstractDateTimeType
+internal class DateTime64Type : AbstractDateTimeType, IInstantReader
 {
     public int Scale { get; set; }
 
     public override string Name => "DateTime64";
 
     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,15 @@ public override ParameterizedType Parse(SyntaxTreeNode node, Func<SyntaxTreeNode
 
     public override object Read(ExtendedBinaryReader reader) => FromClickHouseTicks(reader.ReadInt64());
 
+    bool IInstantReader.ReportsInstant => true;
+
+    object IInstantReader.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)));

ClickHouse.Driver/Types/DateTimeType.cs

@@ -5,7 +5,7 @@
 
 namespace ClickHouse.Driver.Types;
 
-internal class DateTimeType : AbstractDateTimeType
+internal class DateTimeType : AbstractDateTimeType, IInstantReader
 {
     public override string Name => "DateTime";
 
@@ -21,7 +21,18 @@ public override ParameterizedType Parse(SyntaxTreeNode node, Func<SyntaxTreeNode
         return new DateTimeType { TimeZone = timeZone };
     }
 
-    public override object Read(ExtendedBinaryReader reader) => ToDateTime(Instant.FromUnixTimeSeconds(reader.ReadUInt32()));
+    public override object Read(ExtendedBinaryReader reader) => ToDateTime(ReadInstant(reader));
+
+    bool IInstantReader.ReportsInstant => true;
+
+    object IInstantReader.ReadWithInstant(ExtendedBinaryReader reader, out Instant? instant)
+    {
+        var decoded = ReadInstant(reader);
+        instant = decoded;
+        return ToDateTime(decoded);
+    }
+
+    private static Instant ReadInstant(ExtendedBinaryReader reader) => Instant.FromUnixTimeSeconds(reader.ReadUInt32());
 
     protected override void WriteChecked<T>(ExtendedBinaryWriter writer, DateTimeOffset dto, T value)
     {

ClickHouse.Driver/Types/IInstantReader.cs

@@ -0,0 +1,25 @@
+using ClickHouse.Driver.Formats;
+using NodaTime;
+
+namespace ClickHouse.Driver.Types;
+
+/// <summary>
+/// Implemented by types that decode an absolute instant, so a reader can obtain the instant a value was
+/// stored as instead of re-deriving it from the decoded wall-clock value. A wall clock inside a DST
+/// fall-back hour occurs at two offsets, so that derivation cannot preserve the instant.
+/// </summary>
+internal interface IInstantReader
+{
+    /// <summary>
+    /// Whether <see cref="ReadWithInstant"/> can report an instant. Transparent wrappers report what their
+    /// underlying type does, so <c>Nullable(Date)</c> is false while <c>Nullable(DateTime)</c> is true.
+    /// </summary>
+    bool ReportsInstant { get; }
+
+    /// <summary>
+    /// Reads a value exactly like <see cref="ClickHouseType.Read(ExtendedBinaryReader)"/>, additionally
+    /// reporting in <paramref name="instant"/> the instant it decoded, or <see langword="null"/> when the
+    /// value carries none (a NULL, for example).
+    /// </summary>
+    object ReadWithInstant(ExtendedBinaryReader reader, out Instant? instant);
+}