ClickHouse / ClickHouse/ClickHouse.EntityFrameworkCore

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

未关闭
#54 0 条评论 0 个 reaction 已指派 0 人 在 GitHub 查看
主要语言
C#
星标
23
派生
7
平均合并
14 天 3 小时
30 天内合并 PR
1

描述

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

贡献指南

这个仓库没有索引到贡献指南

调研方向

从 src/EFCore.ClickHouse/Update/Internal/ClickHouseModificationCommandBatch.cs 第 104 行开始,检查如何从列类型映射中获取行值。为 enum、Uri 和显式 HasConversion 属性添加 SaveChanges 覆盖,然后根据文档中的映射,并在可用时使用真实的 ClickHouse 服务器,验证插入操作。完成的标准是,经过转换的 provider 值能够到达 driver,且新测试通过。

由索引模型根据 Issue 内容生成。

评估

技术栈
csharp
领域
database
Issue 类型
缺陷
难度
3/5
预计耗时
1-2 天
活跃度
冷清
描述清晰度
描述清楚
新手友好度
76/100

把新 issue 发到你的邮箱

精选适合新手参与的 GitHub issue 摘要。