Skip to content

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

Description

@alex-clickhouse

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:

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

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<byte>, 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.

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