Closed pmiddleton closed 1 year ago
It is the visitor which expands navigation. 😄 Sub-Expressions:
Sub-expression visitors:
Overall process: When we start with query root, we create NEE with EntityReference. We apply defining query/query filters as needed. Then we translate other queryable methods. Mainly
There is bunch of code here and there to special case certain things like
Above is high level overview of how it works. Some of the things could change based on bugs but not expecting a lot of change in this area. Let me know if you have questions about how to handle specific method.
Ok you are either the worlds faster typist or you had that stored away in a word document :)
Thanks this will help me work through the issues I'm having in here better .
Typed on the fly lol!
@smitpatel, @ajcvickers
Just an update. I have about half of my unit tests working for queryable functions. My plan is to finish things up over xmas break so I can get PR ready in Jan.
I had to break down and get a new computer. My poor 8 year old Core 2 2600k was taking 2-3 minutes to build and start a unit test :)
@pmiddleton Glad you powered-up. Looking forward to the PR!
@smitpatel - Design question for you.
I have this test query I'm digging through while trying to figure out how things are working (it uses the data in the UDF tests)
var t = (from c in context.Customers
select new
{
Order = c.Orders.Where(o => o.QuantitySold > 2).ToList()
}).ToList();
Why do queries which have collection projections need to go down the _clientEval route in RelationalProjectionBindingExpressionVisitor? There is no actual method eval client side. Why couldn't the regular eval path be modified to make this work?
What am I missing here.
Projection Contains a collection. SelectExpression's select clause cannot contain collections (only scalar values), hence we need to generate a join and iterate over multiple rows on client side to product correct results. Which makes it client eval. Another way to look at it, anything which is non-client eval, means there is one-to-one mapping between member access path (ProjectionMember) to SqlExpression so that if you access that particular member in binding later (let's say OrderBy), then you can use it. But when you think about collection projection, can you really put that collection in OrderBy? Hence after this projection, the query is pretty much non-composible, hence client eval.
Thanks Smit. I knew that mapping had to happen during materialization, I just didn't connect that with the _clientEval flag. My brain was stuck thinking that only meant evaluating method calls client side when they were not translatable.
@smitpatel @ajcvickers
I need some ideas.
I have run into a problem materializing child collections which originate from a queryable function. The result type of a queryable function might not have a primary key defined (nor may it be possible to define one). CustomShaperCompilingExpressionVisitor expects the child collection to have a key that it can use to identify which collection any given part of a result row needs to be placed into.
I've been trying to figure out a way to either generate a temp key, or work around needing one.
I've been able to get things working if you only project a single collection by using the parents id as the key. This quickly fails if you introduce a second collection.
Have you run into this scenario before and if so how did you deal with it?
For reference this is the type of query I am talking about.
var results = (from c in context.Customers
select new
{
c.Id,
OrderCountYear = context.GetCustomerOrderCountByYear(c.Id).ToList()
}).ToList();
Likely dupe of https://github.com/aspnet/EntityFrameworkCore/issues/15873
It is not possible to project out a group if you don't have a way to correlate with outer row.
Yea from looking at the code that was what I was afraid of. I was holding out hope there was something I was missing :)
I can either leave in the support I have for a single collection, or remove it and blanket say all projected collections require an id.
What do you think?
What does single collection query look like?
That is the example I posted above. You can have a single QF collection with no other collections.
You key off of the parent id. When that changes you know to start another collection.
So the sql for the above generates
SELECT [c].[Id], [c].[LastName], [o].[Count], [o].[CustomerId], [o].[Year]
FROM [Customers] AS [c]
OUTER APPLY [dbo].[GetCustomerOrderCountByYear]([c].[Id]) AS [o]
ORDER BY [c].[Id]
With this data from the unit tests.
Id LastName Count CustomerId Year
1 One 2 1 2000
1 One 1 1 2001
2 Two 2 2 2000
3 Three 1 3 2001
4 Four NULL NULL NULL
I'm not sure if that will be useful enough vs the confusion it will generate when someone tries to do multiple collections.
Not supported.
Status update.
I got all of my query unit tests passing over xmas vacation. I now just need to finish up work on some model validation changes.
I should still be on track to have a PR ready sometime in Jan.
The return type of a QueryableFunction needs to be registered as an Entity in order for everything to work. However a table shouldn't be created in the database as this is not a persisted type.
I'm looking for a way to flag the entity so as to not generate a table. Is there something like this already in place?
@smitpatel @ajcvickers @bricelam - Can anyone point me in the right direction for any existing code for not generating a table for entities? This is one of the last things I have yet to do.
We need #2725. We hacked it in ToView using a marker annotation...
...and ignoring it in the model differ.
(Note well, viewDefinition
below is the annotation object, not the annotation's value)
@ajcvickers @bricelam Does anyone there have issues with this solution when running ReSharper? When I have ReSharper on I keep running out of memory issues. I had to turn it off to keep things stable.
@pmiddleton Not sure if anyone uses R# routinely on the team. A couple of us use Rider and I'm not aware of any issues with it.
@ajcvickers - Ok thanks. Yea R# uses up a ton of ram and I'm bumping into the 2 GB 32 bit process limit. If you could walk down the hall to the VS team and have them build as 64 bit :) haha
@pmiddleton They will just say to uninstall R#. Visual Studio has been making a big push over the last few years to be natively productive-- that is, not require third-party extensions for productivity. The reason being that R# (and probably similar extensions) use a lot of resources.
Weird, I was sure R# ran out-of-process precisely to avoid the 2GB limit... But who knows exactly what's going on there.
There's a free/open alternative called Roslynator which may use less resources.
@smitpatel
Can you give me a brief overview of what NavigationExpandingExpressionVisitor does.
Queryable functions needs some changes in here and it would save me some time to know the purpose of this class.
Thanks