Open hakanaltindis opened 2 years ago
What an interesting problem....
It returns an array with 3 items but @odata.count is 4. This is the my issue.
This is happening because OData is probably removing the entire "Where" clause from the expression and then checking how many in total there are.
Seems to me like you found a legitimate bug here: OData should NOT remove EF entity/global filters since you don't want to expose those filtered entities to the outside in the first place.
I'm honestly surprised this problem exists... I thought the EF provider would inject that "where" clause at a level very close to the actual DB call, meaning it would be impossible to manipulate it from the outside. Looks like that's not the case after all?
Why do OData not join $expand to SQL query for getting count?
Because the expanded relationships do not matter for counting how many of the original ones there are. The expansions are removed to reduce the number of joins and make the count query faster overall.
My example may be not valuable. But you can think this issue with authorization.
I assume you mean resource-based authorization here? Like, "some users not having permission to see a subset of the resources"? I don't think OData has any support for this whatsoever to be honest. As far as I know, resource-based authorization itself is fairly "manual", since you have to code the checks yourself, and they are not done at the database level (which actually kinda sucks).
More on that here:
Now, if you were referring to multi-tenancy... then I think it would also have a very similar issue, since it is also usually coded with a global filter in EF.
Do you think I did something wrong?
Nope, I think what you were expecting here was correct.
How can I solve this problem? What can I do?
You might have to deal with the "include total count" functionality here manually, by injecting ODataQueryOptions
instead of relying on [EnableQuery]
. But even then, it's not 100% clear to me exactly how you'd do it to be honest.
Another option could be to create a view in the database that performs the filter for you, and then map the view in EF instead of mapping the original table. However, that will obviously be a lot more cumbersome than just using EF filters, so it would be less than ideal. I'm pretty sure OData would work fine though in that situation, since the source of the data is the view itself and the count would honor only non-deleted items since only those exist in the view.
EDIT: I actually missed something from your original post.... what you are counting is not the customers, but the activities. There are 4 activities, so returning 4 is correct to me. However, it looks like EF filters will filter transitive relationships as well? I'm actually surprised that's the case.
Can you try a few things?
If you query "customers" instead of "activities" with OData, does the total count then match with the entities?
Is it possible to create a filter in the activity entity explicitly, so that it only returns Where(activity => !activity.Customer.IsDeleted)
? I wonder if OData would then honor that.
Thank you @julealgon for your explainer answer.
Why do OData not join $expand to SQL query for getting count?
Because the expanded relationships do not matter for counting how many of the original ones there are. The expansions are > removed to reduce the number of joins and make the count query faster overall.
OData uses just main table for counting faster. It seems to be right at first view. But I want count of my url query what I mentioned before.
So in my opinion, OData should include $expand to Sql Query. Because if OData include $expand, EF HasQueryFilters will added automatically By EF.
You might have to deal with the "include total count" functionality here manually, by injecting ODataQueryOptions instead of relying on [EnableQuery]. But even then, it's not 100% clear to me exactly how you'd do it to be honest.
I am actually using both of them. And the issue still alive :(
More on that here:
Thank you, for your sharing. I read this article before. I used this approached in somewhere. That approach solve authorization problem in memory. And we get unnecessary data from database. I think, in this case, it may affect application performance negative way.
Another option could be to create a view in the database that performs the filter for you, and then map the view in EF instead of mapping the original table.
This is my last option. But you said that it is not good way to solve.
If you query "customers" instead of "activities" with OData, does the total count then match with the entities?
Yes, it will match. HasQueryFilters come into play because of customer entity is used in sql query.
Is it possible to create a filter in the activity entity explicitly, so that it only returns Where(activity => !activity.Customer.IsDeleted)? I wonder if OData would then honor that.
How can I do that? I dont understand clearly.
OData uses just main table for counting faster. It seems to be right at first view. But I want count of my url query what I mentioned before.
So, the way $count
works is that it has this well defined semantic: it will "count all records that exist beside your filtering". It feels to me like if you just want to count the actual entries, you could add this to the consumer side.
So in my opinion, OData should include $expand to Sql Query. Because if OData include $expand, EF HasQueryFilters will added automatically By EF.
That's an interesting point.... the expand is what is causing EF to ignore the transitive entity right? So yeah... this is tricky... OData doesn't keep the $expands by default and it would be hard to special case this I think.
What happens in your query if you do NOT expand Customer
when fetching activities? Will it EF fetch the one with the deleted customer?
Thank you, for your sharing. I read this article before. I used this approached in somewhere. That approach solve authorization problem in memory. And we get unnecessary data from database. I think, in this case, it may affect application performance negative way.
Pretty much yeah.... I'm not that big of a fan of it too exactly because it is not done at the DB layer automatically.
Is it possible to create a filter in the activity entity explicitly, so that it only returns Where(activity => !activity.Customer.IsDeleted)? I wonder if OData would then honor that.
How can I do that? I dont understand clearly.
I actually don't know if this works, but in the configuration for your Activity
entity, you could add this and see how it behaves:
modelBuilder.Entity<Activity>().HasQueryFilter(a => !a.Customer.IsDeleted);
I'm not saying that would be a "good" workaround, but it might just work. However, the problem is that one would have to replicate this on every possible relationship and it will also probably slow down some queries :(
Hi again @julealgon,
So, the way $count works is that it has this well defined semantic: it will "count all records that exist beside your filtering". It feels to me like if you just want to count the actual entries, you could add this to the consumer side.
I understood. But I dont know how can I count in client/consumer side.
Let me explain you with an example:
There are 1000 records in database according to my web url query. But I just take top 10 because of application performance. But OData returns me 2000 with @odata.count
. And I consider how many page are there my pagination system in list view according to the number given to me, that is 2000. So I will mislead my users. I will show more page numbers than it should be.
I hope I was able to explain.
What happens in your query if you do NOT expand Customer when fetching activities? Will it EF fetch the one with the deleted customer?
Yes, fetch deleted customers' activities into list.
I actually don't know if this works, but in the configuration for your
Activity
entity, you could add this and see how it behaves:modelBuilder.Entity<Activity>().HasQueryFilter(a => !a.Customer.IsDeleted);
I'm not saying that would be a "good" workaround, but it might just work. However, the problem is that one would have to replicate this on every possible relationship and it will also probably slow down some queries :(
Your opinion made me very curious. And I tried it. And it worked interestingly :) But I think, this solution is not sustainable. Because I have to think every combinations between entities while implementing this. And as you said, queries will run too slow.
I understood. But I dont know how can I count in client/consumer side. Let me explain you with an example: There are 1000 records in database according to my web url query. But I just take top 10 because of application performance. But OData returns me 2000 with
@odata.count
. And I consider how many page are there my pagination system in list view according to the number given to me, that is 2000. So I will mislead my users. I will show more page numbers than it should be.
Oh I thought you just needed the total returned, and not the overall total. Ok, I understand your issue really is that you don't want to ever consider the filtered entities in your total, which would be consistent with EF's behavior.
Yes, fetch deleted customers' activities into list.
Thanks for getting back on this. I personally had no idea entity filtering worked like that... it almost seems "wrong" in a way... What if I just wanted to see all activities with their customers, even if the customer was soft deleted? That would basically be impossible right? Do you think there is a chance that the bug is in EF's behavior here, and not in OData itself? Should it really not return the parent activity just because its customer is soft-deleted?
I know this is more of a question to the EF team, but anyways...
Your opinion made me very curious. And I tried it. And it worked interestingly :)
I see... so it is consistent indeed. Thanks for trying that.
But I think, this solution is not sustainable. Because I have to think every combinations between entities while implementing this. And as you said, queries will run too slow.
Yep... totally agree this is not sustainable at all. I just wanted to be sure that my logic was correct in that what matters for OData is the initial entity filter.
I honestly have no idea how this problem can be fixed now... I realized I said something wrong earlier, implying that the where part of the query is removed to fetch the total, but that of course is not the case: only joins and pagination are removed.
I believe that what is happening is that the expansion itself is causing the exclusion of entities by the EF logic, because the filter is included when the customer entity is accessed, but since the filter only applies because the expansion is specified, somehow OData just disregards that filter when fetching the count... (i.e. removing the expansion ends up removing that global where condition with it...).
I'll leave this to be investigated by someone with deeper knowledge of EF filters. It is most definitely a bug IMHO.
What if I just wanted to see all activities with their customers, even if the customer was soft deleted?
We decided to use another DbContext to show soft-deleted items.
Do you think there is a chance that the bug is in EF's behavior here, and not in OData itself?
I considered your idea before. In my opinion, OData's behavior is responsible for that. And we found this link System Query Option $count They said that:
The $count system query option ignores any $top, $skip, or $expand query options
I cannot understand why OData ignore $expand option.
Should it really not return the parent activity just because its customer is soft-deleted?
In some scenarios, yes they should return, in some scenarios no they should not return. It can change depend on what you need.
@hakanaltindis I found some evidence in the documentation that the situation you are seeing could be considered an error. Here:
Notice how this describes something very similar to your problem:
The problem can be observed when executing two queries:
var allPosts = db.Posts.ToList(); var allPostsWithBlogsIncluded = db.Posts.Include(p => p.Blog).ToList();
With above setup, the first query returns all 6 Posts, however the second query only returns 3. This mismatch happens because Include method in the second query loads the related Blog entities.
This leads me to believe that this is the situation you have: you must've configured Activity
to have a 'required' dependency on Consumer
, correct?
What would make the most sense to me, in your case, is to do as the article says and configure the relationship as Optional instead. This would not cause the expands to influence the total: the only difference is that the soft-deleted customer would be treated as 'null' (instead of causing the entire associated activity to not show in the list).
By changing the relationship to Optional, the counts from OData would match again.
The article also goes through basically what I suggested, of creating the same filter "on the other side of the relationship", but as we discussed that's a PITA and "we should not do that™".
With that in mind.... I believe what we have here is not actually an OData bug, but an EF quirk, so I'd personally close it (after you confirm it works fine for you).
Hi @julealgon,
Sorry for my late answer.
And sorry again to disappoint you. :( Because your evidence is not exactly match my issue. I understood what you mean. But according to in my situation, the return list is correct. I can get correct items what they should be. So I think EF Core is working right. But I cannot get correct number for @odata.count
. OData sets the property, so I still think OData is not working correctly.
So that there is no misunderstanding, My issue is about value of @odata.count
, Not items. The items is filled correctly.
Because your evidence is not exactly match my issue.
@hakanaltindis I'd humbly disagree. Now that I understand the full situation, I don't consider this an OData bug anymore.
The EF article states this very clearly, including a warning at the top:
Caution
Using required navigation to access entity which has global query filter defined may lead to unexpected results.
The situation you are seeing, where the inclusion of a relationship causes the parent to vanish from the query results, is described by them as "unexpected result" and "error", and I would agree. Even before reading the article, I was surprised to see the behavior you were getting and even questioned if this was an EF issue to begin with.
The suggested approach by the EF team is to change the relationship to Optional in this case so that it will still include the parent object in the query even if the underlying relationship is not there due to the global filter. Once the relationship is marked as Optional, you could then change your query to stop returning activities where the customer is deleted explicitly, and then the OData count property on that would match again.
So that there is no misunderstanding, My issue is about value of
@odata.count
, Not items. The items is filled correctly.
They are inherently linked to each other. The only reason you are seeing a mismatching count, is because of that weird behavior that causes the parent entity to disappear.
Now... while I don't personally consider this an OData bug (and would thus not attempt to special case the logic for this scenario), keep in mind that I'm not part of the OData team. You can still try to get OData folks to keep looking into this one and try some sort of fix in the OData side.
@xuzhg FYI
Hi again @julealgon,
I developed an OData Sample project for test. You can reach the source code with the link
These are my seed data:
I use this odata web query link for the test. http://localhost:5000/odata/CustomerActivity?$expand=Customer($select=FirstName)&$filter=customerid%20eq%203&$count=true
Trid tests wity your suggestions:
First:
builder.HasMany(c => c.CustomerActivities).WithOne(ca => ca.Customer).IsRequired();
I configured with above code and I called the odata query. The result is:
Generated query to count:
SELECT COUNT(*)
FROM odata."CustomerActivities" AS c
WHERE c."CustomerId" = @__TypedProperty_0
Generated query to get items:
SELECT c."Id", c."ActivityId", c."CustomerId", c."IsDeleted", t."FirstName", t."Id", FALSE
FROM odata."CustomerActivities" AS c
INNER JOIN (
SELECT c0."Id", c0."FirstName"
FROM odata."Customers" AS c0
WHERE NOT (c0."IsDeleted")
) AS t ON c."CustomerId" = t."Id"
WHERE c."CustomerId" = @__TypedProperty_0
Second:
builder.HasMany(c => c.CustomerActivities).WithOne(ca => ca.Customer).IsRequired(false);
I configured with above code and I called the odata query. The result is:
Generated query to count:
SELECT COUNT(*)
FROM odata."CustomerActivities" AS c
WHERE c."CustomerId" = @__TypedProperty_0
Generated query to get items:
SELECT c."Id", c."ActivityId", c."CustomerId", c."IsDeleted", t."FirstName", t."Id", (t."Id" IS NULL)
FROM odata."CustomerActivities" AS c
LEFT JOIN (
SELECT c0."Id", c0."FirstName"
FROM odata."Customers" AS c0
WHERE NOT (c0."IsDeleted")
) AS t ON c."CustomerId" = t."Id"
WHERE c."CustomerId" = @__TypedProperty_0
But The result I expected the count and the items are zero. But with your suggestion, they are 2. What can I do to get the result I expected?
What can I do to get the result I expected?
As far as I know and based on the recommendations in the EFCore article, you'd need to define the relational query filter explicitly as I mentioned earlier:
modelBuilder.Entity<Activity>().HasQueryFilter(a => !a.Customer.IsDeleted);
Now, my understanding is that you really want to treat everything in the system that has a relationship to a deleted customer as if the relationship itself also didn't exist. While keeping the relationship as "Required" achieves that, it doesn't seem to be a "supported" scenario by the EF team and more of a quirk behavior due to the change from LEFT JOIN
to INNER JOIN
.
I don't know of a truly general solution right now to what you want.
Really grateful for your helpful behavior, @julealgon
As far as I know and based on the recommendations in the EFCore article, you'd need to define the relational query filter explicitly as I mentioned earlier:
modelBuilder.Entity<Activity>().HasQueryFilter(a => !a.Customer.IsDeleted);
I am thinking to evaluate your suggestion. But my some of Global Query Filters is not simple like this sample. I worry about sustainable.
Any ideas from OData Team?
Thanks @hakanaltindis for reporting the issue. I believe the model that EF is exposing is inconsistent, and that it's a known issue, and that there's no one-size-fits-all solution to the issue... Let's consider this part of model configuration for a sec
modelBuilder.Entity<Activity>(e => {
e.HasOne(a => a.Customer)
.WithMany(c => c.Activities)
.HasForeignKey(a => a.CustomerId);
What you're specifying here is that every activity should be associated with a non-deleted customer.
The question then becomes, if one queries for activities particularly, should all non-optional relationships be applied when determining the "correct" list of activities to return? One customer might be okay with that behavior while another customer would probably want all activities to be returned irrespective of any query filters that would otherwise kick in if the non-optional relationships were considered. No solution fits all use cases.
For your particular requirement, the model that EF is exposing should not allow an activity that is not associated with a non-deleted customer to be returned.
This is not the behavior that EF has implemented. Querying activities without including any explicit associations returns all the records in the Activities
table. On the one hand, the model EF is exposing says that every activity has to be associated with a non-deleted customer, while on the other OData is able make queries against the same model to count the number of activities and EF is returning activities not associated with a non-deleted customer.
Like @julealgon observed, EF team knows about the issue and they included a caution in their documentation.
That's an inconsistency that's not easy to cure from the OData side.
Luckily, the customer is able to massage the model to behave the way they want. The one who'd like everything to be returned can pass false
to IsRequired
predicate, while the one who'd like only activities associated with non-deleted customers can apply the HasQueryFilter
predicate that @julealgon suggested. Obviously it means a lot more work and it may not be elegant.
The same dilemma that the EF team might have faced is the same one that the OData team would face if they attempted to deal with the inconsistency in some way.
@gathogojr, thanks for your reply.
I understood every relationships between entities what you want to say about them.(HasQueryFilter, IsRequired etc.) I know what will happen when I use them.
But there is one point what I could not get. That is:
I call the this query http://localhost:5000/odata/activity?$expand=customer($select=name)&$select=id,name&$top=10&$count=true
I think, OData does something like that to get data
_dbContext.Activities.Include(a => a.Customer).Select(...).Take(10);
But OData ignores expands to get count
_dbContext.Activities.Count();
Why does OData ignore expands getting count? What is their mind? I can not understand. They include filter or search, but they ignore expands. Why?
They said this in the link,
The $count system query option ignores any $top, $skip, or $expand query options, and returns the total count of results across all pages including only those results matching any specified $filter and $search.
Can you explain the thinking behind this?
@hakanaltindis Maybe @mikepizzo could chime in to clarify the OData TC committee's thinking around ignoring $expand
query option but let's think about it for a sec.
Should the following 2 queries return different values for @odata.count
?
Because if we try to accommodate your requirement, we'll apply $expand
in the first case and return @odata.count
as 3 while in the second case we'd return @odata.count
as 4. That behavior could also be considered inconsistent.
Like I said, there may not be one correct solution to the issue
Why does OData ignore expands getting count? What is their mind? I can not understand. They include filter or search, but they ignore expands. Why?
As I mentioned in one of the initial posts, this is very likely done for efficiency. Semantically, expanded properties should never affect the total count of records, so they are obviously eliminated to make the TotalCount call as fast as possible by removing unnecessary joins, which can be expensive. The filters are kept because you need to know "how many records there are with those conditions", for paging, etc, to be consistent.
Now, the fact that the global filter makes it so that expanding can change the results of the parent shouldn't influence this decision: it is, as the EF team itself put it, an error.
The reason OData ignores $expand is because $count counts the items at a particular level -- i.e., how many activities there are (not how many total records there are). This is the same for $top, $skip, etc. -- these operate at a particular level, not across the entire result. Note that, because $count counts at a particular level, you can nest $count in $expand to count the nested records, as in:
which will return the count of activities and the count of customers (and their names) for each activity.
For some reason, I had no idea $count
worked in nested queries! I guess it only makes sense. You learn something everyday with OData it seems lol. Thanks for the note @mikepizzo
Hi everyone,
Thanks for sharing your idea.
@gathogojr
Should the following 2 queries return different values for
@odata.count
?
- http://localhost:5000/odata/activity?$expand=customer&$count=true
- http://localhost:5000/odata/activity?$count=true
Because if we try to accommodate your requirement, we'll apply
$expand
in the first case and return@odata.count
as 3 while in the second case we'd return@odata.count
as 4. That behavior could also be considered inconsistent.
It seems to be inconsistent. But the other point view, I added Global Query Filter
on purpose. So I know It runs like this. And if I dont use Global Query Filter, the both queries will return @odata.count
as 4.
@julealgon
Semantically, expanded properties should never affect the total count of records What is the
$expand
meaning in SQL? I think it is meaning ÌNNER JOIN`, so after two tables are inner joined, you can see just matching items as result. For this reason, I think $expand should affect the total count of records.
@mikepizzo,
http://localhost:5000/odata/activity?$expand=customer($select=name;$count=true)&$select=id,name&$top=10&$count=true This sample does not work for me because I try to reach to Customer from Activities which means Activity does not have any collection property.
And all you, According to my sample, I used IsDeleted property. Maybe it is misleading you. But if we consider it was tenantId, I will display wrong number to tenants. To explain that, The tenant should only able to see data of their own records.
That's why I'm still not completely convinced.
What is the $expand meaning in SQL? I think it is meaning ÌNNER JOIN`, so after two tables are inner joined, you can see just matching items as result. For this reason, I think $expand should affect the total count of records.
To be honest, I actually am not 100% sure, but IIRC expand will end up being translated as a Select
somewhere to force the data to be fetched. The this select will end up forcing a JOIN if there isn't one, so that the columns on the relationship are accessible.
The distinction between INNER vs LEFT join depends on the type of relationship (required or optional).
I don't see how expanding by itself should impact the total number of records. The difference between a query with and without the expand is that one of the related properties will be included or not included in the final payload, but we are still talking about the same space of objects.
According to my sample, I used IsDeleted property. Maybe it is misleading you.
Speaking for myself here, but your example/scenario is pretty clear to me. I'm very used to both soft-deletion and multi-tenancy.
But if we consider it was tenantId, I will display wrong number to tenants. To explain that, The tenant should only able to see data of their own records.
If you were doing multi-tenancy, every table would have the tenantId
column, not only the Customer
table, so Activity
would be excluded because of that and the counts would again match. It wouldn't make sense for just one related table to be multi-tenant, but it's parent in the relationship to not be, as that would be a tenant violation by itself.
The distinction between INNER vs LEFT join depends on the type of relationship (required or optional).
Yes, it can be change according to configuration between entities.
I don't see how expanding by itself should impact the total number of records. The difference between a query with and without the expand is that one of the related properties will be included or not included in the final payload, but we are still talking about the same space of objects.
One query without $expand
returned a value. The query with $expand
will return smaller or same value than first one. The value may change, so the count of record will change.
If you were doing multi-tenancy, every table would have the
tenantId
column, not only theCustomer
table, soActivity
would be excluded because of that and the counts would again match. It wouldn't make sense for just one related table to be multi-tenant, but it's parent in the relationship to not be, as that would be a tenant violation by itself.
You are right. This sample was not appropriate. But my issue still there :)
One query without
$expand
returned a value. The query with$expand
will return smaller or same value than first one. The value may change, so the count of record will change.
Sure, I know this is what is happening with your scenario right now, but normally it doesn't. What you are experiencing is a quirk of EFCore's global filters and shouldn't be relied upon. Under any other circumstance, expansion of a property by itself does not reduce the number of parent entries.
Think as if you are using an unsupported behavior from EFCore: you can't sensibly expect dependent libraries (like OData) to adapt to such unsupported behaviors. I at least don't think it should.
But my issue still there :)
I totally understand. Not trying to dismiss your scenario, just want to make it clear that you are relying on something which I would not recommend anyone to rely on. Even if it sucks from a maintenance standpoint, I feel like adding the explicit filters on the parent entity is the way to go in your case.
Again, I don't speak for the OData team of course, so they will have to opt to either pursue some sort of fix to this situation, or decide that it shouldn't be touched.
Hi,
My situation may be rare topic. I did not find anything about that. I try to tell with a sample.
And consider records of the tables in database like the below
I configured everything well. The web application perfectly run with odata. But some conditions return wrong value.
Let's tell you that,
I called the below query to show on a list.
http://localhost:5000/odata/activity?$expand=customer($select=name)&$select=id,name&$top=10&$count=true
Response to the request is the below:
It returns an array with 3 items but
@odata.count
is 4. This is the my issue.You know that, OData call 2 SQL Query for this url. First one to get data. Second one to get count. I checked SQL Queries in console.
First Query like that:
Second Query like that:
Why do OData not join $expand to SQL query for getting count?
My example may be not valuable. But you can think this issue with authorization.
In addition, this issue break my pagination. Because I consider count of pages according to
@odata.count
How can I solve this problem? What can I do? Do you think I did something wrong?