Handle expressions that result in top-level case statements in Search… #7331
Conversation
…ConditionTranslatingVisitor Addresses dotnet#7271
|
Hi @tuespetre, I'm your friendly neighborhood .NET Foundation Pull Request Bot (You can call me DNFBOT). Thanks for your contribution! The agreement was validated by .NET Foundation and real humans are currently evaluating your PR. TTYL, DNFBOT; |
|
|
||
| if (_isSearchCondition) | ||
| if (expression.IsLogicalOperation()) |
There was a problem hiding this comment.
simplify this as _isSearchCondition = expression.IsLogicalOperation()
| .Where(o => o.OrderID > 54321 | o.OrderID > 98765 | o.OrderID > 1234567)); | ||
| } | ||
|
|
||
| [ConditionalFact] |
There was a problem hiding this comment.
Just for coverage - Test for binary and with binary and.
| @@ -2029,6 +2029,43 @@ public virtual void Where_Is_on_same_type() | |||
| } | |||
|
|
|||
| [ConditionalFact] | |||
There was a problem hiding this comment.
Also add same tests in AsyncQueryTestBase. You don't need to assert SQL gen for those.
|
@tuespetre - Please update PR with requested changes and rebase it on latest dev. |
|
@tuespetre are you planning to address the feedback on this PR? |
|
Oh, sorry -- I forgot to reply. Yes, I am planning to. |
|
@smitpatel looks like this is obsoleted by your new PR, want me to close this one? |
|
@tuespetre - Thank you for your efforts. We had been pending doing a refactor in that part of query pipeline to improve accuracy since we had seen good incoming traffic of bugs in that area. Due to being closer to correct solution, the underlying issue was getting resolved in refactoring. Since the refactored code was different approach from the one outlined in this PR (which is along the line of older code), we couldn't take this in as refactoring would be changing all of it. Though thank you for your help. |
|
Anytime! |
…ConditionTranslatingVisitor
Addresses #7271