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

Expression predicates combined with bitwise OR do not correctly compose with subsequent predicates #3061

Closed
ymotton opened this issue Jan 15, 2024 · 5 comments

Comments

@ymotton
Copy link

ymotton commented Jan 15, 2024

Context:
We're investigating porting our saas-platform from sql-server to postgresql.
We use an ExpressionBuilder helper class to programatically combine multiple lambda expressions.
This allows us to more easily combine complex filtering criteria into a larger compound expression.

For instance:

//Trivialized example for brevity
Expression<Func<Product, bool>> predicate1 = x => x.Brand == brand1;
Expression<Func<Product, bool>> predicate2 = x => x.Brand == brand2;
Expression<Func<Product, bool>> compoundPredicate = ExpressionBuilder.Or(predicate1, predicate2);

var matchingProducts = myDbContext.Set<Product>().Where(compoundPredicate).ToArray();

Build-up:
The ExpressionBuilder's "Or" incorrectly combines the two expression bodies with Expression.Or instead of Expression.OrElse.
The former is the Bitwise Or-operator and the latter is the logical Or-operator.
So in essence compoundPredicate translates to x => x.Brand == brand1 | x.Brand == brand2; instead of x => x.Brand == brand1 || x.Brand == brand2;.

The Issue:
The issue arises, not in how the library handles this bitwise OR. This seems to generate a logical OR in the SQL-statement as expected, but rather in how it composes with subsequent Where() filters.

// This returns Products of either brand1 or brand2, as expected
var matchingProducts = myDbContext.Set<Product>()
    .Where(compoundPredicate)
    .ToArray();

// You would expect this to return all products of either brand1 or brand2 with a price higher than 10
// It does not: it returns ALL Products of brand1 and only Products of brand2 with a price higher than 10
var otherProducts =  myDbContext.Set<Product>()
    .Where(compoundPredicate)
    .Where(x => x.Price > 10M)
    .ToArray();

What happened:
The first Where was not correctly surrounded with parentheses when combined with the second Where in SQL:

SELECT *
FROM products AS p
WHERE p.brand = 'brand1' OR p.brand = 'brand2' AND p.price > 10

When I fixed the way the expressions get combined (Using logical OR) the problem was fixed:

SELECT *
FROM products AS p
WHERE (p.brand = 'brand1' OR p.brand = 'brand2') AND p.price > 10

But I felt I should mention it, as this does not seem like correct behavior.

@ymotton ymotton changed the title Expression predicates combined with Expression.Or do not correctly compose with subsequent predicates Expression predicates combined with bitwise OR do not correctly compose with subsequent predicates Jan 15, 2024
@roji
Copy link
Member

roji commented Jan 16, 2024

Duplicate of dotnet/efcore#30248

@roji roji marked this as a duplicate of dotnet/efcore#30248 Jan 16, 2024
@roji
Copy link
Member

roji commented Jan 16, 2024

This is an EF-side issue; using Or rather than OrElse is generally not the right thing, which is why a quicker fix hasn't been prioritized.

The ExpressionBuilder's "Or" incorrectly combines the two expression bodies with Expression.Or instead of Expression.OrElse.

Note that Or being bitwise and OrElse being logical is by-design; this is how the APIs were designed (there's history there).

@roji roji closed this as not planned Won't fix, can't repro, duplicate, stale Jan 16, 2024
@ymotton
Copy link
Author

ymotton commented Jan 16, 2024

@roji
Is there a reason this issue only reared its head as we switched to Npgsql.EntityFrameworkCore.PostgreSQL?
The SqlServer provider seemed to handle this correctly.

@roji
Copy link
Member

roji commented Jan 16, 2024

There could be some operator precedence differences between the two databases... Can you post a minimal, runnable console app that works on SQL Server but fails on PG?

@ymotton
Copy link
Author

ymotton commented Jan 16, 2024

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

No branches or pull requests

2 participants