From e2b3d7270efbca2790311b503b8512d8872ebdd7 Mon Sep 17 00:00:00 2001 From: Shay Rojansky Date: Sun, 5 Oct 2025 14:00:32 +0200 Subject: [PATCH] Fix NpgsqlArrayConverter for one-directional conversions Fixes #3050 --- .../ValueConversion/NpgsqlArrayConverter.cs | 54 +++++++++++-------- .../PrimitiveCollectionsQueryNpgsqlTest.cs | 25 +++++++++ 2 files changed, 56 insertions(+), 23 deletions(-) diff --git a/src/EFCore.PG/Storage/ValueConversion/NpgsqlArrayConverter.cs b/src/EFCore.PG/Storage/ValueConversion/NpgsqlArrayConverter.cs index b4eda1bad..4d8fe6477 100644 --- a/src/EFCore.PG/Storage/ValueConversion/NpgsqlArrayConverter.cs +++ b/src/EFCore.PG/Storage/ValueConversion/NpgsqlArrayConverter.cs @@ -131,25 +131,15 @@ private static Expression> ArrayConversionExpression ArrayAccess(input, i); break; - // Input is typed as an IList - we can get its length and index into it + // Input is typed as an ICollection - we can get its length, but we can't index into it case { IsGenericType: true } when inputInterfaces.Append(input.Type) - .Any(i => i.IsGenericType && i.GetGenericTypeDefinition() == typeof(IList<>)): + .Any(i => i.IsGenericType && i.GetGenericTypeDefinition() == typeof(ICollection<>)): { getInputLength = Property( input, input.Type.GetProperty("Count") - // If TInput is an interface (IList), its Count property needs to be found on ICollection + // If TInput is an interface (ICollection), its Count property needs to be found on ICollection ?? typeof(ICollection<>).MakeGenericType(input.Type.GetGenericArguments()[0]).GetProperty("Count")!); - indexer = i => Property(input, input.Type.FindIndexerProperty()!, i); - break; - } - - // Input is typed as an ICollection - we can get its length, but we can't index into it - case { IsGenericType: true } when inputInterfaces.Append(input.Type) - .Any(i => i.IsGenericType && i.GetGenericTypeDefinition() == typeof(ICollection<>)): - { - getInputLength = Property( - input, typeof(ICollection<>).MakeGenericType(input.Type.GetGenericArguments()[0]).GetProperty("Count")!); indexer = null; break; } @@ -176,6 +166,31 @@ private static Expression> ArrayConversionExpression NewArrayBounds(outputElementType, lengthVariable), + var t when typeof(TConcreteOutput).GetConstructor([typeof(int)]) is ConstructorInfo ctorWithLength => New(ctorWithLength, lengthVariable), + var t when typeof(TConcreteOutput).GetConstructor([]) is not null => New(typeof(TConcreteOutput)), + + _ => null + }; + + if (instantiateOutput is null) + { + // If the output type can't be instantiated (no public parameterless constructor), we can't value convert to it. + // If we simply throw and prevent the array converter from being instantiated, that would block scenarios where the user is + // only *writing* an uninstantiable type, but never reading it (see #3050). To allow for such "one-directional" value converters, + // we instead return a lambda that throws, so that if converter is actually ever used to read, it will throw at that point. + // See test Parameter_collection_Dictionary_Valuees_with_value_converter_Contains. + return Lambda>( + Throw( + New( + typeof(InvalidOperationException).GetConstructor([typeof(string)])!, + Constant($"Type {typeof(TConcreteOutput)} cannot be instantiated as it does not have a public parameterless constructor.")), + typeof(TOutput)), + input); + } + expressions.AddRange( [ // Get the length of the input array or list @@ -184,14 +199,7 @@ private static Expression> ArrayConversionExpression NewArrayBounds(outputElementType, lengthVariable), - var t when typeof(TConcreteOutput).GetConstructor([typeof(int)]) is ConstructorInfo ctorWithLength => New(ctorWithLength, lengthVariable), - _ => New(typeof(TConcreteOutput)) - }) + Assign(output, instantiateOutput) ]); if (indexer is not null) @@ -200,7 +208,7 @@ var t when typeof(TConcreteOutput).GetConstructor([typeof(int)]) is ConstructorI // the element converter on each element. // for (var i = 0; i < length; i++) // { - // result[i] = input[i]; + // result[i] = convert(input[i]); // } var counter = Parameter(typeof(int), "i"); @@ -232,7 +240,7 @@ elementConversionExpression is null // counter = 0; // while (enumerator.MoveNext()) // { - // output[counter] = enumerator.Current; + // output[counter] = convert(enumerator.Current); // counter++; // } var enumerableType = typeof(IEnumerable<>).MakeGenericType(inputElementType); diff --git a/test/EFCore.PG.FunctionalTests/Query/PrimitiveCollectionsQueryNpgsqlTest.cs b/test/EFCore.PG.FunctionalTests/Query/PrimitiveCollectionsQueryNpgsqlTest.cs index e60bbd9de..773dd5090 100644 --- a/test/EFCore.PG.FunctionalTests/Query/PrimitiveCollectionsQueryNpgsqlTest.cs +++ b/test/EFCore.PG.FunctionalTests/Query/PrimitiveCollectionsQueryNpgsqlTest.cs @@ -533,6 +533,31 @@ public virtual async Task Parameter_collection_HashSet_with_value_converter_Cont """); } + [ConditionalFact] + public virtual async Task Parameter_collection_Dictionary_Values_with_value_converter_Contains() + { + Dictionary enums = new() + { + [0] = MyEnum.Value1, + [1] = MyEnum.Value4 + }; + + // Dictionary<>.ValuesCollection doesn't have a public parameterless constructor, so NpgsqlArrayConverter can't convert to it + // (see #3050). We still allow NpgsqlArrayConverter to be built to allow one-directional conversion: in the query below, + // we only need to write enum.Values as a parameter (never read it). + await AssertQuery(ss => ss.Set().Where(c => enums.Values.Contains(c.Enum))); + + AssertSql( + """ +@enums_Values={ '0' +'3' } (DbType = Object) + +SELECT p."Id", p."Bool", p."Bools", p."DateTime", p."DateTimes", p."Enum", p."Enums", p."Int", p."Ints", p."NullableInt", p."NullableInts", p."NullableString", p."NullableStrings", p."NullableWrappedId", p."NullableWrappedIdWithNullableComparer", p."String", p."Strings", p."WrappedId" +FROM "PrimitiveCollectionsEntity" AS p +WHERE p."Enum" = ANY (@enums_Values) +"""); + } + public override async Task Parameter_collection_ImmutableArray_of_ints_Contains_int() { await base.Parameter_collection_ImmutableArray_of_ints_Contains_int();