Skip to content

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

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

Closed
hunsra opened this issue Jan 6, 2024 · 5 comments
Closed
Labels
bug Something isn't working

Comments

@hunsra
Copy link
Contributor

hunsra commented Jan 6, 2024

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

  • 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 6, 2024
@hunsra
Copy link
Contributor Author

hunsra commented Jan 6, 2024

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

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:

foreach (var orderer in _orderers)
{
var nullPosAttr = orderer.NullPosition.GetAttribute<MapToAttribute>();
var orderingAttr = orderer.Ordering.GetAttribute<MapToAttribute>();
if (nullPosAttr == null || orderingAttr == null) continue;
var key = !string.IsNullOrEmpty(orderer.ForeignTable) ? $"{orderer.ForeignTable}.order" : "order";
query.Add(key, $"{orderer.Column}.{orderingAttr.Mapping}.{nullPosAttr.Mapping}");
}

@hunsra
Copy link
Contributor Author

hunsra commented Jan 7, 2024

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

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

hunsra commented Jan 7, 2024

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.

hunsra added a commit to hunsra/postgrest-csharp that referenced this issue Jan 7, 2024
….GenerateUrl() to produce a single order query parameter with comma-separated expressions for each chained .Order() method rather than producing a separate order query parameter for each.
@hunsra hunsra mentioned this issue Jan 7, 2024
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

2 participants