Skip to content

Problem with cursor paging and sorting by a field with null values #270

Closed

Description

@abrenoch

Describe the bug
We discovered an issue when sorting by a field with null values while using the PagingStrategies.CURSOR strategy. The issue manifests itself as no additional items loading beyond the first query, but upon further investigation it seems the query used to recover the 'cursor' item actually contains a small syntax problem (that doesn't cause an error - at least in MariaDB).

Have you read the Contributing Guidelines?

yes

To Reproduce
Using the following QueryOptions on a DTO:

@QueryOptions({
  pagingStrategy: PagingStrategies.CURSOR,
  enableTotalCount: true,
  maxResultsSize: 10,
})

Steps to reproduce the behavior:

  1. Apply sorting on a nullable field as so:
sorting: [
    {field: "someNullableField", direction: "ASC"}
]
  1. The first response will indicate more page available:
            "pageInfo": {
                "hasNextPage": true,
                "hasPreviousPage": false,
                "endCursor": "eyJ0eXBlIjoia2V5c2V0IiwiZmllbGRzIjpbeyJmaWVsZCI6Imdhcm1lbnRTdHlsZSIsInZhbHVlIjpudWxsfSx7ImZpZWxkIjoiaWQiLCJ2YWx1ZSI6OTY5NjM1fV19",
                "startCursor": "eyJ0eXBlIjoia2V5c2V0IiwiZmllbGRzIjpbeyJmaWVsZCI6Imdhcm1lbnRTdHlsZSIsInZhbHVlIjpudWxsfSx7ImZpZWxkIjoiaWQiLCJ2YWx1ZSI6OTY5NDQ4fV19",
                "__typename": "PageInfo"
            },
  1. But using that endCursor to get the next page of data, no items are returned in the query:
// supplied paging: {first: 10, after: $startCursor} (using endCursor from above for $startCursor)

{
    "data": {
        "getAssets": {
            "edges": [],
            "pageInfo": {
                "hasNextPage": false,
                "hasPreviousPage": true,
                "endCursor": null,
                "startCursor": null,
                "__typename": "PageInfo"
            },
            "__typename": "AssetConnection",
            "totalCount": 247941
        }
    }
}

Expected behavior
The next page of queried items is returned

Screenshots
NA

Desktop (please complete the following information):

  • Node Version: 20.11
  • Nestjs-query: 6.1.1

Additional context
The problem appears to be that at:

equalities.push({ [keySetField.field]: { eq: keySetField.value } } as Filter<DTO>)

The query produces an "eq": null filter, which at least on mysql should be "is": null.

Changing that line to:

equalities.push({ [keySetField.field]: { [keySetField.value === null ? 'is' : 'eq']: keySetField.value } });

Appears to resolve the issue for mysql - but I'm not sure if this would be correct for other drivers or if some other methodology should be considered.

Activity

  1. TriPSs commented on Jun 13, 2024

    @TriPSs
    Owner

    Could you make a PR? Then we will see if some tests fail.

  2. abrenoch commented on Jun 13, 2024

    @abrenoch
    ContributorAuthor

    @TriPSs

    So I'm trying to make this work but am running into a typescript problem with that change:

    Conversion of type '{ [x: string]: { is: DTO[keyof DTO]; }; }' to type 'Filter<DTO>' may be a mistake because neither type sufficiently overlaps with the other. If this was intentional, convert the expression to 'unknown' first.
      Type '{ [x: string]: { is: DTO[keyof DTO]; }; }' is not comparable to type 'FilterComparisons<DTO>'
    

    Looking at the types for FilterComparisons and FilterFieldComparison, it appears that FilterFieldComparison extends BooleanFieldComparisons... But using the is operator doesn't seem to be permitted.

    Seems like this is the only issue with the change - any thoughts on why this might be the case?

    Edit: Here is the branch I'm working with if it helps

  3. TriPSs commented on Jun 14, 2024

    @TriPSs
    Owner

    @abrenoch thanks, can you already create the PR? Will take a look.

  4. abrenoch commented on Sep 18, 2024

    @abrenoch
    ContributorAuthor

    Hey there, was circling back around to this issue.

    The tyescript errors mentioned above appear to be avoided if pushing the filter objects in separate conditions:

    if (keySetField.value === null) {
      equalities.push({ [keySetField.field]: { is: null } } as Filter<DTO>)
    } else {
      equalities.push({ [keySetField.field]: { eq: keySetField.value } } as Filter<DTO>)
    }
    

    Updated the PR to reflect this - I hope it is fully compatible!

  5. added a commit that references this issue on Sep 25, 2024
  6. abrenoch commented on Sep 25, 2024

    @abrenoch
    ContributorAuthor

    Fixed in #271

  7. added 4 commits that reference this issue on Sep 8, 2026
    3bb0f41
    e14586f
    28b9549
    eac719d
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions