$skipToken is replaced with $skip when $compute is used

[cppandcsharp]

Assemblies affected ASP.NET Core OData 8.x

Describe the bug NextLink does not have $skipToken even when configured, but instead has $skip when a $compute expression is used in the request.

Reproduce steps

1. Enable SkipToken for your controllers.
2. Hit your API service with a $compute expression:

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

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:2000/beta/students?$orderby=lowername&$compute=tolower(name) as lowername

Expected behavior NextLink in the response should have a $skipToken, but it instead has a $skip.

Screenshots NA

Additional context

1. The default SkipToken handler is used.
2. The same code returns a $skipToken when used without the $compute expression and even with just the $orderby expression.

[aboryczko]

I don't think that using compute values in skip tokens is correct. Looking at it from a database perspective, we use skip token so that we don't have to do row skipping calculations which don't scale.

Since you are using a function in your ordering it would require to calculate the tolower value for all items hence skip token would not yield the expected performance boost but even a significant degradation because now instead of calculating comparisons for skip+take items you need to calculate tolower for all items.

Looking at your previously reported issue my question is: why do you need case-(in)sensitive ordering? do you really need to support both? you can get one or the other by using an appropriate collation on the database, either per column or database. if you really need to support both I would consider an additional column which would always be lower case. Otherwise you will have performance issues once you have more records.

[cppandcsharp]

@aboryczko - One of my database indexes is created for the lowercase version of a column. To use this database index to the fullest and save sequential scans, it is required that I use the lowercase version wherever appropriate in ORDER BY, WHERE, etc. Isn't a use-case for a case insensitive ordering a legitimate one?

Collations are what has been recommended, but I would not want to do that to my database just to get around this limitation when it should have been a capability as per OData spec.

And, I am not specifically concerned about whether having $skipToken in $compute is correct or not. Enabling $skipToken for my OData routes eliminates support for $skip (even for endpoints where this may be ideal over $skipToken), and using $compute introduces that back out of nowhere. I do not know if that is by-design, but if $compute is being recommended to compensate for tolower() in the $orderby, there needs to be an equivalent capability to fill the feature gap.

[aboryczko]

You could try to implement your own SkipTokenHandler based of the DefaultSkipTokenHandler and if you succeed you could make a PR out of it.

[cppandcsharp]

@aboryczko - I did explore that as well. But, from my limited experience on this, SkipTokenHandlers cannot be configured per endpoint unless there is some factory logic inside the handler that decides which exact version of the handler to really call based on the endpoint. If true, that to me, is another area that has a scope for support.

[aboryczko]

the handler is a singleton, I would go the way where you "fix" the compute use case and leave the rest as is. I think that supporting plain $orderby=tolower(property) is much more effort with all the cases and I wouldn't expect it to be done anytime soon.

[cppandcsharp]

Got it.

I did explore other options too. But, my Controller extends ODataController and returns an IQueryable (I don't want to change that), because of which any EF OrderBy is overridden by what is provided in the $orderbyclause. And there isn't a direct way this can be done at the EF level too (without getting into collations, etc.), to translate all Where and OrderBy on a specific property to enforce use of lower-case.

[aboryczko]

I wanted to add one more detail. $compute properties are considered open type properties and in the DefaultSkipTokenHandler you can see

if (orderByNodes.OfType<OrderByOpenPropertyNode>().Any())
{
    //SkipToken will not support ordering on dynamic properties
    return null;
}

either way it's a lot of work to make this happen :worried:

[cppandcsharp]

cc: @xuzhg for the RCA on why $skipToken does not work with $compute.

[xuzhg]

cc: @xuzhg for the RCA on why $skipToken does not work with $compute.

It seems it's a feature gap when implementing SkipTokenHandler. We'd find resource to fix it.