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:
  WHERE a < b = 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.

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.

See the related links for more information.

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
- npgsql#323 (review)
  • Loading branch information
austindrenski committed May 25, 2018
1 parent c1abc10 commit c0c7693
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 c0c7693

Please sign in to comment.