aloneguid / parquet-dotnet

Fully managed Apache Parquet implementation
https://aloneguid.github.io/parquet-dotnet/
MIT License
542 stars 140 forks source link

Do not try to set readonly properties when deserializing #514

Closed Pragmateek closed 1 month ago

Pragmateek commented 1 month ago

Issue description

Hello,

By default, when deserializing, Parquet.Net tries to set readonly properties which of course raises a LINQ Expression exception:

Unhandled exception. System.FieldAccessException: failed to compile 'N (System.Int32)'
 ---> System.ArgumentException: Expression must be writeable (Parameter 'left')
   at System.Linq.Expressions.Expression.RequiresCanWrite(Expression expression, String paramName)
   at System.Linq.Expressions.Expression.Assign(Expression left, Expression right)
   at Parquet.Serialization.Dremel.FieldAssemblerCompiler`1.InjectLevel(Expression rootVar, Type rootType, Field parentField, Field[] levelFields, List`1 path)

How to make Parquet.Net read them when serializing and ignore them when deserializing?

Thanks in advance,

Mickael

using Parquet.Serialization;

using var ms = new MemoryStream();
await ParquetSerializer.SerializeAsync([new A()], ms);

ms.Seek(0, SeekOrigin.Begin);

await ParquetSerializer.DeserializeAsync<A>(ms);

class A
{
    public int N => 1;
}
aloneguid commented 1 month ago

It's a bug in caching, I was using single cache for read schemas and write schemas, so if you read first, readable schema has read-only property and sets in the cache, which is then used in writer incorrectly. Please try -pre.4 version.

Pragmateek commented 1 month ago

Thanks for the fix. It's working now, just had to add a field as otherwise it would have nothing to serialize.

using Parquet.Serialization;

using var ms = new MemoryStream();
await ParquetSerializer.SerializeAsync([new A()], ms);

ms.Seek(0, SeekOrigin.Begin);

var a = (await ParquetSerializer.DeserializeAsync<A>(ms)).Single();

Console.WriteLine(a.N);

class A
{
    public int Phony { get; set; }
    public int N => 1;
}