ClickHouse / ClickHouse/clickhouse-cs

Binary write: Date/Date32/DateTime/DateTime64 reject string values with a bare NotSupportedException, unlike every other scalar type

Open
#614 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
C#
Stars
94
Forks
22
Avg merge
11h 26m
Merged PRs (30d)
22

Description

## Description

`ClickHouseType.Write` accepts a `string` for almost every scalar column type — the integer types go through `Convert.ToXxx(value, CultureInfo.InvariantCulture)`, `DecimalType`/`Float32Type`/`Float64Type` likewise, `UuidType.ExtractGuid` has an explicit `new Guid((string)data)` branch, and `Enum8Type`/`Enum16Type` look the string up as an enum name.

The four date/time types do not. `AbstractDateTimeType.CoerceToDateTimeOffset(object)` switches over `DateOnly` / `DateTimeOffset` / `DateTime` / `OffsetDateTime` / `ZonedDateTime` / `Instant` and falls through to a bare `throw new NotSupportedException()` — no message, no mention of the value or the target type. So `Date`, `Date32`, `DateTime`/`DateTime32` and `DateTime64` are the only scalar types where a string value fails, and when it does the caller gets `System.NotSupportedException: Specified method is not supported.`

Through `InsertBinaryAsync` this surfaces as `ClickHouseBulkCopySerializationException: Error when serializing data` with the message-less `NotSupportedException` as the inner exception, which gives no hint about which column or value was at fault.

This also blocks `Map(Date, V)` / `Map(DateTime, V)` columns for any caller whose map keys are strings — the key reaches `MapType.Write` -> `DateType.Write` as a `string`.

This is the .NET counterpart of https://github.com/ClickHouse/clickhouse-java/issues/3132. Note that the other two halves of that report do **not** apply here: `UInt64Type.Write` uses `Convert.ToUInt64`, which accepts the full unsigned range (`"18446744073709551615"` -> `FF FF FF FF FF FF FF FF`) and throws `OverflowException` on `"-1"` rather than silently wrapping it; and `UuidType` already handles strings.

## ClickHouse server version

`26.8.3.105` (verified end-to-end against a running server; the unit-level repro below needs no server).

## Reproduction

Unit level, `ClickHouse.Driver.Tests` (NUnit), no server required:

```csharp
using System;
using System.IO;
using ClickHouse.Driver.Formats;
using ClickHouse.Driver.Types;
using NUnit.Framework;

public class StringCoercionTests
{
private static void Write(string clickHouseType, object value)
{
var type = TypeConverter.ParseClickHouseType(clickHouseType, TypeSettings.Default);
using var stream = new MemoryStream();
using var writer = new ExtendedBinaryWriter(stream);
type.Write(writer, value);
}

[Test]
public void StringIsAcceptedByEveryScalarType()
{
// These all pass today
Assert.DoesNotThrow(() => Write("Int32", "42"));
Assert.DoesNotThrow(() => Write("UInt64", "18446744073709551615"));
Assert.DoesNotThrow(() => Write("Decimal(9, 2)", "1.23"));
Assert.DoesNotThrow(() => Write("UUID", "61f0c404-5cb3-11e7-907b-a6006ad3dba0"));

// These all throw NotSupportedException today
Assert.DoesNotThrow(() => Write("Date", "2020-01-01"));
Assert.DoesNotThrow(() => Write("Date32", "2020-01-01"));
Assert.DoesNotThrow(() => Write("DateTime", "2020-01-01 12:34:56"));
Assert.DoesNotThrow(() => Write("DateTime64(3)", "2020-01-01 12:34:56.789"));
}
}
```

Observed (each of the four date/time writes):

```
Date <- "2020-01-01" THREW NotSupportedException: Specified method is not supported.
Date32 <- "2020-01-01" THREW NotSupportedException: Specified method is not supported.
DateTime <- "2020-01-01 12:34:56" THREW NotSupportedException: Specified method is not supported.
DateTime64(3) <- "2020-01-01 12:34:56.789" THREW NotSupportedException: Specified method is not supported.
Map(Date, String) <- Dictionary { ["2020-01-01"] = "x" }
THREW NotSupportedException: Specified method is not supported.
```

while for comparison, on the same run:

```
Int32 <- "42" OK: 2A-00-00-00
UInt64 <- "18446744073709551615" OK: FF-FF-FF-FF-FF-FF-FF-FF
Decimal(9,2) <- "1.23" OK: 7B-00-00-00
UUID <- "61f0c404-..." OK: E7-11-B3-5C-04-C4-F0-61-A0-DB-D3-6A-00-A6-7B-90
String <- "abc" OK: 03-61-62-63
```

End-to-end through the public API, against `http://localhost:8123`:

```csharp
using var client = new ClickHouseClient("Host=localhost;Port=8123");
await client.ExecuteNonQueryAsync("DROP TABLE IF EXISTS str_date");
await client.ExecuteNonQueryAsync("CREATE TABLE str_date (id Int32, d Date, u UInt64, g UUID) ENGINE Memory");

// succeeds
await client.InsertBinaryAsync("str_date", new[] { "id", "u", "g" },
new[] { new object[] { "1", "18446744073709551615", "61f0c404-5cb3-11e7-907b-a6006ad3dba0" } });

// throws
await client.InsertBinaryAsync("str_date", new[] { "id", "d" },
new[] { new object[] { "2", "2020-01-01" } });
```

Actual: `ClickHouse.Driver.Copy.ClickHouseBulkCopySerializationException: Error when serializing data`, inner `NotSupportedException: Specified method is not supported.`
Expected: either the row inserts (parsing the string as a date), or the failure names the column, the value and the reason.

## Suggested fix

`ClickHouse.Driver/Types/AbstractDateTimeType.cs`, `CoerceToDateTimeOffset(object)` (the switch ending in `_ => throw new NotSupportedException()`):

1. Add a `string s` branch that parses with `CultureInfo.InvariantCulture` — `DateTimeOffset.TryParse` with `DateTimeStyles.AssumeUniversal`/`AdjustToUniversal` when the text carries an offset, otherwise `DateTime.TryParse` with `DateTimeStyles.None` so the result is `Unspecified` and goes through the existing wall-clock-in-column-timezone path in `CoerceToDateTimeOffset(DateTime)`. That keeps a string like `"2020-01-01 12:34:56"` consistent with an `Unspecified` `DateTime` carrying the same wall clock, and matches how `Convert.ToXxx(..., InvariantCulture)` handles strings for the numeric types. This is purely additive: these inputs throw today, so nothing that currently succeeds changes behaviour.
2. Independently of (1), give the fallthrough a message — `throw new NotSupportedException($"Cannot convert {value?.GetType()} to a value for {this}")`. The current message-less throw is what turns a wrong-type cell into an unusable `ClickHouseBulkCopySerializationException`.

## Related, same family (separate from the above)

`AbstractBigIntegerType.Write` (`ClickHouse.Driver/Types/AbstractBigIntegerType.cs:37`) ends its switch with `_ => new BigInteger(Convert.ToInt64(value, CultureInfo.InvariantCulture))`, so a string for `Int128`/`UInt128`/`Int256`/`UInt256` is silently capped at the `Int64` range:

```
Int128 <- "170141183460469231731687303715884105727" THREW OverflowException: Value was either too large or too small for an Int64.
UInt256 <- "1157920892373161954235709850086879078532699846656405640394575840079131296399 35" (same)
```

`BigInteger.Parse(s, CultureInfo.InvariantCulture)` for the string case would cover the full width. Happy to split this into its own issue if preferred.

## Link

Upstream report: https://github.com/ClickHouse/clickhouse-java/issues/3132

Contributor guide

Open the contributing guide

Research direction

Start in ClickHouse.Driver/Types/AbstractDateTimeType.cs at CoerceToDateTimeOffset(object), then run the unit-level StringCoercionTests reproduction from the issue. Add coverage for string values across Date, Date32, DateTime, and DateTime64, and ensure unsupported values produce a descriptive exception; the date strings should serialize successfully without a server.

Written by the indexing model from the issue text.

Assessment

Tech stack
csharp
Domain
database
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
78/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.