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

NH-3851 - using Skip(aNumber).Count() where aNumber is greater than total count of records results in ArgumentOutOfRangeException #1342

Open
nhibernate-bot opened this issue Oct 13, 2017 · 9 comments

Comments

@nhibernate-bot
Copy link
Collaborator

nhibernate-bot commented Oct 13, 2017

John Thornborrow created an issue — 26th February 2016, 21:59:14:

Happens with any mapped entity, where aNumber is greater than the number of results:

Session.Query<Entity>().Skip(aNumber).Count()

Or an example case for us in pagination:

var recordsOnThisPage = Session.Query<Entity>().Skip((pageNumber - 1) * recordsPerPage).Take(recordsPerPage).Count()

Will result in:

System.ArgumentOutOfRangeException : Index was out of range. Must be non-negative and less than the size of the collection.
Parameter name: index

   at System.ThrowHelper.ThrowArgumentOutOfRangeException(ExceptionArgument argument, ExceptionResource resource)
   at System.Collections.Generic.List`1.System.Collections.IList.get_Item(Int32 index)
   at NHibernate.Linq.DefaultQueryProvider.ExecuteQuery(NhLinqExpression nhLinqExpression, IQuery query, NhLinqExpression nhQuery)
   at NHibernate.Linq.DefaultQueryProvider.Execute(Expression expression)
   at NHibernate.Linq.DefaultQueryProvider.Execute<TResult>(Expression expression)
   at System.Linq.Queryable.Count<TSource>(IQueryable`1 source)

Ricardo Peres added a comment — 27th February 2016, 11:01:10:

Are you sure on this? I cannot reproduce it!
Please submit a failing unit test.


John Thornborrow added a comment — 24th March 2016, 22:25:50:

https://github.com/Jestar342/NHibernateIssues/blob/master/NHibernateIssues/SkipCount.cs

@hazzik
Copy link
Member

hazzik commented Jun 14, 2018

This is SQLite and SQL Server 2012 (and some others?) behaviour:

select count(*) from A limit 100 offset 100;

Would be executed like:

select * from (select count(*) from A) limit 100 offset 100;

instead of expected:

select count(*) from (select * from A limit 100 offset 100);

@hazzik
Copy link
Member

hazzik commented Jun 14, 2018

It does not throw the exception anymore, but returns invalid results (always 0), which is even worse.

@fredericDelaporte
Copy link
Member

fredericDelaporte commented Jun 14, 2018

I do not think that the mentioned "expected" query is really what should be expected from the mentioned starting SQL. That is a matter of knowing what is the priority of the limit and offset operators (or fetch/offset for SQL Server, ...). I expect them to have lowest priority, so to be executed on the select count(*) from A result-set rather than applied on A table directly.

By example the PostgreSQL documentation states:

LIMIT and OFFSET allow you to retrieve just a portion of the rows that are generated by the rest of the query

Which I can only interpret as them being applied last.

Same for SQLite:

The LIMIT clause is used to place an upper bound on the number of rows returned by the entire SELECT statement.

(SQL-Server documentation is quite lame in its current state on docs: offset/fetch are barely documented as a side note of order by. There is no usable information there for our trouble here.)

The trouble lies in the Linq translation in my opinion. It should not swap the Skip/Take out, applying the count directly on the entity table instead of applying it to the paginated query. In other words the Linq translation should generate a sub-query explicitly here, counting from the paginated query.

@hazzik
Copy link
Member

hazzik commented Jun 14, 2018

Subqueries in the from clause are not supported in HQL, unfortunately. And this is the HUGE change in the parser.

@fredericDelaporte
Copy link
Member

Then it should either throw a "not supported exception" or swap to in-memory evaluation of the aggregation done on the paginated results.

@fredericDelaporte
Copy link
Member

Reducing the priority to minor: the opener use case is not efficient and common. Counting the number of results actually present on a page is better done in memory in most cases, since usually the page data is loaded or about to be loaded. Or else the global number of results can be counted (so without paging), and the actual number of elements expected for a given page can then be computed in memory.

@mcessna
Copy link

mcessna commented Sep 6, 2019

We encountered the same Exception using Any():

The query:
return session.Query()
.Any(integration => integration.Id == id && (bool) integration.Enabled);
The SQL:
SELECT TOP 1 >snip< from INTEGRATION integratio0_ where integratio0_.INTEGRATIONID='ICRMION00001' and integratio0_.ENABLED='T'

The stack:
at System.ThrowHelper.ThrowArgumentOutOfRangeException(ExceptionArgument argument, ExceptionResource resource)
at System.Collections.Generic.List1.System.Collections.IList.get_Item(Int32 index) at NHibernate.Linq.DefaultQueryProvider.ExecuteQuery(NhLinqExpression nhLinqExpression, IQuery query, NhLinqExpression nhQuery) at NHibernate.Linq.DefaultQueryProvider.Execute(Expression expression) at NHibernate.Linq.DefaultQueryProvider.Execute[TResult](Expression expression) at System.Linq.Queryable.Any[TSource](IQueryable1 source, Expression`1 predicate)

The protected virtual object ExecuteQuery(NhLinqExpression nhLinqExpression, IQuery query, NhLinqExpression nhQuery) method in DefaultQueryProvider.cs assumes the call to query.List() will return one or more results; if it returns nothing then the call to results[0] throws the ArgumentOutOfRangeException. I rewrote the query to use RowCount() to resolve the issue.

@backstromjoel
Copy link

backstromjoel commented Nov 22, 2022

Can confirm it just returns the wrong amount, as described in #1081.

Any ideas for workarounds or how to solve this?

@bahusoid
Copy link
Member

fredericDelaporte wrote

In other words the Linq translation should generate a sub-query explicitly here, counting from the paginated query.

hazzik wrote

Subqueries in the from clause are not supported in HQL, unfortunately. And this is the HUGE change in the parser.

So at least now it's fixable as subquery support is already implemented in #2551

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants