Efcore: Query: OrderBy constant generates incorrect query for integer/boolean

[smitpatel]

At present in query,
OrderBy(c => true) translates to ORDER BY 1
OrderBy(c => 3) translates to ORDER BY 3
In SqlServer, ORDER BY integer means order by that column number from projection which introduces unintended ordering and gives different results compared to what linq gives.
We are also using creating Order by when we do group join with default if empty using the left side of join condition which also faces above issue based on the join condition.
See tests QuerySqlServerTest.OrderBy_true, QuerySqlServerTest.OrderBy_integer & GearsOfWarQuerySqlServerTest.Left_join_predicate_value_equals_condition
Possible work-arounds:
We can remove the order by clause if it is using constant. Results would match this way with linq but for paging cases, order by clause is necessary.
We can introduce dummy order by as follows:
Order by @@ROWCOUNT
Order by @parameter + 1 where @parameter is passed. Value doesn't matter.
@divega

[divega]

I think the cases of ORDER BY with a non-numeric and non-Boolean constants and ORDER BY with variables/parameters could also be interesting. Assuming we are using the same naive translation we use for numeric and Boolean constants, SQL Server will throw:

• For non-numeric constants:

A constant expression was encountered in the ORDER BY list, position 1.

• For variables:

The SELECT item identified by the ORDER BY number 2 contains a variable as part of the expression identifying a column position. Variables are only allowed when ordering by an expression referencing a column name.

It seems that the general approach could be to swallow any elements in the ORDER BY list that are either variables or constants to prevent generating SQL that would cause an exception (in the variable case and for non-numeric constants) or results ordered by some arbitrary column (in the numeric constant case). That said there are cases that are a bit more complicated that will still cause an exception.

It is true if there are paging operations and we remove all elements of the ORDER BY list, we will have to introduce a dummy ORDER BY. Maybe that can be handled by the existing logic that introduces ORDER BY @@ROWCOUNT?

(BTW if anyone finds anything other than @@ROWCOUNT that still works and produces stable ordering that would be great. I almost prefer @@SPID to @@ROWCOUNT because the latter can give you the false impression that it represents something in the query)

[kunnis]

@divega Have you considered ORDER BY (SELECT 1)?

[divega]

@kunnis actually no. Great tip!

@smitpatel @anpete this seems to works. I think we should consider doing it here as well as when we need an arbitrary ORDER BY for paging. Thoughts?

[ErikEJ]

Won't work with SQL ce..,

[anpete]

@divega Yes. I recently also discovered this trick when working on the improved GroupJoin prototype.

[divega]

@ErikEJ does SQL CE have all the dame issues? E.g.is it already broken for these scenarios?

[kunnis]

@divega I think it currently has the same problem.

https://github.com/ErikEJ/EntityFramework.SqlServerCompact/blob/master/src/Provider40/Query/Sql/Internal/SqlCeQuerySqlGenerator.cs#L76 Uses ORDER BY GETDATE() to get around the fact that the Order By clause is required.

CE doesn't allow for a scalar_subquery (which is what (select 1) is) in the Order By (See https://technet.microsoft.com/en-us/library/ms174501(v=sql.110).aspx ) where MSSQL does (See https://msdn.microsoft.com/en-us/library/ms190286.aspx )

I setup SqlCE earlier today and ran a statement ending in ORDER BY @@rowcount. That threw an exception that @@rowcount wasn't allowed there. @@spid doesn't work either. I tried several different variations and I couldn't find a clause that was clear about not doing anything and worked in both MSSQL and SqlCE.

[ErikEJ]

@divega - what @kunnis says (thanks)

[smitpatel]

The same issue persists in SQLite too. Fixing it for relational provider in general.