supabase-community / postgrest-csharp

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

Using multiple `.Order()` methods when querying doesn't work as expected. #85

Closed hunsra closed 10 months ago

hunsra commented 10 months ago

Bug report

Describe the bug

Using more than one .Order() method in a query doesn't seem to work. Only the first .Order() method is honored.

To Reproduce

Given a model with the following structure (and a 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; }
}

Making the following query:

response = await client.From<Contact>()
            .Order("last_name", Ordering.Ascending)
            .Order("first_name", Ordering.Ascending)
            .Order("middle_name", Ordering.Ascending)
            .Get();

Results in a list of Contact models that are only ordered by the "last_name" column.

Expected behavior

The resulting list should be ordered by the "last_name" column, then by the "first_name" column, then by the "middle_name" column.

Screenshots

The Supabase Edge API Network Log seems to indicate the proper ?order= query parameter is being created, but it is not returning records int he correct order, as described above:

"search": "?order=last_name.asc.nullsfirst&order=first_name.asc.nullsfirst&order=middle_name.asc.nullsfirst",

System information

Additional context

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

hunsra commented 10 months ago

Here's a screenshot of the resulting Models array from the debugger. As you can see, the first two items are in the wrong order:

image
acupofjose commented 10 months ago

I believe it's a matter of it being a comma separated query list instead of an additional order query variable! (see here).

If you want to do a PR, that'd be awesome! Otherwise I'll get to it when I can. The relevant code should be around here in Table.cs:

https://github.com/supabase-community/postgrest-csharp/blob/20240e65ba20768eddce361a4b85914e2c1b17d0/Postgrest/Table.cs#L658-L667

hunsra commented 10 months ago

Thanks @acupofjose.

Ah. I see. So, the query parameter needs to be formatted as a single, comma-separated list of order expressions, as in:

?order=last_name.asc.nullsfirst,first_name.asc.nullsfirst,middle_name.asc.nullsfirst

not as multiple chained query parameters, as in:

?order=last_name.asc.nullsfirst&order=first_name.asc.nullsfirst&order=middle_name.asc.nullsfirst

I'll try to see if I can make a PR to address it, but I can't promise anything as I've never made any edits to this codebase. Fingers crossed.

acupofjose commented 10 months ago

You’ve got it! That’s the necessary change. Like I said, the PR is appreciated (we love new contributors) but if you can wait, I’m happy to get to it myself.

hunsra commented 10 months ago

Understood, thanks.

Since I'm using the project, I'd like to be able to contribute. I'll give it a shot and make a PR if I can get it working.