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

Allow creating an Expression<Func<>> instead of a IQueryable Projection #733

Open
Tvde1 opened this issue Sep 7, 2023 · 7 comments
Open
Labels
enhancement New feature or request

Comments

@Tvde1
Copy link

Tvde1 commented Sep 7, 2023

It is possible to get a IQueryable<B> ProjectToB(IQueryable<A> query) which can be used as

context.ASet.ProjectToB().ToListAsync();

Is your feature request related to a problem? Please describe.

We have a generic class like follows:

class BaseRepository<TEntity, TModel>
{
    public BaseRepository(DbSet<TEntity> dbSet, Func<IQueryable<TEntity>, IQueryable<TModel>> projectToModel) { ... }

    public async Task<List<TModel>> GetAll()
    {
        return await _projectToModel(
                _dbSet.Where(x => x.SomeCondition)
            )
            .Where(x => x.IsSomething)
            .ToListAsync();
    }
}

class UserRepository : BaseRepository<UserEntity, UserModel>
{
    public UserRepository(DbContext c) : base(c.Users, UserMapper.ProjectToModel) {}
}

static partial class UserMapper
{
    public static partial IQueryable<UserModel> ProjectToModel(this IQueryable<UserEntity> q);
}

Describe the solution you'd like
I would prefer being able to create an Expression<Func<A, B>> and give it to my base repository.

class BaseRepository<TEntity, TModel>
{
    public BaseRepository(DbSet<TEntity> dbSet, Expression<Func<TEntity, TModel>> projectToModel) { ... }

    public async Task<List<TModel>> GetAll()
    {
        return await _dbSet
            .Where(x => x.SomeCondition)
            .Select(_projectToModel)
            .Where(x => x.IsSomething)
            .ToListAsync();
    }
}

class UserRepository : BaseRepository<UserEntity, UserModel>
{
    public UserRepository(DbContext c) : base(c.Users, UserMapper.CreateToModelProjection()) {}
}

static partial class UserMapper
{
    public static partial Expression<Func<UserModel, UserEntity>> CreateToModelProjection()
}

This will make the query cleaner and easier to read.

Describe alternatives you've considered
The aforementioned code works, but I'd prefer the new variant.

Additional context
I'm happy to hear your opinions on this matter.

@latonz
Copy link
Contributor

latonz commented Sep 8, 2023

I understand your use case and I think this could probably be helpful in other scenarios. However, as I think this is an edge case, it is not a priority for us. But I would be happy to accept a PR that implements this.

@latonz latonz added the enhancement New feature or request label Sep 8, 2023
@TimothyMakkison
Copy link
Collaborator

TimothyMakkison commented Sep 8, 2023

I'd be happy to do this, should be easy to add 🤞

@TimothyMakkison
Copy link
Collaborator

nvm might be a bit trickier than I thought, sorry

@Tvde1
Copy link
Author

Tvde1 commented Sep 9, 2023

The Expression must already be created in order to use IQueryable<>, so I figured it might not be too difficult. If I find some free time this month I might dive into the code

@TimothyMakkison
Copy link
Collaborator

TimothyMakkison commented Sep 9, 2023

Yeah, I think the logic is identical to IQueryable. I'd naively thought it would be a quick, simple check for expressions and then write some tests.
I hadn't registered that I'd need to edit the method extractor, maybe add a new object method builder and maybe change an interface.

It's possible to add, but not the quick, 15 minute change I wanted 😆

@ranma42
Copy link

ranma42 commented Nov 22, 2023

I started experimenting on this. I stumbled on some assumptions that currently seem to be baked in, but so far nothing major; in fact with some hacks I managed to get it somewhat working:

//HintName: Mapper.g.cs
// <auto-generated />
#nullable enable
public partial class Mapper
{
    private partial global::System.Linq.IQueryable<global::B> Map(global::System.Linq.IQueryable<global::A> source)
    {
        return System.Linq.Queryable.Select(source, MapToExpression(42));
    }

    private global::System.Linq.Expressions.Expression<global::System.Func<global::A, global::B>> MapToExpression(int source)
    {
#nullable disable
        return x => new global::B()
        {
            Parent = x.Parent != null ? new global::B() : default,
        };
#nullable enable
    }
}

I'll try experimenting some more, but I would like to know if this direction looks somewhat reasonable. I am aiming for something like:

//HintName: Mapper.g.cs
// <auto-generated />
#nullable enable
public partial class Mapper
{
    private partial global::System.Linq.IQueryable<global::B> Map(global::System.Linq.IQueryable<global::A> source)
    {
        return System.Linq.Queryable.Select(source, MapToExpression());
    }

    private global::System.Linq.Expressions.Expression<global::System.Func<global::A, global::B>> MapToExpression()
    {
#nullable disable
        return x => new global::B()
        {
            Parent = x.Parent != null ? new global::B() : default,
        };
#nullable enable
    }
}

(aka no hacked source). Later on I would like to memoize the expression, but that might not be really relevant (it would be a minor optimization for the allocations, definitely optional).

@latonz
Copy link
Contributor

latonz commented Nov 24, 2023

@ranma42 looks good so far 😊 Thank you for your efforts.
IMO the externalized expression method only needs to be generated if there is a partial method definition for it:

[Mapper]
public partial class Mapper
{
    public partial IQueryable<B> Map(this IQueryable<A> source);
}

should generate

[Mapper]
public partial class Mapper
{
    public partial global::System.Linq.IQueryable<global::B> Map(this global::System.Linq.IQueryable<global::A> source)
    {
#nullable disable
          return System.Linq.Queryable.Select(source, x => ...);
#nullable enable
    }
}

but

[Mapper]
public partial class Mapper
{
    public partial Expression<Func<A, B>> Mapper();
}

should generate

[Mapper]
public partial class Mapper
{
    public partial Expression<Func<A, B>> Mapper()
    {
#nullable disable
          return x => ...;
#nullable enable
    }
}

If both are declared, the queryable method could call the expression builder, but IMO this would be optional for a first version and could be an optimisation later on.

@ranma42 ranma42 mentioned this issue Nov 25, 2023
7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants