chkimes / graphql-net

Convert GraphQL to IQueryable
MIT License
891 stars 86 forks source link

Query field, resolve or queryableGetter is being executed twice #79

Closed diegoagd10 closed 7 years ago

diegoagd10 commented 7 years ago

Hi I'm trying to create an example with MongoDb and I realize the method FindFiles is being executed twice, I wonder if I made something wrong.

This is the implemented code:

var schema = GraphQL<MongoRepository<Metadata>>.CreateDefaultSchema(() => new 
MongoRepository<Metadata>(metadataCollection));

// Adding type to schema
schema.AddType<Metadata>("MetadataType").AddAllFields();

// Adding query to schema
schema.AddListField("Metadatas", new Arguments(), 
    (repository, arguments) => FindFiles(repository, arguments));

schema.Complete();

var query = $@"{{
    Metadatas(relatedTo: ""text:10002"") {{
        id,
        fileName,
        extension,
    }},
    Metadatas(relatedTo: ""text:10001"") {{
        id,
        fileName,
        extension
    }}
}}";

var gql = new GraphQL<MongoRepository<Metadata>>(schema);
var result = gql.ExecuteQuery(query);

Console.WriteLine(JsonConvert.SerializeObject(result, Formatting.Indented));

Console.ReadKey();
chkimes commented 7 years ago

@diegoagd10

This is working as designed (though maybe not as expected). When I originally put this library together, I treated top-level queries (fields on the schema) differently than I treated fields on user types. Fields on the top-level schema are executed once for every time they are shown in the query, but child fields should be automatically joined in that query.

I don't particularly like this design choice, but this was how I put it together before I understood GraphQL fully. I'm going to leave this issue open since it's a change I would like to make in the future.

diegoagd10 commented 7 years ago

I'm okey whith your design but I forget to say something, in that example each query is being executed twice, so in that example, ExecuteQuery is calling 4 times the method FindFiles.

Metadatas(relatedTo: ""text:10001"") => is calling FindFiles twice

Metadatas(relatedTo: ""text:10002"") => is calling FindFiles twice

chkimes commented 7 years ago

Ah, now that's interesting. I don't believe that should be happening. I'll take a closer look when I get home tonight.

diegoagd10 commented 7 years ago

Thanks πŸ‘

chkimes commented 7 years ago

@diegoagd10

So I took a closer look at this, and I realized that this actually should be happening and is expected. The important thing to pay attention to on your end is to make sure that the FindFiles method should be returning an IQueryable, but it should not be executing a database call. I should be able to call FindFiles any number of times without actually impacting the database.

Even though the library calls FindFiles twice, it only executes the IQueryable once.

The reason that this happens is because different IQueryable providers require queries to be shaped in different ways. So for Entity Framework we do things slightly differently than for NHibernate, or even for executing against an in-memory provider. However, in order to find out what type of IQueryable provider we're working with, we need the actual IQueryable so we can check the type. That happens here:

https://github.com/ckimes89/graphql-net/blob/master/GraphQL.Net/Executor.cs#L27

We use that information to determine how to glue all the expressions together (i.e. selectors for child fields), then we get the IQueryable a second time here:

https://github.com/ckimes89/graphql-net/blob/master/GraphQL.Net/Executor.cs#L43

and execute it here:

https://github.com/ckimes89/graphql-net/blob/master/GraphQL.Net/Executor.cs#L51-L60

Again, the important point of all this is to make sure that your FindFiles method is returning an IQueryable and not executing a query against the database.

diegoagd10 commented 7 years ago

super, thanks @ckimes89 πŸ‘. I think this issue could be closed.