dotnet / efcore

EF Core is a modern object-database mapper for .NET. It supports LINQ queries, change tracking, updates, and schema migrations.
https://docs.microsoft.com/ef/
MIT License
13.64k stars 3.15k forks source link

OrderBy NULL value throws "Incorrect syntax near the keyword IS NULL" error #8591

Closed paritosharya408 closed 7 years ago

paritosharya408 commented 7 years ago

I have following entity in my project (.net core1.0/ef core1.0/vs2015)-

public class News
    {
        public int Id { get; set; }
        public string NewsType { get; set; }
        public DateTime? Date { get; set; }
        public int? UserDefinedRank { get; set; }
        public string DocumentName { get; set; }
        public string DocumentLocation { get; set; }
        public string BigImage { get; set; }
        public string SmallImage { get; set; }
        public string Header { get; set; }
        public string SummaryText { get; set; }
        public string DetailedText { get; set; }
        public string ArticleText { get; set; }
        public DateTime? EventDate { get; set; }
        public string EventAddress { get; set; }
        public bool? RegistrationClosed { get; set; }
        public string EventRegistrationUrl { get; set; }
        public string LinkType { get; set; }
        public int? LinkedNewsId { get; set; }
        public int? LinkedGroupId { get; set; }
        public int? GroupNewsOrder { get; set; }
        public bool? HideOnListPage { get; set; }
        public string CustomUrl { get; set; }
        public bool? IsCustomUrl { get; set; }
        public bool? IsJobClosed { get; set; }
        public virtual Group Group { get; set; }
        public virtual ICollection<Registration> Registrations { get; set; }
    }

Now I want to return all news items first ordered by NULL rank and then by any integer value and lastly also ordered by date descending. This is the code in my repository that returns this data return await context.News.OrderBy(x => x.UserDefinedRank == null).ThenBy(x => x.UserDefinedRank).ThenByDescending(x => x.Date).ToListAsync();

This generates following SQL which is incorrect (viewed in SQL profiler):

SELECT [x].[Id], [x].[ArticleText], [x].[BigImage], [x].[CustomUrl], [x].[Date], [x].[DetailedText], [x].[DocumentLocation], [x].[DocumentName], [x].[EventAddress], [x].[EventDate], [x].[EventRegistrationUrl], [x].[GroupId], [x].[GroupNewsOrder], [x].[Header], [x].[HideOnListPage], [x].[IsCustomUrl], [x].[IsJobClosed], [x].[LinkType], [x].[LinkedGroupId], [x].[LinkedNewsId], [x].[NewsType], [x].[RegistrationClosed], [x].[SmallImage], [x].[SummaryText], [x].[UserDefinedRank]
FROM [News] AS [x]
ORDER BY [x].[UserDefinedRank] IS NULL, [x].[UserDefinedRank], [x].[Date] DESC

The same code in EF6.0 works flawlessly and generates:

SELECT 
    [Project1].[Id] AS [Id], 
    [Project1].[NewsType] AS [NewsType], 
    [Project1].[Date] AS [Date], 
    [Project1].[UserDefinedRank] AS [UserDefinedRank], 
    [Project1].[DocumentName] AS [DocumentName], 
    [Project1].[DocumentLocation] AS [DocumentLocation], 
    [Project1].[BigImage] AS [BigImage], 
    [Project1].[SmallImage] AS [SmallImage], 
    [Project1].[Header] AS [Header], 
    [Project1].[SummaryText] AS [SummaryText], 
    [Project1].[DetailedText] AS [DetailedText], 
    [Project1].[ArticleText] AS [ArticleText], 
    [Project1].[EventDate] AS [EventDate], 
    [Project1].[EventAddress] AS [EventAddress], 
    [Project1].[RegistrationClosed] AS [RegistrationClosed], 
    [Project1].[EventRegistrationUrl] AS [EventRegistrationUrl], 
    [Project1].[LinkType] AS [LinkType], 
    [Project1].[LinkedNewsId] AS [LinkedNewsId], 
    [Project1].[LinkedGroupId] AS [LinkedGroupId], 
    [Project1].[GroupNewsOrder] AS [GroupNewsOrder], 
    [Project1].[HideOnListPage] AS [HideOnListPage], 
    [Project1].[CustomUrl] AS [CustomUrl], 
    [Project1].[IsCustomUrl] AS [IsCustomUrl], 
    [Project1].[IsJobClosed] AS [IsJobClosed]
    FROM ( SELECT 
        CASE WHEN ([Extent1].[UserDefinedRank] IS NULL) THEN cast(1 as bit) ELSE cast(0 as bit) END AS [C1], 
        [Extent1].[Id] AS [Id], 
        [Extent1].[NewsType] AS [NewsType], 
        [Extent1].[Date] AS [Date], 
        [Extent1].[UserDefinedRank] AS [UserDefinedRank], 
        [Extent1].[DocumentName] AS [DocumentName], 
        [Extent1].[DocumentLocation] AS [DocumentLocation], 
        [Extent1].[BigImage] AS [BigImage], 
        [Extent1].[SmallImage] AS [SmallImage], 
        [Extent1].[Header] AS [Header], 
        [Extent1].[SummaryText] AS [SummaryText], 
        [Extent1].[DetailedText] AS [DetailedText], 
        [Extent1].[ArticleText] AS [ArticleText], 
        [Extent1].[EventDate] AS [EventDate], 
        [Extent1].[EventAddress] AS [EventAddress], 
        [Extent1].[RegistrationClosed] AS [RegistrationClosed], 
        [Extent1].[EventRegistrationUrl] AS [EventRegistrationUrl], 
        [Extent1].[LinkType] AS [LinkType], 
        [Extent1].[LinkedNewsId] AS [LinkedNewsId], 
        [Extent1].[LinkedGroupId] AS [LinkedGroupId], 
        [Extent1].[GroupNewsOrder] AS [GroupNewsOrder], 
        [Extent1].[HideOnListPage] AS [HideOnListPage], 
        [Extent1].[CustomUrl] AS [CustomUrl], 
        [Extent1].[IsCustomUrl] AS [IsCustomUrl], 
        [Extent1].[IsJobClosed] AS [IsJobClosed]
        FROM [dbo].[News] AS [Extent1]
    )  AS [Project1]
    ORDER BY [Project1].[C1] ASC, [Project1].[UserDefinedRank] ASC, [Project1].[Date] DESC

If I comment the OrderBy(x => x.UserDefinedRank == null), it returns data but without the first condition that i really want.

Steps to reproduce

Use OrderBy clause in LINQ with NULL condition.

Exception message:
Stack trace:
System.Data.SqlClient.SqlException (0x80131904): Incorrect syntax near the keyword 'IS'.

   at System.Data.SqlClient.SqlCommand.<>c.<ExecuteDbDataReaderAsync>b__174_0(Task`1 result)

   at System.Threading.Tasks.ContinuationResultTaskFromResultTask`2.InnerInvoke()

   at System.Threading.Tasks.Task.Execute()

--- End of stack trace from previous location where exception was thrown ---

   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()

   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)

   at Microsoft.EntityFrameworkCore.Storage.Internal.RelationalCommand.<ExecuteAsync>d__20.MoveNext()

--- End of stack trace from previous location where exception was thrown ---

   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()

   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)

   at Microsoft.EntityFrameworkCore.Query.Internal.AsyncQueryingEnumerable.AsyncEnumerator.<MoveNext>d__8.MoveNext()

--- End of stack trace from previous location where exception was thrown ---

   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()

   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)

   at Microsoft.EntityFrameworkCore.Query.Internal.AsyncLinqOperatorProvider.ExceptionInterceptor`1.EnumeratorExceptionInterceptor.<MoveNext>d__5.MoveNext()

--- End of stack trace from previous location where exception was thrown ---

   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()

   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)

   at Microsoft.EntityFrameworkCore.EntityFrameworkQueryableExtensions.<ToListAsync>d__129`1.MoveNext()

--- End of stack trace from previous location where exception was thrown ---

   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()

   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)

   at System.Runtime.CompilerServices.TaskAwaiter`1.GetResult()

   at GoalEnvision.Public.DAL.Repositories.NewsRepository.<GetAsync>d__2.MoveNext() in D:\Project\GitHub\Regent\GoalEnvision.Public\src\GoalEnvision.Public\DAL\Repositories\NewsRepository.cs:line 39

--- End of stack trace from previous location where exception was thrown ---

   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()

   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)

   at System.Runtime.CompilerServices.TaskAwaiter`1.GetResult()

   at GoalEnvision.Public.Areas.Admin.Controllers.NewsController.<Index>d__3.MoveNext() in D:\Project\GitHub\Regent\GoalEnvision.Public\src\GoalEnvision.Public\Areas\Admin\Controllers\NewsController.cs:line 41

--- End of stack trace from previous location where exception was thrown ---

   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()

   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)

   at Microsoft.AspNetCore.Mvc.Internal.ObjectMethodExecutor.<CastToObject>d__40`1.MoveNext()

--- End of stack trace from previous location where exception was thrown ---

   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()

   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)

   at Microsoft.AspNetCore.Mvc.Internal.ControllerActionInvoker.<InvokeActionFilterAsync>d__28.MoveNext()

--- End of stack trace from previous location where exception was thrown ---

   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()

   at Microsoft.AspNetCore.Mvc.Internal.ControllerActionInvoker.<InvokeAsync>d__18.MoveNext()

--- End of stack trace from previous location where exception was thrown ---

   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()

   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)

   at Microsoft.AspNetCore.Builder.RouterMiddleware.<Invoke>d__4.MoveNext()

--- End of stack trace from previous location where exception was thrown ---

   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()

   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)

   at Microsoft.AspNetCore.Authentication.AuthenticationMiddleware`1.<Invoke>d__18.MoveNext()

--- End of stack trace from previous location where exception was thrown ---

   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()

   at Microsoft.AspNetCore.Authentication.AuthenticationMiddleware`1.<Invoke>d__18.MoveNext()

--- End of stack trace from previous location where exception was thrown ---

   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()

   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)

   at Microsoft.AspNetCore.Authentication.AuthenticationMiddleware`1.<Invoke>d__18.MoveNext()

--- End of stack trace from previous location where exception was thrown ---

   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()

   at Microsoft.AspNetCore.Authentication.AuthenticationMiddleware`1.<Invoke>d__18.MoveNext()

--- End of stack trace from previous location where exception was thrown ---

   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()

   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)

   at Microsoft.AspNetCore.Authentication.AuthenticationMiddleware`1.<Invoke>d__18.MoveNext()

--- End of stack trace from previous location where exception was thrown ---

   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()

   at Microsoft.AspNetCore.Authentication.AuthenticationMiddleware`1.<Invoke>d__18.MoveNext()

--- End of stack trace from previous location where exception was thrown ---

   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()

   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)

   at Microsoft.AspNetCore.Authentication.AuthenticationMiddleware`1.<Invoke>d__18.MoveNext()

--- End of stack trace from previous location where exception was thrown ---

   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()

   at Microsoft.AspNetCore.Authentication.AuthenticationMiddleware`1.<Invoke>d__18.MoveNext()

--- End of stack trace from previous location where exception was thrown ---

   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()

   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)

   at Microsoft.AspNetCore.Authentication.AuthenticationMiddleware`1.<Invoke>d__18.MoveNext()

--- End of stack trace from previous location where exception was thrown ---

   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()

   at Microsoft.AspNetCore.Authentication.AuthenticationMiddleware`1.<Invoke>d__18.MoveNext()

--- End of stack trace from previous location where exception was thrown ---

   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()

   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)

   at Microsoft.AspNetCore.Diagnostics.DeveloperExceptionPageMiddleware.<Invoke>d__6.MoveNext()

ClientConnectionId:3d2b4628-a4d7-476e-b089-0f5d34a367b4
public async Task<IEnumerable<News>> GetAsync()
        {
            //THIS FAILS
            //return await context.News.OrderBy(x => x.UserDefinedRank == null).ThenBy(x => x.UserDefinedRank).ThenByDescending(x => x.Date).ToListAsync();

            //THIS WORKS
            return await context.News.OrderBy(x => x.UserDefinedRank).ThenByDescending(x => x.Date).ToListAsync();
        }

Further technical details

EF Core version: 1.0.0 Database Provider: Microsoft.EntityFrameworkCore.SqlServer Operating system: Windows 10 IDE: Visual Studio 2015

smitpatel commented 7 years ago

We should generate a case block because IS NULL is condition and ordering should return a value. Based on refactoring of BooleanConditionTranslatingExpressionVisitor this should work in current dev.

smitpatel commented 7 years ago

Generated SQL

Executed DbCommand (21ms) [Parameters=[], CommandType='Text', CommandTimeout='30']
      SELECT [x].[Id], [x].[ArticleText], [x].[BigImage], [x].[CustomUrl], [x].[Date], [x].[DetailedText], [x].[DocumentLocation], [x].[DocumentName], [x].[EventAddress], [x].[EventDate], [x].[EventRegistrationUrl], [x].[GroupNewsOrder], [x].[Header], [x].[HideOnListPage], [x].[IsCustomUrl], [x].[IsJobClosed], [x].[LinkType], [x].[LinkedGroupId], [x].[LinkedNewsId], [x].[NewsType], [x].[RegistrationClosed], [x].[SmallImage], [x].[SummaryText], [x].[UserDefinedRank]
      FROM [News] AS [x]
      ORDER BY CASE
          WHEN [x].[UserDefinedRank] IS NULL
          THEN CAST(1 AS BIT) ELSE CAST(0 AS BIT)
      END, [x].[UserDefinedRank], [x].[Date] DESC

fixed by #7554