Add Integration for Marten

[jabellard]

Integration for Marten

Generally, the features from the HotChocolate.Data package should work with any LINQ provider from which some IQueryable<T> can be retrieved. However, this is not the case with Marten. Pagination and projections work out of the box as expected, but filtering and sorting do not. LINQ expressions generated for filtering and sorting must first be translated in a format that is digestible for the Marten LINQ provider before they are applied to the IQueryable<T> object. This integration provides custom configurations to seamlessly integrate Marten with the HotChocolate.Data package.

Closes #5282

[jabellard]

@michaelstaib , Given that this is my first contribution, it seems like I'll have to sign a CLA. I get that from you?

[michaelstaib]

@jabellard yes ... I will send it to you to sign on slack.

[CLAassistant]

All committers have signed the CLA.

[michaelstaib]

@jabellard we have now everything setup for the CLA, click on the link in the CLA bot message and sign the agreement.

[jabellard]

@michaelstaib Ok. Just signed.

[jabellard]

Also I just noticed that The CodeQl check is failing after merging in the main branch:

Error: /home/runner/work/hotchocolate/hotchocolate/src/StrawberryShake/SourceGenerator/test/CodeGeneration.CSharp.Analyzers.Tests/StarWars/Test.cs(9,18): error CS1061: 'IServiceCollection' does not contain a definition for 'AddStarWarsClient' and no accessible extension method 'AddStarWarsClient' accepting a first argument of type 'IServiceCollection' could be found (are you missing a using directive or an assembly reference?) [/home/runner/work/hotchocolate/hotchocolate/src/StrawberryShake/SourceGenerator/test/CodeGeneration.CSharp.Analyzers.Tests/StrawberryShake.CodeGeneration.CSharp.Analyzers.Tests.csproj]

This is not related to the changes in this PR, so I'm not sure what's going on there.

[michaelstaib]

The issue with CodeQL has to do with source generators... will have a look in a separate PR

[jabellard]

@michaelstaib , can you take a look? I want to work on finishing this up.

[michaelstaib]

Its waiting for a review by @PascalSenn

[jabellard]

@PascalSenn can you take a look?

[PascalSenn]

@jabellard Sorry i didnt complete the review so the comments were still pending 🤦 I have a couple of questions.

[jabellard]

@PascalSenn Responded to comments. Just let me know if you have any other questions.

[PascalSenn]

@jabellard

So i have changed the implementation to use InMemory and the default QueryableProvider. With this implementation we are now also able to use marten with resolver that return "IEnumerable" or just IQueryable without breaking them.

I have also added tests for sorting and filtering. We have a set of regression tests that we run for each provider.

I fixed already a bunch of issues but i see three main points that are not working with the Marten SQL transaltor:

1. in Expressions like where: {foo: {bar: { in: [1,2,3]}}} (i think i know what this issue is)
2. custom expressions (new feature in v13) e.g. x => x.FirstName + " " + x.LastName

Can you have a look with the marten team if we can make these expressions work (or otherwise what a alternative would be):

"_s0 => (_s0.Bars.Count == 10)"

Marten Command Failure:$\nWITH mt_temp_id_list3CTE as (\nselect ctid, unnest(CAST(ARRAY(SELECT jsonb_array_elements(CAST(d.data -\u003E\u003E \u0027Bars\u0027 as jsonb))) as jsonb[])) as data from public.mt_doc_queryablefiltervisitorexpressiontests_foo as d\n)\n  , mt_temp_id_list4CTE as (\nselect ctid, count(*) as data from mt_temp_id_list3CTE as d group by ctid having count(*) =:p0\n)\n select d.id, d.data from public.mt_doc_queryablefiltervisitorexpressiontests_foo as d where d.ctid in (select ctid from mt_temp_id_list4CTE)$\n$\n22023: cannot extract elements from an object

[PascalSenn]

Also a major issues are the List operations. Most of them seem not to work It seems like most of the list operations will not work: https://martendb.io/v3/documentation/documents/querying/linq

Should we disable the array operations for marten?

[PascalSenn]

@michaelstaib
It seems like marten does not support most of the list operations all any some and also has limited support for custom expressions.

My suggestion would be deactivating list filtering for the marten queryable provider and remove the tests for the custom expressions (as it is more a db queryable provider limitation rather than a hc limitation)

[jabellard]

@michaelstaib It seems like marten does not support most of the list operations all any some and also has limited support for custom expressions.

My suggestion would be deactivating list filtering for the marten queryable provider and remove the tests for the custom expressions (as it is more a db queryable provider limitation rather than a hc limitation)

@PascalSenn : I think you have been looking at an old version of the Marten docs. At the moment, Marten does not have support for the all quantifier operation, but does have support for the any and some operations: https://martendb.io/documents/querying/linq/child-collections.html#quantifier-operations-within-child-collections

[PascalSenn]

well it seems like the expressions that we compile do not work with marten at least ;) there are failing tests in the repo, you can check it out and run the tests. maybe you have an idea what goes wrong

[oskardudycz]

Hey folks, thanks for the effort on making the initial Marten+HotChocolate integration. I just skimmed the discussion and PR, I plan to have a wider look soon. A few comments from my side:

It seems like most of the list operations will not work: https://martendb.io/v3/documentation/documents/querying/linq

Those docs are for v3; we have the right now v5, which had a big revamp around Linq. Check the docs at https://martendb.io/documents/querying/linq/operators.html for the latest version. Still, even for the v3 we had pretty good support for the most common scenarios. Could you expand on what's not working or link me to the failing scenarios? Of course, we cannot support all the possible permutation of scenarios as an in-memory provider, but still, I think that majority of the common should work.

Regarding returning to memory and then making filtering, I think that's not the best approach. It'd be similar to the initial EF Core release, which just simulated the queries and was not performant. My recommendation would be to try to see what we can do to make the scenarios work, and if some don't, then list them and fail with the specific error message, the user will get at least an understanding of the limitations.

Thoughts?

[michaelstaib]

I also would like to avoid doing in-memory filtering as it could be very heavy on memory and yield suboptimal performance. Lets, try to get as many things in as possible and just remove the things that do not work from the provider in HC.

[PascalSenn]

Hi @oskardudycz

We do project to expressions that then are interpreted by marten. On the HotChocolate side we do not really filter in memory whenever possible.

So the List.Contains stuff is already fixed, @jabellard is now looking into how to solve the issues that we have with the operations all some none and any

• any This is a true/false value that indicates if the collection is empty. I think this can be easy worked around with foo.Bars.Count() >0
• some matchs am element of the collection when one of the elements of the nested collection matchs the condition. So in expression terms foos.Where(x => x.Bars.Any(x => x.Baz == 1 && x.Qux == 2)
• none is the negation of some. (so should be easy once some is solved)
• all matchs an element of the collection when all of the elements of the nested collection match the condition. So in expression terms foos.Where(x => x.Bars.All(x => x.Baz == 1 && x.Qux == 2)

[jabellard]

Yes! @oskardudycz , as @PascalSenn mentioned, filtering as as well as the other features such as sorting provided by Hot Chocolate will execute queries natively in the database as opposed to in memory. The references to InMemory in the thread, if I remember correctly, are about how Hot Chocolate will generate specialized expressions that, for instance, will include null guards when in memory LINQ provider is used.

Regarding the list operators. I anticipate that all of them, except the all operator, which Marten does not support at the moment, will work. I got them working in a project that I have. The tests that @PascalSenn added are not working yet, but from what I can tell so far, this is due to an issue with how the documents saved to the database during the test session are serialized, hence the wierd error:

Marten Command Failure:$\nWITH mt_temp_id_list3CTE as (\nselect ctid, unnest(CAST(ARRAY(SELECT jsonb_array_elements(CAST(d.data -\u003E\u003E \u0027Bars\u0027 as jsonb))) as jsonb[])) as data from public.mt_doc_queryablefiltervisitorexpressiontests_foo as d\n)\n , mt_temp_id_list4CTE as (\nselect ctid, count(*) as data from mt_temp_id_list3CTE as d group by ctid having count(*) =:p0\n)\n select d.id, d.data from public.mt_doc_queryablefiltervisitorexpressiontests_foo as d where d.ctid in (select ctid from mt_temp_id_list4CTE)$\n$\n22023: cannot extract elements from an object

I should get to the bottom of why that's happening very soon and fix those tests.

As far as the all operator is concerned, for now, I think we should just keep it disabled until support for it is added to Marten.

[jabellard]

@PascalSenn, I fixed the issue with serializing the test data. There's one more thing that we need to figure out. A query like :

{
  root(where: {
      fooNested: {
          some: {
              bar: {
                  eq: "a"
              }
          }
      }
  }){
      fooNested {
          bar
      }
  }
}

Should work like a charm. I know because I've tested similar queries in a project that I have. However, the test for it is not passing. This is because the sql generated in the test environment is malformed. The generated sql for the query above should be:

WITH mt_temp_id_list1CTE as (
select ctid, unnest(CAST(ARRAY(SELECT jsonb_array_elements(CAST(d.data ->> 'FooNested' as jsonb))) as jsonb[])) as data from public.mt_doc_queryablefiltervisitorlisttests_foo as d
 )
   , mt_temp_id_list2CTE as (
 select ctid, data from mt_temp_id_list1CTE as d where d.data ->> 'Bar' = 'a'
 )
  select d.id, d.data from public.mt_doc_queryablefiltervisitorlisttests_foo as d where d.ctid in (select ctid from mt_temp_id_list2CTE)

However, what is generated in the test environment is (with added comments):

.param set @__p_0 'a'

--- not needed============================================================
SELECT "d"."Id"
FROM "Data" AS "d"
WHERE EXISTS (
    SELECT 1
    FROM "FooNested" AS "f"


-- what is actually needed============================================================
 WITH mt_temp_id_list1CTE as (
 select ctid, unnest(CAST(ARRAY(SELECT jsonb_array_elements(CAST(d.data ->> 'FooNested' as jsonb))) as jsonb[])) as data from public.mt_doc_queryablefiltervisitorlisttests_foo as d
 )
   , mt_temp_id_list2CTE as (
 select ctid, data from mt_temp_id_list1CTE as d where d.data ->> 'Bar' = :p0
 )
  select d.id, d.data from public.mt_doc_queryablefiltervisitorlisttests_foo as d where d.ctid in (select ctid from mt_temp_id_list2CTE)



 --- not needed============================================================
    WHERE "d"."Id" = "f"."FooId" AND "f"."Bar" = @__p_0)

The part in the middle is what we need. We don't need the parts at the top and bottom. Can we fix this by configuring the test setup differently?

[jabellard]

@PascalSenn : Never mind. I got all the list tests working. I just had to get rid of the old snapshot files to get things working.

[jabellard]

All tests for filtering are passing if I run each test class in that project separately. However, if I try running all the tests in that project at once, then I get some snapshot mismatch errors.

[jabellard]

All tests for filtering are passing if I run each test class in that project separately. However, if I try running all the tests in that project at once, then I get some snapshot mismatch errors.

I take that back. The in/nin tests are failing which is weird, since you fixed them earlier.

[jabellard]

I fixed the issue with the test setup. All tests are now passing except for tests regarding:

• Nullable enum values
• Custom expressions

[jabellard]

@PascalSenn , @oskardudycz : We're super close to finishing up this integration.

There's been a few outstanding issues holding up this PR:

• Invoking custom handlers that @PascalSenn implemented as opposed to the default ones in the test environment: I reconfigured the test setup, and resolved that.

• Failing tests for the all list operator At the moment, Marten has no support for that operator, so I removed those tests. @PascalSenn , is there a way to disable that operation when the Marten filtering provider is used?

• Failing tests for custom expressions That's expected. Marten can't support every possible permutation of building custom expressions, so I removed the tests for the ones that are not supported.

• Failing tests for in and nin queries for nullable enum values This is now the only outstanding issue that's holding off this PR. The tests are failing because of a limitattion in Marten for parsing expressions used to filter nullable enum values. I resolved that issue, and submitted a PR for it: https://github.com/JasperFx/weasel/pull/70 . @oskardudycz , Can you take a look? I've already tested this locally, and got all the tests passing. One this gets merged in, we can just reference the latest version of Marten in this project to pass the outstanding tests.

[PascalSenn]

@oskardudycz : Do you have any suggestion how we could get all to work with marten?

[oskardudycz]

Folks, I apologise for the lack of feedback. I was (and still am) a bit too busy last week. I'll try to investigate that the next week.

[jabellard]

@PascalSenn , @oskardudycz : I took a look into implementing the All operation for Marten, and have a very good idea on how to go about doing it. I'll give it a shot this weekend.