-
Notifications
You must be signed in to change notification settings - Fork 228
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
Translation of string.Join overload used with List<string> parameter missing #3105
Comments
I'm not that experienced with EF Core's inner workings, but looking at efcore.pg/src/EFCore.PG/Query/ExpressionTranslators/Internal/NpgsqlStringMethodTranslator.cs Lines 91 to 115 in 2382b8b
|
private static readonly MethodInfo String_Join2 = | |
typeof(string).GetMethod(nameof(string.Join), [typeof(string), typeof(string[])])!; |
List<int>
ArrayRatingsJoined = string.Join(", ", b.RatingsArray)
uses this overload: public static string Join<T> (string? separator, System.Collections.Generic.IEnumerable<T> values);
which is handled by String_Join_generic1
efcore.pg/src/EFCore.PG/Query/ExpressionTranslators/Internal/NpgsqlStringMethodTranslator.cs
Lines 103 to 108 in 2382b8b
private static readonly MethodInfo String_Join_generic1 = | |
typeof(string).GetTypeInfo().GetMethods(BindingFlags.Public | BindingFlags.Static | BindingFlags.DeclaredOnly) | |
.Single( | |
m => m is { Name: nameof(string.Join), IsGenericMethod: true } | |
&& m.GetParameters().Length == 2 | |
&& m.GetParameters()[0].ParameterType == typeof(string)); |
List<string>
ListTagsJoined = string.Join(", ", b.TagsList)
uses this overload: public static string Join (string? separator, System.Collections.Generic.IEnumerable<string?> values);
This overload does not seem to be covered in NpgsqlStringMethodTranslator
.
Thanks for the good PR @georg-jung! |
…l#3105) This string.Join overload is required for translations of calls with a List<string> parameter, but the overload allows more general IEnumerable<string> parameters too which can not be translated.
After they changed due to ee8925d
…sql#3105) string.Join translations are just valid if their parameter's TypeMapping is NpgsqlArrayTypeMapping.
After they changed due to 909f589
After they changed due to 909f589
For array type mapping, https://www.npgsql.org/efcore/mapping/array.html reads:
In my experience, this works great with
int[]
s orList<int>
. I recently came accross a strange inconsistency though, ifT
isstring
specificly (edit: if used withstring.Join
).The array type mapping seems to work fine forI noticed this when trying to translate a query that containsstring[]
but seems to be broken forList<string>
.string.Join
with aList<string>
. I expected this to be translated asarray_to_string
, which isn't what happens.This example hopefully clarifies what I experienced:
On my machine, this prints (before it throws and some of the
Console.WriteLines
aren't executed anymore):Details of the exception thrown:
When executing the same code while depeding on
7.0.11
instead, the behaviour is equivalent, so this doesn't seem to be a recent regression. (qWorksAsExpected
is translated slightly different, but I don't think that has anything to do with this issue.) Output with7.0.11
:Please let me know if there's anything I should further clarify, answer or test.
Not sure if this is related to #3074 or #3092.Probably not, given #3105 (comment).Same as above but cloneable: https://github.com/georg-jung/NpgsqlEfCoreArrayTranslationRepro
The text was updated successfully, but these errors were encountered: