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

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 this issue Jan 5, 2024 · 5 comments
Labels
bug Something isn't working

Comments

@hunsra
Copy link
Contributor

hunsra commented Jan 5, 2024

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

  • OS: iOS 17.0 simulator (also happens on Windows 11)
  • Version of supabase-csharp: 0.14.0

Additional context

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

@hunsra hunsra added the bug Something isn't working label Jan 5, 2024
@acupofjose acupofjose changed the title Can't use Table<TModel>.Where(Expression<Func<TModel, bool>> predicate) with more than 2 or'ed conditions in the predicate expression Can't use Table<TModel>.Where(Expression<Func<TModel, bool>> predicate) with more than 2 or'ed conditions in the predicate expression Jan 5, 2024
@acupofjose acupofjose transferred this issue from supabase-community/supabase-csharp Jan 5, 2024
@acupofjose
Copy link
Member

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
Copy link
Contributor Author

hunsra commented Jan 5, 2024

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
Copy link
Member

@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
Copy link
Contributor Author

hunsra commented Jan 14, 2024

@acupofjose, Thanks. This is excellent!

@kaushalkumar86
Copy link

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants