ClickHouse / ClickHouse/ClickHouse.EntityFrameworkCore

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

Abierto
#54 0 comentarios 0 reacciones 0 asignados Ver en GitHub
Lenguaje dominante
C#
Estrellas
23
Forks
7
Merge medio
14 d 3 h
PR fusionados (30 d)
1

Descripción

## 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.

Guía de contribución

No hay ninguna guía de contribución indexada para este repositorio

Línea de trabajo

Comienza en src/EFCore.ClickHouse/Update/Internal/ClickHouseModificationCommandBatch.cs en la línea 104 e inspecciona cómo se obtiene el valor de la fila a partir del mapeo del tipo de columna. Añade cobertura de SaveChanges para propiedades enum, Uri y HasConversion explícitas, y después verifica los inserts con los mapeos documentados y con un servidor ClickHouse real si está disponible. Se considera terminado cuando los valores de proveedor convertidos llegan al driver y las nuevas pruebas pasan.

Escrito por el modelo de indexación a partir del texto del issue.

Evaluación

Stack tecnológico
csharp
Área
database
Tipo de issue
Error
Dificultad
3/5
Tiempo estimado
1-2 días
Estado de actividad
Tranquilo
Claridad
Bien especificado
Aptitud para principiantes
76/100

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.