Improve final DB query "size"

[ApocDev]

The current implementation of the "Query" fields in this library are potentially very heavy.

Each query that eventually gets built is the equivalent of a SELECT * FROM mytable;, instead of leveraging IQueryable and expression building to generate a "thin" select based on requested fields.

This is bad news when tables can contain very large fields (think news articles, or large file data during a "summary" query).

Solution?

Build the select expressions dynamically via expression building to handle only selecting required fields from the database.

This means that the Navigation query system will need to basically be "rewritten" to account for the total change from using .Include("Navigation") to something more akin to new { Navigation = new Navigation { ... } }.

Possibly this can be optional on a per-query basis?

We should be able to cache the select queries in some way based on the incoming query structure, so we only have a one-time build performance hit. (Though, larger query sets, could cause potential memory consumption issues?)

I have already written code locally to handle the base "select only this table" builder, when navigation properties are not used. This vastly speeds up DB access, and dramatically lowers bandwidth requirements at the same time.

Only question is, where/how would we approach the navigation property issue?

[SimonCropp]

i was aware of this problem, but since it didnt effect my data model i put it in the "too hard for now" basket.

I have been planning on cracking open the EFCore codebase and see if i can create a new query API that can be programmatically constructed based on strings rather than expressions. The current EFCore API is purely based on expressions, which makes sense for top level code that has access to all the types. but it is horrible for anyone writing extensions. this has hit me a few times on diff projects and i was hoping to come up with a re-usable solution that doesnt involve the complexity of programmatically creating expressions

[ApocDev]

As far as creating the expressions for simple properties, that part is trivial, it's just a MemberInit expression, with some Binds.

Example: https://gist.github.com/ApocDev/5e6001d18083e78d0f852885ee99e797

The real work comes when you need to "subquery". I have run into an issue where I'm not sure how to get the "parent" from the selected field, and continue building the sub-select expressions to build up navigation fields, or collections.

Lastly, custom resolvers are.... difficult.

Possibly https://github.com/StefH/System.Linq.Dynamic.Core would be of help to dynamically build the queries more easily?

[Shane32]

I'd suggest using reflection to create an expression that is based off all the selected fields, using the resolve expression from the Field() method to populate the anonymous type (or object[] array, etc). Then it can be passed to EF as a single Select(). The issue is that some resolve functions MUST run client side (vs in SQL server). EF Core currently allows for automatic detection of what needs to run client-side, and EF Core 3.0 will also allow code in the Select expression to run client side. So this should be feasible.

This would be way cool, but it does seem like a lot of work. It would be most particularly useful when adding specialized indexes for tables which have included columns. (Note: creating an index with included columns natively are a new feature of EF Core 2.2, although can be done in prior versions manually)

Here's another idea: what if you added an includedFields property to Field (call it AddField), and then upon execution, generate a Select function with only those fields populated? Then the resolve functions can still run client side. Sorta a middle ground approach that would likely be easier to implement and more reliable than passing all the resolve functions to EF. Still have to figure out the subqueries....

[fisapras]

@SimonCropp are you thinking about GraphQL to SQL parser?

[SimonCropp]

@fisapras nope. more like a "graphql to (EF compat) expression parser"

[famblard86]

On this topic I think try to use the same approach of Automapper to create the projection expression (thought for Odata) can give some result. If I have some time, I'll test.

[SimonCropp]

anyone want to have a go at a PR for this?

[Shane32]

I can’t, but I will say that it is possible to dynamically create select arguments which create dictionaries for the selected members, but it would require a rewrite of the entire project except the argument parsing code.

Also, performance with EF 3.0 is poor with certain queries due to cartesian results being returned. This is easier patched with the existing codebase than with dynamically generated queries, due to EF auto stitching tracked objects. No patching is necessary with EF 2.x because it splits up queries as necessary, albeit sometimes quite inefficiently.

[SimonCropp]

ok closing this for now. always happy to consider PRs that improve the query size

[thesobercoder]

I tried using System.Linq.Dynamic.Core in a non GraphQL.EntityFramework project and it works fine, just that if PK field is not selected in the graph then there is no way to query based on that. But nonetheless this would be an absolutely fantastic feature to have. Enterprise data that I work with has 300 columns in a single table and no way would I want to get them all just for seeing 2 or 3 fields.

[SimonCropp]

@thesobercoder happy to consider a PR.

[Shane32]

It can be written without System.Linq.Dynamic.Core btw. It's just a matter of storing all the expressions from each EfField into metadata and then splicing them together into a select expression for Linq to process. Since anonymous types cannot be constructed at runtime (that I'm aware of), you need to create a select array or select dictionary expression. I suggest a dictionary with string keys representing the field names. (The resulting SQL does not contain these string keys.) You will find numerous bugs with EF 2.x's handling of complex queries, but EF 3 works fine. However, EF 3 will return cartesian results when pulling from multiple one-to-many subtables. This cannot be avoided easily. Until EF 5 drops with multi-query support, you'll need to use dataloaders to improve performance. (Same issue with EF3 exists with existing codebase, as it is an EF 3 issue.)

[Intecpsp]

@ApocDev did you get your gist working? I'm looking into this in my project and that looks pretty sound.

[douglasg14b]

Any movement on this from the folks in the room closely familiar with EF Core's workings? This hit me hard in testing, pulling a single column from a few million rows on a wide table definitely made things slow down...

Can the queries that this generates be setup for projection in some manner? Or failing that, customized for certain tables to manually avoid this sort of issue?

[Shane32]

If you want to work on this feature, I can help if you have questions. I guarantee you that it is possible to add this feature in the manner I described above. (I am unfamiliar with the Automapper technique or if that even works.)

[douglasg14b]

Honestly, between not being deeply familiar with EF Core's innards, and this projects code being somewhat obfuscated I wouldn't want to work on it. I could dig into EF Core, but struggling through this codebase is my hard stop, I've already been referencing it to sort out some of it's ambiguity and it's been... frustrating.

(Not to rag on you Simon! Just calling out why it is undesirable, a clean and commented codebase is MUCH easier to contribute to)

Aside from that projection isn't just an automapper thing, It's a native EF thing!

var authors = context.Authors
    .Select(author => new
    {
    	Id = author.AuthorId,
    	FullName = author.FirstName + " " + author.LastName,
    }).ToList();

Will only generate the query for the AuthorId, FirstName , and LastName, rather than pulling all columns. If this can currently be done for certain tables/entities with this lib, that would work out fine

Edit: This can also project to say a DTO rather than an anonymous type.

[Shane32]

It's not so much about knowing EF Core as it is working with expression trees. Once you complete the code it would work just as well with linq2db as it would with EF, as it's just iterating through the requested graph and building a Select expression from it for the appropriate table. But working with expressions requires an iron will.

[douglasg14b]

@Shane32 See my updated comment. EF already does this for you, that may influence the proposed solution?

[Shane32]

The trick is getting EF to know what fields you want and don't want.

If your graphs only use a small subset of the available database fields, you could just write a dbcontext that only maps the fields you actually use. This would make it so that while it still returns all the fields for any table, "all" is only a small subset that you're interested in.

[Shane32]

Another thing is if you skip the ability to use "Includes", a main feature of this library, and instead always rely on dataloaders, then with a lot less work you can probably write some code that examines what fields are required and pulls those fields only. I have not thought it all the way through, but it should be a lot less complex than a complete overhaul that builds expression trees.

[douglasg14b]

That's a workable workaround.

As for getting EF to know which fields you want, could we piggy back off of the behavior that already exists with projection? I have never delved into this, can anonymous types be defined at runtime that are selected?

Good callout that piggying off that pmuch eliminates the ability to perform include. Though if memory serves me right, you can pass an array of strings that are property paths and it will include those in the projection.

[Shane32]

The trick is getting EF to know what fields you want and don't want.

This isn't EF specific, mind you -- EF simply looks at the .Select() expression just before the .ToList() or similar that serializes results. Any ORM that uses LINQ will operate similarly.

[Shane32]

Anonymous types cannot be defined at runtime. (Well, it's possible, but only by loading a dynamically-built assembly at runtime. Not for the faint of heart.)

For my implementation, I have it return a dictionary with keys as graphql field names.

So when querying:

products {
  id
  name
}

It produces the following select expression:

var results = await db.Products.Select(x => new Dictionary<string, object>() {
    { "id", x.Id },
    { "name", x.Name }
}).ToListAsync();

And then your field resolver pulls from the dictionary like this:

Field("id", resolve: context => context.Source["id"]);

Of course this all gets built automatically, but that's what it boils down to

[Shane32]

If you requested id, name, and also color, then it would build that into the dictionary (inside the expression), and when the field resolver executed, it would be able to find a color dictionary entry and return it.

[Shane32]

It sounds like a lot of extra cpu cycles happening. That's true, but it can save a lot of time if you are using a lot of "wide" tables from your data source. A bit of a balance game here. Good news is that the strings are constants, so they are never encoded more than once - it's just copying references to them all over the place.

[Shane32]

I would like to use arrays instead of dictionaries, but then the field resolvers would need to know which index in the array is necessary to return the correct data...

[douglasg14b]

I didn't consider just projecting to a dictionary, that's a pretty obvious stepping stone!

As for extra cycles, wouldn't be a problem if it's an optional feature, use it when you need otherwise it operates like normal.

Where this loses me is how to actually define the entries for the dictionary at runtime, I've dabbled in expression trees for dynamically building queries before, but it was years ago, and I distinctly remember it being a nightmare.

[Shane32]

And that's why it took me two months of intense effort to write our implementation...

[Shane32]

Hmm...re-reading the original issue, it seems I was just having a problem with navigation entries. Well, test it and see, I guess!

[SimonCropp]

@douglasg14b

I could dig into EF Core, but struggling through this codebase is my hard stop, I've already been referencing it to sort out some of it's ambiguity and it's been... frustrating. (Not to rag on you Simon! Just calling out why it is undesirable, a clean and commented codebase is MUCH easier to contribute to)

A new codebase often seems confusing when u first look at it. the problem with this project is it deal primarily with reflection and expression construction. it is not code that equates well with simplicity and ease of readability. When i find a new codebase new or daunting, i generally assume it is due to my lack of context, not a fault of the code. But that is just me. If you find ways to make it more readable, i am happy to consider pull requests.

Also would your comment have been any less effective by omitting your critical options of the code quality

As for commented codebase, i prefer the approach of "comment when doing something unexpected, or when the it is not clear what some code is doing". the problem is, when working on a codebase alone, and u wrote all the code, what is "not clear" is subjective, ie if i wrote some code it is difficult for me to tell when it will not be clear to someone else. And again, i am happy to accept pull requests.

[Shane32]

Good point @SimonCropp -- you can easily spend as much time commenting as you do writing the code. The more you know the codebase, the less comments are necessary. So it really becomes quite difficult to know how many comments to put in.

I find that having xml comments on public members are key to helping others understand the code -- sometimes even for private members. I've been working on the graphql-dotnet repo to help add xml comments there, but it has a long way to go. After that, so long as classes, methods and variables are named logically, as @SimonCropp says, limited comments usually are adequate, understanding of course that there will always be a learning curve for someone new to the codebase.

[douglasg14b]

I've separated my reply to another thread to keep this topic on track. @SimonCropp to make it clear I'm showing care and criticality for this because I want to contribute to this codebase. Not because I get off on being critical.

[douglasg14b]

Preface: I know very little about GraphQL.Net I decided to try and switch from REST endpoint hell to GraphQL this week. Please excuse me if some things are obvious on that side, and I'm just missing it.

---

Alright, so, messed around a bit with this and got a built a working lambda that can be tossed into the Select(). This means that we get an IQueryable<Dictionary<string, object>> return from the resolver. I'm doing this outside of this lib for now to see if I can just pass in an IQueryable that already sets up the projection, hacking through it till I have something that works to later refine and fix up.

I ran into a couple upstream issues, some resolved (noting them here for clarification). Shane may have words for some of this?

---

1. the graphType must be set as the Dictonaty<string, object> (If that's the dictionary type you are using) else it throws the "... cannot be coerced effectively to a GraphQL type" exception
2. The graphType must be of IGraphType (As constrained by GraphQL.Net). If you construct a lambda for a Dictionary, this throws an exception when attempting to create the generic, since it violates the constraint. For the purpose of just getting this working, I put together a GraphDictionary<TKey, TValue> : Dictionary<TKey, TValue>, IGraphType. This seems pretty hacky and there is probably a much more elegant way to go about that?
   3. Alright, now the next error is one I'm still trying to figure out a cause & workaround for:

• 'Output type 'AppQuery' can have fields only of output types: ScalarGraphType, ObjectGraphType, InterfaceGraphType, UnionGraphType or EnumerationGraphType. Field 'customers' has an input type. (Parameter 'fieldType')'
• This seems obvious enough, I don't have a proper output type. However, how I make that actionable is out of my reach right now.
  • This is based on the check here: https://github.com/graphql-dotnet/graphql-dotnet/blob/cc693b421245fac2357a770a2cb1447def5e6292/src/GraphQL/Types/Composite/ComplexGraphType.cs#L72
  • Which is set up to check for the types listed here: https://github.com/graphql-dotnet/graphql-dotnet/blob/cc693b421245fac2357a770a2cb1447def5e6292/src/GraphQL/GraphQLExtensions.cs#L64
    • Which is essentially getting the IGraphType and pattern matching it to the types listed
  • Do I need to implement say IObjectGraphType on my GraphDictionary?

3. [Revised] Re-implementing GraphDictionary to inherit from ObjectGraphType and jut exposing Dictionary & IENumerable Methods/Props/Indexers through a private backer got me past that.
4. No more exceptions \o/. However, I think I hit a snag: Cannot query field \"id\" on type \"GraphDictionary\".. Crap, GraphQL.Net uses reflection to grab fields doesn't it? I mean, ofc it does. Is this a hard wall?

[Shane32]

1. I suggest defining a EfSource<T> that inherits from Dictionary<string, object> just a convenience so that you have a specific source type for each model type:

class EfSource<T> : Dictionary<string, object>
{
    //add default constructors
    public EfSource(...) : base(...) { }
}

2. You'll want a graph type for each model type which inherits from ObjectGraphType<EfSource<T>>. See sample below.

3. Right; see point 2.

4. You'll need to define the fields for each model.

class PersonType : ObjectGraphType<EfSource<T>>
{
    public PersonType()
    {
        Field("id", resolve: context => context.Source["id"], graphType: typeof(NonNullGraphType<IdGraphType>));
        Field("name", resolve: context => context.Source["name"], graphType: typeof(StringGraphType));
    }
}

You could of course write code to use reflection to examine each model type and create matching graph type fields. I chose to be explicit about the graph type members.

[Shane32]

Keep in mind that the dictionary becomes the source for the graph type, not the graph type itself. So your main query will look something like this:

class Query : ObjectGraphType
{
    public Query()
    {
        Field("people", resolve: async context => {
            // examine the IResolveFieldContext to determine which fields were queried
            // build the select query (using EfSource<Person> as the return type, not Dictionary<string, object>)
            // execute the query and return a list of dictionaries
            IEnumerable<EfSource<Person>> ret = await db.People.Select(selectQuery).ToListAsync();
            return ret;
        }, graphType: typeof(ListGraphType<PersonType>));
    }
}

You can of course just use Dictionary<string, object> if you like. It will still work just as well. You still need individual graph types for each model type, no matter what. But if you want auto generating graph types you could do this:

class EfObjectType<T> : ObjectGraphType<Dictionary<string, object>> //or ObjectGraphType<EfSource<T>>
{
    public EfObjectType() {
        //build fields dynamically by using reflection on typeof(T)
    }
}

class PersonType : EfObjectType<Person>
{
}

Edit: or with the auto generating sample, you could use the generic type on the field type, so in the above example:

Field(..., graphType: typeof(EfObjectType<Person>));

[douglasg14b]

Damn Shane, it's late!

---

Ohh, the picture is starting to come together nicely now.

The EfSource<T> is MUCH more elegant than my hacked together workaround, for some reason I was thinking I needed to keep the Tkey and TValue of the dictionary generic from it's inherited type, which I don't, it will always be <string, object> for this purpose.

Taking that a step further with the EfObjectType is icing on the cake, using reflection and mapping those fields is easytown after that.

Of course, I see some serious performance concerns with using reflection so much, especially when building the lambda. However, I'm 90% sure that I've solved this by caching before, and that I could do that here, just gotta find my old code. Either way, not a concern right now.

I'll take what you've explained and work with that tomorrow evening and see if I can get something that works end-to-end. Idk if I can properly integrate it into this lib once it works, but once I get it to a working and clean state I can present it at the very least.

[Shane32]

Good news is that the graph types are singletons, and so the reflection building the fields only happens once. Your performance/reflection considerations are going to come into play while building the select lambda. Caching will help of course. You can of course cache an entire lambda if you can create a cache key that's based on the selected fields for the specific model.

[Shane32]

I should add lambda caching to my code... lol

[douglasg14b]

Yeah, the potential variations in fields may hose that up I think. Each difference in selected fields will result in a different cache key, which would take some time to warm up and fill in the cache. I could solve that with more intelligent caching, or caching some intermediate steps in the lambda creation (If that's possible) to offset the warm up cost.

That or the warm up period is negligible in the real world as common queries would be warmed up fast anyways, and infrequent ones would only be slow once.

Edit: This is exciting, I just wanna hop on this now, but sleep is important. I'll check back in when I've made more progress towards a working solution! Thanks for all the help this far.

[Shane32]

In practice, I've found the speed of the building the lambda to be insignificant compared to the overall execution time of the query. However, EF also needs to compile the select queries to SQL statements, and I believe it uses caching to speed this process up. Potentially if the lambda is cached, it may be able to identify a cache match (or quicker) vs otherwise. So there may be a EF Core speed benefit to caching the lambda.

[douglasg14b]

Hm. Issue I'm running into is that only IENumerable not IQueryable has a .Select() method that accepts a Func<TSource, TResult>. Which hoses things up with this lib as it needs an IQueryable that implements IAsyncEnumerable. Which, it seems the return of IEnumerable.AsQueryable(), does not implement IAsyncEnumerable. Pmuch AsQueryable() doesn't return an EF flavored IQueryable it seems.

So I believe I need to get an IQueryable that implements IAsyncEnumerable out of the IENumerable returned by the Select(). The returned IEnumerable hasn't been resolved, no query has been performed at that point. Which makes me wonder why you can't perform async operations on it like normal, I can't quite follow the type juggling there.

Edit: That or figure out how to get a working generated Expression<Func<TSource, TResult>> into the IQuerable.Select() that doesn't break EF Core.

Seems related to https://stackoverflow.com/questions/47744663/how-to-use-func-in-a-linq-query-that-provide-an-iqueryable-output

[Shane32]

You for sure need to generate a Expression<Func<TSource, TResult>>. It’s the only way EF will be able to translate it into a proper SQL query. Any solution involving IEnumerable.Select will cause the full table row to be returned before executing the Func.

[douglasg14b]

That's what I thought, However any Expression<Func<TSource, TResult>> I generate produces some error from EF. Unfortunatey my expression building knowledge is pretty bad atm, maybe the expressions are just built wrong? For instance:

• .Select(x => lambda(x)) looked sane to me, but results in Processing of the LINQ expression ... failed. This may indicate either a bug or a limitation in EF Core.
• Just to poke at it: Instead of compiling is (lambda.Compile()) , use the expression I already have Select(lambda) results in:
  • System.NotImplementedException: 'Unhandled expression node type: Block' at Microsoft.EntityFrameworkCore.Query.Internal.ExpressionEqualityComparer.GetHashCode(Expression obj)
  • It seems to not match the patterns defined here? https://github.com/dotnet/efcore/blob/0b3165096d6b55443fc06ae48404c2b037dd73e7/src/EFCore/Query/ExpressionEqualityComparer.cs#L40
    • Even though the lamdba.NodeType is Lambda. That switch is not happy with it (What is actually going on there, what's that switch actually making a comparison against?). It has all the properties of the LambdaExpression match, though unsure of how to get my expression to trigger that case.

---

Any solution involving IEnumerable.Select will cause the full table row to be returned before executing the Func.

That's what I thought as well, or rather that's what I expected. However, when i get the IENumerable back, no query has been performed (Logging EF queries). The query only occurs when I call ToList() or similar method.

[Shane32]

However, when i get the IEnumerable back, no query has been performed (Logging EF queries).

It will execute as soon as you start to enumerate the query. For instance:

var query = db.People.Where(x => x.Name == "john doe");
// query is an IQueryable<Person> and has not been executed yet
var queryEnumerable = query.AsEnumerable();
// queryEnumerable is an IEnumerable<Person> and has not been exectued yet
var selectedEnumerable = queryEnumerable.Select(x => x.Id);
// selectedEnumerable is an IEnumerable<int> which still has not been executed yet

// anything that enumerates the query will trigger the db call, like these statements:
foreach (var id in selectedEnumerable) { ... }
var list = selectedEnumerable.ToList();
var hashset = selectedEnumerable.ToHashSet();
// in the above sample, the database actually executes 3 different times
// also, the query is processed at the last point that it was an IQueryable; so, the full Person row is returned for each record

// note that Where can also be an IEnumerable, so this sample here actually pulls EVERY record from the database:
var result = db.People.AsEnumerable().Where(x => x.Id == 345).Single();

[Shane32]

Unhandled expression node type: Block

It's telling you that EF Core cannot process a BlockExpression, which you copied from the sample you linked me to at stackoverflow. Use ListInitExpression instead, as I outlined here: #145 (comment)

It will largely go together the same way, but basically, instead of writing an expression that represents this:

var dic = new Dictionary<string, object>();
dic.Add("id", x => x.Id);
dic.Add("string", x => x.Name);
return dic;

You instead want to write an expression that represents this:

return new Dictionary<string, object> { { "id", x => x.Id }, { "name", x => x.Name } };

As you can see, the body of the individual expressions you're adding to the dictionary have not changed. It's just how you create and initialize the dictionary.

[Shane32]

I would suggest you try the following:

Expression<Func<Person, Dictionary<string, object>>> lambda = x => new Dictionary<string, object> { { "id", x => x.Id }, { "name", x => x.Name } };
var test = db.People.Select(lambda).First();

Then verify that test returns the correct data. Then use the debugger to view the expression tree contained within the variable lambda, and simply work to recreate that expression by hand with the methods in the Expression namespace. Eventually you'll get there. Then work to make it dynamically create an expression tree based on inputs, and voila!

You may find this free tool handy for visualizing expressions:

https://marketplace.visualstudio.com/items?itemName=vs-publisher-1232914.ReadableExpressionsVisualizers

[Shane32]

FYI, none of these solutions require the use of IAsyncEnumerable. You should end up with an IQueryable<Dictionary<string, object>> upon which you can execute var ret = await query.ToListAsync(cancellationToken). Since graphql-dotnet doesn't stream results back to the caller (which would be near-impossible while still meeting the full graphql spec), results will have to get stored in a list or array anyway. So I use ToListAsync.

[douglasg14b]

I was hoping I would be able to copy/paste/modify/fix my way through the Expressions portion of this. Looks like I'll have to properly sit down and relearn their fundamentals again and do this right.

Unfortunately I cannot commit to that right now, I've got a project looming. I will come back around to this when I find a nice slow or free week, or if it turns into a more urgent problem in this project.

Thanks for the advice and knowledge thus far. I'll be back around sometime in the near future!

[Intecpsp]

@douglasg14b any luck so far? I'm very interested in how this issue turns out

[expcat]

I hope this issue can be improved. (dotnet/core#5852)

[SimonCropp]

happy to consider a PR

[douglasg14b]

To follow up on this being closed.

This is no longer an issue?