Use OFFSET syntax to make pageInfo.hasPrevious page work

[legomind]

I have submitted #169 to start addressing MSSQL server support, but then I discovered that a where clause is being used to window the query instead of offset.

The COUNT(1) OVER() statement will only return the count constrained by the where clause, so pageInfo.hasPrevious will never return true.

If the OFFSET was used instead, the count query would return the entire count, so hasPrevious would work as designed.

The only way that I can see this working is with a query like this:

SELECT [id]
    , COUNT(1) OVER () AS [full_count]

FROM [Users] AS [User]

--Include where clause here if specified by user

ORDER BY [User].[id]

ASC OFFSET (
    SELECT ISNULL(
        (SELECT ROW_NUMBER() OVER(ORDER BY Id) FROM dbo.Users WHERE id = @LastId), --Include where clause here if specified by user
        (SELECT COUNT(*) FROM dbo.Users) --Include where clause here if specified by user
    )
) ROWS
FETCH NEXT 1 ROWS ONLY

[mickhansen]

@brad-decker has a PR that changes cursors to use offset instead (to solve another issue): #165

My only concern is a slight perf decrease since OFFSET still fetches all rows and then filters out.

[legomind]

It depends on the sql server. mssqlserver will compute row windows for the entire query, but only fetch the rows within the offset and limit range.

[borderBite]

@legomind @mickhansen With MSSQL, and maybe other dbs too, COUNT(1) OVER () AS [full_count] causes the DB to iterate through all rows compute full_count.

If you're not filtering the results, it's especially painful, because it's doing the equivalent of:
SELECT COUNT(*) FROM XXXX
but it can take WAY longer on a large table. On one of my tables with only 2M records it takes 10x longer.

I think there are more performant solutions we could implement: #521

[jedwards1211]

@mickhansen so why was pagination changed to use offsets for all DBs, instead of just MSSQL?
Would you be open to a PR to add back an option to opt-in to the previous windowing strategy instead of using offsets?

[mickhansen]

@jedwards1211 Simplicity probably. Yes open to that.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.