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.

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

Latest builds incompatible with some PostgreSQL array scenarios #17374

Closed
roji opened this issue Aug 22, 2019 · 20 comments · Fixed by #18666
Closed

Latest builds incompatible with some PostgreSQL array scenarios #17374

roji opened this issue Aug 22, 2019 · 20 comments · Fixed by #18666
Assignees
Labels
closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. type-bug
Milestone

Comments

@roji
Copy link
Member

roji commented Aug 22, 2019

When syncing EFCore.PG to latest preview9, some array scenarios are now breaking.

Exception:

Processing of the LINQ expression 'AsQueryable<int>(NavigationTreeExpression
    Value: EntityReferenceSomeArrayEntity
    Expression: (Unhandled parameter: s).SomeArray)' by 'NavigationExpandingExpressionVisitor' failed. This may indicate either a bug or a limitation in EF Core. See https://go.microsoft.com/fwlink/?linkid=2101433 for more detailed information.
   at Microsoft.EntityFrameworkCore.Query.Internal.NavigationExpandingExpressionVisitor.VisitMethodCall(MethodCallExpression methodCallExpression) in /home/roji/projects/EFCore/src/EFCore/Query/Internal/NavigationExpandingExpressionVisitor.cs:line 573

Query:

var count = ctx.SomeEntities.Count(e => e.SomeArray.Any());

Expression tree before NavigationExpandingExpressionVisitor:

value(Microsoft.EntityFrameworkCore.Query.Internal.EntityQueryable`1[Npgsql.EntityFrameworkCore.PostgreSQL.Query.ArrayQueryTest+SomeArrayEntity]).Count(e => e.SomeArray.AsQueryable().Any())

Basically NavigationExpandingExpressionVisitor freaks out when it sees a queryable method (AsQueryable) on an array.

/cc @smitpatel

@roji roji changed the title Latest builds Latest builds incompatible with some PostgreSQL array scenarios Aug 22, 2019
@smitpatel
Copy link
Contributor

How is the query getting translated to server eventually? (if this error did not happen)

@roji
Copy link
Member Author

roji commented Aug 22, 2019

I've got some pattern matching logic in a simple method translator which identifies it and produces the following:

SELECT COUNT(*)::INT FROM ""SomeEntities"" AS s WHERE array_length(s.""SomeArray"", 1) > 1

Basically I need the the expression to make it all the way to translation...

@ajcvickers
Copy link
Contributor

@roji How breaking is this for Postgres? (i.e. We need to decide whether we're going to try to take this in ask mode, or whether we fix it in 3.1.)

@roji
Copy link
Member Author

roji commented Aug 22, 2019

Well, it prevents translation of several array operations, including inside JSON documents (the new feature). Specifically, this prevents translating Any and Contains.

It would definitely be a shame to not have this - but I guess it's not completely critical if this goes into 3.1 instead?

@SonicGD
Copy link

SonicGD commented Sep 5, 2019

In our projects we are using pg arrays and jsonb fields extensively. For us this is a breaking change that would prevent us from upgrading to preview 9 and 3.0 release. For now, on preview 8 all working fine.

SonicGD added a commit to BioWareRu/BioEngine.BRC.Common that referenced this issue Sep 5, 2019
SonicGD added a commit to BioWareRu/BioEngine.Core that referenced this issue Sep 5, 2019
@roji
Copy link
Member Author

roji commented Sep 5, 2019

@SonicGD just making sure - as far as I know this is only blocking the translation of Contain on arrays (as Any can be rewritten with a length check). This is the functionality blocking you from upgrading?

@SonicGD
Copy link

SonicGD commented Sep 5, 2019

Yes. We have many queries like

query.Where(e => e.SiteIds.Contains(site.Id));

and they are failing with

Processing of the LINQ expression 'AsQueryable<Guid>(NavigationTreeExpression
    Value: EntityReferencePost
    Expression: (Unhandled parameter: p).SiteIds)' by 'NavigationExpandingExpressionVisitor' failed. This may indicate either a bug or a limitation in EF Core. See https://go.microsoft.com/fwlink/?linkid=2101433 for more detailed information.

@roji
Copy link
Member Author

roji commented Sep 5, 2019

Thanks for confirming. Note that 3.1 will be coming out quite quickly after 3.0, and this is definitely planned to be fixed for that release - so even at worse case you're not going to be waiting for long.

@thepra
Copy link

thepra commented Sep 24, 2019

Having similar regression here too after updating to 3.0 and preview9 of PostgreSQL in array join such as:

var userPictures = await _userService.AppUserPicturesStatic.Join(allUsersIds, up => up.AppUserId, userId => userId, (up, userId) => up).ToListAsync();//allUsersIds = string[]

and the error from this is:

[23:18:01 ERR] Error from source=[Admin] with message=[Processing of the LINQ expression 'Join<AppUserPicture, string, string, AppUserPicture>( outer: DbSet<AppUserPicture>, inner: (Unhandled parameter: __p_0), outerKeySelector: (up) => up.AppUserId, innerKeySelector: (userId) => userId, resultSelector: (up, userId) => up)' by 'NavigationExpandingExpressionVisitor' failed. This may indicate either a bug or a limitation in EF Core. See https://go.microsoft.com/fwlink/?linkid=2101433 for more detailed information.]

@smitpatel
Copy link
Contributor

@thepra - join with client side array is not supported since that join has to be client eval. It has no correlation with PostgreSQL arrays.

@thepra
Copy link

thepra commented Sep 24, 2019

@smitpatel what's the alternative then in this brave new world?

@smitpatel
Copy link
Contributor

@thepra - That query was doing client evaluation before. And now we don't do that implicitly. Query should be rewritten for explicit client eval.

var userPictures = (await _userService.AppUserPicturesStatic.ToListAsync()).Join(allUsersIds, up => up.AppUserId, userId => userId, (up, userId) => up).ToList();

@thepra
Copy link

thepra commented Sep 25, 2019

@smitpatel mmmmm, quite inefficient then pulling all the records and then joining it, but I guess this is how it was before implicitly.
If you can help me with this, what efficient way would be to write this query and make it do the join database side?

@roji
Copy link
Member Author

roji commented Sep 25, 2019

@thepra yes, the important point is that this is the same behavior as in 2.x, but the inefficiency was being hidden by client evaluation - it is now much easier to understand what's going on.

If I'm reading your code correctly, allUsersIds is an array parameter. You can't join a table with an array, but it seems like the main point of the query is to filter out rows whose ID isn't in the array. If that's the case, the query can simply be rewritten as follows:

_userService.AppUserPicturesStatic.Where(a => allUsersIds.Contains(a.AppUserId)).ToList()

Unfortunately Contains on an array currently doesn't work (that's what this issue is about), but this is planned to be fixed for 3.1.0.

Another more extreme option is to use the unnest PostgreSQL function to expand the array to a table-like set of rows, and join on that:

CREATE TABLE numbers_table (num INT, name TEXT);
INSERT INTO numbers_table (num, name) VALUES (1, 'name1'), (2, 'name2'), (3, 'name3');
SELECT num, name FROM numbers_table JOIN (SELECT unnest(ARRAY[1, 3]) AS arr_num) AS arr_table ON arr_num = num;

EF Core will not generate this kind of advanced, PostgreSQL-specific SQL, and most probably never will. However, you can use raw SQL to write it yourself and have EF materialize the results back to entity instances.

@thepra
Copy link

thepra commented Sep 25, 2019

Unfortunately Contains on an array currently doesn't work (that's what this issue is about), but this is planned to be fixed for 3.1.0.

@roji There's no issue on that, I can alter that array to a list. Still, moving to 3.0 have broken many of such similar queries.
Fortunately filtering out the solution for ".Join(" can easily point me to the parts that I need to rework 😌.
So right now the correct way left to use the .Join() method is to use the virtual properties in the model to generate the query with those entities in mind, am I right?

@roji
Copy link
Member Author

roji commented Sep 25, 2019

Still, moving to 3.0 have broken many of such similar queries.

This is true and we're aware of the one-time porting pain that disabling client evaluation may cause. We do believe it's the better way for EF Core to function, as it makes perf issues much more visible and improves compatibility across versions.

So right now the correct way left to use the .Join() method is to use the virtual properties in the model to generate the query with those entities in mind, am I right?

You should be able to use Join, but you must use it on entity types rather than on array parameters as you tried above.

@roji
Copy link
Member Author

roji commented Sep 25, 2019

@thepra can you please open a new issue with a minimal code sample (and schema) that shows the problem?

@thepra
Copy link

thepra commented Sep 25, 2019

@roji I deleted that answer because I found a solution, I just needed to apply the fluent configuration between the entities to make it work properly. This thanks to another open issue.

@roji
Copy link
Member Author

roji commented Sep 25, 2019

Great, thanks for confirming.

smitpatel added a commit that referenced this issue Oct 30, 2019
Our EnumerableToQueryable converter works on all the methods. But during nav expansion, we actually inject query sources.
If item is not a query source (like postgre arrays or collection property mapped as scalar via value conversion), then we convert it to enumerable again to be handled by provider.

Resolves #17374
@smitpatel smitpatel added the closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. label Oct 30, 2019
smitpatel added a commit that referenced this issue Oct 30, 2019
Our EnumerableToQueryable converter works on all the methods. But during nav expansion, we actually inject query sources.
If item is not a query source (like postgre arrays or collection property mapped as scalar via value conversion), then we convert it to enumerable again to be handled by provider.

Resolves #17374
smitpatel added a commit that referenced this issue Oct 30, 2019
Our EnumerableToQueryable converter works on all the methods. But during nav expansion, we actually inject query sources.
If item is not a query source (like postgre arrays or collection property mapped as scalar via value conversion), then we convert it to enumerable again to be handled by provider.

Resolves #17374
smitpatel added a commit that referenced this issue Oct 30, 2019
Our EnumerableToQueryable converter works on all the methods. But during nav expansion, we actually inject query sources.
If item is not a query source (like postgre arrays or collection property mapped as scalar via value conversion), then we convert it to enumerable again to be handled by provider.

Resolves #17374
@wuchang
Copy link

wuchang commented Nov 21, 2019

好大一个坑,好惨

@ajcvickers ajcvickers modified the milestones: 3.1.0-preview3, 3.1.0 Dec 2, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. type-bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants