MySQL Bugs: #45723: Entity Framework DbSortExpression not processed when using Skip & Take

Bug #45723	Entity Framework DbSortExpression not processed when using Skip & Take
Submitted:	24 Jun 2009 17:58	Modified:	5 Aug 2009 13:23
Reporter:	Lynn Eriksen	Email Updates:
Status:	Closed	Impact on me:	None
Category:	Connector / NET	Severity:	S2 (Serious)
Version:	6.04	OS:	Windows (.net 3.5 sp1)
Assigned to:		CPU Architecture:	Any

Description:
I have encounterd a situation where the EF provider is not correctly processing the a DBSortExpression when Skip & Take is called. I have done some research into the source code and can validate that method 'Visit(DbSortExpression)' is not being called when a Skip value is set in the DBLimitExpression. I have validated this by setting break points and watching SQL construction take place using the various 'Visit' override methods on the SelectGenerator class.

How to repeat:
I have create an EntityContext based on the attached data.

1) If I execute the following statement: 
TestModel.tblquarantine.OrderByDescending(q => q.MsgDate).Take(100).ToList();

The following 'Visit' methods are called in this order on SelectGenerator:
Visit(DbLimitExpression)
Visit(DbSortExpression)

This yields correctly sorted data.

2) If I execute the following statement:
TestModel.tblquarantine.OrderByDescending(q => q.MsgDate).Skip(100).Take(100).ToList();

Visit(DbLimitExpression)
Visit(DbSkipExpression)

The 'Visit(DbSortExpression)' is never called and the data is not sorted.

What I cannot determine if the DBSortExpression is if there is a DbSortExpression in the expression tree be evaluated.

Test data to demonstrate error

Attachment: testing_data.zip (application/x-zip-compressed, text), 426.63 KiB.

I believe I have solved the issue. 

When Skip() and Take() are used in conjunction the sorting information is located in the DbSkipExpression object instead of a seperate DbSortExpression object. Specifically, sorting information is stored as collection DbSortClause in the  DbSkipExpression 'SortOrder' property.

In the 6.04 provider the DbSortClause objects inside the DbSkipExpression 'SortOrder' property are not being evaluated. This can be fixed by adding a for loop to evaluate DbSortClause in the 'Visit(DbSkipExpression)' method of SelectGenerator.cs.

-------------------------------------------------------

>>> Old Code

SelectStatement select = VisitInputExpressionEnsureSelect(expression.Input.Expression, expression.Input.VariableName,
                expression.Input.VariableType);

>>> New Code (suggested fix)

SelectStatement select = VisitInputExpressionEnsureSelect(expression.Input.Expression, expression.Input.VariableName,
                expression.Input.VariableType);

foreach (DbSortClause sortClause in expression.SortOrder)
{
    select.AddOrderBy(new SortFragment(
                    sortClause.Expression.Accept(this), sortClause.Ascending));
}

-------------------------------------------------------

This fix is producing correct sql after several tests,working correctly thus far after several tests.

Tonci, I can supply you with a sample project if necessary.

Lynn, please do. Thanks.

Lynn, what about that project?

Tonci, have been very busy.

I will try to submit by weeks end.

Added solution using FTP.

See: bug_solution_45723.zip

Lynn, are we talking of 
public override SqlFragment Visit(DbSortExpression expression)? Or there's some other routine you're talking about? Each SqlFragment Visit(DbSortExpression expression) in trunk (newest code) has:
            foreach (DbSortClause sortClause in expression.SortOrder)
            {
                select.AddOrderBy(new SortFragment(
                    sortClause.Expression.Accept(this), sortClause.Ascending));
            }
            return select;
part. Or is there an error in internal logic when both skip&take are used?

Btw, can you attach smaller test case just for showing this error?

Hello Tonci,

The method in question is 'SqlFragment Visit(DbSkipExpression expression)'.

This method is called is using Skip & Take together in the LINQ statement. If not using Skip then method 'Visit(DbSortExpression expression)' is called instead. Both methods must provide sort information for SQl generation.

The fix for method 'SqlFragment Visit(DbSkipExpression expression)' is to add the same ForEach loop that appends the the SortClauses to the select statement. Here is that loop:

foreach (DbSortClause sortClause in expression.SortOrder)
            {
                select.AddOrderBy(new SortFragment(
                    sortClause.Expression.Accept(this), sortClause.Ascending));
            }

I'll try to submit a test solution that illustrates the problem rather than the fix, so it should be smaller. Right now I am very busy so it make take me a day or two.

Lynn, then I got it right. There is exact code you posted in latest sources. Please check trunk if you can.

Tonci, I have not done this before. I you point me to a page that points me in the right direction?

Lynn, sources are available on launchpad. Please check http://bazaar.launchpad.net/~mysql-clr-team/connectornet/trunk/changes for changes in files you are referring to. If you have bzr installed then do "lp:connectornet/x.y" where x.y is 5.1, 5.2, 6.0 and 6.1 and, latest sources, lp:connectornet/trunk.

Hello Tonci,

No. The patch is not there - but lauchpad repositiory will allow me to point you the right way.

Here is the link:

http://bazaar.launchpad.net/~mysql-clr-team/connectornet/trunk/annotate/head%3A/MySql.Data...

On line 253:  public override SqlFragment Visit(DbSkipExpression expression)

This method needs to have the same ForEach loop over the SortOrder collection  in the method as this method is responsible for adding sorting to the sql syntax when skip is used.

Just above 'return select;' (line 260) add the following code:

foreach (DbSortClause sortClause in expression.SortOrder)
{
 
   select.AddOrderBy(new SortFragment(
   sortClause.Expression.Accept(this), sortClause.Ascending));
 
}

This is the same code from 'public override SqlFragment Visit(DbSortExpression expression)' and should fix things. I will try to send a revised solution that shows the problem today or monday.

Tonci,

I have uploaded a project name 'Bug 45723 - Take 2.zip' to the ftp. This contains a solution with a project and output for a test database. All you need to do is setup the dabase and the configuration. 

If you run Default.aspx in the web project you'll see a grid with paging and sorting hooked of to an EntityDataSource. It will not sort becasue it is missing the code for Visit(DbSkipExpression).

Lynn, first of all, I got your test case working. Problems with test case:
  - edit sln file and fix paths to projects
  - add startup project
  - change attached DDL script as it was not "USE"ing proper database
and so on. But main thing is it's working.

Now, I have troubles understanding where SELECT comes from and how to affect it. Looking through solution, I do not see OrderBy being called at any time. I presume LinQ takes that over but I do not know how to check/modify that. Can you help me there?
Finally, there is no ORDER BY in resulting query (only LIMIT X,10) but I can not say problem's verified before I know how to affect the query issued to be sure there *is* an "order by"...

Hello Tonci,

Thank you for your patience and I am glad that you got the project running. Looks like you have several questions so I'll try to answer as clearly as I can.

1) The main project of interest in the web site project. This leverages the library where the entity framework .edmx file is contained.

2) Inside the the web project there is a default.aspx file that is our main focus. This default.aspx file contains a grid view control and an entity data source control. The entity data source control supplies data to the grid view, and inturn the grid view supplies paging and sorting information to the entity data source control. The entity data source control creates linq statements against the entity object context.

3) If you try sorting the gridview you will see it will not sort. This is becasue the select generator in the driver is not generating the correct sql.

4) If you put a break point in the method 'public override SqlFragment Visit(DbSkipExpression expression)' you'll see that the expression supplies items in the SortOrder class but there is not code to handle them. But it's an easy fix as the same For Each loop from 'Visit (DbSortExpression expression)' does the job.

If the use of the asp.net project and entity data source are getting too much in the way I will create a console app that is more expressly to the point.

Thanks for your patience, and I apologize for the difficulty. I appreciate you guys greatly! :)

Note: I will get you a console app as soon as I can.

Tonci you're AWESOME!

A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/80087

744 Reggie Burnett	2009-08-04
      - fixed DbSkipExpression so that it would respect any sort clauses that are attached
        (thanks Lynn!) (bug #45723)

fixed in 6.0.5 and 6.1.1

An entry was added to both the 6.0.5 and 6.1.1 changelogs:

The Entity Framework provider was not calling DBSortExpression correctly when the Skip and Take methods were used, such as in the following statement:

TestModel.tblquarantine.OrderByDescending(q => q.MsgDate).Skip(100).Take(100).ToList();

This resulted in the data being unsorted.

Lynn, thanks for kind words and many reports like this one catching true problems before anyone else.
And thanks Reggie for a quick fix.