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

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

Closed
OliverRC opened this issue Feb 20, 2024 · 15 comments
Closed
Assignees

Comments

@OliverRC
Copy link

OliverRC commented Feb 20, 2024

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

  • EF version: [EF Core v8.0.2]
  • EF Plus version: [EF Plus Core v8.102.1]
  • Database Server version: [SQL Server 2022 / MariaDB 11.2.3]
  • Database Provider version (NuGet): [Microsoft.EntityFrameworkCore.SqlServer v8.0.2 / Pomelo.EntityFrameworkCore.MySql v8.0.0]
@JonathanMagnan JonathanMagnan self-assigned this Feb 20, 2024
@JonathanMagnan
Copy link
Member

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
Copy link
Author

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
Copy link
Author

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

@JonathanMagnan
Copy link
Member

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
Copy link
Author

I have created a minimal example

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

@JonathanMagnan
Copy link
Member

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
Copy link
Author

OliverRC commented Mar 18, 2024

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
Copy link
Author

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
Copy link
Author

OliverRC commented Mar 27, 2024

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
Copy link
Member

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
Copy link
Member

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
Copy link
Author

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
Copy link
Member

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
Copy link
Author

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
Copy link
Member

Awesome,

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

Best Regards,

Jon

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants