Skip to content

Commit

Permalink
Patch for custom binary operators that use standard comparison symbols
Browse files Browse the repository at this point in the history
When a custom binary operator uses a standard comparison operator (`=`, `<>`, `<`, `>`, `<=`, `>=`)
EF Core will generate a pattern that causes a syntax error in PostgreSQL.

Example SQL:
```sql
WHERE x."Inet" < @__inet_1 = TRUE
```

This patch adds logic to check if the custom binary operator is one of the standard
comparison operators. If so, it wraps the expression in parenthesis.

Example SQL (patched):
```sql
WHERE (x."Inet" < @__inet_1) = TRUE
```

While it seems like this could be avoided by having the custom translator construct
a standard `Expression.LessThan(Expression,Expression)`, this causes an error to be
thrown...because the binary operator isn't defined.

Example stack trace:

```
System.InvalidOperationException : The binary operator LessThanOrEqual is not defined
  for the types 'System.Net.IPAddress' and 'System.Net.IPAddress'.
  at System.Linq.Expressions.Expression.GetUserDefinedBinaryOperatorOrThrow(...)
  at System.Linq.Expressions.Expression.GetComparisonOperator(...)
  at System.Linq.Expressions.Expression.LessThanOrEqual(...)
```

Related:

- dotnet/efcore#9143
- #323 (review)
  • Loading branch information
austindrenski authored and roji committed May 26, 2018
1 parent 38d2176 commit c302579
Showing 1 changed file with 26 additions and 0 deletions.
26 changes: 26 additions & 0 deletions src/EFCore.PG/Query/Sql/Internal/NpgsqlQuerySqlGenerator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
#endregion

using System;
using System.Collections.Generic;
using System.Diagnostics;
using System.Linq.Expressions;
using System.Text.RegularExpressions;
Expand All @@ -41,6 +42,15 @@ public class NpgsqlQuerySqlGenerator : DefaultQuerySqlGenerator
{
readonly bool _reverseNullOrderingEnabled;

/// <summary>
/// The collection of standard comparison operators.
/// </summary>
/// <remarks>
/// See: https://github.com/aspnet/EntityFrameworkCore/issues/9143
/// </remarks>
[NotNull] static readonly HashSet<string> StandardComparisonSymbols =
new HashSet<string>(new[] { "=", "<>", "<", ">", "<=", ">=" });

protected override string TypedTrueLiteral => "TRUE::bool";
protected override string TypedFalseLiteral => "FALSE::bool";

Expand Down Expand Up @@ -322,12 +332,28 @@ public virtual Expression VisitCustomBinary(CustomBinaryExpression expression)
{
Check.NotNull(expression, nameof(expression));

//--------------------------------------------------------------------------------------------
// BUG: custom relational operators cause Postgres to throw 42601: syntax error at or near "="
// Example:
// source.Where(x => EF.Functions.LessThan(x.Inet, inet));
// WHERE x."Inet" < @__inet_1 = TRUE
//
// See: https://github.com/aspnet/EntityFrameworkCore/issues/9143
//--------------------------------------------------------------------------------------------
var operatorIsComparisonSymbol = StandardComparisonSymbols.Contains(expression.Operator);

if (operatorIsComparisonSymbol)
Sql.Append('(');

Visit(expression.Left);
Sql.Append(" ");
Sql.Append(expression.Operator);
Sql.Append(" ");
Visit(expression.Right);

if (operatorIsComparisonSymbol)
Sql.Append(')');

return expression;
}

Expand Down

0 comments on commit c302579

Please sign in to comment.