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

OFFSET used by mistake for SQL Server 2012. #365

Open
ghost opened this issue May 30, 2015 · 2 comments
Open

OFFSET used by mistake for SQL Server 2012. #365

ghost opened this issue May 30, 2015 · 2 comments

Comments

@ghost
Copy link

ghost commented May 30, 2015

The following SQL query is not valid from point of view of syntax:

SELECT DISTINCT ColumnA, ColumnB, ColumnC
FROM SomeTable
ORDER BY ColumnD, ColumnC
OFFSET 0 ROWS FETCH NEXT 50 ROWS ONLY 

because ColumnD is not in distinct select list.

But such pagination (Skip and Take) and ordering (OrderBy) are valid and possible.
This issue is addressed only to OFFSET syntax available in SQL Server 2012 (and some others).

MsSql2008SqlProvider works fine because BuildAlternativeSql property is set to true.
True for that property will force using of AlternativeBuildSql method.
So this method is used to create another version of select query with row number.
"ROW_NUMBER() OVER" syntax allows ordering with DISTINCT queries and pagination works fine.
Of course row number is slower than offset. So we need to create some solution to consider both methods.

Potential way to do it is overriding of the following method for MsSql2012SqlProvider:

protected override void BuildSql(StringBuilder sb)
        {
            if (BuildAlternativeSql)
                AlternativeBuildSql(sb, true, base.BuildSql);
            else
                base.BuildSql(sb);
        }

There we need to detect distinct query and compare columns in select and in order by.
But I am not sure how it is better to do... project structure. Maybe somebody who is more experienced in the project structure can suggest detection logic or refactoring of BuildAlternativeSql property.

Fast temp fix is related to disabling of OFFSET syntax. There are 2 ways:

  1. Return "true" instead of "false":
protected override bool   BuildAlternativeSql { get { return false;             } }
  1. Change database compatibility version to 100 (SQL Server 2008):
ALTER DATABASE [My Database] SET COMPATIBILITY_LEVEL = 100

In 2nd case SqlDataProviderVersionResolver will instantiate 2008 version of sql provider.

@ili
Copy link
Collaborator

ili commented Jun 22, 2015

I see... SQL Server demand to include order by columns into select closure..
may be we can check and if it is not so use old syntax....

@ghost
Copy link
Author

ghost commented Jun 22, 2015

SQL Server demands to include all columns from ORDER BY to SELECT DISTINCT clause. But LINQ expressions can be made in the way when we do not need to include it. In my example we need to sort items by ColumnD and after that perform distinct selection of top 50 rows using columns not related to ColumnD. From point of view of Linq experssion it is possible and it works if ROW_NUMBER syntax is used. I guess we need to switch to ROW_NUMBER instead of OFFSET only for queries with DISTINCT, ORDER BY and pagination at the same time and only in case when select and order by columns are different in expression. In all other cases we can use OFFSET because it works a bit faster.

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

No branches or pull requests

1 participant