-
-
Notifications
You must be signed in to change notification settings - Fork 87
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Some bug about Guid,Guid? filter and Include query #85
Comments
I'm not sure I'm following what problem you are trying to describe. It may be helpful if you could include the test data for those entities so that I can replicate what you are doing. It may also be related to #76 so please check there to see if it's the same issue. I do not have a solution to that issue yet. It does seem like you are saying there is an issue with the null checks on the filters? Instead of casting to (Guid?) and then comparing to "null", could you try using the ".HasValue" property and see if that make a difference? So change "(Guid?)t.TenantId == null" to "!t.TenantId.HasValue". |
Tks for reply. |
First question ,linq query like this,have an include method to load refer nav property
If Userinfo have an ISoftDelete filter,and type is bool |
Second when we want to user a nullable filter,if we set the filter value to null So entity will query fail... |
And third NOTE:I have change the DynamicFilter parameter to a simple name like @TenantID to read easy |
The last i found the sql has too many no useful nesting sub query in verision >=2.0.0 than 1.4.11 Hope my poor english explains clearly.I add the InitData code in topic |
I'm going to try to answer each of these posts based on the "question number" you referenced in them. With 4 being your "last one".
2 & 3) Please try changing your filter to "!t.TenantId.HasValue" as I suggested in my first response and see if that makes a difference. I'm wondering if the cast to Guid? may be causing the problem and this will tell me if that's the case.
|
Here is question 1 linq query,
1.4.11 sql in profiler
The join is correct 2.4.0 sql in profile
The outer apoly join is
This join will cause parent load fail,the correct should be
UserInfo4's parent is UserInfo1,the result is correct! |
I reproduced your first problem and tracked it down. I have a fix but need to do a little more testing and ran out of time tonight. I'll try to get it pushed out tomorrow. |
Glad for see your reply ,when question 1 fixed ,I will explain 2 and 3 |
Could you please explain it now? If there's a change I need to make, I would rather push out once than multiple times. |
OK, Now i explain Q2
the sql will have sql like
Filter set is the correct sql should be
Such as my IMayHaveTenant filter,but if the filter value is int? long? the sql is correct like
I have test in ef with no filter |
Now is Q3,very like Q2
And real filter query i have see
As above ,(CAST(NULL AS uniqueidentifier)) will never get a right value to compare |
For Q2 & Q3: I cannot reproduce the bad sql problems you are seeing - including seeing "cast" in the sql. I cut-and-pasted your code exactly as you posted it above. However, in the code you posted, you have this statement:
This is referencing the "tenantId" parameter name incorrectly. Case matters here - that needs to be "tenantId" not "TenantId". Change that to the following and the set parameter will match correctly.
Or you can remove the parameter name from that statement completely since there is only 1 parameter. Also, try this for your filter:
Notice this is not doing any null checks at all. The library sees that you are working with a nullable type. So it automatically includes the necessary "is null" and "is not null" checks as necessary. This is the resulting SQL I get for both tests (one with tenantId set to a guid value and one with it set to null):
Please make these adjustments and re-test. If you still see problems, please double check your actual code against what you posted to see if there are any discrepancies. |
Just pushed out a new NuGet package with the fix for your first issue. Let me know how that works. |
Glad for release pack, |
Q2,Q3 i have test in 2.5.0 and now is no problem,i just have some suggestion in sql Here is entity
Filter :
InitData
DbContext filter set,both used,and same effect in sql:
Now is the first linq query:
I have see 2.5.0 can get entity ,so Q2 now just some suggestion for optimize ,sql is
The suggest sql is
|
Now continute for Q3 and up
and use 2.5.0,sql is
and suggest sql
|
I have test 2.5.0,found Q2 and Q3 is miss, so just some sql optimize suggestion. |
I don't have fine grained control over how EF builds the actual SQL. All I can do is construct it using what EF exposes (which is database agnostic) and it will generate the SQL as best it can (which is database specific). So while the SQL you are suggesting is certainly how I would write it myself, I can't make EF do that. Just to be clear: The change I made was to fix Q1 but you didn't mention that. Is that fixed for you? And the changes did not affect Q2 & Q3 (I don't think) but you say that is also working correctly now? So everything is now working other than the non-optimal SQL which I can't control? |
Yes , I test 2.5.0 Q2 and Q3 is working corredtly now. And now i'll try to up version to 2.5.0 ,TKS! |
I believe this is all resolved except for the non-optimal SQL (the "outer apply") which is covered by issue #72. So closing this for now - if there is other information or still problems, please reopen or post a new issue. |
And see the test code
The text was updated successfully, but these errors were encountered: