pdevito3 / QueryKit

🎛️ QueryKit is a .NET library that makes it easier to query your data by providing a fluent and intuitive syntax for filtering and sorting.
Apache License 2.0
147 stars 13 forks source link

feat - add collection support for collections. ONLY Equals operator #7

Closed royston-c-oa closed 1 year ago

royston-c-oa commented 1 year ago

@pdevito3 This is an extremely rough idea and will need to be rewritten. But it adds support for collections. I have only added support for the equals operator. If you like this route we would need to assess what the best linq statements are for each operator.

I'm not sure I like that we would need to alter every operator to support Collections but I can't see another way to create the Expressions on an IEnumable.

For instance Ingredients.Preparations.Text == "<VALUE>" on a Recipe would have to translate to either:

.Where(x => x.Ingredients.Any(y => y.Preparations.Any(z => z.Text == "<VALUE>")))
.Where(x => x.Ingredients.SelectMany(y => y.Preparations).Select(y => y.Text).Any(y => y == "<VALUE>"))

I opted for selecting the nested property and then applying the operator to avoid nested expressions., which would require you to know the operator before you start or some complex evaluation/manipulation of the expression after the "left" expression had been calculated.

pdevito3 commented 1 year ago

This is great! I wouldn't worry about it. Even if there's something more eloquent this should still be maintainable enough I think it's worth it for the value add. I've been trying to do a recursive op inside the leftexprparser and it's been such a pain. This seems totally reasonable! Will be interesting to see how it holds up to more complex scenarios but I like this so far.

Thanks for the fresh eyes and effort! 🚀

royston-c-oa commented 1 year ago

I was thinking over this and we might have an issue or potential feature request in the future for all vs any of the collection evaluating to an operator. For instance:

Ingredients.Name == "potato" Ingredients.Measure > 1

Given the context of a recipe you clearly want an Any Name equals potato. However in the second we you could easily want Any or All ingredients to have measures greater than 1.

OData handle this like:

Ingredients/any(i:i/Name eq "potato") Ingredients/all(i:i/Measure > 1)

Given the way the library parses the query this type of syntax would require a large overhaul (I think, correct me if I'm wrong).

So the question is how would QueryKit support this?

Maybe:

Ingredients.Name any == "potato" any(Ingredients.Name == "potato")

Given the way the query is currently parsed we'd only allow 1 operator on the inner object at once. So it would be as concise as odata such as:

Ingredients/any(i:i/Name eq "potato" AND i/Measure > 1)

It would be:

Ingredients.Name any == "potato" AND Ingredients.Measure any > 1

One of the nice things is that we could supply a default and assume that if the any or all term is left out we default to any.

Ingredients.Name == "potato" Equivalent to: Ingredients.Name any == "potato"

royston-c-oa commented 1 year ago

I think either way it might be best to have a different set of operator classes for collections. Thoughts?

royston-c-oa commented 1 year ago

Also, its worth noting that the in-memory performance of the linq queries will be worse if we explode out the inner value first via SelectMany and Select. I have a feeling the primary use case for this library will be to use it with libraries like EF, so this will matter less but worth noting.

pdevito3 commented 1 year ago

I'll think about it some more and follow up, but here's my pre-coffee thoughts ☕️

I like having a default of any. i think that's usually how things operate, but I can definitely see use cases coming up for all. I think we should try and stick to the left, operator, right pattern if at all possible though.

The question then is, what's the syntax for this? A couple things come to mind Prompt: All ingredients with measure > 1 Goal: Recipes.Where(x => x.Ingredients.All(x => x.Measure > 1))

O1: Ingredients-Measure > 1 Use a - or some other separator between properties... this would require parser updates though and might be a big lift. this would probably grow better with nesting though?

O2: Ingredients.Measure #> 1 Adding a new comparison operator prefix for designating all. any would just happen by default like Ingredients.Measure > 1

Here's a larger list of examples we can do tests for.


As far as the selects and perf, good call out. i think let's get it working like this first, get it as performant as we can with this solution, and if it becomes enough of a problem in the future, it can be addressed then.

pdevito3 commented 1 year ago

and btw, i made a develop branch to work on without messing up the main README. Might want to rebase on that. I did some soundex work in there yesterday but it shouldn't clash much if at all with what you've done so far

pdevito3 commented 1 year ago

👋 might dive into this a bit this weekend if you're busy. any thoughts on a particular approach?

pdevito3 commented 1 year ago

hey, think i have a plan for this. closing in favor of #9 but cherry picked what you had as a base. planning on getting a release out for this soon. Thanks so much for the inspiration and effort here! 🚀

pdevito3 commented 1 year ago

crap... i thought the cherry pick would add you as a contributor to the repo but for some reason it doesn't, even though the commit definitely still logs you as an author.

maybe it doesn't matter to you, but if it does, please feel free to at least submit a comment PR or something to get your name on the contributor list. you've earned it @royston-c-oa!