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

Support tolower() in $orderby option #1126

Closed
cppandcsharp opened this issue Dec 5, 2023 · 10 comments · Fixed by #1150
Closed

Support tolower() in $orderby option #1126

cppandcsharp opened this issue Dec 5, 2023 · 10 comments · Fixed by #1150

Comments

@cppandcsharp
Copy link

cppandcsharp commented Dec 5, 2023

Assemblies affected
Which assemblies and versions are known to be affected e.g. ASP.NET Core OData 8.x
Microsoft.AspNetCore.OData 8.2.3

Describe the bug
I am not able to use a tolower(text_field_in_model) in an $orderby clause.

Reproduce steps
Hit a request against a model that has a search string:

http://localhost/api/students?$orderby=tolower(name)

Data Model

public class Student
{
    [Key]
    public string Id {get; set;}
    public string Name {get; set;}
}

EDM (CSDL) Model

<EntityType Name="Student">
    <Key>
        <PropertyRef Name="id" />
    </Key>
    <Property Name="id" Type="Edm.String" Nullable="false" />
    <Property Name="name" Type="Edm.String" Nullable="false" />
</EntityType>

Request/Response
http://localhost/api/students?$orderby=tolower(name)

Expected behavior
Results should be ordered by name (the property used in the $orderby clause)

Actual behavior

The query specified in the URI is not valid. Only ordering by properties is supported for non-primitive collections. Expressions are not supported.

Screenshots
NA

Additional context
NA

I just checked the code and it appears only SingleValuePropertyAccessNode is supported, not SingleValueFunctionCallNode.

P.S.: This is a valid OData construct - https://services.odata.org/V4/TripPinService/People?$orderby=tolower(FirstName)%20desc,UserName

@cppandcsharp cppandcsharp added the bug Something isn't working label Dec 5, 2023
@julealgon
Copy link
Contributor

Does this work if you use $compute to create a new property that is the result of tolower(FirstName) and then order on that? Not dismissing the ask here, just curious (and maybe it could serve as a temporary workaround for you @holagathan ).

@xuzhg
Copy link
Member

xuzhg commented Dec 5, 2023

@holagathan The suggestion from @julealgon to use $compute is what I want to suggest.

http://localhost/api/students?$orderby=lowername&$compute=tolower(name) as lowername

More details about $compute, you can refer to this

@cppandcsharp
Copy link
Author

@xuzhg, @julealgon - Thank you for getting back.

I will explore this work-around for the short term. But, are there plans to support an explicit tolower()? If the APIs that I am working on go public, it may not be possible to expect all clients to use this combination as is.

@gathogojr gathogojr added feature and removed bug Something isn't working labels Dec 5, 2023
@gathogojr
Copy link
Contributor

Thanks @holagathan for reporting this issue. We currently don't support expressions in the $orderby query option. This is a feature gap.

Report back after trying out $compute as a possible workaround.

We'll add the issue to our backlog. We also accept pull request contribution if you're in a position to contribute.

@cppandcsharp
Copy link
Author

cppandcsharp commented Dec 5, 2023

@gathogojr, @xuzhg - $compute works, but not with $skipToken. I have support for $skipToken enabled, and am able to use that for all other options, except this.

I am able to get the needed results in the first page, but the next link does not have a $skipToken but only $skip. With no other changes, using just an $orderby expression returns the next link with $skipToken in it.

@julealgon
Copy link
Contributor

I am able to get the needed results in the first page, but the next link does not have a $skipToken but only $skip. With no other changes, using just an $orderby expression returns the next link with $skipToken in it.

That feels like a bug to me. Can you check if there is already an issue for that, and if not, create one?

@cppandcsharp
Copy link
Author

cppandcsharp commented Dec 6, 2023

@julealgon - No bug exists already and I have created this - #1127 - $skipToken is replaced with $skip when $compute is used.

Given the both options aren't working (tolower() in $orderby and $skipToken with $compute, what is the workaround?

@bogdan-patraucean
Copy link

bogdan-patraucean commented Dec 6, 2023

I need support for expresions in orderby too. There are times when you need to sort something that is case insensitive and tolower was the option I expected to work.

The problem is you can't order by a string type property and have this result:
A,B,C,c,D, but instead you will have A,B,C,D,c

@Vaduva-Calin
Copy link

I'm encountering the same issue as @bogdan-patraucean. Is there a work-around for this particular case?

@gathogojr gathogojr added the P2 label Dec 13, 2023
xuzhg added a commit that referenced this issue Jan 11, 2024
* Fixes #1126: enable built-in expression in $orderby

* Fixes the failing test case

* Add the comments

* Update src/Microsoft.AspNetCore.OData/Query/Validator/OrderByQueryValidator.cs

Co-authored-by: John Gathogo <[email protected]>

* Apply suggestions from code review

Co-authored-by: John Gathogo <[email protected]>

---------

Co-authored-by: John Gathogo <[email protected]>
@cppandcsharp
Copy link
Author

@xuzhg - Thank you for fixing this. I will give a try when available.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants