invoices: fix SQL invoice query pagination #8854
Conversation
|
Important Review skippedAuto reviews are limited to specific labels. Labels to auto review (1)
Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
bhandras
force-pushed
the
invoices-limit-offset-fixup
branch
from
cc5a59e to
37390a3
Compare
| @@ -24,12 +23,6 @@ import ( | |||
| "github.com/stretchr/testify/require" | |||
| ) | |||
|
|
|||
| // sqliteConstructorMu is used to ensure that only one thread can call the | |||
| // sqldb.NewTestSqliteDB constructor at a time. This is a temporary workaround | |||
| // Temporary replace until the next version of sqldb is tagged. | ||
| replace github.com/lightningnetwork/lnd/sqldb => ./sqldb |
There was a problem hiding this comment.
cool - not sure we want to merge with this thought right?
So I think might need to open separate PR that points to this (so we can verify complete CI run) and then once this PR is merged and tagged, then update that PR to point to the tagged version
There was a problem hiding this comment.
So the next two migration PRs both touch sqldb and other packages depending on it so i think we can't avoid. Maybe can move this to the next PR (#8855) though.
bhandras
force-pushed
the
invoices-limit-offset-fixup
branch
from
37390a3 to
a22424d
Compare
Previously paginated queries offseted the add_index_get, add_index_let, settle_index_get and settle_index_let parameters with the paginators current page offset, however this was incorrect as we can just use SQL's LIMIT/OFFSET to paginate. This commit fixes this issue and adds an optional parameter to the constructor of the invoice SQL store to set page size. This is useful when testing as we can now cover pagination correctly with our existing unit tests.
bhandras
force-pushed
the
invoices-limit-offset-fixup
branch
from
a22424d to
b35f060
Compare
Previously paginated queries offseted the add_index_get, add_index_let, settle_index_get and settle_index_let parameters with the paginators current page offset, however this was incorrect as we can just use SQL's LIMIT/OFFSET to paginate. This commit fixes this issue and adds an optional parameter to the constructor of the invoice SQL store to set page size. This is useful when testing as we can now cover pagination correctly with our existing unit tests.