Skip to content

Fix skiptoken paging issue when combined with $orderby involving nullable property - #859

Merged
xuzhg merged 1 commit into
OData:mainfrom
gathogojr:fix/1943-skiptoken-paging-issue-when-combined-with-orderby
Mar 27, 2023
Merged

Fix skiptoken paging issue when combined with $orderby involving nullable property#859
xuzhg merged 1 commit into
OData:mainfrom
gathogojr:fix/1943-skiptoken-paging-issue-when-combined-with-orderby

Conversation

@gathogojr

@gathogojr gathogojr commented Mar 14, 2023

Copy link
Copy Markdown
Contributor

This pull request fixes OData/WebApi#1943, fixes OData/WebApi#2041, and fixes OData/WebApi#2561

Description

Consider a simple scenario involving a data sample comprising of Id key field and a non-unique nullable field CreditLimit:

Id CreditLimit
1 null
2 2
3 null
4 30
5 null
6 35
7 5
8 50
9 25

From the controller action, we could configure a page size of 2:

[EnableQuery(PageSize = 2)]
public ActionResult<IEnumerable<S1Customer>> Get()
{
    return new List<S1Customer>
    {
        new S1Customer { Id = 1, CreditLimit = null },
        new S1Customer { Id = 2, CreditLimit = 2 },
        new S1Customer { Id = 3, CreditLimit = null },
        new S1Customer { Id = 4, CreditLimit = 30 },
        new S1Customer { Id = 5, CreditLimit = null },
        new S1Customer { Id = 6, CreditLimit = 35 },
        new S1Customer { Id = 7, CreditLimit = 5 },
        new S1Customer { Id = 8, CreditLimit = 50 },
        new S1Customer { Id = 9, CreditLimit = 25 },
    };
}

When the data sample is ordered by CreditLimit field ($orderby=CreditLimit), and then internally by Id (for uniqueness and predictability - stable ordering), the ordered data sample would look at follows:

CreditLimit Id
null 1
null 3
null 5
2 2
5 7
25 9
30 4
35 6
50 8

If we query for the S1Customers entity set and order by CreditLimit, we get the following response:

GET: http://localhost:5000/odata/S1Customers?$orderby=CreditLimit

Response:

{
    "@odata.context": "http://localhost:5000/odata/$metadata#S1Customers",
    "value": [
        {
            "Id": 1,
            "CreditLimit": null
        },
        {
            "Id": 3,
            "CreditLimit": null
        }
    ],
    "@odata.nextLink": "http://localhost:5000/odata/S1Customers?$orderby=CreditLimit&$skiptoken=CreditLimit-null,Id-3"
}

While the response is correct (including the next-link), if we attempt to use the returned next link to fetch the next batch of records, the following error message is returned:

The query specified in the URI is not valid. The binary operator GreaterThan is not defined for the types 'System.Nullable`1[System.Decimal]' and 'Microsoft.OData.ODataNullValue'.

What is happening here is that we’re mishandling the null value and attempting to apply a greater-than operator between the nullable CreditLimit field and the ODataNullValue.

This pull request fixes this issue by ensuring that a valid Where expression is composed as follows:

  • for skiptoken value CreditLimit-null,Id-3 in ascending order scenario - $orderby=CreditLimit, we compose the Where expression as: (CreditLimit ne null OR (CreditLimit eq null AND Id gt 3))
  • for skiptoken value CreditLimit-2,Id-2 in ascending order scenario - $orderby=CreditLimit, we compose the Where expression as: (CreditLimit gt 2 OR (CreditLimit eq 2 AND Id gt 2))
  • for skiptoken value CreditLimit-null,Id-3 in descending order scenario - $orderby=CreditLimit desc, we compose the Where expression as: (CreditLimit eq null AND Id gt 3)
  • for skiptoken value CreditLimit-35,Id-6 in descending order scenario - $orderby=CreditLimit desc, we compose the Where expression as: (CreditLimit lt 35 OR (CreditLimit eq 35 AND Id gt 6))

Consider also a more advanced scenario involving a data sample comprising of Id key field, a non-nullable field Grade, and a non-unique nullable field CreditLimit:

Id Grade CreditLimit
1 A null
2 B null
3 A 10
4 C null
5 A 30
6 C null
7 B 5
8 C 25
9 B 50
10 D 50
11 F 35
12 F 30
13 F 55

From the controller action, we would configure a page size of 4 as follows:

[EnableQuery(PageSize = 4)]
public ActionResult<IEnumerable<S2Customer>> Get()
{
    return new List<S2Customer>
    {
        new S2Customer { Id = 1, Grade = "A", CreditLimit = null },
        new S2Customer { Id = 2, Grade = "B", CreditLimit = null },
        new S2Customer { Id = 3, Grade = "A", CreditLimit = 10 },
        new S2Customer { Id = 4, Grade = "C", CreditLimit = null },
        new S2Customer { Id = 5, Grade = "A", CreditLimit = 30 },
        new S2Customer { Id = 6, Grade = "C", CreditLimit = null },
        new S2Customer { Id = 7, Grade = "B", CreditLimit = 5 },
        new S2Customer { Id = 8, Grade = "C", CreditLimit = 25 },
        new S2Customer { Id = 9, Grade = "B", CreditLimit = 50 },
        new S2Customer { Id = 10, Grade = "D", CreditLimit = 50 },
        new S2Customer { Id = 11, Grade = "F", CreditLimit = 35 },
        new S2Customer { Id = 12, Grade = "F", CreditLimit = 30 }
    };
}

When the data sample is ordered by Grade and then by CreditLimit field ($orderby=Grade,CreditLimit), and then internally by Id (for uniqueness and predictability - stable ordering), the ordered data sample would look at follows:

Grade CreditLimit Id
A null 1
A 10 3
A 30 5
B null 2
B 5 7
B 50 9
C null 4
C null 6
C 25 8
D 50 10
F 30 12
F 35 11
F 55 13

If we query for the S2Customers entity set and orderby Grade and then by CreditLimit, we get the following response:

GET: http://localhost:5000/odata/S2Customers?$orderby=Grade,CreditLimit

Response

{
    "@odata.context": "http://localhost:5000/odata/$metadata#S2Customers",
    "value": [
        {
            "Id": 1,
            "Grade": "A",
            "CreditLimit": null
        },
        {
            "Id": 3,
            "Grade": "A",
            "CreditLimit": 10.00
        },
        {
            "Id": 5,
            "Grade": "A",
            "CreditLimit": 30.00
        },
        {
            "Id": 2,
            "Grade": "B",
            "CreditLimit": null
        }
    ],
    "@odata.nextLink": "http://localhost:5000/odata/S2Customers?$orderby=Grade%2CCreditLimit&$skiptoken=Grade-%27B%27,CreditLimit-null,Id-2"
}

Again, the response is correct (including the next-link), but if we attempt to use the returned next link to fetch the next batch of records, the following error message is returned:

The query specified in the URI is not valid. The binary operator GreaterThan is not defined for the types 'System.Nullable`1[System.Decimal]' and 'Microsoft.OData.ODataNullValue'.

Again, this is due to mishandling of the null value.

In this pull request we ensure that a valid Where expression is composed as follows:

  • for skiptoken value of Grade-B,CreditLimit-null,Id-2 in ascending order scenario - $orderby=Grade,CreditLimit, we compose the Where expression as: Grade gt B OR (Grade eq B AND (CreditLimit ne null OR (CreditLimit eq null AND Id gt 2)))
  • for skiptoken value of Grade-F,CreditLimit-30,Id-12 in ascending order scenario - $orderby=Grade,CreditLimit, we compose the Where expression as: Grade gt F OR (Grade eq F AND (CreditLimit gt 30 OR (CreditLimit eq 30 AND Id gt 12)))
  • for skiptoken value of CreditLimit-null,Grade-C,Id-6 in ascending order scenario - $orderby=CreditLimit,Grade, we compose the Where expression as: CreditLimit ne null OR (CreditLimit eq null AND (Grade gt C OR (Grade eq C AND Id gt 6)))
  • for skiptoken value of CreditLimit-30,Grade-A,Id-5 in ascending order scenario - $orderby=CreditLimit,Grade, we compose the Where expression as: CreditLimit gt 30 OR (CreditLimit eq 30 AND (Grade gt A OR (Grade eq A AND Id gt 5)))

@gathogojr gathogojr changed the title Fix paging issue when combined with involving nullable property Fix paging issue when combined with $orderby involving nullable property Mar 14, 2023
@gathogojr gathogojr changed the title Fix paging issue when combined with $orderby involving nullable property Fix skiptoken paging issue when combined with $orderby involving nullable property Mar 14, 2023
@gathogojr
gathogojr force-pushed the fix/1943-skiptoken-paging-issue-when-combined-with-orderby branch from f0e3f67 to 05eb664 Compare March 14, 2023 11:39
@gathogojr
gathogojr force-pushed the fix/1943-skiptoken-paging-issue-when-combined-with-orderby branch from 05eb664 to e30be1f Compare March 15, 2023 05:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants