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

Invalid SQL with CROSS APPLY generated on SQLite #19178

Closed
ixtreon opened this issue Dec 5, 2019 · 13 comments · Fixed by #21591
Closed

Invalid SQL with CROSS APPLY generated on SQLite #19178

ixtreon opened this issue Dec 5, 2019 · 13 comments · Fixed by #21591
Assignees
Labels
area-query closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. customer-reported poachable type-enhancement
Milestone

Comments

@ixtreon
Copy link

ixtreon commented Dec 5, 2019

After updating EF Core from 2.2.3 to 3.1.0-preview3.19554.8, a specific query started failing when executed via the Sqlite provider. While we can manually work around this issue, the second query should be identical to the first one yet yields different SQL.

To Reproduce

The model in use:

public class Conversation
{
    public int Id { get; set; }

    public DateTime? DateLastSeenByFirst { get; set; }
    public DateTime? DateLastSeenBySecond { get; set; }

    public virtual ICollection<Message> Messages { get; set; }
}

public class Message
{
    public int Id { get; set; }

    public DateTime DateSent { get; set; }

    public int? ConversationId { get; set; }
    public virtual Conversation Conversation { get; set; }
}

The goal is to return all messages not yet seen by a given participant, noting they may have never seen the conversation. This is our existing implementation:

bool isFirstParty = false;  // change to true if second party

var thisFails = db.Set<Conversation>()
    .Select(x => new
    {
        Conversation = x,
        LastSeen = isFirstParty ? x.DateLastSeenByFirst : x.DateLastSeenBySecond,
    })
    .SelectMany(x => x.Conversation.Messages.Where(m => x.LastSeen == null || x.LastSeen < m.DateSent))
    .ToList();

When run via the SQLite provider, this generates the following query:

SELECT "t"."ID", "t"."DateSent", "t"."ConversationId"
FROM "Conversations" AS "m"
CROSS APPLY (
    SELECT "i"."ID", "i"."DateSent", "i"."ConversationId"
    FROM "Messages" AS "i"
    WHERE ("m"."DateLastSeenBySecond" IS NULL OR ("m"."DateLastSeenBySecond" < "i"."DateSent")) AND ("m"."Id" = "i"."ConversationId")
) AS "t"

In turn throwing the following exception:

Microsoft.Data.Sqlite.SqliteException
  HResult=0x80004005
  Message=SQLite Error 1: 'near "(": syntax error'.
  Source=<Cannot evaluate the exception source>
  StackTrace:
<Cannot evaluate the exception stack trace>

More Details

My limited understanding is that the APPLY operator is specific to SQL Server and is not available in SQLite.

When the query is re-written as follows, EF generates a query with a JOIN as expected:

var thisWorks = db.Set<Message>()
    .Select(x => new
    {
        Message = x,
        LastSeen = isFirstParty ? x.Conversation.DateLastSeenByFirst : x.Conversation.DateLastSeenBySecond,
    })
    .Where(x => x.LastSeen == null || x.LastSeen < x.Message.DateSent)
    .Select(x => x.Message)
    .ToList();
SELECT "i"."ID", "i"."DateSent", "i"."ConversationId"
FROM "Messages" AS "i"
LEFT JOIN "Conversations" AS "m" ON "i"."ConversationId" = "m"."Id"
WHERE "m"."DateLastSeenBySecond" IS NULL OR ("m"."DateLastSeenBySecond" < "i"."DateSent")

The same happens if the LastSeen expression is repeated twice in the Where block, instead of selecting it initially.

Running the same examples using the SQL Server provider yields similarly looking SQL - first one using APPLY and the second one, a JOIN. I would've naively expected the same output from both queries, considering they are virtually identical.

Additional context

Microsoft.Data.Sqlite version: 3.1.0-preview3.19554.8
Target framework: net461
Operating system: Windows 10

@smitpatel
Copy link
Member

This issue would improve the exception message pointing to users that query cannot be translated because it uses cross apply/outer apply.

@stevendarby
Copy link
Contributor

stevendarby commented Mar 10, 2020

Isn't it a bug that SQL is generated for SQLite that isn't SQLite compatible? Is a join not an option?

Given that Microsoft recommend using SQLite in unit tests, what is their recommendation for when a query works in SQL Server (in production) but not SQLite (in unit tests). I'm loathe to change code just to satisfy a unit test.

@ajcvickers
Copy link
Member

@Snappyfoo We need to update that guidance to be more detailed. See dotnet/EntityFramework.Docs#430 and dotnet/EntityFramework.Docs#1304, and also the discussion on #18457

Full fidelity when testing is only possible when using the same database backend that you use in production. Every step away from that results in some level of mismatch. The SQLite provider is closer to SQL Server because they are both relational providers, but it still has different capabilities and behaviors in some cases.

@stevendarby
Copy link
Contributor

stevendarby commented Mar 10, 2020

@Snappyfoo We need to update that guidance to be more detailed. See dotnet/EntityFramework.Docs#430 and dotnet/EntityFramework.Docs#1304, and also the discussion on #18457

Full fidelity when testing is only possible when using the same database backend that you use in production. Every step away from that results in some level of mismatch. The SQLite provider is closer to SQL Server because they are both relational providers, but it still has different capabilities and behaviors in some cases.

I can understand that the database might behave differently, but what I don’t quite follow in this particular case is why the SQL produced when targeting SQLite isn’t compatible with SQLite. Perhaps I don’t get something about how EF works, but isn’t it open to support for a number of database providers and the process of translating LINQ expressions to SQL is specific to each database provider? Isn’t there a bug in that process for SQLite?

I do understand that what I’m asking for is a situation where the SQL executed would be different when against SQL Server vs SQLite but I would expect the result to be the same in most cases, and I’m not trying to test EF in that respect. I’m only aware of the SQL it produces because it doesn’t work.

@ajcvickers
Copy link
Member

@Snappyfoo Yes, this is a bug. (We label bugs with type-bug.)

@stevendarby
Copy link
Contributor

@Snappyfoo Yes, this is a bug. (We label bugs with type-bug.)

Oh, right you are. Sorry, I misinterpreted one of the comments. Thanks for clarifying that.

@smitpatel
Copy link
Member

Duplicate of #18871

Changing this to enhancement. The reason we generate non-join (outer apply) is non-equality comparison being referenced in predicate. In the absence of which we would generate a proper join.

@CarlosLanderas
Copy link

CarlosLanderas commented Apr 12, 2020

Having the same issue in the HealthChecks UI repository, but with OUTER APPLY:

https://github.com/Xabaril/AspNetCore.Diagnostics.HealthChecks/blob/storage-providers/src/HealthChecks.UI/Middleware/UIApiEndpointMiddleware.cs#L48-L61

Error:

 Microsoft.EntityFrameworkCore.Database.Command[20102]
      Failed executing DbCommand (1ms) [Parameters=[@__item_Name_1='?' (Size = 9), @___settings_MaximumExecutionHistoriesPerEndpoint_0='?'], CommandType='Text', CommandTimeout='30']
      SELECT "t"."Id", "t"."DiscoveryService", "t"."LastExecuted", "t"."Name", "t"."OnStateFrom", "t"."Status", "t"."Uri", "h"."Id", "h"."Description", "h"."Duration", "h"."HealthCheckExecutionId", "h"."Name", "h"."Status", "t0"."Id", "t0"."Description", "t0"."HealthCheckExecutionId", "t0"."Name", "t0"."On", "t0"."Status"
      FROM (
          SELECT "e"."Id", "e"."DiscoveryService", "e"."LastExecuted", "e"."Name", "e"."OnStateFrom", "e"."Status", "e"."Uri"
          FROM "Executions" AS "e"
          WHERE "e"."Name" = @__item_Name_1
          LIMIT 2
      ) AS "t"
      LEFT JOIN "HealthCheckExecutionEntries" AS "h" ON "t"."Id" = "h"."HealthCheckExecutionId"
      OUTER APPLY (
          SELECT "h0"."Id", "h0"."Description", "h0"."HealthCheckExecutionId", "h0"."Name", "h0"."On", "h0"."Status"
          FROM "HealthCheckExecutionHistories" AS "h0"
          WHERE "h0"."HealthCheckExecutionId" = "t"."Id"
          ORDER BY "h0"."On" DESC
          LIMIT @___settings_MaximumExecutionHistoriesPerEndpoint_0
      ) AS "t0"
      ORDER BY "t"."Id", "h"."Id", "t0"."On" DESC, "t0"."Id"
fail: Microsoft.EntityFrameworkCore.Query[10100]
      An exception occurred while iterating over the results of a query for context type 'HealthChecks.UI.Core.Data.HealthChecksDb'.
      Microsoft.Data.Sqlite.SqliteException (0x80004005): SQLite Error 1: 'near "(": syntax error'.
         at Microsoft.Data.Sqlite.SqliteException.ThrowExceptionForRC(Int32 rc, sqlite3 db)
         at Microsoft.Data.Sqlite.SqliteCommand.PrepareAndEnumerateStatements(Stopwatch timer)+MoveNext()
         at Microsoft.Data.Sqlite.SqliteCommand.GetStatements(Stopwatch timer)+MoveNext()
         at Microsoft.Data.Sqlite.SqliteDataReader.NextResult()
         at Microsoft.Data.Sqlite.SqliteCommand.ExecuteReader(CommandBehavior behavior)
         at Microsoft.Data.Sqlite.SqliteCommand.ExecuteReaderAsync(CommandBehavior behavior, CancellationToken cancellationToken)
         at Microsoft.Data.Sqlite.SqliteCommand.ExecuteDbDataReaderAsync(CommandBehavior behavior, CancellationToken cancellationToken)
         at Microsoft.EntityFrameworkCore.Storage.RelationalCommand.ExecuteReaderAsync(RelationalCommandParameterObject parameterObject, CancellationToken cancellationToken)
         at Microsoft.EntityFrameworkCore.Storage.RelationalCommand.ExecuteReaderAsync(RelationalCommandParameterObject parameterObject, CancellationToken cancellationToken)
         at Microsoft.EntityFrameworkCore.Storage.RelationalCommand.ExecuteReaderAsync(RelationalCommandParameterObject parameterObject, CancellationToken cancellationToken)
         at Microsoft.EntityFrameworkCore.Query.Internal.QueryingEnumerable`1.AsyncEnumerator.InitializeReaderAsync(DbContext _, Boolean result, CancellationToken cancellationToken)
         at Microsoft.EntityFrameworkCore.Query.Internal.QueryingEnumerable`1.AsyncEnumerator.MoveNextAsync()
Microsoft.Data.Sqlite.SqliteException (0x80004005): SQLite Error 1: 'near "(": syntax error'.

@smitpatel
Copy link
Member

@maumar - This should be working now.

@maumar
Copy link
Contributor

maumar commented Jul 10, 2020

@smitpatel I'm using this to track the exception message improvement (PR coming soon)

maumar added a commit that referenced this issue Jul 10, 2020
Adding logic that checks for APPLY and throws meaningful exception
maumar added a commit that referenced this issue Jul 10, 2020
Adding logic that checks for APPLY and throws meaningful exception

Fixes #19178
@maumar maumar added the closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. label Jul 10, 2020
maumar added a commit that referenced this issue Jul 13, 2020
Adding logic that checks for APPLY and throws meaningful exception

Fixes #19178
maumar added a commit that referenced this issue Jul 13, 2020
Adding logic that checks for APPLY and throws meaningful exception

Fixes #19178
@maumar maumar closed this as completed in b4728bb Jul 13, 2020
@ajcvickers ajcvickers modified the milestones: 5.0.0, 5.0.0-preview8 Jul 14, 2020
@jons-aura
Copy link

I'm pretty sure I just hit this bug but it looks like -preview8 isn't out yet. The newest I see when checking "Include prerelease" in Nuget Package Manager is 5.0.0-preview7.20365.15. Is there an ETA on preview 8 so I can see if it resolves my issue?

@bricelam
Copy link
Contributor

Any day now. We aim for monthly previews.