Add translation of string.Join overload used with List<string> parame…

…ter (#3106) Fixes #3105
npgsql · Feb 24, 2024 · 707c89e · 707c89e
1 parent 2382b8b
commit 707c89e
Show file tree

Hide file tree

Showing 8 changed files with 229 additions and 131 deletions.
diff --git a/src/EFCore.PG/Query/ExpressionTranslators/Internal/NpgsqlStringMethodTranslator.cs b/src/EFCore.PG/Query/ExpressionTranslators/Internal/NpgsqlStringMethodTranslator.cs
@@ -1,5 +1,6 @@
 using Npgsql.EntityFrameworkCore.PostgreSQL.Query.Expressions.Internal;
 using Npgsql.EntityFrameworkCore.PostgreSQL.Storage.Internal;
+using Npgsql.EntityFrameworkCore.PostgreSQL.Storage.Internal.Mapping;
 using static Npgsql.EntityFrameworkCore.PostgreSQL.Utilities.Statics;
 using ExpressionExtensions = Microsoft.EntityFrameworkCore.Query.ExpressionExtensions;
 
@@ -100,6 +101,9 @@ public class NpgsqlStringMethodTranslator : IMethodCallTranslator
     private static readonly MethodInfo String_Join4 =
         typeof(string).GetMethod(nameof(string.Join), [typeof(char), typeof(string[])])!;
 
+    private static readonly MethodInfo String_Join5 =
+        typeof(string).GetMethod(nameof(string.Join), [typeof(string), typeof(IEnumerable<string>)])!;
+
     private static readonly MethodInfo String_Join_generic1 =
         typeof(string).GetTypeInfo().GetMethods(BindingFlags.Public | BindingFlags.Static | BindingFlags.DeclaredOnly)
             .Single(
@@ -334,8 +338,10 @@ public NpgsqlStringMethodTranslator(NpgsqlTypeMappingSource typeMappingSource, I
                 || method == String_Join2
                 || method == String_Join3
                 || method == String_Join4
+                || method == String_Join5
                 || method.IsClosedFormOf(String_Join_generic1)
-                || method.IsClosedFormOf(String_Join_generic2)))
+                || method.IsClosedFormOf(String_Join_generic2))
+            && arguments[1].TypeMapping is NpgsqlArrayTypeMapping)
         {
             // If the array of strings to be joined is a constant (NewArrayExpression), we translate to concat_ws.
             // Otherwise we translate to array_to_string, which also supports array columns and parameters.

diff --git a/test/EFCore.PG.FunctionalTests/Query/ArrayArrayQueryTest.cs b/test/EFCore.PG.FunctionalTests/Query/ArrayArrayQueryTest.cs
diff --git a/test/EFCore.PG.FunctionalTests/Query/ArrayListQueryTest.cs b/test/EFCore.PG.FunctionalTests/Query/ArrayListQueryTest.cs
diff --git a/test/EFCore.PG.FunctionalTests/Query/ArrayQueryFixture.cs b/test/EFCore.PG.FunctionalTests/Query/ArrayQueryFixture.cs
@@ -56,8 +56,10 @@ public ISetSource GetExpectedData()
                         Assert.Equal(ee.NullableText, ee.NullableText);
                         Assert.Equal(ee.NonNullableText, ee.NonNullableText);
                         Assert.Equal(ee.EnumConvertedToInt, ee.EnumConvertedToInt);
-                        Assert.Equal(ee.ValueConvertedArray, ee.ValueConvertedArray);
-                        Assert.Equal(ee.ValueConvertedList, ee.ValueConvertedList);
+                        Assert.Equal(ee.ArrayOfStringConvertedToDelimitedString, ee.ArrayOfStringConvertedToDelimitedString);
+                        Assert.Equal(ee.ListOfStringConvertedToDelimitedString, ee.ListOfStringConvertedToDelimitedString);
+                        Assert.Equal(ee.ValueConvertedArrayOfEnum, ee.ValueConvertedArrayOfEnum);
+                        Assert.Equal(ee.ValueConvertedListOfEnum, ee.ValueConvertedListOfEnum);
                         Assert.Equal(ee.IList, ee.IList);
                         Assert.Equal(ee.Byte, ee.Byte);
                     }

diff --git a/test/EFCore.PG.FunctionalTests/Query/ArrayQueryTest.cs b/test/EFCore.PG.FunctionalTests/Query/ArrayQueryTest.cs
@@ -503,12 +503,20 @@ public virtual Task Concat(bool async)
     // Note: see NorthwindFunctionsQueryNpgsqlTest.String_Join_non_aggregate for regular use without an array column/parameter
     [ConditionalTheory]
     [MemberData(nameof(IsAsyncData))]
-    public virtual Task String_Join_with_array_parameter(bool async)
+    public virtual Task String_Join_with_array_of_int_column(bool async)
         => AssertQuery(
             async,
             ss => ss.Set<ArrayEntity>()
                 .Where(e => string.Join(", ", e.IntArray) == "3, 4"));
 
+    [ConditionalTheory]
+    [MemberData(nameof(IsAsyncData))]
+    public abstract Task String_Join_with_array_of_string_column(bool async);
+
+    [ConditionalTheory]
+    [MemberData(nameof(IsAsyncData))]
+    public abstract Task String_Join_disallow_non_array_type_mapped_parameter(bool async);
+
     #endregion Other translations
 
     #region Support

diff --git a/test/EFCore.PG.FunctionalTests/TestModels/Array/ArrayEntity.cs b/test/EFCore.PG.FunctionalTests/TestModels/Array/ArrayEntity.cs
@@ -23,8 +23,10 @@ public class ArrayEntity
     public SomeEnum EnumConvertedToString { get; set; }
     public SomeEnum? NullableEnumConvertedToString { get; set; }
     public SomeEnum? NullableEnumConvertedToStringWithNonNullableLambda { get; set; }
-    public SomeEnum[] ValueConvertedArray { get; set; } = null!;
-    public List<SomeEnum> ValueConvertedList { get; set; } = null!;
+    public SomeEnum[] ValueConvertedArrayOfEnum { get; set; } = null!;
+    public List<SomeEnum> ValueConvertedListOfEnum { get; set; } = null!;
+    public string[] ArrayOfStringConvertedToDelimitedString { get; set; } = null!;
+    public List<string> ListOfStringConvertedToDelimitedString { get; set; } = null!;
     public IList<int> IList { get; set; } = null!;
     public byte Byte { get; set; }
 }
diff --git a/test/EFCore.PG.FunctionalTests/TestModels/Array/ArrayQueryContext.cs b/test/EFCore.PG.FunctionalTests/TestModels/Array/ArrayQueryContext.cs
@@ -27,10 +27,28 @@ protected override void OnModelCreating(ModelBuilder modelBuilder)
                 e.Property(ae => ae.NullableEnumConvertedToStringWithNonNullableLambda)
                     .HasConversion(new ValueConverter<SomeEnum, string>(w => w.ToString(), v => Enum.Parse<SomeEnum>(v)));
 
-                e.PrimitiveCollection(ae => ae.ValueConvertedArray)
+                e.Property(ae => ae.ListOfStringConvertedToDelimitedString)
+                    .HasConversion(
+                        v => string.Join(",", v),
+                        v => v.Split(',', StringSplitOptions.None).ToList(),
+                        new ValueComparer<List<string>>(
+                            (c1, c2) => c1.SequenceEqual(c2),
+                            c => c.Aggregate(0, (a, v) => HashCode.Combine(a, v.GetHashCode())),
+                            c => c.ToList()));
+
+                e.Property(ae => ae.ArrayOfStringConvertedToDelimitedString)
+                    .HasConversion(
+                        v => string.Join(",", v),
+                        v => v.Split(',', StringSplitOptions.None).ToArray(),
+                        new ValueComparer<string[]>(
+                            (c1, c2) => c1.SequenceEqual(c2),
+                            c => c.Aggregate(0, (a, v) => HashCode.Combine(a, v.GetHashCode())),
+                            c => c.ToArray()));
+
+                e.PrimitiveCollection(ae => ae.ValueConvertedArrayOfEnum)
                     .ElementType(eb => eb.HasConversion(typeof(EnumToStringConverter<SomeEnum>)));
 
-                e.PrimitiveCollection(ae => ae.ValueConvertedList)
+                e.PrimitiveCollection(ae => ae.ValueConvertedListOfEnum)
                     .ElementType(eb => eb.HasConversion(typeof(EnumToStringConverter<SomeEnum>)));
 
                 e.HasIndex(ae => ae.NonNullableText);

diff --git a/test/EFCore.PG.FunctionalTests/TestModels/Array/ArrayQueryData.cs b/test/EFCore.PG.FunctionalTests/TestModels/Array/ArrayQueryData.cs
@@ -45,8 +45,10 @@ public static IReadOnlyList<ArrayEntity> CreateArrayEntities()
                 EnumConvertedToString = SomeEnum.One,
                 NullableEnumConvertedToString = SomeEnum.One,
                 NullableEnumConvertedToStringWithNonNullableLambda = SomeEnum.One,
-                ValueConvertedArray = [SomeEnum.Eight, SomeEnum.Nine],
-                ValueConvertedList = [SomeEnum.Eight, SomeEnum.Nine],
+                ValueConvertedArrayOfEnum = [SomeEnum.Eight, SomeEnum.Nine],
+                ValueConvertedListOfEnum = [SomeEnum.Eight, SomeEnum.Nine],
+                ArrayOfStringConvertedToDelimitedString = ["3", "4"],
+                ListOfStringConvertedToDelimitedString = ["3", "4"],
                 IList = new[] { 8, 9 },
                 Byte = 10
             },
@@ -71,8 +73,10 @@ public static IReadOnlyList<ArrayEntity> CreateArrayEntities()
                 EnumConvertedToString = SomeEnum.Two,
                 NullableEnumConvertedToString = SomeEnum.Two,
                 NullableEnumConvertedToStringWithNonNullableLambda = SomeEnum.Two,
-                ValueConvertedArray = [SomeEnum.Nine, SomeEnum.Ten],
-                ValueConvertedList = [SomeEnum.Nine, SomeEnum.Ten],
+                ValueConvertedArrayOfEnum = [SomeEnum.Nine, SomeEnum.Ten],
+                ValueConvertedListOfEnum = [SomeEnum.Nine, SomeEnum.Ten],
+                ArrayOfStringConvertedToDelimitedString = ["5", "6", "7", "8"],
+                ListOfStringConvertedToDelimitedString = ["5", "6", "7", "8"],
                 IList = new[] { 9, 10 },
                 Byte = 20
             }