Skip to content

HTTP client: DateTime64(9) silently stores a wrapped (wrong) value for dates outside 1677-2262 #711

Open

Description

@claude

Description

The HTTP client's binary write path for DateTime64 scales the instant with an unchecked 64-bit multiply, so a DateTime/DateTimeOffset outside the Int64-representable range of the column's scale wraps around and a wrong value is stored, with no error.

ClickHouse.Driver/Types/DateTime64Type.cs:25:

public long ToClickHouseTicks(Instant instant) => MathUtils.ShiftDecimalPlaces(instant.ToUnixTimeTicks(), Scale - 7);

ClickHouse.Driver/Utility/MathUtils.cs:31 does value * factor outside a checked block, so the multiply wraps silently. For Scale = 9 the factor is 100, and the representable window is only 1677-09-21 .. 2262-04-11 — exactly the window the ClickHouse docs give for precision 9. Scale = 8 wraps above roughly year 4890.

DateTime64Type.cs:54-56 even asserts the opposite:

// 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)));

Note the HTTP parameter path is not affected: HttpParameterFormatter.FormatDateTime64InTargetTimezone emits an ISO string, so DateTime64(7) parameters round-trip fine. The native TCP client is also not affected — ClickHouse.Driver.Tcp/Types/Codecs/DateTime64ColumnCodec.cs:179-196 range-checks before scaling up and throws ArgumentOutOfRangeException.

Unlike the Java driver (which converts every precision through epoch nanos, breaking even DateTime64(7)), here scales 0-7 are correct; only scales 8 and 9 are affected.

ClickHouse server version

26.10.3.7 (local, HTTP on 8123). Verified against a running server.

Reproduction

Scratch NUnit test in ClickHouse.Driver.Tests:

using System;
using System.Linq;
using System.Threading.Tasks;
using ClickHouse.Driver.Copy;
using ClickHouse.Driver.Utility;

namespace ClickHouse.Driver.Tests.Types;

public class ScratchDateTime64RangeTests : AbstractConnectionTestFixture
{
    [TestCase(7, "1500-06-01")]
    [TestCase(7, "3000-01-01")]
    [TestCase(9, "1500-06-01")]
    [TestCase(9, "3000-01-01")]
    [TestCase(8, "3000-01-01")]
    public async Task Scratch_OutOfRangeDateTime64(int scale, string date)
    {
        var value = new DateTimeOffset(DateTime.Parse(date + "T00:00:00Z").ToUniversalTime(), TimeSpan.Zero);
        var targetTable = CreateTableName($"scratch_dt64_{scale}_{date.Replace("-", "")}");
        await connection.ExecuteStatementAsync($"CREATE TABLE {targetTable} (value DateTime64({scale}, 'UTC')) ENGINE Memory");

        using var bulkCopy = new ClickHouseBulkCopy(connection) { DestinationTableName = targetTable };
        await bulkCopy.WriteToServerAsync(Enumerable.Repeat(new object[] { value }, 1));

        var stored = await connection.ExecuteScalarAsync($"SELECT toString(value) FROM {targetTable}");
        TestContext.Progress.WriteLine($"scale={scale} sent={value:yyyy-MM-dd HH:mm:ss} stored={stored}");
    }
}

Actual output (all five cases "pass" — nothing throws):

scale=7 sent=1500-06-01 00:00:00 stored=1500-06-01 00:00:00.0000000
scale=7 sent=3000-01-01 00:00:00 stored=3000-01-01 00:00:00.0000000
scale=9 sent=1500-06-01 00:00:00 stored=2084-12-18 23:34:33.709551616   <-- wrong
scale=9 sent=3000-01-01 00:00:00 stored=1830-11-23 00:50:52.580896768   <-- wrong
scale=8 sent=3000-01-01 00:00:00 stored=3000-01-01 00:00:00.00000000

Expected: either the value round-trips, or the write throws a clear out-of-range error naming the value and the column. Actual: a silently wrapped value is stored.

Suggested fix

Range-check before scaling up in DateTime64Type.ToClickHouseTicks (ClickHouse.Driver/Types/DateTime64Type.cs:25), mirroring what the TCP codec already does in DateTime64ColumnCodec.CountFromDateTimeOffset (ClickHouse.Driver.Tcp/Types/Codecs/DateTime64ColumnCodec.cs:179-196): when Scale > 7, compare against long.MaxValue / scaleUp and long.MinValue / scaleUp and throw ArgumentOutOfRangeException with the value and TypeName. WriteChecked already carries the un-coerced original for exactly this kind of error, and its "No range check" comment needs updating. Alternatively (or additionally) make MathUtils.ShiftDecimalPlaces use a checked multiply.

Link

Relayed from ClickHouse/clickhouse-java#3292

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions