zzzprojects / EntityFramework-Plus

Entity Framework Plus extends your DbContext with must-haves features: Include Filter, Auditing, Caching, Query Future, Batch Delete, Batch Update, and more
https://entityframework-plus.net/
MIT License
2.27k stars 318 forks source link

Query Future does not seem to work as expected. Two DB calls being made #793

Closed OliverRC closed 6 months ago

OliverRC commented 9 months ago

1. Description

I followed the documentation on Query Future in order to optimize my pagination queries. However, I am not able to get the desired behavior to work.

The expectation was that both queries would be executed together in a single execute step.

2. Example code

app.MapGet("/people", (AppDbContext dbContext, 
    [FromQuery] int page = 1, 
    [FromQuery] int pageSize = 10) =>
{
    var query = dbContext.People.AsQueryable();

    var futureItems = query.Skip((page - 1) * pageSize)
        .Take(pageSize)
        .Future();

    var futureCount = query.DeferredCount().FutureValue();

    var items = futureItems.ToList();
    var count = futureCount.Value;

    return new { count, items };
});

3. Issue

However, I see two separate database calls and executions that waterfall one after the other.

I have tested with both ~MSSQL~ and MariaDB (MySql)

To aid in debugging, I added .NET Aspire to a minimal example, and the following is the database call trace.

MariaDB image

~MSSQL~ image image

As you can see, despite following the documentation, two separate calls are still being made.

4. Project

I've tried to produce a minimal example. Here is the GitHub project. Might require the latest VS2022 preview for aspire preview 3 bits.

https://github.com/OliverRC/QueryFutureIssue

Further technical details

JonathanMagnan commented 9 months ago

Hello @OliverRC ,

Thank you for reporting. Looking at the code, Pomelo has been disabled for the QueryFuture feature. However, Pomelo was disabled years ago, so we will look for the reason, as perhaps we had to do it when we started this library with EF Core 2 and since then, a lot of time has passed.

Best Regards,

Jon

OliverRC commented 9 months ago

Hey,

I was wondering if maybe it was a MySql related issue.

This is why I tried MSSQL hoping for a better result. Unfortunately, as you can see it doesn't seem to work there either.

It would be a real pity if the Plus library doesn't work with Pomelo as your Extensions library is working perfectly with Pomelo (MySql)

OliverRC commented 9 months ago

Do you have any feedback as this feature did not seem to work on MSSQL which is EF's primary database driver?

JonathanMagnan commented 9 months ago

Hello @OliverRC ,

It should already work on MSSQL. If you have an issue with this provider, could you create a runnable project with the issue? It doesn’t need to be your project, just a new solution with the minimum code to reproduce the issue.

However, it was indeed disabled for Pomelo as it caused an issue in EF Core 3. I still have to review the code made by my developer, but if accepted, we will start to support Pomelo for EF Core 5+

OliverRC commented 9 months ago

I have created a minimal example

Link is https://github.com/OliverRC/QueryFutureIssue

JonathanMagnan commented 9 months ago

Hello @OliverRC ,

It worked great on our side for SQL Server:

exec sp_executesql N'-- EF+ Query Future: 1 of 2
SELECT [p].[Id], [p].[Email], [p].[Name], [p].[ShirtSize]
FROM [People] AS [p]
ORDER BY (SELECT 1)
OFFSET @Z_1___p_0 ROWS FETCH NEXT @Z_1___p_1 ROWS ONLY
;

-- EF+ Query Future: 2 of 2
SELECT COUNT(*)
FROM [People] AS [p]
;

',N'@Z_1___p_0 int,@Z_1___p_1 int',@Z_1___p_0=0,@Z_1___p_1=10

Only 1 SQL has been executed.

OliverRC commented 8 months ago

Hi @JonathanMagnan

[1] I ran SQL Server Profiler on my test project and I can verify I am seeing the same results as you from the trace

exec sp_executesql N'-- EF+ Query Future: 1 of 2
SELECT [p].[Id], [p].[Email], [p].[Name], [p].[ShirtSize]
FROM [People] AS [p]
ORDER BY (SELECT 1)
OFFSET @Z_1___p_0 ROWS FETCH NEXT @Z_1___p_1 ROWS ONLY
;

-- EF+ Query Future: 2 of 2
SELECT COUNT(*)
FROM [People] AS [p]
;

',N'@Z_1___p_0 int,@Z_1___p_1 int',@Z_1___p_0=0,@Z_1___p_1=10

So it look like the MSSQL integration is indeed working. However, the way EF "sees" it's execution threw me off because in the console it still "looks" to be two separate commands See below

image

I am going to dig a little deeper into tracing/profiling MariaDB/MySQL.

[2] Regarding Pomelo support, I would be interested in this. We are paid customers to your EF6 and EF Core Bulk Extensions library which DOES support Pomelo and so just assumed the EntityFramework-Plus library would as it's from the same vendor and EF wizards :)

OliverRC commented 8 months ago

Looking at the MariaDB log:

SET GLOBAL general_log=1;  
SET GLOBAL log_output='TABLE';  

SELECT * FROM mysql.general_log;

It looks like two separate queries

event_time user_host thread_id server_id command_type argument
2024-03-18 13:56:32.606767 [root] @ [172.17.0.1] 12 1 Connect root@172.17.0.1 on appdb using TCP/IP
2024-03-18 13:56:32.607276 root[root] @ [172.17.0.1] 12 1 Query SET NAMES utf8mb4
2024-03-18 13:56:32.608268 root[root] @ [172.17.0.1] 12 1 Query SELECT `p`.`Id`, `p`.`Email`, `p`.`Name`, `p`.`ShirtSize`
FROM `People` AS `p`
LIMIT 10 OFFSET 0
2024-03-18 13:56:32.609626 root[root] @ [172.17.0.1] 12 1 Query SET NAMES utf8mb4
2024-03-18 13:56:32.610149 root[root] @ [172.17.0.1] 12 1 Query SELECT COUNT(*)
FROM `People` AS `p`
OliverRC commented 8 months ago

I am following up as I haven't had any responses to my latest findings. TLDR:

[1] EF Core seems to log the queries as two separate commands even though Futures work on MSSQL?

[2] We are a paid customer of EF Extensions which does support Pomelo any reason EF-Plus does not?

JonathanMagnan commented 8 months ago

Hello @OliverRC ,

My bad, this request was a little bit lost on our side.

We were waiting for the answer about the SQL Server provided, which you confirmed last week was working.

The fix for Pomelo in the future has been merged with our master. We will support it only starting from EF Core 5+ (EF Core 2 and EF Core 3 still have an issue).

However, the new version will only be released in around 2 or 3 weeks (Should be April 9 or April 16).

Best Regards,

Jon

JonathanMagnan commented 7 months ago

Hello @OliverRC ,

The v8.102.2.2 is now available.

Could you try it and let us know if everything is now working as expected?

Best Regards,

Jon

OliverRC commented 7 months ago

Hey @JonathanMagnan so I've done some testing.

[1] It definitely looks like version 8.102.2.2 is now collapsing the SQL statements

-- EF+ Query Future: 1 of 2
SELECT `p`.`Id`, `p`.`Email`, `p`.`Name`, `p`.`ShirtSize`
FROM `People` AS `p`
ORDER BY `p`.`Id`
LIMIT @Z_1___p_1 OFFSET @Z_1___p_0
;

-- EF+ Query Future: 2 of 2
SELECT COUNT(*)
FROM `People` AS `p`
;

Performance locally seems worse

That is great. However I am noticing what seems to be something odd.

On the newer version, even though the SQL is merged, the actual performance (albeit on a very simple example locally) seems slower.

image

The old version is faster with the two separate calls.

image

Benchmarks

I was going to run benchmarks however I think that the version I was using 8.102.1 has been pulled from Nuget, and so I am no longer able to run a comparison using BenchmarkDotNet

JonathanMagnan commented 7 months ago

Hello @OliverRC ,

You can turn off query batch with the following options: https://github.com/zzzprojects/EntityFramework-Plus/blob/master/src/shared/Z.EF.Plus.QueryFuture.Shared/QueryFutureManager.cs#L54

Version are always available on NuGet, just hidden: https://www.nuget.org/packages/Z.EntityFramework.Plus.EFCore/8.102.1

As for the performance issue, there is not much we can do here. We know that some providers take longer to create the query plan as the query gets more complex.

Best Regards,

Jon

OliverRC commented 7 months ago

Thanks for the guidance on the configuration.

I'd like to run the updated version with Future on our actual codebase and maybe compare performance in both modes to see what suits our performance profile best.

As for performance, I guess that is interesting that it falls to the provider building the query plan.

I guess that's what that "empty" space is on the trace.

Thank you for your detailed assistance, it is greatly appreciated.

JonathanMagnan commented 7 months ago

Awesome,

Let me know about your results; I would be interested to hear your thoughts.

Best Regards,

Jon