TriPSs / nestjs-query

Easy CRUD for GraphQL.
https://tripss.github.io/nestjs-query/
MIT License
152 stars 43 forks source link

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

Open abrenoch opened 3 months ago

abrenoch commented 3 months ago

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"}
    ]
  2. The first response will indicate more page available:
            "pageInfo": {
                "hasNextPage": true,
                "hasPreviousPage": false,
                "endCursor": "eyJ0eXBlIjoia2V5c2V0IiwiZmllbGRzIjpbeyJmaWVsZCI6Imdhcm1lbnRTdHlsZSIsInZhbHVlIjpudWxsfSx7ImZpZWxkIjoiaWQiLCJ2YWx1ZSI6OTY5NjM1fV19",
                "startCursor": "eyJ0eXBlIjoia2V5c2V0IiwiZmllbGRzIjpbeyJmaWVsZCI6Imdhcm1lbnRTdHlsZSIsInZhbHVlIjpudWxsfSx7ImZpZWxkIjoiaWQiLCJ2YWx1ZSI6OTY5NDQ4fV19",
                "__typename": "PageInfo"
            },
  3. 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:  
https://github.com/TriPSs/nestjs-query/blob/0f599ed1b432b3c4a6575dfc5b1fc5fee9e92289/packages/query-graphql/src/types/connection/cursor/pager/strategies/keyset.pager-strategy.ts#L113
The query produces an `"eq": null` filter, which at least on mysql should be `"is": null`.

Changing that line to:
```ts
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.

TriPSs commented 3 months ago

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

abrenoch commented 3 months ago

@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

TriPSs commented 3 months ago

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