ClickHouse / ClickHouse/ClickHouse.EntityFrameworkCore

SaveChanges fails for value-converted properties: bulk insert path does not apply the converter

Offen
#54 0 Kommentare 0 Reaktionen 0 zugewiesene Personen Auf GitHub ansehen
Vorherrschende Sprache
C#
Sterne
23
Forks
7
Ø Merge
14 T. 3 Std.
Gemergte PRs (30 T.)
1

Beschreibung

## Problem

`SaveChanges` fails for every property that has a value converter. The bulk insert path sends the model value to the driver. It does not apply the converter from the type mapping.

`src/EFCore.ClickHouse/Update/Internal/ClickHouseModificationCommandBatch.cs:104`:

```csharp
row[i] = writeColumns[i].Value ?? DBNull.Value;
```

`IColumnModification.Value` gives the model-level value. The code never calls `RelationalTypeMapping.Converter`, so the driver receives a CLR type that it cannot write to the column.

## Effect

C# enums are affected. The README documents enums as a supported type (`README.md`, type mapping table), and the provider maps them on purpose with `ClickHouseEnumTypeMapping` + `EnumToStringConverter`. Enum inserts fail.

Any property with an explicit `HasConversion(...)` is also affected.

## How to reproduce

```csharp
public enum Colour { Red, Green, Blue }

public class Row
{
public long Id { get; set; }
public Colour Colour { get; set; }
}

// ...
await ctx.Database.EnsureCreatedAsync(); // creates: colour String
ctx.Rows.Add(new Row { Id = 1, Colour = Colour.Green });
await ctx.SaveChangesAsync(); // throws
```

Error:

```
ClickHouse.Driver.Copy.ClickHouseBulkCopySerializationException : Error when serializing data
---- System.ArgumentException : String requires string, byte[], ReadOnlyMemory, or Stream, got Colour
at ClickHouse.Driver.Types.StringType.Write(ExtendedBinaryWriter writer, Object value)
at ClickHouse.Driver.Copy.Serializer.RowBinarySerializer.Serialize(Object[] row, ClickHouseType[] types, ExtendedBinaryWriter writer)
```

Confirmed against a real ClickHouse server for `enum`, `Uri`, and `DateTimeOffset`.

## Why queries are not affected

The query parameter path goes through `RelationalTypeMapping.CreateParameter`, which applies the converter. Only the insert path skips it.

## Suggested fix

Apply the converter when the row is built, for example with `ConvertToProvider` from the column type mapping.

Also add `SaveChanges` test coverage for an enum property, a `Uri` property, and a property with an explicit `HasConversion`. The current enum tests only assert converter behaviour on the mapping object. They never do an insert, which is why this defect was not found.

## Notes

Found while I investigated #53. The two problems are independent. #53 does not need this fix, because a native `DateTimeOffset` mapping carries no converter.

Beitragsleitfaden

Für dieses Repository ist kein Beitragsleitfaden indexiert

Rechercherichtung

Start in src/EFCore.ClickHouse/Update/Internal/ClickHouseModificationCommandBatch.cs at line 104 and inspect how the row value is obtained from the column type mapping. Add SaveChanges coverage for enum, Uri, and explicit HasConversion properties, then verify inserts against the documented mappings and a real ClickHouse server if available. Done means converted provider values reach the driver and the new tests pass.

Vom Indexierungsmodell aus dem Issue-Text verfasst.

Bewertung

Tech-Stack
csharp
Bereich
database
Issue-Typ
Bug
Schwierigkeit
3/5
Geschätzter Aufwand
1-2 Tage
Aktivitätsstatus
Ruhig
Klarheit
Klar beschrieben
Anfängerfreundlichkeit
76/100

Neue Issues direkt in Ihr Postfach

Eine kurze Übersicht über anfängerfreundliche GitHub-Issues.