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

ArgumentOutOfRangeException upon certain queries with includes #19616

Closed
joshcomley opened this issue Jan 17, 2020 · 24 comments · Fixed by #20207 or #20340
Closed

ArgumentOutOfRangeException upon certain queries with includes #19616

joshcomley opened this issue Jan 17, 2020 · 24 comments · Fixed by #20207 or #20340
Assignees
Labels
closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. customer-reported Servicing-approved type-bug
Milestone

Comments

@joshcomley
Copy link

Certain queries with includes are causing an ArgumentOutOfRangeException.

When using OData, almost (or possibly) all queries with more than one expand of a collection and translating to SQL fails with the following error:

screencapture-localhost-15580-odata-Movies-2020-01-17-05_07_47

Steps to reproduce

A very quick repro, download this zip, extract it in the src folder, open All.sln, add the zipped project as an existing project, make sure the project references are set correctly and run. It will create a local SQL database on your machine. And it will fail to run the following query:

http://localhost:15580/odata/Movies?$expand=Stars,Writers

If you remove either one of the expands, it works. I wasn't sure whether to post to OData or here, but I think the issue lies here.

Further technical details

EF Core version: 3.1.1
Database provider: Microsoft.EntityFrameworkCore.SqlServer
Target framework: .NET Core 3.0
Operating system: Windows 10
IDE: Visual Studio 2019 16.5 Preview 1

@joshcomley
Copy link
Author

joshcomley commented Jan 17, 2020

I've managed to significantly simplify the reproduction of this error:

var movies = _context.Movies.Select(movie => new 
    {
        Value1 = movie.Stars.Select(star => new
        {
            Anything = true
        }),
        Value2 = movie.Stars.Select(writer => new
        {
            AnythingElse = true
        })
    })
    .ToList();

This code will throw it, you just need to substitute Movies and Movies.Stars for whatever example data structure you'd like, where Stars is a many side of a one-to-many relationship.

If you remove either one of Anything or AnythingElse, the query works.

Edit: removed unnecessary extra entity type

@joshcomley
Copy link
Author

Useful additional info (the ones that fail have the same compile-time value for the projected property):

Fails

var movies = _context.Movies.Select(movie => new 
    {
        Value1 = movie.Stars.Select(star => new
        {
            Anything = 1
        }),
        Value2 = movie.Stars.Select(writer => new
        {
            AnythingElse = 1
        })
    })
    .ToList();


var movies = _context.Movies.Select(movie => new 
    {
        Value1 = movie.Stars.Select(star => new
        {
            Anything = false
        }),
        Value2 = movie.Stars.Select(writer => new
        {
            AnythingElse = false
        })
    })
    .ToList();

Succeeds

var movies = _context.Movies.Select(movie => new 
    {
        Value1 = movie.Stars.Select(star => new
        {
            Anything = 1
        }),
        Value2 = movie.Stars.Select(writer => new
        {
            AnythingElse = 2
        })
    })
    .ToList();


var movies = _context.Movies.Select(movie => new 
    {
        Value1 = movie.Stars.Select(star => new
        {
            Anything = true
        }),
        Value2 = movie.Stars.Select(writer => new
        {
            AnythingElse = false
        })
    })
    .ToList();

@joshcomley
Copy link
Author

I believe I have fixed this bug, I am just organising a pull-request

@joshcomley
Copy link
Author

Could someone please help guide me where would be best to write a test for this? I am looking in EFCore.Relational.Tests but am unable to find exactly where or how I should put this in. I know I'm supposed to submit a test with every PR, but it's going to take me half a day just to work out the test structure!

@ajcvickers
Copy link
Member

@maumar Can you point @joshcomley to a good place for a test?

@maumar
Copy link
Contributor

maumar commented Jan 17, 2020

ComplexNavigationsQueryTestBase is a good place, it has the model structure needed.

@joshcomley
Copy link
Author

Thanks @maumar - is there any way to actually debug the test? I want to see what the output SQL for the SQL test is, but it's all very abstracted. I managed to run tests via command line, but I get a lot of Cannot open database "Northwind" requested by the login errors.

@maumar
Copy link
Contributor

maumar commented Jan 17, 2020

The process I follow is run build.cmd on the efcore repo, then open Runtime.sln using startvs.cmd, then open Test Explorer, find the test you want to run, right click -> debug.

To see the generated sql, you can override the test method in ComplexNavigationsQuerySqlServerTest, call base and use AssertSql("") method afterwards. The generated sql is compared against the baseline and if they don't match you can see the actually generated sql in QueryBaseline.txt file in the root directory of efcore repo.

Do you have localdb installed in your environment? Tests run against it by default and should automatically create all the necessary databases.

@smitpatel smitpatel self-assigned this Feb 5, 2020
@cornem
Copy link

cornem commented Feb 14, 2020

Could this please be fixed and released ASAP? Right now this is blocking anyone that relies on OData from upgrading to .NET Core 3.1. We cannot upgrade our APIs, because client apps are doing multiple expands against our endpoints, which apparently no longer work because of this bug.

So we're essentially stuck on the - now deprecated - .NET Core 2.2 until this is fixed.

@ajcvickers
Copy link
Member

@maumar @smitpatel Are there any workarounds here?

@cornem There is a community contribution to fix this from @joshcomley. Now that @smitpatel is back we are working through that PR and hope to get it merged soon. We don't want to jump in and take over because that can be quite demoralizing to the contributor who has been working hard on a fix.

Once we have the fix we will consider if for a patch release as described in the release planning process.

@smitpatel
Copy link
Contributor

No work-around since query is generated by ODATA dynamically.

@jasonsheridan79
Copy link

I'm with @cornem , I'm completely blocked by this. OData calls that were fine in 2.2 are now seeing "No coercion operator is defined between types 'System.Int32' and 'System.Boolean'" and " Index was out of range. Must be non-negative and less than the size of the collection." errors.

@DFSko
Copy link

DFSko commented Feb 21, 2020

my solution has 10 projects, 9 of them upgraded to 3.1 and 1 on 2.2 version (because Odata blocking update). Checking this issue everyday

@Landsec-HadrianP
Copy link

This is also a blocker for our application upgrade to 3.1, desperately waiting for a fix.

@nikolai-mb
Copy link

I also have several projects blocked by this. @ajcvickers it seems like @joshcomley might not be available as he was last active on github january 8. Is it possible for @smitpatel to take over this issue in order to speed up the process of fixing this?

@ajcvickers
Copy link
Member

@nikolai-mb Yes, @smitpatel will take over here.

smitpatel added a commit that referenced this issue Mar 7, 2020
When joining 2 tables in relational, we lift the table from inner select expression and copy over projections. If one of this projection is same as any outer projection then we reuse the same projection. That means the associated indexes in inner shaper will go to different indexes rather than in linear fashion.
Fix is to remember which index each projection went and use that rather than applying an offset.

Resolves #19616
@ajcvickers ajcvickers added this to the 3.1.x milestone Mar 9, 2020
@ajcvickers ajcvickers modified the milestones: 3.1.x, 3.1.4 Mar 13, 2020
smitpatel added a commit that referenced this issue Mar 19, 2020
When joining 2 tables in relational, we lift the table from inner select expression and copy over projections. If one of this projection is same as any outer projection then we reuse the same projection. That means the associated indexes in inner shaper will go to different indexes rather than in linear fashion.
Fix is to remember which index each projection went and use that rather than applying an offset.

Resolves #19616
smitpatel added a commit that referenced this issue Mar 20, 2020
When joining 2 tables in relational, we lift the table from inner select expression and copy over projections. If one of this projection is same as any outer projection then we reuse the same projection. That means the associated indexes in inner shaper will go to different indexes rather than in linear fashion.
Fix is to remember which index each projection went and use that rather than applying an offset.

Resolves #19616
@smitpatel smitpatel reopened this Mar 20, 2020
@smitpatel
Copy link
Contributor

Re-opening to make sure that we merge this in 3.1 too.

smitpatel added a commit that referenced this issue Mar 30, 2020
When joining 2 tables in relational, we lift the table from inner select expression and copy over projections. If one of this projection is same as any outer projection then we reuse the same projection. That means the associated indexes in inner shaper will go to different indexes rather than in linear fashion.
Fix is to remember which index each projection went and use that rather than applying an offset.

Resolves #19616
@jannikbuschke
Copy link

This fix seems to not been released, despite being a blocker for many to upgrade to dotnet 3.1. The issue was reported on January. Now is mai. The fix seems to be merged already, but 3.1.4 is not released yet. Can this be prioritized please?

@ajcvickers
Copy link
Member

@jannikbuschke Patch releases scheduled are unified for all of .NET Core. 3.1.4 is being validated as part of this process and will be shipped with the other 3.1.x patches.

@jannikbuschke
Copy link

Sorry for ranting a bit here, but I hope in the future odata, aspnetcore and sql server ef core will be better streamlined and the basic usages such as described in this issue will not break.

And I hope for 3.1.4 to happen soonish, there is no planned date yet or?

@ajcvickers
Copy link
Member

@jannikbuschke I'm not at liberty to give an exact date, but it is planned for this month.

@gunter79
Copy link

gunter79 commented May 13, 2020

With the 3.1.4 release available, has anybody got it working ? I still get the same error when querying two lists using expands in my query.

@ajcvickers
Copy link
Member

@gunter79 We may have missed something, or you may be hitting a different bug. Please file a new issue and attach a small, runnable project or post a small, runnable code listing that reproduces what you are seeing so that we can investigate.

@gunter79
Copy link

👍
I must apologize, after upgrading all my nugget packages (Microsoft.EntityFrameworkCore and
Microsoft.EntityFrameworkCore.SqlServer), the 3.1.4 release solves it.
I'm an happy camper now. Thank you very much, you did an awesome job guys !

@ajcvickers ajcvickers added the closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. label May 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment