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

Support Linq GroupJoin Operator #1599

Closed
dapalmi opened this issue Nov 15, 2020 · 9 comments
Closed

Support Linq GroupJoin Operator #1599

dapalmi opened this issue Nov 15, 2020 · 9 comments
Labels

Comments

@dapalmi
Copy link
Contributor

dapalmi commented Nov 15, 2020

I am using the joining of related documents according to this documentation:
https://martendb.io/documentation/documents/querying/include/

Maybe I am using it wrong but there are a few things with this approach which don't work for me.

  1. It seems like the following example results in three queries to the database:
    var issues = query.Query<Issue>().Include<User>(x => x.AssigneeId, list).ToArray();
create temp table mt_temp_id_list1 as (
select id, CAST(d.data ->> 'AssigneeId' as uuid) as id1 from public.mt_doc_issue as d
); 
select d.data, d.mt_doc_type from public.mt_doc_user as d where id in (select id1 from mt_temp_id_list1);
select d.data from public.mt_doc_issue as d where id in (select id from mt_temp_id_list1);

This doesn't seem efficient.

  1. I don't think this solution would work in workflows where the IQueryable goes through different modules, since the resulting list of related documents will be lost.
    For example one method defines the base query:
public IQueryable<Issue> BaseQuery()
{
    var list = new List<User>();
    return _querySession.Query<Issue>().Include<User>(x => x.AssigneeId, list);
}

And another method takes care of other LINQ extensions like Where(), Skip(), Take(), Order() etc.:

public IQueryable<Issue> OrderQuery( IQueryable<Issue> issueQuery)
{
    return issueQuery.OrderBy(x => x.FirstName);
}

I understand that some don't like a non mapped attribute to be part of the aggregate, but with someone coming from Entity Framework, that is the known approach.

I tried to add for example a User attribute together with the foreign key in the aggregate and fill it with a query like this:
query.Query<Issue>("select i.data || jsonb_build_object('User', u.data) from mt_doc_issue as i, mt_doc_user as u where i.data ->> 'AssigneeId' = u.data ->> 'Id'");
But the raw SQL approach only returns an IReadOnlyList which is not ideal.

I would like to request following LINQ extension for Marten:
IMartenQueryable<T> Include<TInclude>(Expression<Func<T, object>> idSource);
Therefore someone can run a query like this without the callback...
_querySession.Query<Issue>().Include<User>(x => x.AssigneeId);
which would result in a query like this...
select i.data || jsonb_build_object('User', u.data) from mt_doc_issue as i, mt_doc_user as u where i.data ->> 'AssigneeId' = u.data ->> 'Id'
The additional field (User) and the table name can be determined via the <User> and the foreign key via the idSource parameter.

I am just asking because I saw that there is major development in the LINQ area anyway right now.

@jeremydmiller
Copy link
Member

@dapalmi:

"I don't think this solution would work in workflows where the IQueryable goes through different modules, since the resulting list of related documents will be lost." -- can you explain that more? I don't understand why that matters here. All the filtering is on the root Issue document, with User coming in just from the references.

"It seems like the following example results in three queries to the database" -- it's batched up. And there's no way to pull off the Include() with a one to many relationship without multiple recordsets. Doing the JOIN syntax when we were limited to 1 to 1 had all kind of issues on the code side. All in all, I don't see this making a significant perf difference. The big change is that it actually works, we support one to many like folks have wanted for years, and the internals are simpler.

Just not really sure what you're wanting to happen here. Are you concerned about the SQL being generated, or wanting some other additional functionality?

@dapalmi
Copy link
Contributor Author

dapalmi commented Nov 17, 2020

@jeremydmiller Thanks for the quick response. I understand your approach. I basically would like to build together an IQueryable chain via several modules which only hits the database when executing e.g. a ToList() at the very end. Also I don't like the related documents being separate from the actual query result. I don't know how I would marry those two results again in a generic manner in my project (Sorry. I hope you understand my thinking process. English is not my first language). But maybe , like you mentioned, the Join(), GroupJoin() feature is more what I have in mind. Is that feature on the roadmap?

@jeremydmiller
Copy link
Member

"which only hits the database when executing e.g. a ToList() at the very end" -- All the queries to the database happen when you call ToList() and not when you call Include(), so it's one database round trip that brings back multiple recordsets.

We don't have the GroupJoin feature in the backlog right now, but I think it makes sense to do as a usability thing. If we're gonna do it, I'd rather do it soon while the Linq internals are fresh in my mind. Would you mind adding that issue to the backlog?

@dapalmi
Copy link
Contributor Author

dapalmi commented Nov 17, 2020

Thanks a lot. I saw that there's been work done with the Linq internals, which I really appreciate. What do you mean with "adding that issue to the backlog"?

@jeremydmiller
Copy link
Member

Create a new GitHub issue, or change this one to "Support Linq GroupJoin Operator". But man, that looks like an ugly syntax: https://docs.microsoft.com/en-us/dotnet/api/system.linq.enumerable.groupjoin?view=net-5.0

@dapalmi
Copy link
Contributor Author

dapalmi commented Nov 17, 2020

"But man, that looks like an ugly syntax" - That's true. Maybe since Marten is doing it's own thing with the Include syntax anyway, maybe you can do another Include extension, which acts as a prettier GroupJoin()?

@dapalmi dapalmi changed the title Including Related Documents Support Linq GroupJoin Operator Nov 17, 2020
@jeremydmiller
Copy link
Member

Possibly, and that's already in the backlog from way back. Something like "multi document queries" I believe.

@dapalmi
Copy link
Contributor Author

dapalmi commented Nov 17, 2020

This one looks related: #1357

@jeremydmiller jeremydmiller added this to the 7.0.0 milestone Sep 8, 2023
@jeremydmiller jeremydmiller removed this from the 7.0.0 milestone Dec 5, 2023
@jeremydmiller
Copy link
Member

I think this is very unlikely to ever happen.

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

No branches or pull requests

2 participants