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

ToQueryString on a split query only shows the first query #22080

Open
ajcvickers opened this issue Aug 15, 2020 · 15 comments
Open

ToQueryString on a split query only shows the first query #22080

ajcvickers opened this issue Aug 15, 2020 · 15 comments

Comments

@ajcvickers
Copy link
Contributor

For the query below, ToQueryString returns

SELECT [c].[Id]
FROM [Chunky] AS [c]
ORDER BY [c].[Id]

I would expect it to show all three queries that eventually get logged:

SELECT [c].[Id]
FROM [Chunky] AS [c]
ORDER BY [c].[Id]

SELECT [c0].[Id], [c0].[ChunkyId], [c].[Id]
FROM [Chunky] AS [c]
INNER JOIN [Cherry] AS [c0] ON [c].[Id] = [c0].[ChunkyId]
ORDER BY [c].[Id], [c0].[Id]

SELECT [m].[Id], [m].[CherryId], [c].[Id], [c0].[Id]
FROM [Chunky] AS [c]
INNER JOIN [Cherry] AS [c0] ON [c].[Id] = [c0].[ChunkyId]
INNER JOIN [Monkey] AS [m] ON [c0].[Id] = [m].[CherryId]
ORDER BY [c].[Id], [c0].[Id]

Code:

public class SomeDbContext : DbContext
{
    private static readonly ILoggerFactory
        Logger = LoggerFactory.Create(x => x.AddConsole());//.SetMinimumLevel(LogLevel.Debug));

    protected override void OnModelCreating(ModelBuilder modelBuilder)
    {
        modelBuilder.Entity<Chunky>();
    }
    
    protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
        => optionsBuilder
            .UseLoggerFactory(Logger)
            .EnableSensitiveDataLogging()
            .UseSqlServer(Your.SqlServerConnectionString);
}

public static class Program
{
    public static void Main()
    {
        using (var context = new SomeDbContext())
        {
            context.Database.EnsureDeleted();
            context.Database.EnsureCreated();

            context.Add(new Chunky
            {
                Cherries = new List<Cherry>
                {
                    new Cherry
                    {
                        Monkeys = new List<Monkey> {new Monkey()}
                    }
                }
            });

            context.SaveChanges();
        }

        using (var context = new SomeDbContext())
        {
            var queryable = context.Set<Chunky>().Include(e => e.Cherries).ThenInclude(e => e.Monkeys).AsSplitQuery();
            Console.WriteLine(queryable.ToQueryString());
            queryable.ToList();
        }
    }
}

public class Chunky
{
    public int Id { get; set; }
    public ICollection<Cherry> Cherries { get; set; }
}

public class Cherry
{
    public int Id { get; set; }
    public ICollection<Monkey> Monkeys { get; set; }
}

public class Monkey
{
    public int Id { get; set; }
}
@smitpatel
Copy link
Contributor

By design. Other queries are not executed till first query actually enumerates and find a row to materialize. This was the reason ToQueryString feature was not implemented pre-3.0

@ajcvickers
Copy link
Contributor Author

Note from triage: current plan is to do this when we implement query batching.

@Enngage
Copy link

Enngage commented Nov 15, 2020

Is there a way to get some unique hash from the IQueryable? I was using the sql text as a cache key, but this doesn't work when split queries are used. Or maybe there is some other alternative (other then using the single query)?

@ajcvickers
Copy link
Contributor Author

@Enngage What, exactly, do you want to hash? Should it be the same if the query has different parameter values? What about constants? What about when different IQueryables generate the same SQL? Hashing a query is a complex question; even internally to EF we do it more than one way.

Have you considered using query tags? https://docs.microsoft.com/en-us/ef/core/querying/tags

@Enngage
Copy link

Enngage commented Nov 16, 2020

@aaronralls Thank you for getting back to me so quickly!

My question is probably a bit ambiguous. I was using the entire SQL query as a cache key, but this is not possible to do with the split query option in 5.0 because the ToQueryString gets only first SQL and therefore it would lead to multiple IQueryable objects having the same cache key.

Based on the conversion above, it is not yet possible to get all SQL queries so I was hoping there is a way to get reasonably unique hash from the IQueryable object that I could use instead of the full SQL query.

Tagging is a cool feature, but I don't think it helps in this case as I wouldn't know how to generate the tag in the first place.

@Phylum123
Copy link

I just solved that issue by using ".Expression.ToString();" on the IQueryable. Gives you a string representation of the query code itself.

@Enngage
Copy link

Enngage commented Nov 24, 2020

@Phylum123, sadly that doesn't work because it also does not provide unique string representation of query. There are missing variables / columns and probably other things as well. For example one extremely simple query I just tried gives these results:

ToQueryString()

DECLARE @__teamLeaderUserId_0 int = 3;

SELECT [u].[Id]
FROM [Users] AS [u]
WHERE [u].[TeamLeaderId] = @__teamLeaderUserId_0

Expression.ToString()

[Microsoft.EntityFrameworkCore.Query.QueryRootExpression].Where(m => (m.TeamLeaderId == Convert(value(Service.Base.BaseService`1+<>c__DisplayClass129_0[Entity.User]).teamLeaderUserId, Nullable`1))).AsNoTracking()

If, in this case, I make another query with different value in teamLeaderUserId property (other than 3 in my sample) I get the same expression string.

@ajcvickers
Copy link
Contributor Author

@Enngage That's why I asked, "What, exactly, do you want to hash?" Because typically in EF Core, the same query but a different parameter value would be considered the same query when we hash. You can write an ExpressionVistor that will traverse the expression tree and generate whatever hash you want from it, but you'll have to decide for things like parameters whether a different value signifies a different hash or not.

@Phylum123
Copy link

Phylum123 commented Nov 25, 2020

@Enngage Here is the solution to your problem. I scouted around the internet, because I wanted the same solution and I beleive I found it. It essentially changes every constant or variable in the query to its actual value.

//This does NOT work if your condition is a method. For instance:
dbContext.Invoices.Where(i => i.InvoiceId <= MethodToGetMaxInvoiceId());

//Work around:
int maxInvoiceId = MethodToGetMaxInvoiceId();

dbContext.Invoices.Where(i => i.InvoiceId <= maxInvoiceId );

//However, cases like:
dbContext.Invoices.Where(i => i.InvoiceId <= SomeClass.SomeProperty);
//OR
dbContext.Invoices.Where(i => i.InvoiceId <= SomeLocalInt);

//Work just fine

Usage:

var yourQuery = //Query Code
var visitor = new ChangeVarsToLiteralsVisitor()
var changedExpression = visitor.Visit(yourQuery.Expression);
var newQueryString = changedExpression.ToString();

Code:

    public class ChangeVarsToLiteralsVisitor : ExpressionVisitor
    {
        protected override Expression VisitMember(MemberExpression memberExpression)
        {
            // Recurse down to see if we can simplify...
            var expression = Visit(memberExpression.Expression);

            // If we've ended up with a constant, and it's a property or a field,
            // we can simplify ourselves to a constant
            if (expression is ConstantExpression)
            {
                object container = ((ConstantExpression)expression).Value;
                var member = memberExpression.Member;

                if (member is FieldInfo)
                {
                    object value = ((FieldInfo)member).GetValue(container);
                    return Expression.Constant(value);
                }
                
                if (member is PropertyInfo)
                {
                    object value = ((PropertyInfo)member).GetValue(container, null);
                    return Expression.Constant(value);
                }

            }

            return base.VisitMember(memberExpression);
        }
}

Credit goes to the poster: Jon Skeet, who posted below the accepted answer
https://stackoverflow.com/questions/6998523/how-to-get-the-value-of-a-constantexpression-which-uses-a-local-variable

@domagojmedo

This comment was marked as resolved.

@roji

This comment was marked as resolved.

@Giorgi
Copy link
Contributor

Giorgi commented Feb 20, 2024

On a related noted, would it make sense to add CreateDbBatch method that creates a DbBatch with commands for split queries?

@roji
Copy link
Member

roji commented Feb 20, 2024

@Giorgi EF doesn't actually use the DbBatch abstraction yet (e.g. in SaveChanges, where we do batch but not with DbBatch); I've been wanting to do that for a while now but haven't yet found the time (#18990). Also, split query doesn't currently batch even without DbBatch, but rather does multiple roundtrips (#10878). Once those two things are done, we can definitely expose a CreateDbBatch like we expose CreateCommand, yeah.

@Giorgi
Copy link
Contributor

Giorgi commented Feb 20, 2024

Yes, I understand that it doesn't use DbBatch at the moment but because there is no way (in case of split queries) to programmatically get either query texts with ToQueryString or commands with CreateDbCommand why not expose CreateDbBatch that returns the commands executed by the current implementation?

@roji
Copy link
Member

roji commented Feb 20, 2024

We could; but the real work here is to expose the non-first query - exactly how one does that is less important. One important point is that currently, if the first query has no results, the second isn't even executed; so returning both queries (regardless of via DbBatch or not) wouldn't necessarily even be accurate. Once we do #10878, both queries would always be sent in any case, at which point it makes more sense to also return them in ToQueryString() or whatever.

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

No branches or pull requests

7 participants