Make AsSubQuery public

[InspiringCode]

Hi @virzak,

is there any chance you could make DbFunctionsExtensions.AsSubQuery public?

Unfortunately, EF Core has some limitations (and there will probably always be some limitations) in regard to the way it generates SQL. I am writing a query in my current project, where I need to filter on quite a few subselect projections, which generates horribly inefficient SQL at the time (see this EF core issue).

I was trying to reimplement the AsSubQuery-functionality as a separate extension, but then I can't use your efcore-extensions anymore because we both would replace the NpgsqlQueryableMethodTranslatingExpressionVisitorFactory which doesn't work.

Since I really need this functionality (other attempts to optimize the query have failed), I am a bit out of look. What do you think?

[virzak]

This was explicitly recommended against by the ef core team.

Can you provide minimum repro of where the latest prerelease fails?

[InspiringCode]

You can take the simple example from the first post of the linked issue.

What the EF Core team meant was, that it would not be good to include AsSubQuery in the official EF Core package because the described issue should ideally be solved by the query processor.

But since there is not even a milestone yet when the query generator will be redesigned, we need a solution until then (which might be EF11, EF12 or even later).

And AsSubQuery is the only solution (workaround) I can think of.

[virzak]

Ok, I'll look into it on the weekend

[InspiringCode]

var q1 = ctx
    .Blogs
    .Select(p => new
    {
        p.Name,
        PostCount = p.Posts.Count
    })
    .AsSubQuery()
    .Where(p => p.PostCount > 5)
    .ToList();

Adding AsSubQuery to the LINQ statement above fixes the issue.

[InspiringCode]

@virzak Any news on this? Is there still a good argument left to not make this method public?

[virzak]

@InspiringCode apologies, haven't had the chance to look into this. This weekend. Promise.

In the meantime, if you can't wait, you can easily make it public yourself and throw the packages on myget or something. That's what I do while waiting for other people to resolve issues.

[InspiringCode]

@virzak Any updates on this?

[virzak]

Really sorry, been extremely busy to do anything. Broke my promise too.

Have you made a temp myget package just for yourself?

[InspiringCode]

@virzak I worked on a different project in the meantime. Is there any update on this? I mean it's just a single keyword that would have to be changed (namely internal to public). Would be really helpful if you could do this.

[virzak]

@InspiringCode, that single keyword could mean breaking change if I wanted to remove it later.

This project is set up to upload to myget. You can change from internal to public, pack the package and upload to myget.

efcore-extensions/.github/workflows/build.yml

Lines 139 to 143 in 799e9db

	- name: Push to MyGet
	if: ${{ env.MYGET_PUSH_KEY != '' }}
	run: dotnet nuget push packages\*.nupkg -s https://www.myget.org/F/zomp-efcore-extensions/api/v3/index.json -k ${{ secrets.MYGET_PUSH_KEY }}
	env:
	MYGET_PUSH_KEY: ${{ secrets.MYGET_PUSH_KEY }}

That's the way to go about it. I'll look at this issue in November at the latest.

[sobiemir]

@InspiringCode
If you still need this, you can add this piece of code to your project:

public static class DbFunctionsExtensions
{
    public static IQueryable<TEntity> AsSubQuery<TEntity>(this IQueryable<TEntity> source)
    {
        ArgumentNullException.ThrowIfNull(source);
        return source.Provider.CreateQuery<TEntity>(
            Expression.Call(
                null,
                DbFunctionsExtensions.AsSubQueryMethod.MakeGenericMethod(typeof(TEntity)),
                source.Expression));
    }
    
    internal static readonly MethodInfo AsSubQueryMethod = typeof(DbFunctionsExtensions)
        .GetMethod(nameof(DbFunctionsExtensions.AsSubQuery))
            ?? throw new NullReferenceException();
}

Now you can use AsSubQuery on IQueryable.

The answer to the question "how it works?" is here:
https://github.com/zompinc/efcore-extensions/blob/master/src/Zomp.EFCore.WindowFunctions/Extensions/SubQueryProcessor.cs

To be more specific, here:

if (methodCallExpression.Method.Name == nameof(DbFunctionsExtensions.AsSubQuery))

[Jameak]

If anyone comes across this issue and needs AsSubQuery for scenarios involving ToList-calls on Select projections by using @sobiemir's workaround but get the following exception message:

System.ArgumentException : Expression of type 'System.Collections.Generic.List`1[<>f__AnonymousType8`2[Zomp.EFCore.Testing.TestRow,System.Int64]]' cannot be used for parameter of type 'System.Linq.IQueryable`1[<>f__AnonymousType8`2[Zomp.EFCore.Testing.TestRow,System.Int64]]' of method 'System.Linq.IQueryable`1[<>f__AnonymousType8`2[Zomp.EFCore.Testing.TestRow,System.Int64]] AsSubQuery[<>f__AnonymousType8`2](System.Linq.IQueryable`1[<>f__AnonymousType8`2[Zomp.EFCore.Testing.TestRow,System.Int64]])' (Parameter 'arg0')

Then here is a solution.

Given the following two test-cases (using this libraries test setup):

[Fact]
public void SelectWithSubQueryToList()
{
    var querySub = DbContext.TestRows
        .Select(e => new
        {
            Entity = e,
            Rank = EF.Functions.Rank(EF.Functions.Over().PartitionBy(e.Col1).OrderByDescending(e.Col1)),
        })
        .AsSubQuery();

    var query = DbContext.TestRows
        .Select(outer => new
        {
            Id = outer.Id,
            SubRows = querySub.ToList(), // <-- This works out-of-the-box
        });

    var queryStr = query.ToQueryString();
}

[Fact]
public void SelectWithSubQueryToListWithWhereReferencingOuterParameter()
{
    var querySub = DbContext.TestRows
        .Select(e => new
        {
            Entity = e,
            Rank = EF.Functions.Rank(EF.Functions.Over().PartitionBy(e.Col1).OrderByDescending(e.Col1)),
        })
        .AsSubQuery();

    var query = DbContext.TestRows
        .Select(outer => new
        {
            Id = outer.Id,
            SubRows = querySub.Where(inner => inner.Entity.Id == outer.Id).ToList(), // <-- This fails
        });

    var queryStr = query.ToQueryString();
}

The test SelectWithSubQueryToList works while the test SelectWithSubQueryToListWithWhereReferencingOuterParameter crashes with the abovementioned exception.

The cause of this is that the WindowFunctionsNpgsqlQueryableMethodTranslatingExpressionVisitor class hasn't overridden the CreateSubqueryVisitor method, and when EF calls this method to re-instantiate the visitor for subquery-handling it ends up hitting the NpgsqlQueryableMethodTranslatingExpressionVisitor implementation which instantiates a new NpgsqlQueryableMethodTranslatingExpressionVisitor instead. This doesn't have the Zomp-specific AsSubQuery handling logic and causes the issue.

One way to fix this from your own codebase as a user of this library is to do the following. This assumes Npgsql and EFCore 8. Adjust as needed.

// Visitor implementation which overrides CreateSubqueryVisitor
public class FixedWindowFunctionsNpgsqlQueryableMethodTranslatingExpressionVisitor : NpgsqlQueryableMethodTranslatingExpressionVisitor
{
    public FixedWindowFunctionsNpgsqlQueryableMethodTranslatingExpressionVisitor(
        QueryableMethodTranslatingExpressionVisitorDependencies dependencies,
        RelationalQueryableMethodTranslatingExpressionVisitorDependencies relationalDependencies,
        QueryCompilationContext queryCompilationContext)
        : base(dependencies, relationalDependencies, queryCompilationContext)
    {
    }

    protected FixedWindowFunctionsNpgsqlQueryableMethodTranslatingExpressionVisitor(
        FixedWindowFunctionsNpgsqlQueryableMethodTranslatingExpressionVisitor parentVisitor)
        : base(parentVisitor)
    {
    }

    protected override QueryableMethodTranslatingExpressionVisitor CreateSubqueryVisitor()
    {
        // This MUST call the constructor that takes a parentVisitor, because that sets a "_subquery = true" flag inside npgsql, which changes behavior in ef-core.
        return new FixedWindowFunctionsNpgsqlQueryableMethodTranslatingExpressionVisitor(this);
    }

    // This is identical to the original WindowFunctionsNpgsqlQueryableMethodTranslatingExpressionVisitor implementation
    protected override Expression VisitMethodCall(MethodCallExpression methodCallExpression) =>
        SubQueryProcessor.ProcessSubQuery(this, methodCallExpression) ?? base.VisitMethodCall(methodCallExpression);
}

// Factory implementation that instantiates our visitor class
public class FixedWindowFunctionsNpgsqlQueryableMethodTranslatingExpressionVisitorFactory : NpgsqlQueryableMethodTranslatingExpressionVisitorFactory
{
    private readonly QueryableMethodTranslatingExpressionVisitorDependencies _dependencies;
    private readonly RelationalQueryableMethodTranslatingExpressionVisitorDependencies _relationalDependencies;

    public FixedWindowFunctionsNpgsqlQueryableMethodTranslatingExpressionVisitorFactory(
        QueryableMethodTranslatingExpressionVisitorDependencies dependencies,
        RelationalQueryableMethodTranslatingExpressionVisitorDependencies relationalDependencies)
        : base(dependencies, relationalDependencies)
    {
        _dependencies = dependencies;
        _relationalDependencies = relationalDependencies;
    }

    public override QueryableMethodTranslatingExpressionVisitor Create(QueryCompilationContext queryCompilationContext)
    {
        return new FixedWindowFunctionsNpgsqlQueryableMethodTranslatingExpressionVisitor(_dependencies, _relationalDependencies, queryCompilationContext);
    }
}

// New extension method to overwrite the Zomp visitor
public static class NpgsqlDbContextOptionsBuilderExtensions
{
    public static NpgsqlDbContextOptionsBuilder UseFixedWindowFunctions(this NpgsqlDbContextOptionsBuilder builder) => builder.AddOrUpdateExtension();

    private static NpgsqlDbContextOptionsBuilder AddOrUpdateExtension(
        this NpgsqlDbContextOptionsBuilder builder)
    {
        ArgumentNullException.ThrowIfNull(builder);

        builder.UseWindowFunctions();

        var coreOptionsBuilder = ((IRelationalDbContextOptionsBuilderInfrastructure)builder).OptionsBuilder;
        _ = coreOptionsBuilder.ReplaceService<IQueryableMethodTranslatingExpressionVisitorFactory, FixedWindowFunctionsNpgsqlQueryableMethodTranslatingExpressionVisitorFactory>();

        return builder;
    }
}

Now you can replace any usages of UseWindowFunctions in your codebase with UseFixedWindowFunctions and the problem should be solved.
Subclassing WindowFunctionsNpgsqlQueryableMethodTranslatingExpressionVisitor unfortunately isn't possible from a library-user codebase because the class hasn't declared the parentVisitor constructor-variant which is necessary to make this work.

Implementing this change directly in WindowFunctionsNpgsqlQueryableMethodTranslatingExpressionVisitor in the Zomp library code makes the SelectWithSubQueryToListWithWhereReferencingOuterParameter test-case no longer crash and generates the SQL I would expect. All the other Npgsql-specific Zomp test-cases still pass after this change, though I haven't verified that they generate the exact same SQL.

@virzak I haven't hit this issue without explicit usage of AsSubQuery, and since AsSubQuery isn't part of the public api of this library this probably isn't technically a bug. If you want to go ahead and implement the fix for all supported backends and EFCore versions then feel free. I unfortunately wont have time to contribute a PR with the fix.

[InspiringCode]

@virzak If you find some time, I think adding this small fix from @Jameak would be really useful for others too.

[virzak]

@InspiringCode @Jameak Will look at it this weekend. Sorry, been extremely busy with work.