chkimes / graphql-net

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

NHibernate sample #41

Open gursharan001 opened 7 years ago

gursharan001 commented 7 years ago

I really like your idea of converting GraphQL to IQueryable and have been trying to do a sample using NHibernate.

Do you know any reason why this wouldn't work with NHibernate.

I get following exception in Executor.cs. I am on .Net 4.5 using NHibernate 4

NHibernate.Exceptions.GenericADOException was unhandled HResult=-2146232832 Message=Could not execute query[SQL: SQL not available] Source=NHibernate SqlString=SQL not available StackTrace: at NHibernate.Impl.SessionImpl.List(IQueryExpression queryExpression, QueryParameters queryParameters, IList results) at NHibernate.Impl.AbstractSessionImpl.List(IQueryExpression queryExpression, QueryParameters parameters) at NHibernate.Impl.AbstractQueryImpl2.List() at NHibernate.Linq.DefaultQueryProvider.ExecuteQuery(NhLinqExpression nhLinqExpression, IQuery query, NhLinqExpression nhQuery) at NHibernate.Linq.DefaultQueryProvider.Execute(Expression expression) at NHibernate.Linq.DefaultQueryProvider.Execute[TResult](Expression expression) at Remotion.Linq.QueryableBase1.GetEnumerator() at System.Collections.Generic.List1..ctor(IEnumerable1 collection) at System.Linq.Enumerable.ToList[TSource](IEnumerable1 source) at GraphQL.Net.Executor1.Execute(TContext context, GraphQLField field, ExecSelection1 query) in C:\scratch\graphql-net\GraphQL.Net\Executor.cs:line 56 at GraphQL.Net.Executor1.Execute(GraphQLSchema1 schema, GraphQLField field, ExecSelection1 query) in C:\scratch\graphql-net\GraphQL.Net\Executor.cs:line 17 at GraphQL.Net.GraphQL1.ExecuteQuery(String queryStr) in C:\scratch\graphql-net\GraphQL.Net\GraphQL.cs:line 44 at QueryApi.Program.Program.Main(String[] args) in C:\scratch\QueryApi\QueryApi\Program\Program.cs:line 21 at System.AppDomain._nExecuteAssembly(RuntimeAssembly assembly, String[] args) at System.AppDomain.ExecuteAssembly(String assemblyFile, Evidence assemblySecurity, String[] args) at Microsoft.VisualStudio.HostingProcess.HostProc.RunUsersAssembly() at System.Threading.ThreadHelper.ThreadStart_Context(Object state) at System.Threading.ExecutionContext.RunInternal(ExecutionContext executionContext, ContextCallback callback, Object state, Boolean preserveSyncCtx) at System.Threading.ExecutionContext.Run(ExecutionContext executionContext, ContextCallback callback, Object state, Boolean preserveSyncCtx) at System.Threading.ExecutionContext.Run(ExecutionContext executionContext, ContextCallback callback, Object state) at System.Threading.ThreadHelper.ThreadStart() **InnerException: HResult=-2146233075 Message=Operation could destabilize the runtime. Source=Anonymously Hosted DynamicMethods Assembly StackTrace: at lambda_method(Closure , Object[] ) at NHibernate.Linq.ResultTransformer.TransformTuple(Object[] tuple, String[] aliases) at NHibernate.Hql.HolderInstantiator.Instantiate(Object[] row) at NHibernate.Loader.Hql.QueryLoader.GetResultList(IList results, IResultTransformer resultTransformer) at NHibernate.Loader.Loader.ListIgnoreQueryCache(ISessionImplementor session, QueryParameters queryParameters) at NHibernate.Loader.Loader.List(ISessionImplementor session, QueryParameters queryParameters, ISet1 querySpaces, IType[] resultTypes) at NHibernate.Loader.Hql.QueryLoader.List(ISessionImplementor session, QueryParameters queryParameters) at NHibernate.Hql.Ast.ANTLR.QueryTranslatorImpl.List(ISessionImplementor session, QueryParameters queryParameters) at NHibernate.Engine.Query.HQLQueryPlan.PerformList(QueryParameters queryParameters, ISessionImplementor session, IList results) at NHibernate.Impl.SessionImpl.List(IQueryExpression queryExpression, QueryParameters queryParameters, IList results) InnerException:`**

chkimes commented 7 years ago

This is a pretty fun exception that I ran into a lot when building this library, particularly this little piece:

Message=Operation could destabilize the runtime.

This results from building an expression of the following form:

a => new SomeType {
    ObjectTypedField = a.SomeIntValue
}

C# automatically inserts an cast to object when you write that out as code, but when building it as an expression you need to explicitly add a ConvertExpression. So, the expression actually looks like:

a => new SomeType {
    ObjectTypedField = Convert(a.SomeIntValue) // cast-like expression operation
}

You would think that in this case you should always add the ConvertExpression, but alas that isn't so. The Entity Framework expression parser barfs when it encounters that. However, if you try to just build the expression without the Convert, then you get "Operation could destabilize the runtime" when you execute it in memory. Neither option will support both, so I have an explicit check to sniff out the query backend to set the appropriate options. That code is here:

https://github.com/ckimes89/graphql-net/blob/17d11a4166e0fe33886b30ea76325c96cabd926a/GraphQL.Net/Executor.cs#L192-L213

As you can see, I'm only checking for in-memory queries (LINQ To Objects) and Entity Framework. When running on NHibernate, it's getting the wrong value for CastAssignment (should be true). I expect it will also get the wrong value for NullCheckLists as well, since that should probably be false.

gursharan001 commented 7 years ago

You are right.. an explicit conversion was needed. setting the flag to true did the job.

For selecting a single item, I also had to do following @ https://github.com/ckimes89/graphql-net/blob/17d11a4166e0fe33886b30ea76325c96cabd926a/GraphQL.Net/Executor.cs#L56-L58

results = MapResults(transformed.AsEnumerable().FirstOrDefault(), query.Selections.Values());

otherwise cannot convert IEnumerable to IQueryable exception was being thrown.

chkimes commented 7 years ago

Hmm, that's problematic since I imagine that's forcing evaluation of the full query in SQL instead of doing something like a SELECT TOP 1 ...

What was the exact exception it was giving you?

gursharan001 commented 7 years ago

Exception is below. But yes doing a Enumerable conversion seems to cause duplicate columns in the generated query.

System.ArgumentException was unhandled
  HResult=-2147024809
  Message=Object of type 'System.Linq.EnumerableQuery`1[System.Object]' cannot be converted to type 'System.Linq.IQueryable`1[GraphQL.DynamicObjects.Album419cb039-d091-4232-a7b1-db5eda7a0004]'.
  Source=mscorlib
  StackTrace:
       at System.RuntimeType.TryChangeType(Object value, Binder binder, CultureInfo culture, Boolean needsSpecialCast)
       at System.RuntimeType.CheckValue(Object value, Binder binder, CultureInfo culture, BindingFlags invokeAttr)
       at System.Reflection.MethodBase.CheckArguments(Object[] parameters, Binder binder, BindingFlags invokeAttr, CultureInfo culture, Signature sig)
       at System.Reflection.RuntimeMethodInfo.InvokeArgumentsCheck(Object obj, BindingFlags invokeAttr, Binder binder, Object[] parameters, CultureInfo culture)
       at System.Delegate.DynamicInvokeImpl(Object[] args)
       at System.Delegate.DynamicInvoke(Object[] args)
       at NHibernate.Linq.DefaultQueryProvider.ExecuteQuery(NhLinqExpression nhLinqExpression, IQuery query, NhLinqExpression nhQuery)
       at NHibernate.Linq.DefaultQueryProvider.Execute(Expression expression)
       at NHibernate.Linq.DefaultQueryProvider.Execute[TResult](Expression expression)
       at System.Linq.Queryable.FirstOrDefault[TSource](IQueryable`1 source)
       at GraphQL.Net.Executor`1.Execute(TContext context, GraphQLField field, ExecSelection`1 query) in C:\scratch\graphql-net\GraphQL.Net\Executor.cs:line 58
       at GraphQL.Net.Executor`1.Execute(GraphQLSchema`1 schema, GraphQLField field, ExecSelection`1 query) in C:\scratch\graphql-net\GraphQL.Net\Executor.cs:line 17
       at GraphQL.Net.GraphQL`1.ExecuteQuery(String queryStr) in C:\scratch\graphql-net\GraphQL.Net\GraphQL.cs:line 44
       at QueryApi.Program.Program.Main(String[] args) in C:\scratch\QueryApi\QueryApi\Program\Program.cs:line 20
       at System.AppDomain._nExecuteAssembly(RuntimeAssembly assembly, String[] args)
       at System.AppDomain.ExecuteAssembly(String assemblyFile, Evidence assemblySecurity, String[] args)
       at Microsoft.VisualStudio.HostingProcess.HostProc.RunUsersAssembly()
       at System.Threading.ThreadHelper.ThreadStart_Context(Object state)
       at System.Threading.ExecutionContext.RunInternal(ExecutionContext executionContext, ContextCallback callback, Object state, Boolean preserveSyncCtx)
       at System.Threading.ExecutionContext.Run(ExecutionContext executionContext, ContextCallback callback, Object state, Boolean preserveSyncCtx)
       at System.Threading.ExecutionContext.Run(ExecutionContext executionContext, ContextCallback callback, Object state)
       at System.Threading.ThreadHelper.ThreadStart()
  InnerException: 
chkimes commented 7 years ago

Interesting... at first I thought it was the transformed variable that was for some reason not IQueryable, but I don't think that's right anymore. It should have failed the cast where it was assigned:

var transformed = (IQueryable<object>)expr.Compile().DynamicInvoke(context);

So, I think what's actually happening is that part of the dynamically generated query is being returned as IEnumerable when I'm expecting it to be returned is IQueryable. I'll probably have to get NHibernate and inspect the types while debugging to figure this one out.

Out of curiosity, what does your schema setup code look like?

gursharan001 commented 7 years ago

Here is my context setup -

public GraphQL<IMusicStoreQueryService> CreateDefaultContext()
        {
            var schema = GraphQL<IMusicStoreQueryService>.CreateDefaultSchema(() => _musicStoreQueryService);
            schema.AddType<Album>().AddAllFields();
            schema.AddListField("albums", db => db.Albums);
            schema.AddField("album", new {id = 0},
                (db, args) => db.Albums.FirstOrDefault(album => album.AlbumId == args.id));
            schema.Complete();
            return new GraphQL<IMusicStoreQueryService>(schema);
        }

While debugging I can see that transformed is of type NhQueryable.. Here is the expression debug view -

.Call System.Linq.Queryable.Select(
    .Call System.Linq.Queryable.Where(
        .Constant<NHibernate.Linq.NhQueryable`1[QueryApi.Entities.Album]>(NHibernate.Linq.NhQueryable`1[QueryApi.Entities.Album]),
        '(.Lambda #Lambda1<System.Func`2[QueryApi.Entities.Album,System.Boolean]>)),
    '(.Lambda #Lambda2<System.Func`2[QueryApi.Entities.Album,GraphQL.DynamicObjects.Album1ab8332e-8a57-40fe-8210-b92dcc74a3e1]>))

.Lambda #Lambda1<System.Func`2[QueryApi.Entities.Album,System.Boolean]>(QueryApi.Entities.Album $album) {
    $album.AlbumId == (.Constant<System.Runtime.CompilerServices.StrongBox`1[<>f__AnonymousType0`1[System.Int32]]>(System.Runtime.CompilerServices.StrongBox`1[<>f__AnonymousType0`1[System.Int32]]).Value).id
}

.Lambda #Lambda2<System.Func`2[QueryApi.Entities.Album,GraphQL.DynamicObjects.Album1ab8332e-8a57-40fe-8210-b92dcc74a3e1]>(QueryApi.Entities.Album $p)
{
    .If ($p == null) {
        null
    } .Else {
        .New GraphQL.DynamicObjects.Album1ab8332e-8a57-40fe-8210-b92dcc74a3e1(){
            albumId = (System.Nullable`1[System.Int32])$p.AlbumId,
            title = $p.Title,
            price = (System.Nullable`1[System.Decimal])$p.Price
        }
    }
}

And finally the service -

public class MusicStoreQueryService : IMusicStoreQueryService
    {
        private readonly ISession _session;

        public MusicStoreQueryService(ISession session)
        {
            _session = session;
        }

        public IQueryable<Album> Albums => _session.Query<Album>();
    }
chkimes commented 7 years ago

I'm digging through the NHibernate source to see where the issue might be. I have a few ideas, but I'll need to play around to make sure.

However, I just noticed something about the expression debug view. There's a null-check in there that looks like the null-coalescing expression I had to add in to make queries run in memory. Can you try assigning false to NullCheckLists (like you did for setting CastAssignment to true)?

chkimes commented 7 years ago

I'm assuming you're working with the code from master and not the NuGet package. I just pushed a commit to master that should set the expression options correctly for NHibernate.

gursharan001 commented 7 years ago

Yes, I am working with the code from master. I got the exception after setting NullCheckLists to false (as you had indicated in your first reply).

chkimes commented 7 years ago

Oh man, it's 3:30 AM here but I couldn't sleep until I figured this out. It was rough, but I got it.

As it turns out, NHibernate is relying on the generic parameter T of IQueryable<T> to determine what type to deserialize its results as. Because of the cast to IQueryable<object> that I linked earlier, NHibernate was inferring the object type parameter and creating an EnumerableQuery<object>. However, it then tried to use reflection to pass that into another method that was expecting an IQueryable<TEntity> (where your TEntity there would be Album) which causes the whole thing to blow up.

I feel like I should have caught this sooner, since the error is pretty clear in the exception, but I got sidetracked by a bunch of red herrings.

So, we can't use IQueryable<object> to execute our query, we have to use IQueryable<T>. But how do we do that if we don't know what T is at compile time? Well, we have to use Reflection to generate the correct generic methods to call. This is what this code does:

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

That's a helper method to transform this:

var fod = GenericQueryableCall(transformed, q => q.FirstOrDefault()); // q is of type IEnumerable<object>

into a dynamic method with type determined at runtime that does this:

var fod = transformed.FirstOrDefault();  // transformed is of type IEnumerable<T>

When I set up my test case on NHibernate, I was able to get the exact same exception as you. With this fix I'm no longer seeing that, so I'm pretty confident that this will work for you.

gursharan001 commented 7 years ago

Yep.. that fixed the issue and a single select works now. Thank you for working through this. I will try and create a NHibernate test project similar to your EF project as that seems to cover quite a few scenarios.

Thanks again!

gursharan001 commented 7 years ago

I managed to create NHibernate tests based on EF tests (with dependency on SqlLocalDb). Some of the tests fail. What's the best way to share the failing tests as I am not entirely sure why they are failing.

chkimes commented 7 years ago

Thanks for working on an NHibernate test project. I just created an nhibernate branch, so if you send a pull request to that branch with your changes then I'll merge and check out the failing tests.

chkimes commented 7 years ago

Looks like the errors are coming from an incarnation of a similar problem as I fixed earlier. However, it's only triggered on queries that return entities with a sub-field that is enumerable, i.e.:

{ account(id:1) { users { id } } }

NHibernate would choke on that generated query since users is a list there.

I haven't found the exact cause yet, but I should have more time to look into it over the weekend.

gursharan001 commented 7 years ago

Good to hear that it looks solvable. I am doing a sample project pointing GraphiQL with library hosted in WebAPI.

chkimes commented 7 years ago

Looks like it all boils down to this issue:

https://nhibernate.jira.com/browse/NH-3665

NHibernate has apparently ignored it for the past two years (I don't know how, that seems like a very common way to write a query), so I don't expect them to fix it anytime soon. From my investigations, I actually think it is a problem with Remotion, the library that converts LINQ into a more manageable AST.

I have figured out a workaround, though. Calling .Take(1).ToList().FirstOrDefault() instead of just .FirstOrDefault() will bypass this problem for NHibernate. I'll do some more testing on it and if it fixes the issue I'll incorporate it by sniffing the queryable provider for NHibernate and applying the workaround.

gursharan001 commented 7 years ago

Awesome! With EF Core dependency on Remotion hopefully this will be useful in future.

efarr commented 6 years ago

@gursharan001, Did you ever create your sample app working against NHibernate? Is it available somewhere? Thanks!