Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WindowIterator does not consider scroll direction #2851

Closed
thjanssen opened this issue Jun 12, 2023 · 2 comments
Closed

WindowIterator does not consider scroll direction #2851

thjanssen opened this issue Jun 12, 2023 · 2 comments
Assignees
Labels
type: bug A general bug

Comments

@thjanssen
Copy link

thjanssen commented Jun 12, 2023

I played around with the keyset scrolling feature, and I either don't understand backward scrolling or found an issue.

I expected that this

WindowIterator<ChessPlayer> players = WindowIterator.of(pos -> playerRepo.findFirst2ByLastName("Doe", pos))
                                                                                               .startingAt(ScrollPosition.keyset().backward());

would give me a WindowIterator that puts the resultset in the descending order of the primary key and moves from there in windows of 2 until it reaches the lowest primary key (= last record).

But it only seems to work for the 1st window. The query that fetches the 2nd window is missing the DESC keyword to reverse the order.

2023-06-12T15:27:33.354+02:00 DEBUG 7520 --- [           main] org.hibernate.SQL                        : 
    select
        c1_0.id,
        c1_0.birth_date,
        c1_0.first_name,
        c1_0.last_name,
        c1_0.version 
    from
        chess_player c1_0 
    where
        c1_0.last_name=? 
    order by
        c1_0.id desc fetch first ? rows only
2023-06-12T15:27:33.371+02:00  INFO 7520 --- [           main] c.t.janssen.spring.data.TestPagination   : Doe, John14
2023-06-12T15:27:33.371+02:00  INFO 7520 --- [           main] c.t.janssen.spring.data.TestPagination   : Doe, John15
2023-06-12T15:27:33.381+02:00 DEBUG 7520 --- [           main] org.hibernate.SQL                        : 
    select
        c1_0.id,
        c1_0.birth_date,
        c1_0.first_name,
        c1_0.last_name,
        c1_0.version 
    from
        chess_player c1_0 
    where
        c1_0.last_name=? 
        and (
            c1_0.id>?
        ) 
    order by
        c1_0.id fetch first ? rows only
2023-06-12T15:27:33.383+02:00  INFO 7520 --- [           main] c.t.janssen.spring.data.TestPagination   : Doe, John15
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Jun 12, 2023
@mp911de
Copy link
Member

mp911de commented Jun 12, 2023

Related to spring-projects/spring-data-jpa#2999.

The backward direction isn't retained across windows, instead it is being reset to forward. This is by design, yet I agree that it is weird when using that feature. We need to fix this.

@mp911de mp911de self-assigned this Jun 12, 2023
@mp911de mp911de transferred this issue from spring-projects/spring-data-jpa Jun 13, 2023
@mp911de mp911de changed the title Keyset backward scrolling only seems to work for 1st window WindowIterator does not consider scroll direction Jun 13, 2023
@mp911de mp911de added type: bug A general bug and removed status: waiting-for-triage An issue we've not yet triaged labels Jun 13, 2023
@mp911de
Copy link
Member

mp911de commented Jun 13, 2023

When scrolling backward, we need to use the scroll position of the first item as reference. Right now, we always use the last item causing to behave as in forward-scrolling.

@mp911de mp911de added this to the 3.1.1 (2023.0.1) milestone Jun 13, 2023
mp911de added a commit that referenced this issue Jun 13, 2023
We now consider the scroll direction in the iterator to properly continue Keyset backward scrolling.

Closes #2851
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug A general bug
Projects
None yet
Development

No branches or pull requests

3 participants