From 8766f388da9b642ba4433d8d5d670847b472cff8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lucas=20Lef=C3=A8vre?= Date: Wed, 31 Jul 2019 15:12:10 +0200 Subject: [PATCH] [IMP] base: Allow to better search partners if they are customers/suppliers This commit allows to call `name_search` with context key `res_partner_search_mode` that can take currently two values: "customer" or "supplier". When ordering search results, order partners by the number of SO they have when the value is "customer" and by the number of PO if the value is "supplier". Top suppliers/customers are displayed above in a PO/SO partner dropdown. We have thought about different implementations before selecting this one: - Keep the 2 boolean flags and automatically set them if a SO or PO is created for a customer/supplier (helps a bit but doesn't work well for the first SO/PO). As the onboarding is also a priority, we didn't implement it. - Instead of filtering on the bool flags, sort the name_search() result according to the number of orders they already have (e.g. if you search for "Foo" on a PO, a supplier "SuperFoo" with 32 purchase orders will appear before supplier "HorribleFoo" that has no PO). Problem is to do this in an efficient manner for large databases - possibly we could store a "relevance index" based on the number of recent orders. Storing those values is an issue too, as it doesn't work well for multi company database. Indeed, storing values that depend on the context is certainly not a good idea. - Same as previous alternative but with no stored columns, rather a JOIN in name_search. This is the solution that have been kept. The only concern was about the perfomances. We tested it on our production base, on a sales order, as we have way much more customers than suppliers. After a deep analysis on the querry and the bitmap generated by PostgreSQL, we can conclude that the count(*) works rather efficiently and that the request is not significantly heavier than without the context key. Performance Analysis ==================== The following results have been obtained on our production database. Before this commit ------------------ The original query is the following one: ``` EXPLAIN ANALYSE SELECT res_partner.id FROM "res_partner" WHERE ("res_partner"."customer" = True) AND ("res_partner"."active" = True) AND ((("res_partner"."type" != 'private') OR "res_partner"."type" IS NULL) OR "res_partner"."type" IS NULL ) AND (res_partner.email ilike '%eezee-i%' OR res_partner.display_name ilike '%eezee-i%' OR res_partner.ref ilike '%eezee-i%' OR res_partner.vat ilike '%eezeei%') -- don't panic, trust postgres bitmap ORDER BY res_partner.display_name ilike '%eezee-i%' desc, res_partner.display_name limit 8; ``` Here is the EXPLAIN ANALYSE returned by PostgreSQL: ``` Limit (cost=1625.50..1625.52 rows=8 width=21) (actual time=13.397..13.404 rows=8 loops=1) -> Sort (cost=1625.50..1626.32 rows=326 width=21) (actual time=13.395..13.396 rows=8 loops=1) Sort Key: (((display_name)::text ~~* '%eezee-i%'::text)) DESC, display_name Sort Method: top-N heapsort Memory: 26kB -> Bitmap Heap Scan on res_partner (cost=1282.79..1618.98 rows=326 width=21) (actual time=13.025..13.350 rows=56 loops=1) Recheck Cond: (((email)::text ~~* '%eezee-i%'::text) OR ((display_name)::text ~~* '%eezee-i%'::text) OR ((ref)::text ~~* '%eezee-i%'::text) OR ((vat)::text ~~* '%eezeei%'::text)) Rows Removed by Index Recheck: 1 Filter: (customer AND active AND (((type)::text <> 'private'::text) OR (type IS NULL) OR (type IS NULL))) Rows Removed by Filter: 1 Heap Blocks: exact=58 -> BitmapOr (cost=1282.79..1282.79 rows=328 width=0) (actual time=12.983..12.983 rows=0 loops=1) -> Bitmap Index Scan on res_partner_name_tgm_idx_gin (cost=0.00..322.22 rows=162 width=0) (actual time=5.022..5.022 rows=54 loops=1) Index Cond: ((email)::text ~~* '%eezee-i%'::text) -> Bitmap Index Scan on res_partner_name_tgm_idx_gin (cost=0.00..322.22 rows=162 width=0) (actual time=4.128..4.128 rows=56 loops=1) Index Cond: ((display_name)::text ~~* '%eezee-i%'::text) -> Bitmap Index Scan on res_partner_name_tgm_idx_gin (cost=0.00..321.00 rows=1 width=0) (actual time=2.240..2.240 rows=0 loops=1) Index Cond: ((ref)::text ~~* '%eezee-i%'::text) -> Bitmap Index Scan on res_partner_name_tgm_idx_gin (cost=0.00..317.03 rows=4 width=0) (actual time=1.585..1.585 rows=0 loops=1) Index Cond: ((vat)::text ~~* '%eezeei%'::text) Planning time: 0.766 ms Execution time: 13.489 ms ``` After this commit ----------------- Without the context key, the query looks like this: ``` SELECT res_partner.id FROM "res_partner" WHERE ("res_partner"."active" = True) AND ((("res_partner"."type" != 'private') OR "res_partner"."type" IS NULL) OR "res_partner"."type" IS NULL ) AND (res_partner.email ilike '%eezee-i%' OR res_partner.display_name ilike '%eezee-i%' OR res_partner.ref ilike '%eezee-i%' OR res_partner.vat ilike '%eezeei%') -- don't panic, trust postgres bitmap GROUP BY res_partner.id ORDER BY COUNT(*) DESC, res_partner.display_name ilike '%eezee-i%' desc, res_partner.display_name limit 8; And it quite clear when looking at the EXPLAIN ANALYSE that the request is quite the same, except the aggregation that is made with a quicksort method. Limit (cost=1645.00..1645.02 rows=8 width=29) (actual time=13.200..13.206 rows=8 loops=1) -> Sort (cost=1645.00..1645.82 rows=328 width=29) (actual time=13.197..13.198 rows=8 loops=1) Sort Key: (count(*)) DESC, (((display_name)::text ~~* '%eezee-i%'::text)) DESC, display_name Sort Method: top-N heapsort Memory: 26kB -> GroupAggregate (cost=1631.88..1638.44 rows=328 width=29) (actual time=13.090..13.160 rows=56 loops=1) Group Key: id -> Sort (cost=1631.88..1632.70 rows=328 width=20) (actual time=13.080..13.084 rows=56 loops=1) Sort Key: id Sort Method: quicksort Memory: 29kB -> Bitmap Heap Scan on res_partner (cost=1282.79..1618.17 rows=328 width=20) (actual time=12.793..13.055 rows=56 loops=1) Recheck Cond: (((email)::text ~~* '%eezee-i%'::text) OR ((display_name)::text ~~* '%eezee-i%'::text) OR ((ref)::text ~~* '%eezee-i%'::text) OR ((vat)::text ~~* '%eezeei%'::text)) Rows Removed by Index Recheck: 1 Filter: (active AND (((type)::text <> 'private'::text) OR (type IS NULL) OR (type IS NULL))) Rows Removed by Filter: 1 Heap Blocks: exact=58 -> BitmapOr (cost=1282.79..1282.79 rows=328 width=0) (actual time=12.755..12.755 rows=0 loops=1) -> Bitmap Index Scan on res_partner_name_tgm_idx_gin (cost=0.00..322.22 rows=162 width=0) (actual time=4.823..4.823 rows=54 loops=1) Index Cond: ((email)::text ~~* '%eezee-i%'::text) -> Bitmap Index Scan on res_partner_name_tgm_idx_gin (cost=0.00..322.22 rows=162 width=0) (actual time=3.941..3.941 rows=56 loops=1) Index Cond: ((display_name)::text ~~* '%eezee-i%'::text) -> Bitmap Index Scan on res_partner_name_tgm_idx_gin (cost=0.00..321.00 rows=1 width=0) (actual time=2.186..2.186 rows=0 loops=1) Index Cond: ((ref)::text ~~* '%eezee-i%'::text) -> Bitmap Index Scan on res_partner_name_tgm_idx_gin (cost=0.00..317.03 rows=4 width=0) (actual time=1.798..1.798 rows=0 loops=1) Index Cond: ((vat)::text ~~* '%eezeei%'::text) Planning time: 0.761 ms Execution time: 13.317 ms ``` With the context key, the query looks like this: ``` EXPLAIN ANALYSE SELECT res_partner.id FROM "res_partner" LEFT JOIN sale_order ON res_partner.id = sale_order.partner_id WHERE ("res_partner"."active" = True) AND ((("res_partner"."type" != 'private') OR "res_partner"."type" IS NULL) OR "res_partner"."type" IS NULL ) AND (res_partner.email ilike '%eezee-i%' OR res_partner.display_name ilike '%eezee-i%' OR res_partner.ref ilike '%eezee-i%' OR res_partner.vat ilike '%eezeei%') -- don't panic, trust postgres bitmap GROUP BY res_partner.id ORDER BY COUNT(*) DESC, res_partner.display_name ilike '%eezee-i%' desc, res_partner.display_name limit 8; ``` The only difference is that a nested loop is made for the left join, as postgreSQL has correctly identified the dicriminating table. The correct result is obtained within a similar duration. ``` Limit (cost=2700.68..2700.70 rows=8 width=29) (actual time=12.561..12.568 rows=8 loops=1) -> Sort (cost=2700.68..2701.50 rows=328 width=29) (actual time=12.559..12.560 rows=8 loops=1) Sort Key: (count(*)) DESC, (((res_partner.display_name)::text ~~* '%eezee-i%'::text)) DESC, res_partner.display_name Sort Method: top-N heapsort Memory: 26kB -> GroupAggregate (cost=2687.56..2694.12 rows=328 width=29) (actual time=12.449..12.527 rows=56 loops=1) Group Key: res_partner.id -> Sort (cost=2687.56..2688.38 rows=328 width=20) (actual time=12.408..12.425 rows=305 loops=1) Sort Key: res_partner.id Sort Method: quicksort Memory: 42kB -> Nested Loop Left Join (cost=1283.21..2673.86 rows=328 width=20) (actual time=11.629..12.340 rows=305 loops=1) -> Bitmap Heap Scan on res_partner (cost=1282.79..1618.17 rows=328 width=20) (actual time=11.596..11.862 rows=56 loops=1) Recheck Cond: (((email)::text ~~* '%eezee-i%'::text) OR ((display_name)::text ~~* '%eezee-i%'::text) OR ((ref)::text ~~* '%eezee-i%'::text) OR ((vat)::text ~~* '%eezeei%'::text)) Rows Removed by Index Recheck: 1 Filter: (active AND (((type)::text <> 'private'::text) OR (type IS NULL) OR (type IS NULL))) Rows Removed by Filter: 1 Heap Blocks: exact=58 -> BitmapOr (cost=1282.79..1282.79 rows=328 width=0) (actual time=11.560..11.560 rows=0 loops=1) -> Bitmap Index Scan on res_partner_name_tgm_idx_gin (cost=0.00..322.22 rows=162 width=0) (actual time=4.724..4.724 rows=54 loops=1) Index Cond: ((email)::text ~~* '%eezee-i%'::text) -> Bitmap Index Scan on res_partner_name_tgm_idx_gin (cost=0.00..322.22 rows=162 width=0) (actual time=3.825..3.825 rows=56 loops=1) Index Cond: ((display_name)::text ~~* '%eezee-i%'::text) -> Bitmap Index Scan on res_partner_name_tgm_idx_gin (cost=0.00..321.00 rows=1 width=0) (actual time=1.752..1.752 rows=0 loops=1) Index Cond: ((ref)::text ~~* '%eezee-i%'::text) -> Bitmap Index Scan on res_partner_name_tgm_idx_gin (cost=0.00..317.03 rows=4 width=0) (actual time=1.254..1.254 rows=0 loops=1) Index Cond: ((vat)::text ~~* '%eezeei%'::text) -> Index Only Scan using sale_order_partner_id_index on sale_order (cost=0.42..2.74 rows=48 width=4) (actual time=0.006..0.007 rows=5 loops=56) Index Cond: (partner_id = res_partner.id) Heap Fetches: 12 Planning time: 1.056 ms Execution time: 13.791 ms ``` TaskID: 2031147 Co-authored-by: Yannick Tivisse --- addons/purchase/models/res_partner.py | 8 ++++++++ addons/sale/models/res_partner.py | 8 ++++++++ odoo/addons/base/models/res_partner.py | 14 +++++++++----- 3 files changed, 25 insertions(+), 5 deletions(-) diff --git a/addons/purchase/models/res_partner.py b/addons/purchase/models/res_partner.py index e323fe95658..56d79315966 100644 --- a/addons/purchase/models/res_partner.py +++ b/addons/purchase/models/res_partner.py @@ -53,3 +53,11 @@ class res_partner(models.Model): supplier_invoice_count = fields.Integer(compute='_compute_supplier_invoice_count', string='# Vendor Bills') purchase_warn = fields.Selection(WARNING_MESSAGE, 'Purchase Order', help=WARNING_HELP, default="no-message") purchase_warn_msg = fields.Text('Message for Purchase Order') + + def _get_name_search_join_clause(self): + res = super()._get_name_search_join_clause() + partner_search_mode = self.env.context.get('res_partner_search_mode') + if partner_search_mode == 'supplier': + join_clause = "LEFT JOIN {table} ON res_partner.id = {table}.partner_id".format(table=self.env['purchase.order']._table) + return '%s %s' % (res, join_clause) if res else join_clause + return res diff --git a/addons/sale/models/res_partner.py b/addons/sale/models/res_partner.py index fac5165f55f..fa9e296580f 100644 --- a/addons/sale/models/res_partner.py +++ b/addons/sale/models/res_partner.py @@ -40,3 +40,11 @@ class ResPartner(models.Model): ('state', 'in', ['sent', 'sale', 'done']) ], limit=1) return can_edit_vat and not bool(has_so) + + def _get_name_search_join_clause(self): + res = super()._get_name_search_join_clause() + partner_search_mode = self.env.context.get('res_partner_search_mode') + if partner_search_mode == 'customer': + join_clause = "LEFT JOIN {table} ON res_partner.id = {table}.partner_id".format(table=self.env['sale.order']._table) + return '%s %s' % (res, join_clause) if res else join_clause + return res diff --git a/odoo/addons/base/models/res_partner.py b/odoo/addons/base/models/res_partner.py index 3e84b921882..ce996272ac3 100644 --- a/odoo/addons/base/models/res_partner.py +++ b/odoo/addons/base/models/res_partner.py @@ -172,10 +172,6 @@ class Partner(models.Model): column2='category_id', string='Tags', default=_default_category) credit_limit = fields.Float(string='Credit Limit') active = fields.Boolean(default=True) - customer = fields.Boolean(string='Is a Customer', default=True, - help="Check this box if this contact is a customer. It can be selected in sales orders.") - supplier = fields.Boolean(string='Is a Vendor', - help="Check this box if this contact is a vendor. It can be selected in purchase orders.") employee = fields.Boolean(help="Check this box if this contact is an Employee.") function = fields.Char(string='Job Position') type = fields.Selection( @@ -760,6 +756,9 @@ class Partner(models.Model): return super(Partner, self)._search(args, offset=offset, limit=limit, order=order, count=count, access_rights_uid=access_rights_uid) + def _get_name_search_join_clause(self): + return '' + @api.model def _name_search(self, name, args=None, operator='ilike', limit=100, name_get_uid=None): self = self.with_user(name_get_uid or self.env.uid) @@ -782,16 +781,21 @@ class Partner(models.Model): unaccent = get_unaccent_wrapper(self.env.cr) + join_clause = self._get_name_search_join_clause() + query = """SELECT res_partner.id FROM {from_str} + {join} {where} ({email} {operator} {percent} OR {display_name} {operator} {percent} OR {reference} {operator} {percent} OR {vat} {operator} {percent}) -- don't panic, trust postgres bitmap - ORDER BY {display_name} {operator} {percent} desc, + GROUP BY res_partner.id + ORDER BY COUNT(*) DESC, {display_name} {operator} {percent} desc, {display_name} """.format(from_str=from_str, + join=join_clause, where=where_str, operator=operator, email=unaccent('res_partner.email'),