[FW][FIX] res_currency: add order by to _select_companies_rates query #106095
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
PG12 introduced an optimization for CTEs that automatically inlines
CTEs if they are only refered once in the parent query. Prior to that
CTEs were always materialzed, meaning that PG created a sort of temp
table on the fly to store the result of the CTE's evaluation.
Whereas this leads to performance improvements in general, in the
particular case of _select_companies_rates this inlining becomes a
performance bottleneck. This is because while the currency_rate CTE
is only refered once in both purchase_report and product_margin,
the join condition (cr.date_end is null or cr.date_end > ...)
requires evaluating the CTE's date_end subquery twice. This, combined
with the fact that in PG12 the planner goes for a Nested Loop JOIN instead
of a HASH Join in PG10 makes the performances of the whole query
much worse in PG12 than in PG10.
Adding an ORDER BY (or an OFFSET 0, the resulting plan is the same)
creates a kind of optimization fence that forces PG to evaluate the
subquery first using its own plan. This removes the need to rescan the
subquery each time the Merge JOIN filter has to be applied, which
is a good strategy in this specific situation.
The same result could be achieved by adding the keyword "MATERIALIZED"
in the CTE definition. The issue is that this keyword did not exist
in PG 10 so using it would require to check the PG version at runtime
from python.
Examples of query timings change before and after PR:
Number of POs | Before PR | After PR
2000 | 7s | 345ms
7000 | 23s | 1.1s
opw-2930578
X-original-commit: 8b7a394
|
This PR targets saas-15.3 and is part of the forward-port chain. Further PRs will be created up to master. More info at https://github.com/odoo/odoo/wiki/Mergebot#forward-port |
This was referenced Nov 18, 2022
|
@fw-bot r+ |
robodoo
pushed a commit
that referenced
this pull request
Nov 25, 2022
PG12 introduced an optimization for CTEs that automatically inlines
CTEs if they are only refered once in the parent query. Prior to that
CTEs were always materialzed, meaning that PG created a sort of temp
table on the fly to store the result of the CTE's evaluation.
Whereas this leads to performance improvements in general, in the
particular case of _select_companies_rates this inlining becomes a
performance bottleneck. This is because while the currency_rate CTE
is only refered once in both purchase_report and product_margin,
the join condition (cr.date_end is null or cr.date_end > ...)
requires evaluating the CTE's date_end subquery twice. This, combined
with the fact that in PG12 the planner goes for a Nested Loop JOIN instead
of a HASH Join in PG10 makes the performances of the whole query
much worse in PG12 than in PG10.
Adding an ORDER BY (or an OFFSET 0, the resulting plan is the same)
creates a kind of optimization fence that forces PG to evaluate the
subquery first using its own plan. This removes the need to rescan the
subquery each time the Merge JOIN filter has to be applied, which
is a good strategy in this specific situation.
The same result could be achieved by adding the keyword "MATERIALIZED"
in the CTE definition. The issue is that this keyword did not exist
in PG 10 so using it would require to check the PG version at runtime
from python.
Examples of query timings change before and after PR:
Number of POs | Before PR | After PR
2000 | 7s | 345ms
7000 | 23s | 1.1s
opw-2930578
closes #106095
X-original-commit: 8b7a394
Signed-off-by: Raphael Collet <rco@odoo.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
PG12 introduced an optimization for CTEs that automatically inlines
CTEs if they are only refered once in the parent query. Prior
to that CTEs were always materialized, meaning
that PG created a sort of temp table on the fly to store the result
of the CTE's evaluation.
Whereas this leads to performance improvements in general, in the particular
case of
_select_companies_ratesthis inlining becomes a performancebottleneck. This is because while the currency_rate CTE is
only refered once in both purchase_report and product_margin,
the Merge Join Filter
cr.date_end is null or cr.date_end > ...requires evaluating the CTE's date_end subquery twice. This, combined
with the fact that in PG12 the planner goes for a Nested Loop Join instead
of a Hash Join in PG10 makes the performances of the whole query
much worse in PG12 than in PG10. PG10 query plan vs PG12 query plan.
We can see on the PG12 Query Plan that the Merge Join Node 7 scans the CTE twice (SubPlan 1 and SubPlan 2).
An eazy solution would be to add the keyword
Materializedbefore the CTE definition. This keyword, introducedin PG12, forces the materialization of the CTE, avoiding its inlining and making the PG12 plan the same as the PG10 one. Unfortunately,
Materializedwasn't there in PG10 so we cannot use this solution here since some Odoo instances still run on PG10.Instead, this PR adds an
ORDER BY date_endclause at the end of the CTE. This works like an OFFSET 0as it is kind of an optimizer fence, forcing PG to evaluate the subquery first Ordered PG12 query plan
Speedup
Test DB with 7000 purchase_order, 2666 res_currency_rate and 6 active currencies.
Purchase_report query timings in PG12, changing the number of purchase_orders
Changing the number of res_currency_rate
--
I confirm I have signed the CLA and read the PR guidelines at www.odoo.com/submit-pr
Forward-Port-Of: #98844