Skip to content
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

Fix Issue 13919: Translation error using negative #21389

Merged
merged 5 commits into from
Jun 30, 2020

Conversation

jonlouie
Copy link
Contributor

@jonlouie jonlouie commented Jun 23, 2020

This pull request resolves Issue #13919: Translation error using negative.

Summary of Issue

If a LINQ query references an entity property and uses a negated expression, negation only applies to the first term in the expression.

Example:
db.Entities.Where(e => e.Num1 == -(e.Num2 + 100))
Expected translation:
SELECT ... FROM Entities e WHERE e.Num1 = -(e.Num2 + 100)
Actual translation:
SELECT ... FROM Entities e WHERE e.Num1 = -e.Num2 + 100


Summary of Changes

  • On translation, add opening and closing parentheses around expression if expression type is ExpressionType.Negate
  • Add unit test for SQL Server and SQLite providers

Concerns

  • Are these tests placed in the right place? I couldn't find a set of unit tests for unary expressions.

@dnfadmin
Copy link

dnfadmin commented Jun 23, 2020

CLA assistant check
All CLA requirements met.

@@ -737,8 +737,9 @@ protected override Expression VisitSqlUnary(SqlUnaryExpression sqlUnaryExpressio

case ExpressionType.Negate:
{
_relationalCommandBuilder.Append("-");
_relationalCommandBuilder.Append("-(");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use RequiresBrackets helper, so that we don't add parentheses for simple expressions like negate(column) or negate(constant)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also add tests for negate on a column, as well as negate on like expression

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the valuable feedback; I have implemented your suggestions in my latest commits. Please let me know if there's anything I missed or can improve upon.

@smitpatel
Copy link
Member

@jonlouie - You will need to sign Contributor License Agreement before we can process this PR.

@jonlouie
Copy link
Contributor Author

@smitpatel Thank you for the reminder and your patience- had to secure approval before signing the CLA.

@smitpatel smitpatel removed the blocked label Jun 26, 2020
@maumar maumar merged commit 4795eee into dotnet:master Jun 30, 2020
@maumar
Copy link
Contributor

maumar commented Jun 30, 2020

@jonlouie I have merged the PR. Thank you for the contribution!

@jonlouie
Copy link
Contributor Author

@maumar Amazing! Appreciate the fast turnaround and community inclusion :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants