Fix CreateParameter for collection shape mismatch with element value converters

[hostage2222]

When a primitive collection has an element value converter and a parameter uses a different collection shape (e.g. List vs T[]), type mapping inference for Intersect(...).Any() → && can apply the column mapping to that parameter. CreateParameter skipped materialization whenever a converter was present, so Sanitize threw InvalidCastException (IConvertible).

Normalize mismatched values to TConcreteCollection before Sanitize, without rewriting values that already match the converter model type. There is no extra materialization on existing paths — only the new mismatch cases take that branch; existing tests do not.

Fixes #3805

[Copilot]

Pull request overview

Fixes a failure in array parameter creation when an element value converter is present and the parameter value’s collection shape doesn’t match the inferred mapping shape (e.g. List<T> passed where an T[] mapping is applied), which previously could cause ValueConverter.Sanitize to throw InvalidCastException (notably in Intersect(...).Any() translated to &&).

Changes:

• Update NpgsqlArrayTypeMapping.CreateParameter to normalize non-matching values to the supported concrete collection shape (array or List<T>) even when a converter exists, but without rewriting values that already match the converter’s model CLR type.
• Add unit tests covering CreateParameter with value converters across array/list/immutable-list inputs.
• Add functional tests covering Intersect(...).Any() over value-converted primitive collections with array/list/immutable-list parameters.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File	Description
src/EFCore.PG/Storage/Internal/Mapping/NpgsqlArrayTypeMapping.cs	Normalizes mismatched collection-shape parameter values prior to converter sanitization to avoid invalid casts.
test/EFCore.PG.Tests/Storage/NpgsqlTypeMappingSourceTest.cs	Unit coverage for CreateParameter behavior across list/array/immutable-list inputs with converters.
test/EFCore.PG.FunctionalTests/Query/ArrayListQueryTest.cs	Functional coverage for Intersect(...).Any() over a value-converted list when the parameter shape varies.
test/EFCore.PG.FunctionalTests/Query/ArrayArrayQueryTest.cs	Functional coverage for Intersect(...).Any() over a value-converted array when the parameter shape varies.

[roji]

The approach is sound and correctly targets the collection-shape mismatch; I found no blocking issues.

One residual gap: in NpgsqlArrayTypeMapping.cs, the comment says the value is normalized to TConcreteCollection, but the non-array path always creates a List<TElement>. Concrete model collection types to which List<TElement> is not assignable (for example, HashSet<T>) could therefore still fail conversion. Either narrow the comment to describe the supported array/list normalization or instantiate the actual concrete collection.

Additional tests for the shape-only converter path and an arbitrary enumerable parameter over a value-converted array column would improve coverage.

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

Suppressed comments (5)

test/EFCore.PG.Tests/Storage/NpgsqlTypeMappingSourceTest.cs:374

• This helper asserts the exact order of the produced provider array. Since many callers pass unordered inputs (e.g. HashSet), the enumeration/materialization order is not guaranteed and can cause test flakiness. Compare using a deterministic ordering instead of strict positional equivalence.

    private void CreateParameter_with_value_converter(Type mappingType, object value)
    {
        var mapping = CreateTypeMappingSource().FindMapping(mappingType)!;
        Assert.NotNull(mapping.Converter);
        var parameter = mapping.CreateParameter(new NpgsqlCommand(), "p", value);
        Assert.Equivalent(new[] { "foo", "bar" }, Assert.IsType<string[]>(parameter.Value), strict: true);
    }

test/EFCore.PG.Tests/Storage/NpgsqlTypeMappingSourceTest.cs:382

• This helper assumes List preserves the original element order. When the input is an unordered collection (e.g. HashSet or other non-list IEnumerable), CreateParameter materializes via enumeration which has unspecified ordering, making the strict equivalence assertion potentially flaky. Compare using a deterministic ordering instead.

    private void CreateParameter_without_converter(Type mappingType, object value)
    {
        var mapping = CreateTypeMappingSource().FindMapping(mappingType)!;
        Assert.Null(mapping.Converter);
        var parameter = mapping.CreateParameter(new NpgsqlCommand(), "p", value);
        Assert.Equivalent(new[] { 1, 2 }, Assert.IsType<List<int>>(parameter.Value), strict: true);
    }

test/EFCore.PG.Tests/Storage/NpgsqlTypeMappingSourceTest.cs:340

• This assertion assumes a deterministic element order for a HashSet-based input. HashSet enumeration order is explicitly unspecified, so materializing it to an array can produce a different order and make this test flaky across runtimes/framework versions. Prefer an order-insensitive assertion (e.g. compare sorted sequences) for unordered inputs.

This issue also appears in the following locations of the same file:

• line 368
• line 376

        var parameter = mapping.CreateParameter(new NpgsqlCommand(), "p", new HashSet<int> { 1, 2 });
        Assert.Equivalent(new[] { 1, 2 }, Assert.IsType<int[]>(parameter.Value), strict: true);

test/EFCore.PG.FunctionalTests/Query/ArrayListQueryTest.cs:940

• HashSet enumeration order is unspecified, but this test asserts the parameter values in a specific order in the SQL baseline. This can make the test flaky if the HashSet gets enumerated in a different order. Consider using a deterministically ordered set (e.g. ImmutableSortedSet/SortedSet) while keeping the variable name (so the baseline parameter name stays the same).

    public virtual async Task Intersect_parameter_hash_set_over_value_converted_list()
    {
        HashSet<SomeEnum> toFindHashSet = [SomeEnum.One, SomeEnum.Three, SomeEnum.Eight];

test/EFCore.PG.FunctionalTests/Query/ArrayArrayQueryTest.cs:934

• HashSet enumeration order is unspecified, but this test asserts the parameter values in a specific order in the SQL baseline. This can make the test flaky if the HashSet gets enumerated in a different order. Consider using a deterministically ordered set (e.g. ImmutableSortedSet/SortedSet) while keeping the variable name (so the baseline parameter name stays the same).

    public virtual async Task Intersect_parameter_hash_set_over_value_converted_array()
    {
        HashSet<SomeEnum> toFindHashSet = [SomeEnum.One, SomeEnum.Three, SomeEnum.Eight];

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

Suppressed comments (3)

src/EFCore.PG/Storage/Internal/Mapping/NpgsqlArrayTypeMapping.cs:202

• CreateInstance() calls GetConstructor on every invocation, and the fallback exception message currently mentions only a parameterless constructor even though this method also supports an (int capacity) constructor. Since CreateParameter() can be a hot path, consider caching the constructor infos in static fields (generic-type specific) and update the exception message accordingly.

    private static TConcreteCollection CreateInstance(int? count)
        => (count, typeof(TConcreteCollection)) switch
        {
            ({ } c, var type) when type.GetConstructor([typeof(int)]) is { } ctorWithSize
                => (TConcreteCollection)ctorWithSize.Invoke([c]),
            var (_, type) when type.GetConstructor([]) is { } ctor
                => (TConcreteCollection)ctor.Invoke(null),
            var (_, type) => throw new InvalidOperationException(
                $"Type {type.Name} cannot be instantiated as it does not have a public parameterless constructor")
        };

test/EFCore.PG.FunctionalTests/Query/ArrayListQueryTest.cs:941

• This test captures a HashSet and asserts the exact parameter dump in AssertSql. Since HashSet enumeration order is not guaranteed, the logged parameter values can be nondeterministic across runtimes and make the baseline flaky. Consider using a deterministically-ordered set type here (while keeping separate coverage for HashSet elsewhere).

    public virtual async Task Intersect_parameter_hash_set_over_value_converted_list()
    {
        HashSet<SomeEnum> toFindHashSet = [SomeEnum.One, SomeEnum.Three, SomeEnum.Eight];

        await AssertQuery(ss => ss.Set<ArrayEntity>().Where(e => e.ValueConvertedListOfEnum.Intersect(toFindHashSet).Any()));

test/EFCore.PG.FunctionalTests/Query/ArrayArrayQueryTest.cs:936

• This test captures a HashSet and asserts the exact parameter dump in AssertSql. Since HashSet enumeration order is not guaranteed, the logged parameter values can be nondeterministic across runtimes and make the baseline flaky. Consider using a deterministically-ordered set type here (while keeping separate coverage for HashSet elsewhere).

    [ConditionalFact]
    public virtual async Task Intersect_parameter_hash_set_over_value_converted_array()
    {
        HashSet<SomeEnum> toFindHashSet = [SomeEnum.One, SomeEnum.Three, SomeEnum.Eight];

        await AssertQuery(ss => ss.Set<ArrayEntity>().Where(e => e.ValueConvertedArrayOfEnum.Intersect(toFindHashSet).Any()));

[hostage2222]

Chose instantiating the actual TConcreteCollection rather than narrowing the comment. This covers mutable collections via ICollection<T>.Add; immutable collections aren't handled.
Also expanded coverage: HashSet concrete mappings, shape-only converters, non-collection enumerables, plus matching functional Intersect cases.

Regarding Copilot’s feedback: narrowed the HashSet functional tests to a single element (Assert.Equivalent already ignores collection order even with strict: true - https://xunit.net/releases/v2/2.4.2#new-assertions).