odoo/enterprise#99816
Created by Accounting, Olivier Colson (oco)
Blocked
- Merge method
- Review
-
CI
- legal/cla: Contributor License Agreement check
- ci/runbot: Odoo Test Suite
- ci/upgrade_enterprise: Test upgrades for enterprise master
- ci/style: Optional style check. Ignore it only if strictly necessary.
- ci/security: Required security check. Can only be ignored by security team.
- ci/l10n: (runtime 26s)
- Linked pull requests
- label
- odoo-dev:master-no-loop-aggregations-oco
- head
- 5b17bb6cda006ba0d2edc25010f33477d757143d
| odoo/odoo | odoo/enterprise | odoo/documentation | |
|---|---|---|---|
| master | #236334 missing r+ | #99816 missing r+ | #15342 missing r+ |
[IMP] account_reports, l10n_*_reports: More efficient computation of aggregations
(aka MAGA: Make Aggregations Great Again)
When a report contained many aggregations to evaluate, with complex interdependencies, the way the computation was done was to naïve. This happened with a new Japanese report we're currently implementing, which contains multiple sections referencing each other with cross_report aggregations.
Basically, we used to make a queue of all aggregations to evaluate, pop the first one, try to evaluate it, and if it was still missing some data, enqueue it back. We kept on looping like that until every formula got evaluated. On the (extreme) case we faced, it took 20 seconds just to compute aggregations on a database with only demo data !
We solve this by rewriting the aggregation engine in such a way that it only tries to evaluate the expressions it has enough data to compute. Essentially, we now keep track of the number of dependencies to other aggregations that still need to be resolved before being able to evalutate a formula, and we only evalute the ones whose associated count is 0.
On our test example with the Japanese report on a db with only demo data, the computation of aggregations went from 20 seconds to less than 1 with this commit.
To make this refactoring possible, some restrictions needed to be added to cross_report aggregations:
1) By default, a cross_report aggregation does not force its date_scope to the terms it calls anymore. This was only done for Balance Sheets calling the P&L, and induced a LOT of complexity, and the possibility of many unintended corner cases, some of which working purely by chance, some others simply breaking, none of which seeing any actual use in the standard data.
I it still possible to force the date_scope for an aggregation really needing to do so, but it needs to be specifically asked in the subformula. The syntax for that is now: cross_report(report_id, force_date_scope).
When a cross_report aggregation is to be evaluated without forced date scope, it will use the date scope of each of its terms, just as if all those expressions had been part of the current report. On top of the technical benefits, it's also functionally clearer for the users customizing reports by hand.
2) cross_report aggregations with a forced date_scope do not allow referencing other cross_report aggregations anymore. Doing that would allow for triangle dependencies between reports, and proper propagation of the forced date scopes would require additional complexity that we don't want. We now prevent this with a constraint. None of the standard report needed to be adapted ; if some custom-made report fails because of this change, it's anyway possible to reference directly the expression(s) they want instead of counting on an additional indirection.