supabase-community / postgrest-csharp

A C# Client library for Postgrest
https://supabase-community.github.io/postgrest-csharp/api/Postgrest.html
MIT License
113 stars 22 forks source link

Can't use `Table<TModel>.Where(Expression<Func<TModel, bool>> predicate)` with more than 2 or'ed conditions in the predicate expression #84

Open hunsra opened 5 months ago

hunsra commented 5 months ago

Bug report

Describe the bug

An exception occurs in Table<TModel> Where(Expression<Func<TModel, bool>> predicate) if the predicate includes more than 2 or'ed conditions.

To Reproduce

Given the following model (and corresponding table in a Supabase project):

[Table("Contacts")]
public class Contact : BaseModel
{
    [Column("first_name")]
    public string FirstName { get; set; }

    [Column("middle_name")]
    public string MiddleName { get; set; }

    [Column("last_name")]
    public string LastName { get; set; }
}

Use of the following code:

string filter = "Ran";
var response = await supabase.From<Contact>().Where(c => c.FirstName.Contains(filter) || c.MiddleName.Contains(filter) || c.LastName.Contains(filter)).Get();

Results in the following PostgrestException exception:

Message:
{"code":"PGRST100","details":"unexpected \".\" expecting \"(\"","hint":null,"message":"\"failed to parse logic tree ((or.(first_name.like.*Ran*,middle_name.like.*Ran*),last_name.like.*Ran*))\" (line 1, column 6)"}

Source: "Supabase.Postgrest"

StackTrace:
   at Postgrest.Helpers.<MakeRequest>d__3.MoveNext()
   at Postgrest.Helpers.<MakeRequest>d__2`1.MoveNext()
   at Concord.Services.SupabaseService.<GetContactsByName>d__33.MoveNext() in C:\Repos\Concord\Concord\Services\Supabase.cs:line 328
   at Concord.ViewModels.MainViewModel.<SearchContacts>d__40.MoveNext() in D:\Repos\Concord\Concord\ViewModels\MainViewModel.cs:line 272

If the code is modified to use fewer or'ed conditions, such as in:

string filter = "Ran";
var response = await supabase.From<Contact>().Where(c => c.FirstName.Contains(filter) || c.LastName.Contains(filter)).Get();

The filter succeeds and behaves as expected.

It appears from the exception message that the underlying Postgrest code is improperly formatting the logic tree: ((or.(first_name.like.*Ran*,middle_name.like.*Ran*),last_name.like.*Ran*))

Perhaps it should have been: ((or.(first_name.like.*Ran*,middle_name.like.*Ran*,last_name.like.*Ran*)))

Expected behavior

The query should successfully select records whose first_name, middle_name, or last_name fields contain the specified filter string.

System information

Additional context

This is happening in a .NET MAUI application targeting iOS, Android, Windows, and mac Catalyst.

acupofjose commented 5 months ago

Ah, yes indeed! This is an easily re-duplicated issue. Thanks for the clear bug report!

As an easy (albeit verbose fix) you can use the .Or method like so:

var response = await client.From<Contact>().Or([
    new QueryFilter("first_name", Operator.Contains, filter),
    new QueryFilter("middle_name", Operator.Contains, filter),
    new QueryFilter("last_name", Operator.Contains, filter),
]).Get();

Otherwise, I'm going to externally process a bit, as it's not immediately clear how to go about addressing this issue and it's helpful for me to ramble a bit to organize my thoughts.

The postgrest library declares its own LINQ expression parsers, as it translates a LINQ expression into a query string so that the Postgrest server knows what to do with it. The benefit being (as may be obvious) the LINQ queries are actually translated into server-side filters. Which is a pretty nice deal.

However, it means that translating to a string can be a bit complicated.

The way that Microsoft handles this in C# is using Expressions or Nodes. The most complicated of these is the WhereExpressionVisitor.

Essentially, the expression is broken down into nodes.

So, for an expression like this (EX1):

var query = await client.Table<User>().Where(u => u.Contains("David") || u.Contains("John")).Get()

The expression is broken into a tree (that is, left and right nodes) as it is parsed.

EX1

  1. Classify as a BinaryExpressionNode
  2. Visit Left Side (u.Contains("David")) a MethodExpressionNode and generate its string representation
  3. Visit Right Side (u.Contains("John")) a MethodExpressionNode and generate its string representation
  4. Combine using the operator ||
  5. Generate a QueryFilter that matches the expression

Unfortunately, this becomes complicated when an additional node is added to the expression. As the Left and Right sides become confused. The initial Left becomes a Binary Expression containing the first two elements whereas the Right becomes a standalone element.

So, for an expression like this (EX2):

var query = await client.Table<User>().Where(u => u.Contains("David") || u.Contains("John") || u.Contains("Adam")).Get()

EX2

  1. Classify as a BinaryExpressionNode
  2. Visit Left Side (u.Contains("David") || u.Contains("John")) a BinaryExpressionNode 2a. Visit Left Side (u.Contains("David")) a MethodExpressionNode and generate its string representation 2b. Visit Right Side (u.Contains("John")) a MethodExpressionNode and generate its string representation 2c. Combine using the operator ||
  3. Visit Right Side (u.Contains("Adam")) a MethodExpressionNode and generate its string representation
  4. Combine using the operator ||
  5. Generate a QueryFilter that matches the expression

At the moment, I'm unsure how to bridge the gap between steps 2 and 3 and recognize the whole expression as a series of OR operations.

hunsra commented 5 months ago

Thanks @acupofjose for the fast response! I tried the fix and it works well. I was hoping not to have to use the actual field names (e.g., "first_name") and continue to use the model representation property names (e.g., FirstName) in case the table schema changes in the future, but I can live with that.

I'll continue on with the .Or method fix and check back in the future for different resolution.

Thanks again!

acupofjose commented 5 months ago

@hunsra for the record, with v3.5.0 you should now be able to specify QueryFilters using LINQ expressions:

var filters = new List<IPostgrestQueryFilter> {
    new QueryFilter<Contact, string>(x => x.FirstName, Operator.Contains, filter),
    new QueryFilter<Contact, string>(x => x.MiddleName, Operator.Contains, filter),
    new QueryFilter<Contact, string>(x => x.LastName, Operator.Contains, filter),
}

var response = await client.From<Contact>().Or(filters).Get();

Unfortunately, the generic signature is a little verbose. But it's de-coupled from the Table<TModel> so I don't have a way to infer it!

hunsra commented 5 months ago

@acupofjose, Thanks. This is excellent!

kaushalkumar86 commented 2 months ago

@hunsra for the record, with v3.5.0 you should now be able to specify QueryFilters using LINQ expressions:

var filters = new List<IPostgrestQueryFilter> {
    new QueryFilter<Contact, string>(x => x.FirstName, Operator.Contains, filter),
    new QueryFilter<Contact, string>(x => x.MiddleName, Operator.Contains, filter),
    new QueryFilter<Contact, string>(x => x.LastName, Operator.Contains, filter),
}

var response = await client.From<Contact>().Or(filters).Get();

Unfortunately, the generic signature is a little verbose. But it's de-coupled from the Table<TModel> so I don't have a way to infer it!

This doesn't work, the Operator.Contains need list as criterion (gave errors when used to query array data type), from QueryFilter constructor "List or Dictionary must be used supplied as criteria with filters that accept an array of arguments."

var filters = new List<IPostgrestQueryFilter> {
    new QueryFilter<Contact, List<string>>(x => x.FirstName, Operator.Contains, filter),
    new QueryFilter<Contact, List<string>>(x => x.MiddleName, Operator.Contains, filter),
    new QueryFilter<Contact, List<string>>(x => x.LastName, Operator.Contains, filter),
}

var response = await client.From<Contact>().Or(filters).Get();

should work. took me a few hours get my mix of and and or filters working.. The data type "Players" in the following is text array. now

List<IPostgrestQueryFilter> andFilter = new List<IPostgrestQueryFilter>()
{
    new QueryFilter<SupaGameTable, DateTime>(x => x.CreatedAt, Operator.LessThan, DateTime.Now),
    new QueryFilter<SupaGameTable, DateTime>(x => x.CreatedAt, Operator.GreaterThanOrEqual, DateTime.Now.AddSeconds(-Preferences.findTableWaitTime)),
    new QueryFilter<SupaGameTable, List<string>>(x => x.Players, Operator.Contains, new List<string>() { "-" }),
    new QueryFilter<SupaGameTable, int>(x => x.MinJoinPoints, Operator.LessThan, player.UserGold)
};

with

ModeledResponse<SupaGameTable> tables = await _Supabase.Postgrest.Table<SupaGameTable>()
    .And(andFilter)
    .Not(x => x.Players, Operator.Contains, new List<string>() { player.UserId })
    .Order(x => x.MinJoinPoints, Ordering.Descending)
    .Get();

works as expected.