pengyuthon/odoo_source
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

[IMP] core: better way to set/update magic fields

Browse Source 
The goal of this change is to simplify the code managing `create_date`
and `write_date` in methods `create()` and `write()`, and also to remove
weird behaviors caused by the way those fields were updated.

Assume we update a simple field on a record.  This adds pending updates
for the field and `write_date`.  However, the value of `write_date` is
not known yet: it will be updated as `NOW() AT TIME ZONE 'UTC'` in SQL.
So `write_date` is actually given a dummy value in pending updates, and
it is invalidated from cache, until its value is flushed to the database
and fetched again.

Now assume we access another field on the record, and that field is not
in cache.  The prefetching mechanism will read all column fields,
including `write_date`, and flush them first.

    # this adds pending updates foo: 42, write_uid: 1, write_date: False
    record.foo = 42

    # assume 'bar' is not in cache; this prefetches all column fields,
    # which flushes the pending updates above before reading them back
    result = record.bar

We can avoid flushing pending updates if the values read from database
do not overwrite existing values in cache.  If you assume that the value
of a pending update is in cache (in the example, `foo: 42`), you don't
need to flush the corresponding field.  Indeed, the value of `foo` will
remain 42 in cache, whatever its value in the database.  This assumption
(pending updates are in cache) is true for all fields *except* for
`write_date`: it is invalidated from cache, and given a dummy value in
pending updates.  This branch actually makes this assumption true for
all fields.  The avoidance of flushing pending updates will be done in
another commit.

In order to directly assign `write_date` its value, we use a cache for
the value `NOW() AT TIME ZONE 'UTC'` from the database.  This costs at
most one query per transaction, and potentially saves a few queries.

Co-authored-by: Victor Feyens <vfe@odoo.com>
This commit is contained in:
Raphael Collet
2021-02-22 16:20:55 +00:00
co-authored by Victor Feyens
parent 75d2f995e2
commit 840609975a
8 changed files with 62 additions and 61 deletions
Download Patch File Download Diff File Expand all files Collapse all files
addons/hr_holidays/tests/test_company_leave.py
+1 -1
View File
@@ -313,7 +313,7 @@ class TestCompanyLeave(TransactionCase):
})
company_leave._compute_date_from_to()
with self.assertQueryCount(__system__=735, admin=865):
with self.assertQueryCount(__system__=736, admin=865):
# Original query count: 1987
# Without tracking/activity context keys: 5154
company_leave.action_validate()
addons/test_mail/tests/test_performance.py
+16 -16
View File
@@ -231,7 +231,7 @@ class TestMailAPIPerformance(BaseMailPerformance):
#voip module read activity_type during create leading to one less query in enterprise on action_feedback
category = activity.activity_type_id.category
with self.assertQueryCount(__system__=16, emp=20):
with self.assertQueryCount(__system__=16, emp=18):
activity.action_feedback(feedback='Zizisse Done !')
@users('__system__', 'emp')
@@ -248,7 +248,7 @@ class TestMailAPIPerformance(BaseMailPerformance):
record.write({'name': 'Dupe write'})
with self.assertQueryCount(__system__=17, emp=20):
with self.assertQueryCount(__system__=17, emp=19):
record.action_close('Dupe feedback')
self.assertEqual(record.activity_ids, self.env['mail.activity'])
@@ -270,7 +270,7 @@ class TestMailAPIPerformance(BaseMailPerformance):
'partner_ids': [(4, customer_id)],
})
with self.assertQueryCount(__system__=25, emp=32):
with self.assertQueryCount(__system__=24, emp=31):
composer.send_mail()
@users('__system__', 'emp')
@@ -281,7 +281,7 @@ class TestMailAPIPerformance(BaseMailPerformance):
test_record = self.env['mail.test.ticket'].browse(self.test_record_full.id)
test_template = self.env['mail.template'].browse(self.test_template_full.id)
# TODO FIXME non deterministic, last check 25 Mar 2020. Runbot at most +7 compared to local.
with self.assertQueryCount(__system__=11, emp=13):
with self.assertQueryCount(__system__=11, emp=12):
composer = self.env['mail.compose.message'].with_context({
'default_composition_mode': 'comment',
'default_model': test_record._name,
@@ -290,7 +290,7 @@ class TestMailAPIPerformance(BaseMailPerformance):
}).create({})
composer.onchange_template_id_wrapper()
with self.assertQueryCount(__system__=33, emp=40):
with self.assertQueryCount(__system__=32, emp=39):
composer.send_mail()
# remove created partner to ensure tests are the same each run
@@ -302,7 +302,7 @@ class TestMailAPIPerformance(BaseMailPerformance):
def test_message_assignation_email(self):
self.user_test.write({'notification_type': 'email'})
record = self.env['mail.test.track'].create({'name': 'Test'})
with self.assertQueryCount(__system__=37, emp=38):
with self.assertQueryCount(__system__=36, emp=37):
record.write({
'user_id': self.user_test.id,
})
@@ -355,7 +355,7 @@ class TestMailAPIPerformance(BaseMailPerformance):
def test_message_post_one_email_notification(self):
record = self.env['mail.test.simple'].create({'name': 'Test'})
with self.assertQueryCount(__system__=28, emp=31):
with self.assertQueryCount(__system__=27, emp=30):
record.message_post(
body='<p>Test Post Performances with an email ping</p>',
partner_ids=self.customer.ids,
@@ -481,7 +481,7 @@ class TestMailComplexPerformance(BaseMailPerformance):
'recipient_ids': [(4, pid) for pid in self.partners.ids],
})
mail_ids = mail.ids
with self.assertQueryCount(__system__=7, emp=7):
with self.assertQueryCount(__system__=6, emp=6):
self.env['mail.mail'].sudo().browse(mail_ids).send()
self.assertEqual(mail.body_html, '<p>Test</p>')
@@ -494,7 +494,7 @@ class TestMailComplexPerformance(BaseMailPerformance):
self.container.message_subscribe(self.user_portal.partner_id.ids)
record = self.container.with_user(self.env.user)
with self.assertQueryCount(__system__=63, emp=64):
with self.assertQueryCount(__system__=61, emp=62):
record.message_post(
body='<p>Test Post Performances</p>',
message_type='comment',
@@ -511,7 +511,7 @@ class TestMailComplexPerformance(BaseMailPerformance):
record = self.container.with_user(self.env.user)
template_id = self.env.ref('test_mail.mail_test_container_tpl').id
with self.assertQueryCount(__system__=73, emp=75):
with self.assertQueryCount(__system__=70, emp=71):
record.message_post_with_template(template_id, message_type='comment', composition_mode='comment')
self.assertEqual(record.message_ids[0].body, '<p>Adding stuff on %s</p>' % record.name)
@@ -582,7 +582,7 @@ class TestMailComplexPerformance(BaseMailPerformance):
})
rec1 = rec.with_context(active_test=False) # to see inactive records
self.assertEqual(rec1.message_partner_ids, self.partners | self.env.user.partner_id)
with self.assertQueryCount(__system__=37, emp=38):
with self.assertQueryCount(__system__=36, emp=37):
rec.write({'user_id': self.user_portal.id})
self.assertEqual(rec1.message_partner_ids, self.partners | self.env.user.partner_id | self.user_portal.partner_id)
# write tracking message
@@ -602,7 +602,7 @@ class TestMailComplexPerformance(BaseMailPerformance):
customer_id = self.customer.id
user_id = self.user_portal.id
with self.assertQueryCount(__system__=108, emp=109):
with self.assertQueryCount(__system__=105, emp=106):
rec = self.env['mail.test.ticket'].create({
'name': 'Test',
'container_id': container_id,
@@ -631,7 +631,7 @@ class TestMailComplexPerformance(BaseMailPerformance):
rec1 = rec.with_context(active_test=False) # to see inactive records
self.assertEqual(rec1.message_partner_ids, self.user_portal.partner_id | self.env.user.partner_id)
self.assertEqual(len(rec1.message_ids), 1)
with self.assertQueryCount(__system__=77, emp=77):
with self.assertQueryCount(__system__=75, emp=75):
rec.write({
'name': 'Test2',
'container_id': self.container.id,
@@ -668,7 +668,7 @@ class TestMailComplexPerformance(BaseMailPerformance):
rec1 = rec.with_context(active_test=False) # to see inactive records
self.assertEqual(rec1.message_partner_ids, self.user_portal.partner_id | self.env.user.partner_id)
with self.assertQueryCount(__system__=85, emp=85):
with self.assertQueryCount(__system__=83, emp=83):
rec.write({
'name': 'Test2',
'container_id': container_id,
@@ -701,7 +701,7 @@ class TestMailComplexPerformance(BaseMailPerformance):
rec1 = rec.with_context(active_test=False) # to see inactive records
self.assertEqual(rec1.message_partner_ids, self.partners | self.env.user.partner_id | self.user_portal.partner_id)
with self.assertQueryCount(__system__=29, emp=30):
with self.assertQueryCount(__system__=28, emp=29):
rec.write({
'name': 'Test2',
'customer_id': customer_id,
@@ -951,7 +951,7 @@ class TestMailHeavyPerformancePost(BaseMailPerformance):
]
self.attachements = self.env['ir.attachment'].with_user(self.env.user).create(self.vals)
attachement_ids = self.attachements.ids
with self.assertQueryCount(emp=79):
with self.assertQueryCount(emp=76):
self.cr.sql_log = self.warm and self.cr.sql_log_count
record.with_context({}).message_post(
body='<p>Test body <img src="cid:cid1"> <img src="cid:cid2"></p>',
addons/test_mass_mailing/tests/test_performance.py
+2 -2
View File
@@ -51,7 +51,7 @@ class TestMassMailPerformance(TestMassMailPerformanceBase):
})
# runbot needs +50 compared to local
with self.assertQueryCount(__system__=1714, marketing=1715):
with self.assertQueryCount(__system__=1664, marketing=1664):
mailing.action_send_mail()
self.assertEqual(mailing.sent, 50)
@@ -91,7 +91,7 @@ class TestMassMailBlPerformance(TestMassMailPerformanceBase):
})
# runbot needs +62 compared to local
with self.assertQueryCount(__system__=1992, marketing=1993):
with self.assertQueryCount(__system__=1942, marketing=1942):
mailing.action_send_mail()
self.assertEqual(mailing.sent, 50)
odoo/addons/test_assetsbundle/tests/test_assetsbundle.py
+2 -1
View File
@@ -706,8 +706,9 @@ class TestAssetsBundleWithIRAMock(FileTouchable):
# has really been modified. If we do not update the write_date to a posterior date, we are
# not able to reproduce the case where we compile this bundle again without changing
# anything.
self.env['ir.attachment'].flush(['checksum'])
self.env['ir.attachment'].flush(['checksum', 'write_date'])
self.cr.execute("update ir_attachment set write_date=clock_timestamp() + interval '10 seconds' where id = (select max(id) from ir_attachment)")
self.env['ir.attachment'].invalidate_cache(['write_date'])
# Compile a fourth time, without changes
self._bundle(self._get_asset(), False, False)
odoo/addons/test_new_api/tests/test_new_fields.py
+2 -2
View File
@@ -3115,7 +3115,7 @@ class TestFieldParametersValidation(common.TransactionCase):
def insert(model, *fnames):
""" Return the expected query string to INSERT the given columns. """
columns = ['create_uid', 'create_date', 'write_uid', 'write_date'] + sorted(fnames)
columns = sorted(fnames + ('create_uid', 'create_date', 'write_uid', 'write_date'))
return 'INSERT INTO "{}" ("id", {}) VALUES (nextval(%s), {}) RETURNING id'.format(
model._table,
", ".join('"{}"'.format(column) for column in columns),
@@ -3125,7 +3125,7 @@ def insert(model, *fnames):
def update(model, *fnames):
""" Return the expected query string to UPDATE the given columns. """
columns = sorted(fnames) + ['write_uid', 'write_date']
columns = sorted(fnames + ('write_uid', 'write_date'))
return 'UPDATE "{}" SET {} WHERE id IN %s'.format(
model._table,
", ".join('"{}" = %s'.format(column) for column in columns),
odoo/addons/test_performance/tests/test_performance.py
+7 -7
View File
@@ -136,12 +136,12 @@ class TestPerformance(SavepointCaseWithUserDemo):
self.assertEqual(rec1.line_ids, lines)
# delete N lines: O(1) queries
with self.assertQueryCount(14):
with self.assertQueryCount(15):
rec1.invalidate_cache()
rec1.write({'line_ids': [Command.delete(line.id) for line in lines[0]]})
self.assertEqual(rec1.line_ids, lines[1:])
with self.assertQueryCount(12):
with self.assertQueryCount(13):
rec1.invalidate_cache()
rec1.write({'line_ids': [Command.delete(line.id) for line in lines[1:]]})
self.assertFalse(rec1.line_ids)
@@ -151,12 +151,12 @@ class TestPerformance(SavepointCaseWithUserDemo):
lines = rec1.line_ids
# unlink N lines: O(1) queries
with self.assertQueryCount(14):
with self.assertQueryCount(15):
rec1.invalidate_cache()
rec1.write({'line_ids': [Command.unlink(line.id) for line in lines[0]]})
self.assertEqual(rec1.line_ids, lines[1:])
with self.assertQueryCount(12):
with self.assertQueryCount(13):
rec1.invalidate_cache()
rec1.write({'line_ids': [Command.unlink(line.id) for line in lines[1:]]})
self.assertFalse(rec1.line_ids)
@@ -190,7 +190,7 @@ class TestPerformance(SavepointCaseWithUserDemo):
self.assertEqual(rec2.line_ids, lines)
# empty N lines in rec2: O(1) queries
with self.assertQueryCount(__system__=13, demo=13):
with self.assertQueryCount(14):
rec1.invalidate_cache()
rec2.write({'line_ids': [Command.clear()]})
self.assertFalse(rec2.line_ids)
@@ -210,13 +210,13 @@ class TestPerformance(SavepointCaseWithUserDemo):
self.assertEqual(rec1.line_ids, lines[1:])
self.assertEqual(rec2.line_ids, lines[0])
with self.assertQueryCount(7):
with self.assertQueryCount(6):
rec1.invalidate_cache()
rec2.write({'line_ids': [Command.set(lines.ids)]})
self.assertFalse(rec1.line_ids)
self.assertEqual(rec2.line_ids, lines)
with self.assertQueryCount(5):
with self.assertQueryCount(4):
rec1.invalidate_cache()
rec2.write({'line_ids': [Command.set(lines.ids)]})
self.assertEqual(rec2.line_ids, lines)
odoo/models.py
+22 -32
View File
@@ -3582,6 +3582,12 @@ Fields:
if not(self.env.uid == SUPERUSER_ID and not self.pool.ready):
bad_names.update(LOG_ACCESS_COLUMNS)
# set magic fields
vals = {key: val for key, val in vals.items() if key not in bad_names}
if self._log_access:
vals.setdefault('write_uid', self.env.uid)
vals.setdefault('write_date', self.env.cr.now())
determine_inverses = defaultdict(list) # {inverse: fields}
records_to_inverse = {} # {field: records}
relational_names = []
@@ -3645,24 +3651,9 @@ Fields:
real_recs = self.filtered('id')
# If there are only fields that do not trigger _write (e.g. only
# determine inverse), the below ensures that `write_date` and
# `write_uid` are updated (`test_orm.py`, `test_write_date`)
if self._log_access and self.ids:
towrite = env.all.towrite[self._name]
for record in real_recs:
towrite[record.id]['write_uid'] = self.env.uid
towrite[record.id]['write_date'] = False
self.env.cache.invalidate([
(self._fields['write_date'], self.ids),
(self._fields['write_uid'], self.ids),
])
# for monetary field, their related currency field must be cached
# before the amount so it can be rounded correctly
for fname in sorted(vals, key=lambda x: self._fields[x].type=='monetary'):
if fname in bad_names:
continue
field = self._fields[fname]
field.write(self, vals[fname])
@@ -3722,6 +3713,12 @@ Fields:
# determine records that require updating parent_path
parent_records = self._parent_store_update_prepare(vals)
if self._log_access:
# set magic fields (already done by write(), but not for computed fields)
vals = dict(vals)
vals.setdefault('write_uid', self.env.uid)
vals.setdefault('write_date', self.env.cr.now())
# determine SQL values
columns = [] # list of (column_name, format, value)
@@ -3737,12 +3734,6 @@ Fields:
assert field.column_type
columns.append((name, field.column_format, val))
if self._log_access:
if not vals.get('write_uid'):
columns.append(('write_uid', '%s', self._uid))
if not vals.get('write_date'):
columns.append(('write_date', '%s', AsIs("(now() at time zone 'UTC')")))
# update columns
if columns:
query = 'UPDATE "%s" SET %s WHERE id IN %%s' % (
@@ -3810,6 +3801,15 @@ Fields:
# add missing defaults
vals = self._add_missing_default_values(vals)
# set magic fields
for name in bad_names:
vals.pop(name, None)
if self._log_access:
vals.setdefault('create_uid', self.env.uid)
vals.setdefault('create_date', self.env.cr.now())
vals.setdefault('write_uid', self.env.uid)
vals.setdefault('write_date', self.env.cr.now())
# distribute fields into sets for various purposes
data = {}
data['stored'] = stored = {}
@@ -3817,8 +3817,6 @@ Fields:
data['inherited'] = inherited = defaultdict(dict)
data['protected'] = protected = set()
for key, val in vals.items():
if key in bad_names:
continue
field = self._fields.get(key)
if not field:
raise ValueError("Invalid field %r on model %r" % (key, self._name))
@@ -3919,18 +3917,10 @@ Fields:
other_fields = set() # non-column fields
translated_fields = set() # translated fields
# column names, formats and values (for common fields)
columns0 = [('id', "nextval(%s)", self._sequence)]
if self._log_access:
columns0.append(('create_uid', "%s", self._uid))
columns0.append(('create_date', "%s", AsIs("(now() at time zone 'UTC')")))
columns0.append(('write_uid', "%s", self._uid))
columns0.append(('write_date', "%s", AsIs("(now() at time zone 'UTC')")))
for data in data_list:
# determine column values
stored = data['stored']
columns = [column for column in columns0 if column[0] not in stored]
columns = [('id', "nextval(%s)", self._sequence)]
for name, val in sorted(stored.items()):
field = self._fields[name]
assert field.store
odoo/sql_db.py
+10
View File
@@ -257,6 +257,7 @@ class Cursor(BaseCursor):
self._default_log_exceptions = True
self.cache = {}
self._now = None
def __build_dict(self, row):
return {d.name: row[i] for i, d in enumerate(self._obj.description)}
@@ -444,6 +445,7 @@ class Cursor(BaseCursor):
flush_env(self)
self.precommit.run()
result = self._cnx.commit()
self._now = None
self.prerollback.clear()
self.postrollback.clear()
self.postcommit.run()
@@ -457,6 +459,7 @@ class Cursor(BaseCursor):
self.postcommit.clear()
self.prerollback.run()
result = self._cnx.rollback()
self._now = None
self.postrollback.run()
return result
@@ -468,6 +471,13 @@ class Cursor(BaseCursor):
def closed(self):
return self._closed
def now(self):
""" Return the transaction's timestamp ``NOW() AT TIME ZONE 'UTC'``. """
if self._now is None:
self.execute("SELECT (now() AT TIME ZONE 'UTC')")
self._now = self.fetchone()[0]
return self._now
class TestCursor(BaseCursor):
""" A pseudo-cursor to be used for tests, on top of a real cursor. It keeps