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.

Sign up for GitHub

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

Jump to bottom

Add translation of string.Join overload used with List<string> parameter, fix #3105 #3106

Merged
merged 6 commits into from
Feb 24, 2024
Merged

Add translation of string.Join overload used with List<string> parameter, fix #3105 #3106

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
2818b72
Add tests that demonstrate a string.Join overload is missing (#3105)
georg-jung Feb 20, 2024
ccfdc1f
Add missing overload of string.Join with List<string> arg (#3105)
georg-jung Feb 20, 2024
c63416e
Fix mis-named string.Join translation test names (#3105)
georg-jung Feb 24, 2024
bc15b0e
Ensure string.Join is not translated with non array-mapped param (#3105)
georg-jung Feb 24, 2024
b27e530
Add test String_Join_disallow_non_array_type_mapped_parameter (#3105)
georg-jung Feb 24, 2024
a700ef9
Rewrite baselines (#3105)
georg-jung Feb 24, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions src/EFCore.PG/Query/ExpressionTranslators/Internal/NpgsqlStringMethodTranslator.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,9 @@ private static readonly MethodInfo LastOrDefaultMethodInfoWithoutArgs
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(
Expand Down Expand Up @@ -334,6 +337,7 @@ 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)))
{
Expand Down
23 changes: 21 additions & 2 deletions test/EFCore.PG.FunctionalTests/Query/ArrayArrayQueryTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -854,9 +854,9 @@ WHERE COALESCE(array_position(s."IntArray", 6, 2) - 1, -1) = 1
}

// Note: see NorthwindFunctionsQueryNpgsqlTest.String_Join_non_aggregate for regular use without an array column/parameter
public override async Task String_Join_with_array_parameter(bool async)
public override async Task String_Join_with_array_of_int_parameter(bool async)
{
await base.String_Join_with_array_parameter(async);
await base.String_Join_with_array_of_int_parameter(async);

AssertSql(
"""
Expand All @@ -866,6 +866,25 @@ WHERE array_to_string(s."IntArray", ', ', '') = '3, 4'
""");
}

[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public async Task String_Join_with_array_of_string_parameter(bool async)
{
// This is not in ArrayQueryTest because string.Join uses another overload for string[] than for List<string> and thus
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

// ArrayToListReplacingExpressionVisitor won't work.
await AssertQuery(
async,
ss => ss.Set<ArrayEntity>()
.Where(e => string.Join(", ", e.StringArray) == "3, 4"));

AssertSql(
"""
SELECT s."Id", s."ArrayContainerEntityId", s."Byte", s."ByteArray", s."Bytea", s."EnumConvertedToInt", s."EnumConvertedToString", s."IList", s."IntArray", s."IntList", s."NonNullableText", s."NullableEnumConvertedToString", s."NullableEnumConvertedToStringWithNonNullableLambda", s."NullableIntArray", s."NullableIntList", s."NullableStringArray", s."NullableStringList", s."NullableText", s."StringArray", s."StringList", s."ValueConvertedArray", s."ValueConvertedList", s."Varchar10", s."Varchar15"
FROM "SomeEntities" AS s
WHERE array_to_string(s."StringArray", ', ', '') = '3, 4'
""");
}

#endregion Other translations

public class ArrayArrayQueryFixture : ArrayQueryFixture
Expand Down
23 changes: 21 additions & 2 deletions test/EFCore.PG.FunctionalTests/Query/ArrayListQueryTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -876,9 +876,9 @@ WHERE COALESCE(array_position(s."IntList", 6, 2) - 1, -1) = 1
}

// Note: see NorthwindFunctionsQueryNpgsqlTest.String_Join_non_aggregate for regular use without an array column/parameter
public override async Task String_Join_with_array_parameter(bool async)
public override async Task String_Join_with_array_of_int_parameter(bool async)
{
await base.String_Join_with_array_parameter(async);
await base.String_Join_with_array_of_int_parameter(async);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This actually seems mis-named (before your PR) - the array of int is a column, not a parameter. Can you please rename to String_Join_with_array_of_int_column (also the string variant)? Also,

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if something after your

Also,

was cut off?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, nothing important.


AssertSql(
"""
Expand All @@ -888,6 +888,25 @@ WHERE array_to_string(s."IntList", ', ', '') = '3, 4'
""");
}

[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public async Task String_Join_with_array_of_string_parameter(bool async)
{
// This is not in ArrayQueryTest because string.Join uses another overload for string[] than for List<string> and thus
// ArrayToListReplacingExpressionVisitor won't work.
await AssertQuery(
async,
ss => ss.Set<ArrayEntity>()
.Where(e => string.Join(", ", e.StringList) == "3, 4"));

AssertSql(
"""
SELECT s."Id", s."ArrayContainerEntityId", s."Byte", s."ByteArray", s."Bytea", s."EnumConvertedToInt", s."EnumConvertedToString", s."IList", s."IntArray", s."IntList", s."NonNullableText", s."NullableEnumConvertedToString", s."NullableEnumConvertedToStringWithNonNullableLambda", s."NullableIntArray", s."NullableIntList", s."NullableStringArray", s."NullableStringList", s."NullableText", s."StringArray", s."StringList", s."ValueConvertedArray", s."ValueConvertedList", s."Varchar10", s."Varchar15"
FROM "SomeEntities" AS s
WHERE array_to_string(s."StringList", ', ', '') = '3, 4'
""");
}

#endregion Other translations

public class ArrayListQueryFixture : ArrayQueryFixture
Expand Down
2 changes: 1 addition & 1 deletion test/EFCore.PG.FunctionalTests/Query/ArrayQueryTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -503,7 +503,7 @@ 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_parameter(bool async)
=> AssertQuery(
async,
ss => ss.Set<ArrayEntity>()
Expand Down
Loading