Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

After updating to 8.0, mapping array column to a hashset no longer works #3038

Closed
Atulin opened this issue Dec 22, 2023 · 9 comments
Closed

Comments

@Atulin
Copy link

Atulin commented Dec 22, 2023

I have an entity with the following property:

public HashSet<long> Chapters { get; init; }

Before 8.0 I used this converter

public class NpgHashsetConverter<T> : ValueConverter<HashSet<T>, List<T>>, INpgsqlArrayConverter
{
    public NpgHashsetConverter() : base(
        hs => new List<T>(hs),
        ls => new HashSet<T>(ls)
    ){}

    public ValueConverter ElementConverter => new ValueConverter<T, T>(x => x, x => x);
}

and this model config

builder
    .Property(cr => cr.Chapters)
    .HasConversion(new NpgHashsetConverter<long>())
    .Metadata.SetValueComparer(new ValueComparer<HashSet<long>>(
        (a, b) => a.SetEquals(b),
        l => l.Aggregate(0, (i, l1) => HashCode.Combine(i, l1.GetHashCode())),
        h => h.ToHashSet()
    ));

to make it work. After updating to a pre-release version of NpgSQL 8.0 I would get this exception:

[06:33:59 ERR] An unhandled exception has occurred while executing the request.
System.ArgumentException: Expression of type 'System.Collections.Generic.HashSet`1[System.Int64]' cannot be used for parameter of type 'System.Collections.Generic.IList`1[System.Int64]' of method 'System.Collections.Generic.IList`
1[System.Int64] PopulateList[Int64](System.Collections.Generic.IList`1[System.Int64], System.Collections.Generic.IList`1[System.Int64])' (Parameter 'arg0')

Now, I updated the project wholesale to non-preview 8.0, so both EF Core and NpgSQL are at version 8.0.0. Apparently, said version removed INpgSqlArrayConverter interface, and changed how primitive collections are handled in the first place.

Following the breaking changes note, I modified my model config to be just

builder
	.PrimitiveCollection(cr => cr.Chapters);

however, an exception still occurs:

[00:27:03 ERR] An exception occurred while iterating over the results of a query for context type 'Ogma3.Data.ApplicationDbContext'.
System.InvalidCastException: Reading as 'System.Collections.Generic.HashSet`1[[System.Int64, System.Private.CoreLib, Version=8.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e]]' is not supported for fields having DataTypeName 'bigint[]'
 ---> System.NotSupportedException: Writing is not supported for IEnumerable parameters, use an array or List instead.
   at Npgsql.Internal.ResolverFactories.UnsupportedTypeInfoResolver`1.GetTypeInfo(Type type, Nullable`1 dataTypeName, PgSerializerOptions options)
   at Npgsql.Internal.TypeInfoResolverChain.GetTypeInfo(Type type, Nullable`1 dataTypeName, PgSerializerOptions options)
   at Npgsql.Internal.TypeInfoCache`1.<GetOrAddInfo>g__CreateInfo|6_3(Type type, Nullable`1 typeId, PgSerializerOptions options, Boolean defaultTypeFallback, Boolean validatePgTypeIds)
   at Npgsql.Internal.TypeInfoCache`1.<GetOrAddInfo>g__AddEntryById|6_2(Type type, TPgTypeId pgTypeId, ValueTuple`2[] infos, Boolean defaultTypeFallback)
   at Npgsql.Internal.PgSerializerOptions.GetTypeInfoCore(Type type, Nullable`1 pgTypeId, Boolean defaultTypeFallback)
   at Npgsql.Internal.AdoSerializerHelpers.GetTypeInfoForReading(Type type, PostgresType postgresType, PgSerializerOptions options)
   --- End of inner exception stack trace ---
   at Npgsql.Internal.AdoSerializerHelpers.<GetTypeInfoForReading>g__ThrowReadingNotSupported|0_0(Type type, String displayName, Exception inner)
   at Npgsql.Internal.AdoSerializerHelpers.GetTypeInfoForReading(Type type, PostgresType postgresType, PgSerializerOptions options)
   at Npgsql.BackendMessages.FieldDescription.<GetInfo>g__GetInfoSlow|50_0(Type type, ColumnInfo& lastColumnInfo)
   at Npgsql.BackendMessages.FieldDescription.GetInfo(Type type, ColumnInfo& lastColumnInfo)
   at Npgsql.NpgsqlDataReader.<GetInfo>g__Slow|133_0(ColumnInfo& info, PgConverter& converter, Size& bufferRequirement, Boolean& asObject, <>c__DisplayClass133_0&)
   at Npgsql.NpgsqlDataReader.GetFieldValueCore[T](Int32 ordinal)
   at Npgsql.NpgsqlDataReader.GetFieldValue[T](Int32 ordinal)
   at lambda_method516(Closure, QueryContext, DbDataReader, ResultContext, SingleQueryResultCoordinator)
   at Microsoft.EntityFrameworkCore.Query.Internal.SingleQueryingEnumerable`1.AsyncEnumerator.MoveNextAsync()
System.InvalidCastException: Reading as 'System.Collections.Generic.HashSet`1[[System.Int64, System.Private.CoreLib, Version=8.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e]]' is not supported for fields having DataTypeN
ame 'bigint[]'
 ---> System.NotSupportedException: Writing is not supported for IEnumerable parameters, use an array or List instead.
   at Npgsql.Internal.ResolverFactories.UnsupportedTypeInfoResolver`1.GetTypeInfo(Type type, Nullable`1 dataTypeName, PgSerializerOptions options)
   at Npgsql.Internal.TypeInfoResolverChain.GetTypeInfo(Type type, Nullable`1 dataTypeName, PgSerializerOptions options)
   at Npgsql.Internal.TypeInfoCache`1.<GetOrAddInfo>g__CreateInfo|6_3(Type type, Nullable`1 typeId, PgSerializerOptions options, Boolean defaultTypeFallback, Boolean validatePgTypeIds)
   at Npgsql.Internal.TypeInfoCache`1.<GetOrAddInfo>g__AddEntryById|6_2(Type type, TPgTypeId pgTypeId, ValueTuple`2[] infos, Boolean defaultTypeFallback)
   at Npgsql.Internal.PgSerializerOptions.GetTypeInfoCore(Type type, Nullable`1 pgTypeId, Boolean defaultTypeFallback)
   at Npgsql.Internal.AdoSerializerHelpers.GetTypeInfoForReading(Type type, PostgresType postgresType, PgSerializerOptions options)
   --- End of inner exception stack trace ---
   at Npgsql.Internal.AdoSerializerHelpers.<GetTypeInfoForReading>g__ThrowReadingNotSupported|0_0(Type type, String displayName, Exception inner)
   at Npgsql.Internal.AdoSerializerHelpers.GetTypeInfoForReading(Type type, PostgresType postgresType, PgSerializerOptions options)
   at Npgsql.BackendMessages.FieldDescription.<GetInfo>g__GetInfoSlow|50_0(Type type, ColumnInfo& lastColumnInfo)
   at Npgsql.BackendMessages.FieldDescription.GetInfo(Type type, ColumnInfo& lastColumnInfo)
   at Npgsql.NpgsqlDataReader.<GetInfo>g__Slow|133_0(ColumnInfo& info, PgConverter& converter, Size& bufferRequirement, Boolean& asObject, <>c__DisplayClass133_0&)
   at Npgsql.NpgsqlDataReader.GetFieldValueCore[T](Int32 ordinal)
   at Npgsql.NpgsqlDataReader.GetFieldValue[T](Int32 ordinal)
   at lambda_method516(Closure, QueryContext, DbDataReader, ResultContext, SingleQueryResultCoordinator)
   at Microsoft.EntityFrameworkCore.Query.Internal.SingleQueryingEnumerable`1.AsyncEnumerator.MoveNextAsync()
@DmytroLynda
Copy link

@Atulin I Have the same issue but with IEnumerable collection type.

@QuantumToasted
Copy link

QuantumToasted commented Dec 28, 2023

Note: INpgsqlArrayConverter was replaced with NpgsqlArrayConverter, which appears to function a little bit differently, and the only example I could find in the repo was in the test suite. However, I've had some issues (#2991) with it that I'm unsure are an issue with my code or efcore.pg, so of course exercise caution before jumping straight into it.

@roji
Copy link
Member

roji commented Dec 28, 2023

@Atulin and others, there have been some pretty far-reaching changes in how the provider handles array/collections, due to the work on primitive collections on the EF side. I'll definitely take a look at this when I get some spare time.

@Atulin
Copy link
Author

Atulin commented Dec 28, 2023

Ah, so a somewhat-expected regression, then. In the meantime, could you maybe add a mention of it to the list of 8.0 breaking changes, or the docs about primitive collections?

@roji
Copy link
Member

roji commented Feb 17, 2024

Everyone, sorry it took so long to answer this - and I don't have a great answer; EF 8.0's support for primitive collection currently doesn't support HashSet properties - this is tracked by dotnet/efcore#33115, see there for some explanations on why mapping HashSet isn't straightforward. The workaround is to switch to e.g. List<T> - you can still use e.g. Contains and it will translate just fine.

I'll go ahead and close this as this needs to be resolved on the EF side first and foremost; any additional work on the EFCore.PG side after that will be tracked separately.

@roji roji closed this as not planned Won't fix, can't repro, duplicate, stale Feb 17, 2024
@roji
Copy link
Member

roji commented Feb 17, 2024

BTW if there are specific reasons why mapping HashSet to PG array is needed (over array/List), I'd be interested to hear them... I get that sometimes a PG array is used as if it were a set (e.g. for containment checks only) - and I agree it makes sense to support this - I'm just looking for any other specific reasons I might not be aware of.

@Atulin
Copy link
Author

Atulin commented Feb 17, 2024

The specific reason in my case is probably the most obvious one: being able to map a set of unique values easily. Instead of having to do

thing.Numbers.Add(7);
thing.Numbers = thing.Numbers.Distinct();

or some other variation thereof, where thing.Numbers is a List<int>. Instead, I can just do

thing.Numbers.Add(7);

and with that alone ensure, that thing.Numbers, being a HashSet<int> does not contain a duplicate 7.

@QuantumToasted
Copy link

My usecase is the same as Atulin's - specifically that I find myself in a situation where I wish to take advantage of HashSet<T>'s ability to perform the "add if it doesn't exist in the collection already" function I find myself needing in many an array column. This can be easily replicated with an extension method on List<T> for adding/removing, so if this is indeed not planned, I can adjust my code accordingly, but I prefer using first-party solutions when possible!

@roji
Copy link
Member

roji commented Feb 22, 2024

Thanks @Atulin @QuantumToasted. The use-case is valid, just remember that the PG type you're mapping to (i.e. arrays) really do not correspond to the .NET type you want (HashSet). Specifically, reading database arrays into HashSet loses data (both ordering and duplicates).

But for those scenarios where that's OK, I do agree we should look into supporting them. In the meantime, you can also continue using a HashSet property, but configure it as unmapped with EF, and have a mapped private property that reads/writes to the HashSet property. That should get you to a very similar place, which a bit of extra wrapping logic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants