fix: FindInBatches with offset limit #5255
Conversation
| } | ||
|
|
||
| // reset to offset to 0 in next batch | ||
| tx = tx.Offset(-1).Session(&Session{}) |
There was a problem hiding this comment.
Should we extract this call to another check? The offset is also buggy when using w/o limit.
There was a problem hiding this comment.
Should we extract this call to another check? The
offsetis also buggy when using w/olimit.
What does offset is also buggy mean ? FindInBatches query condition is greater than primary key and limit.
There was a problem hiding this comment.
Should we extract this call to another check? The
offsetis also buggy when using w/olimit.What does
offset is also buggymean ?FindInBatchesquery condition is greater than primary key and limit.
If we only using offset, which scenario will be like select all users which uid is larger than 100, then using dbInstance.Offset(100).OrderBy("user_id").FindInBatches(), it will skip 100 records in each of all subsequent queries.
The corresponding test is TestOffsetClausedFindInBatches in go-gorm/playground#466.
There was a problem hiding this comment.
it will skip 100 records in first sub query.
here is reset offset to zero after first sub query that we can use id > ? and limit = ? to query.
There was a problem hiding this comment.
it will skip 100 records in first sub query. here is reset offset to zero after first sub query that we can use
id > ? and limit = ?to query.
That's correct. But this call has been embedded in the limit check. I mean, we may need to extract it to another check that whether we have a Offset clause and the argument is large than 0 so we can both support only using Offset or togethered with Limit clause.
There was a problem hiding this comment.
Offest and Limit use the same clause.Limit, so it support for only using Offset or togethered
https://github.com/go-gorm/gorm/blob/master/chainable_api.go#L245
There was a problem hiding this comment.
Only Offset cannot be used is caused by here, thank you.
There was a problem hiding this comment.
Only
Offsetcannot be used is caused by here, thank you.
Got it. You're nice :).
What did this pull request do?
I think when the user uses
OffsetinFindInBatchesthe meaning is to skip the previous N records, and user usesLIMITinFindInBatchesthe meaning is find total of N records.close #5252
User Case Description