Increase performance of invoice query #9756
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 Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
| @@ -70,6 +70,11 @@ var ( | |||
| // schema. This is optional and can be disabled by the | |||
| // user if necessary. | |||
| }, | |||
| { | |||
| Name: "000007_invoice_modification", | |||
There was a problem hiding this comment.
This is supposed to add the index? Maybe the file is missing from the PR now as the tests fail.
There was a problem hiding this comment.
Wonder if we should come up with a nice naming convention because we might fix stuff in the future as well.
There was a problem hiding this comment.
Maybe something as simple as 000007_invoices if we do more changes to the schema than adding the index, otherwise 000007_invoices_settle_id_index.
|
Great catch!! |
We remove the offset query variable and instead use the keyset to filter and paginate through the index.
We also make sure AMP subinvoices are also fetched with the pagination limit.
so even if we have an add or settle index you are saying the old way still scanned through everything still and internally discarded before responding to the client? |
The old way used the offset of SQL for pagination instead of the keyset pagination. So it would for every new query increase the offset which is super inefficient for large dbs as we know right now. It is ok for smaller dbs tho, so the approach is logically right but does not account for large dbs. |
|
Perhaps we can make a one off benchmark w/ the postgres backend so we can gauge the tangible perf increase here? |
We used to paginate through the invoice dataset via the offset parameter, however using offset for large databases is very inefficient because it has to scan and discard invoices. Now we use the keyset to paginate through the dataset.
Fixes: #9730