From 5a573c6f18249ca6939733c85df96ab175ccf419 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Voet=20=28ryv=29?= Date: Mon, 17 Jan 2022 13:31:09 +0000 Subject: [PATCH] [IMP] base: don't prefetch translate field by default Issue ----- Via the field prefetch mechanism, when we need a value of one field (not in cache of course), the ORM will prefetch all fields (which has the attribute to `prefetch=True`, the default value of this attribute is `True`) for all record ids in `_prefetch_ids`. Then, for each translate fields (where translate is not a callable) the ORM need to make a `LEFT JOIN` on the `ir_translation` to fetch the translated value. For big model, it leads to a simple `SELECT` with several `LEFT JOIN` on ir_translation but each LEFT JOIN have a cost in the planner time (a small cost in the execution time) of PostgreSQL. By example, for `product.template` (stock/sale/purchase installed), there are 6 LEFT JOIN to get all translated fields (5 of this fields are rarely used). Proposed solution ----------------- Deactivate the prefetch by default for all translate fields expect if this field is the `_rec_name` of the model (which is more likely to be used). In the example on the `product.template`: Without prefetching the translated fields, there is only one LEFT JOIN (the name, which is translated but is the `_rec_name` of the model). With the 6 translated fields to fetch, the query takes 5 ms to plan and 2 ms to execute VS with 1 translate field, it 1 ms to plan and 1.5 ms to execute. Side change note ---------------- - All translate of fields of `website.seo.metadata` should be prefetch to avoid lot of website errors (it is because, website put in cache data in sudo before reading it without sudo) - `description` (`mail.message.subtype`), `subject` (`mail.template`), `body_html` (`mail.template`) should be prefetch to avoid lot of extra query from mail module. - `vat_label` (`res.country`) should be prefetch to avoid a extra query for each website page. - Increase some queryCount (when it is legit, due to `subtitle` of `blog_post` or `description` of `event.type.ticket`, etc) task-2738029 closes odoo/odoo#82896 Signed-off-by: Raphael Collet --- addons/event/models/event_stage.py | 6 +++--- .../tests/test_performance.py | 4 ++-- addons/mail/models/mail_message_subtype.py | 2 +- addons/mail/models/mail_template.py | 6 +++--- addons/sale_stock/tests/test_create_perf.py | 8 ++++---- addons/test_event_full/tests/test_performance.py | 6 +++--- addons/website/models/mixins.py | 8 ++++---- addons/website_blog/tests/test_performance.py | 6 +++--- odoo/addons/base/models/res_country.py | 2 +- odoo/addons/base/tests/test_expression.py | 3 ++- odoo/addons/test_new_api/models/test_new_api.py | 11 +++++++++++ .../test_new_api/security/ir.model.access.csv | 1 + odoo/addons/test_new_api/tests/test_new_fields.py | 14 ++++++++++++++ odoo/fields.py | 14 ++++++++++---- 14 files changed, 62 insertions(+), 29 deletions(-) diff --git a/addons/event/models/event_stage.py b/addons/event/models/event_stage.py index d535ea75051..0b978818572 100644 --- a/addons/event/models/event_stage.py +++ b/addons/event/models/event_stage.py @@ -17,11 +17,11 @@ class EventStage(models.Model): string='End Stage', default=False, help='Events will automatically be moved into this stage when they are finished. The event moved into this stage will automatically be set as green.') legend_blocked = fields.Char( - 'Red Kanban Label', default=lambda s: _('Blocked'), translate=True, required=True, + 'Red Kanban Label', default=lambda s: _('Blocked'), translate=True, prefetch=True, required=True, help='Override the default value displayed for the blocked state for kanban selection.') legend_done = fields.Char( - 'Green Kanban Label', default=lambda s: _('Ready for Next Stage'), translate=True, required=True, + 'Green Kanban Label', default=lambda s: _('Ready for Next Stage'), translate=True, prefetch=True, required=True, help='Override the default value displayed for the done state for kanban selection.') legend_normal = fields.Char( - 'Grey Kanban Label', default=lambda s: _('In Progress'), translate=True, required=True, + 'Grey Kanban Label', default=lambda s: _('In Progress'), translate=True, prefetch=True, required=True, help='Override the default value displayed for the normal state for kanban selection.') diff --git a/addons/hr_work_entry_holidays/tests/test_performance.py b/addons/hr_work_entry_holidays/tests/test_performance.py index dd29322a1db..1d467bbd16e 100644 --- a/addons/hr_work_entry_holidays/tests/test_performance.py +++ b/addons/hr_work_entry_holidays/tests/test_performance.py @@ -47,7 +47,7 @@ class TestWorkEntryHolidaysPerformance(TestWorkEntryHolidaysBase): @users('__system__', 'admin') @warmup def test_performance_leave_create(self): - with self.assertQueryCount(__system__=25, admin=26): + with self.assertQueryCount(__system__=26, admin=27): leave = self.create_leave(datetime(2018, 1, 1, 7, 0), datetime(2018, 1, 1, 18, 0)) leave.action_refuse() @@ -56,7 +56,7 @@ class TestWorkEntryHolidaysPerformance(TestWorkEntryHolidaysBase): def test_performance_leave_confirm(self): leave = self.create_leave(datetime(2018, 1, 1, 7, 0), datetime(2018, 1, 1, 18, 0)) leave.action_draft() - with self.assertQueryCount(__system__=18, admin=19): + with self.assertQueryCount(__system__=19, admin=20): leave.action_confirm() leave.state = 'refuse' diff --git a/addons/mail/models/mail_message_subtype.py b/addons/mail/models/mail_message_subtype.py index a634c9d61c0..ea678124d4b 100644 --- a/addons/mail/models/mail_message_subtype.py +++ b/addons/mail/models/mail_message_subtype.py @@ -20,7 +20,7 @@ class MailMessageSubtype(models.Model): 'change in a process (Stage change). Message subtypes allow to ' 'precisely tune the notifications the user want to receive on its wall.') description = fields.Text( - 'Description', translate=True, + 'Description', translate=True, prefetch=True, help='Description that will be added in the message posted for this ' 'subtype. If void, the name will be added instead.') internal = fields.Boolean( diff --git a/addons/mail/models/mail_template.py b/addons/mail/models/mail_template.py index 2ac4904c8d4..e831054da18 100644 --- a/addons/mail/models/mail_template.py +++ b/addons/mail/models/mail_template.py @@ -31,7 +31,7 @@ class MailTemplate(models.Model): name = fields.Char('Name', translate=True) model_id = fields.Many2one('ir.model', 'Applies to', help="The type of document this template can be used with") model = fields.Char('Related Document Model', related='model_id.model', index=True, store=True, readonly=True) - subject = fields.Char('Subject', translate=True, help="Subject (placeholders may be used here)") + subject = fields.Char('Subject', translate=True, prefetch=True, help="Subject (placeholders may be used here)") email_from = fields.Char('From', help="Sender address (placeholders may be used here). If not set, the default " "value will be the author's email alias if configured, or email address.") @@ -47,12 +47,12 @@ class MailTemplate(models.Model): email_cc = fields.Char('Cc', help="Carbon copy recipients (placeholders may be used here)") reply_to = fields.Char('Reply To', help="Email address to which replies will be redirected when sending emails in mass; only used when the reply is not logged in the original discussion thread.") # content - body_html = fields.Html('Body', render_engine='qweb', translate=True, sanitize=False) + body_html = fields.Html('Body', render_engine='qweb', translate=True, prefetch=True, sanitize=False) attachment_ids = fields.Many2many('ir.attachment', 'email_template_attachment_rel', 'email_template_id', 'attachment_id', 'Attachments', help="You may attach files to this template, to be added to all " "emails created from this template") - report_name = fields.Char('Report Filename', translate=True, + report_name = fields.Char('Report Filename', translate=True, prefetch=True, help="Name to use for the generated report file (may contain placeholders)\n" "The extension can be omitted and will then come from the report type.") report_template = fields.Many2one('ir.actions.report', 'Optional report to print and attach') diff --git a/addons/sale_stock/tests/test_create_perf.py b/addons/sale_stock/tests/test_create_perf.py index 654b19ab203..c53810c0393 100644 --- a/addons/sale_stock/tests/test_create_perf.py +++ b/addons/sale_stock/tests/test_create_perf.py @@ -73,7 +73,7 @@ class TestPERF(common.TransactionCase): @warmup def test_light_sales_orders_batch_creation_perf_without_taxes(self): self.products[0].taxes_id = [Command.set([])] - with self.assertQueryCount(admin=58): + with self.assertQueryCount(admin=59): self.env['sale.order'].create([{ 'partner_id': self.partners[0].id, 'user_id': self.salesmans[0].id, @@ -87,7 +87,7 @@ class TestPERF(common.TransactionCase): @users('admin') @warmup def test_light_sales_orders_batch_creation_perf(self): - with self.assertQueryCount(admin=69): # 68 locally, 69 in nightly runbot + with self.assertQueryCount(admin=70): # 69 locally, 70 in nightly runbot self.env['sale.order'].create([{ 'partner_id': self.partners[0].id, 'user_id': self.salesmans[0].id, @@ -104,7 +104,7 @@ class TestPERF(common.TransactionCase): # NOTE: sometimes more queries on runbot, # do not change without verifying in multi-builds # (Seems to be a time-based problem, everytime happening around 10PM) - self._test_complex_sales_orders_batch_creation_perf(1502) + self._test_complex_sales_orders_batch_creation_perf(1504) @users('admin') @warmup @@ -114,7 +114,7 @@ class TestPERF(common.TransactionCase): self.env.user.groups_id += self.env.ref('product.group_discount_per_so_line') # Verify any modification to this count on nightly runbot builds - self._test_complex_sales_orders_batch_creation_perf(1545) + self._test_complex_sales_orders_batch_creation_perf(1546) def _test_complex_sales_orders_batch_creation_perf(self, query_count): MSG = "Model %s, %i records, %s, time %.2f" diff --git a/addons/test_event_full/tests/test_performance.py b/addons/test_event_full/tests/test_performance.py index d894a6a8fc6..ea4563fd0a7 100644 --- a/addons/test_event_full/tests/test_performance.py +++ b/addons/test_event_full/tests/test_performance.py @@ -70,7 +70,7 @@ class TestEventPerformance(EventPerformanceCase): event_type = self.env['event.type'].browse(self.test_event_type.ids) # complex with type - with freeze_time(self.reference_now), self.assertQueryCount(event_user=785): # tef only: 785 (779) - com runbot: 779 - ent runbot 779 + with freeze_time(self.reference_now), self.assertQueryCount(event_user=786): # tef only: 785 (779) - com runbot: 779 - ent runbot 779 self.env.cr._now = self.reference_now # force create_date to check schedulers event_values = [ dict(self.event_base_vals, @@ -144,7 +144,7 @@ class TestEventPerformance(EventPerformanceCase): has_social = 'social_menu' in self.env['event.event'] # otherwise view may crash in enterprise # type and website - with freeze_time(self.reference_now), self.assertQueryCount(event_user=783): # tef only: 673 - com runbot: 675 + with freeze_time(self.reference_now), self.assertQueryCount(event_user=784): # tef only: 673 - com runbot: 675 self.env.cr._now = self.reference_now # force create_date to check schedulers with Form(self.env['event.event']) as event_form: event_form.name = 'Test Event' @@ -187,7 +187,7 @@ class TestEventPerformance(EventPerformanceCase): event_type = self.env['event.type'].browse(self.test_event_type.ids) # complex with type - with freeze_time(self.reference_now), self.assertQueryCount(event_user=81): # tef only: 81 (75) - com runbot: 75 - ent runbot 75 + with freeze_time(self.reference_now), self.assertQueryCount(event_user=82): # tef only: 81 (75) - com runbot: 75 - ent runbot 75 self.env.cr._now = self.reference_now # force create_date to check schedulers event_values = dict( self.event_base_vals, diff --git a/addons/website/models/mixins.py b/addons/website/models/mixins.py index 970ffc3788c..b7e7d0a9da7 100644 --- a/addons/website/models/mixins.py +++ b/addons/website/models/mixins.py @@ -21,11 +21,11 @@ class SeoMetadata(models.AbstractModel): _description = 'SEO metadata' is_seo_optimized = fields.Boolean("SEO optimized", compute='_compute_is_seo_optimized') - website_meta_title = fields.Char("Website meta title", translate=True) - website_meta_description = fields.Text("Website meta description", translate=True) - website_meta_keywords = fields.Char("Website meta keywords", translate=True) + website_meta_title = fields.Char("Website meta title", translate=True, prefetch=True) + website_meta_description = fields.Text("Website meta description", translate=True, prefetch=True) + website_meta_keywords = fields.Char("Website meta keywords", translate=True, prefetch=True) website_meta_og_img = fields.Char("Website opengraph image") - seo_name = fields.Char("Seo name", translate=True) + seo_name = fields.Char("Seo name", translate=True, prefetch=True) def _compute_is_seo_optimized(self): for record in self: diff --git a/addons/website_blog/tests/test_performance.py b/addons/website_blog/tests/test_performance.py index 688e48dbc41..4a94dc9f682 100644 --- a/addons/website_blog/tests/test_performance.py +++ b/addons/website_blog/tests/test_performance.py @@ -13,7 +13,7 @@ class TestBlogPerformance(UtilPerf): self.env['website'].search([]).channel_id = False def test_10_perf_sql_blog_standard_data(self): - self.assertEqual(self._get_url_hot_query('/blog'), 26) + self.assertEqual(self._get_url_hot_query('/blog'), 27) def test_20_perf_sql_blog_bigger_data_scaling(self): BlogPost = self.env['blog.post'] @@ -25,8 +25,8 @@ class TestBlogPerformance(UtilPerf): for blog_post in blog_posts: blog_post.tag_ids += blog_tags blog_tags = blog_tags[:-1] - self.assertEqual(self._get_url_hot_query('/blog'), 26) - self.assertEqual(self._get_url_hot_query(blog_post[0].website_url), 29) + self.assertEqual(self._get_url_hot_query('/blog'), 27) + self.assertEqual(self._get_url_hot_query(blog_post[0].website_url), 31) def test_30_perf_sql_blog_bigger_data_scaling(self): BlogPost = self.env['blog.post'] diff --git a/odoo/addons/base/models/res_country.py b/odoo/addons/base/models/res_country.py index 9bbbbe359e1..87749082f12 100644 --- a/odoo/addons/base/models/res_country.py +++ b/odoo/addons/base/models/res_country.py @@ -69,7 +69,7 @@ class Country(models.Model): ('after', 'After Address'), ], string="Customer Name Position", default="before", help="Determines where the customer/company name should be placed, i.e. after or before the address.") - vat_label = fields.Char(string='Vat Label', translate=True, help="Use this field if you want to change vat label.") + vat_label = fields.Char(string='Vat Label', translate=True, prefetch=True, help="Use this field if you want to change vat label.") state_required = fields.Boolean(default=False) zip_required = fields.Boolean(default=True) diff --git a/odoo/addons/base/tests/test_expression.py b/odoo/addons/base/tests/test_expression.py index 80b391b40ec..08560b14e1a 100644 --- a/odoo/addons/base/tests/test_expression.py +++ b/odoo/addons/base/tests/test_expression.py @@ -1647,6 +1647,7 @@ class TestMany2many(TransactionCase): ''']): self.User.search([('groups_id', 'in', group.ids)], order='id') + group_color = group.color with self.assertQueries([''' SELECT "res_users".id FROM "res_users" @@ -1659,7 +1660,7 @@ class TestMany2many(TransactionCase): )) ORDER BY "res_users"."id" ''']): - self.User.search([('groups_id.color', '=', group.color)], order='id') + self.User.search([('groups_id.color', '=', group_color)], order='id') with self.assertQueries([''' SELECT "res_users".id diff --git a/odoo/addons/test_new_api/models/test_new_api.py b/odoo/addons/test_new_api/models/test_new_api.py index ec5b2091012..3efc9158548 100644 --- a/odoo/addons/test_new_api/models/test_new_api.py +++ b/odoo/addons/test_new_api/models/test_new_api.py @@ -1514,3 +1514,14 @@ class PrecomputeRequired(models.Model): partner_id = fields.Many2one('res.partner', required=True) name = fields.Char(related='partner_id.name', precompute=True, store=True, required=True) + + +class PrefetchTranslateField(models.Model): + _name = 'test_new_api.prefetch.translate' + _description = 'A model with some translate fields to check prefetch' + + name = fields.Char('Name', translate=True) + description = fields.Char('Description', translate=True, prefetch=True) + html_description = fields.Html('Styled description', translate=True, prefetch=True) + rare_description = fields.Char('Rare Description', translate=True) + rare_html_description = fields.Html('Rare Styled description', translate=True) diff --git a/odoo/addons/test_new_api/security/ir.model.access.csv b/odoo/addons/test_new_api/security/ir.model.access.csv index 9250f277377..1a464f3d24e 100644 --- a/odoo/addons/test_new_api/security/ir.model.access.csv +++ b/odoo/addons/test_new_api/security/ir.model.access.csv @@ -84,3 +84,4 @@ access_test_new_api_precompute_line,access_test_new_api_precompute_line,model_te access_test_new_api_precompute_combo,access_test_new_api_precompute_combo,model_test_new_api_precompute_combo,,1,0,0,0 access_test_new_api_precompute_editable,access_test_new_api_precompute_editable,model_test_new_api_precompute_editable,,1,0,0,0 access_test_new_api_precompute_required,access_test_new_api_precompute_required,model_test_new_api_precompute_required,,1,0,0,0 +access_test_new_api_prefetch_translate,access_test_new_api_prefetch_translate,model_test_new_api_prefetch_translate,,1,0,0,0 diff --git a/odoo/addons/test_new_api/tests/test_new_fields.py b/odoo/addons/test_new_api/tests/test_new_fields.py index ec96fbf351c..f4ff27b3871 100644 --- a/odoo/addons/test_new_api/tests/test_new_fields.py +++ b/odoo/addons/test_new_api/tests/test_new_fields.py @@ -2522,6 +2522,20 @@ class TestFields(TransactionCaseWithUserDemo): with self.assertRaises(AccessError): record_user.read(['tags']) + def test_98_prefetch_translate(self): + Model = self.registry['test_new_api.prefetch.translate'] + + # translated '_rec_name' field should be prefetched + self.assertTrue(Model.name.prefetch) + + # parameter 'prefetch' can be always overridden + self.assertTrue(Model.description.prefetch) + self.assertTrue(Model.html_description.prefetch) + + # translated fields should be prefetch=False by default + self.assertFalse(Model.rare_description.prefetch) + self.assertFalse(Model.rare_html_description.prefetch) + class TestX2many(common.TransactionCase): def test_definition_many2many(self): diff --git a/odoo/fields.py b/odoo/fields.py index ff2a896591b..536d33e9314 100644 --- a/odoo/fields.py +++ b/odoo/fields.py @@ -1601,11 +1601,17 @@ class _String(Field): kwargs['translate'] = bool(kwargs['translate']) super(_String, self).__init__(string=string, **kwargs) - def _setup_attrs(self, model_class, name): - super()._setup_attrs(model_class, name) + def setup_nonrelated(self, model): + super().setup_nonrelated(model) if self.prefetch is None: - # do not prefetch complex translated fields by default - self.prefetch = not callable(self.translate) + # translated fields are not prefetched by default except for _rec_name + self.prefetch = not self.translate or model._rec_name == self.name + + def setup_related(self, model): + super().setup_related(model) + if self.prefetch is None: + # translated fields are not prefetched by default except for _rec_name + self.prefetch = not self.translate or model._rec_name == self.name _related_translate = property(attrgetter('translate'))