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

Conditional Order By with collection include generates incorrect SQL #6591

Closed
anna-git opened this issue Sep 22, 2016 · 4 comments
Closed

Conditional Order By with collection include generates incorrect SQL #6591

anna-git opened this issue Sep 22, 2016 · 4 comments
Assignees
Labels
closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. type-bug
Milestone

Comments

@anna-git
Copy link

anna-git commented Sep 22, 2016

Steps to reproduce

Quite a simple query with a nested include though:

  var test = (from post in dbContext.Posts
                     orderby (post.Language == "fr") ? 1 : 2
                     select post).Include(p => p.PostTags).ThenInclude(pt => pt.Tag);
            var testResult = test.ToList();

Knowing that a post has a collection of posttags which themselves own a tag.

The issue

The query can't be materialized because generated sql is wrong:

If you are seeing an exception, include the full exceptions details (message and stack trace).

Exception message:
Additional information: No column name was specified for column 1 of 'post0'.
The multi-part identifier "post.Language" could not be bound.
Stack trace:
{System.Data.SqlClient.SqlException: No column name was specified for column 1 of 'post0'.
The multi-part identifier "post.Language" could not be bound.
   at System.Data.SqlClient.SqlConnection.OnError(SqlException exception, Boolean breakConnection, Action`1 wrapCloseInAction)
   at System.Data.SqlClient.TdsParser.ThrowExceptionAndWarning(TdsParserStateObject stateObj, Boolean callerHasConnectionLock, Boolean asyncClose)
   at System.Data.SqlClient.TdsParser.TryRun(RunBehavior runBehavior, SqlCommand cmdHandler, SqlDataReader dataStream, BulkCopySimpleResultSet bulkCopyHandler, TdsParserStateObject stateObj, Boolean& dataReady)
   at System.Data.SqlClient.SqlDataReader.TryConsumeMetaData()
   at System.Data.SqlClient.SqlDataReader.get_MetaData()
   at System.Data.SqlClient.SqlCommand.FinishExecuteReader(SqlDataReader ds, RunBehavior runBehavior, String resetOptionsString)
   at System.Data.SqlClient.SqlCommand.RunExecuteReaderTds(CommandBehavior cmdBehavior, RunBehavior runBehavior, Boolean returnStream, Boolean async, Int32 timeout, Task& task, Boolean asyncWrite, SqlDataReader ds)
   at System.Data.SqlClient.SqlCommand.ExecuteReader(CommandBehavior behavior)
   at Microsoft.EntityFrameworkCore.Storage.Internal.RelationalCommand.Execute(IRelationalConnection connection, String executeMethod, IReadOnlyDictionary`2 parameterValues, Boolean openConnection, Boolean closeConnection)
   at Microsoft.EntityFrameworkCore.Storage.Internal.RelationalCommand.ExecuteReader(IRelationalConnection connection, IReadOnlyDictionary`2 parameterValues, Boolean manageConnection)
   at Microsoft.EntityFrameworkCore.Query.Internal.QueryingEnumerable.Enumerator.MoveNext()
   at Microsoft.EntityFrameworkCore.Query.Internal.IncludeCollectionIterator.<GetRelatedValues>d__4.MoveNext()
   at System.Linq.Enumerable.SelectEnumerableIterator`2.MoveNext()
   at System.Linq.Enumerable.WhereEnumerableIterator`1.MoveNext()
   at System.Collections.Generic.List`1..ctor(IEnumerable`1 collection)
   at System.Linq.Enumerable.ToList[TSource](IEnumerable`1 source)
   at Microsoft.EntityFrameworkCore.Query.Internal.QueryBuffer.Include(QueryContext queryContext, Object entity, IReadOnlyList`1 navigationPath, IReadOnlyList`1 relatedEntitiesLoaders, Int32 currentNavigationIndex, Boolean queryStateManager)
   at Microsoft.EntityFrameworkCore.Query.Internal.QueryBuffer.Include(QueryContext queryContext, Object entity, IReadOnlyList`1 navigationPath, IReadOnlyList`1 relatedEntitiesLoaders, Boolean queryStateManager)
   at Microsoft.EntityFrameworkCore.Query.QueryMethodProvider.<_Include>d__30`1.MoveNext()
   at Microsoft.EntityFrameworkCore.Query.Internal.LinqOperatorProvider.<_TrackEntities>d__15`2.MoveNext()
   at Microsoft.EntityFrameworkCore.Query.Internal.LinqOperatorProvider.ExceptionInterceptor`1.EnumeratorExceptionInterceptor.MoveNext()
   at System.Collections.Generic.List`1..ctor(IEnumerable`1 collection)
   at System.Linq.Enumerable.ToList[TSource](IEnumerable`1 source)
[ MyProject.ToList()]

Generated sql:

SELECT [post].[Id], [post].[Content], [post].[CreationDate], [post].[Language], [post].[Rating], [post].[UpdateDate], [post].[UserId]
FROM [Posts] AS [post]
ORDER BY CASE
    WHEN [post].[Language] = N'fr'
    THEN 1 ELSE 2
END, [post].[Id]
Microsoft.EntityFrameworkCore.Storage.Internal.RelationalCommandBuilderFactory:Information: Executed DbCommand (1ms) [Parameters=[], CommandType='Text', CommandTimeout='30']
SELECT [p].[Id], [p].[PostId], [p].[TagId], [p].[Words], [t].[Id], [t].[Key], [t].[Name], [t].[ParentTagId]
FROM [PostTags] AS [p]
INNER JOIN (
    SELECT DISTINCT CASE
        WHEN [post].[Language] = N'fr'
        THEN 1 ELSE 2
    END, [post].[Id]
    FROM [Posts] AS [post]
) AS [post0] ON [p].[PostId] = [post0].[Id]
INNER JOIN [Tags] AS [t] ON [p].[TagId] = [t].[Id]
ORDER BY CASE
    WHEN [post].[Language] = N'fr'
    THEN 1 ELSE 2
END, [post0].[Id]

you can see at the end of the query, there's still post identifier whereas this should be "post0"

Nevertheless, it works as expected with a normal order by, without the condition inside I mean.

Further technical details

EF Core version: (found in project.json or packages.config) 1.0.0 or 1.0.1
Operating system: Win 8
Visual Studio version: (e.g. VS 2013 or n/a) vs 2015

@smitpatel
Copy link
Contributor

smitpatel commented Sep 22, 2016

This is a bug in our LiftToOrderBy & UpdateColumnExpression.
Even though they are fixed the query may not be giving correct results due to #6145

@anna-git
Copy link
Author

Oh ok thanks. Do you think it is related to the same thing, because when the query orders by 1, sure it's the column index, but when there's a condition with "order by case...then", which returns 1 or 2 and then sort by those values, it's not about column indexes anymore, as for example http://sqlfiddle.com/#!6/05a45/2, we could return any integer in the case, it would sort on that integer value. Here it's not telling "The ORDER BY position number 3 is out of range of the number of items in the select list." because it's really the value here.
And it's working as expected without the nested include, otherwise, it completely crashes, because "post" identifier isn't the right one anymore, since the previous inner joins.

@rowanmiller
Copy link
Contributor

rowanmiller commented Sep 23, 2016

@smitpatel should this be closed as a dupe of #6145.

@smitpatel
Copy link
Contributor

@rowanmiller - This one is different. I tested with Sql Server, as @annayafi said, Order By case block which returns integer is not assumed column indices. So #6145 is not longer needed for this.
The bug here is incorrect SQL.

SELECT [c].[Id], [c].[ParentId]
FROM [Children] AS [c]
INNER JOIN (
    SELECT DISTINCT CASE
        WHEN [p].[Language] = N'fr'
        THEN 1 ELSE 2
    END, [p].[Id]
    FROM [Parents] AS [p]
) AS [p0] ON [c].[ParentId] = [p0].[Id]
ORDER BY CASE
    WHEN [p].[Language] = N'fr'
    THEN 1 ELSE 2
END, [p0].[Id]

The orderby clause references column from [p] which is no longer in context. Further [p].[Language] is not projected out from inner subquery. The case block in inner subquery should be aliased and order by clause should use the alias with appropriate table reference.
This bug happens with conditional order by and collection include.

@smitpatel smitpatel reopened this Sep 23, 2016
@smitpatel smitpatel changed the title ThenInclude and conditional orderby not working together Conditional Order By with collection include generates incorrect SQL Sep 23, 2016
@smitpatel smitpatel self-assigned this Sep 23, 2016
@divega divega added this to the 1.1.0 milestone Sep 26, 2016
@smitpatel smitpatel added the closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. label Oct 1, 2016
@ajcvickers ajcvickers modified the milestones: 1.1.0-preview1, 1.1.0 Oct 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. type-bug
Projects
None yet
Development

No branches or pull requests

5 participants