-
Notifications
You must be signed in to change notification settings - Fork 808
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
fix(SqlExecutionRepository): fixed bug in sql repository in orca-sql … #4697
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1119,12 +1119,16 @@ class SqlExecutionRepository( | |
type, | ||
conditions = { | ||
if (cursor == null) { | ||
it.where("1=1") | ||
if (where == null) { | ||
it.where("1=1") | ||
} else { | ||
where(it) | ||
} | ||
} else { | ||
if (where == null) { | ||
it.where(field("id").gt(cursor)) | ||
it.where(field("id").lt(cursor)) | ||
} else { | ||
where(it).and(field("id").gt(cursor)) | ||
where(it).and(field("id").lt(cursor)) | ||
Comment on lines
+1129
to
+1131
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The pagination is broken, which makes it only show the first page, for example, in the unit tests on Then, iteration 1 returns the first 10 elements in desc order: --> 20, ..., 11 (10 elements) And cursor = 11 Then the second page gets the elements greater than the cursor, which are --> 20, ..., 12 (9 elements) Then the stop condition in the PagedIterator is triggered: isLastPage = currentPage.size < pageSize Thus, the second page is never returned and only 10 elements are present, when there should be 12. When we change greater than, for lesser than, we get: Iteration 1: --> 20, ..., 11 (10 elements) Iteration 2, elements lesser than 11: --> 1, 2 Then we get the correct response. |
||
} | ||
} | ||
}, | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For the case in which the cursor is null (it always starts as null in the first iteration), the custom where statements that are passed to the function are not used on the query, effectively not filtering the first page.