[FIX] core: new record shouldn't force fetching inverse x2many fields

In e0297bdac4, the creation of a new
record always patches the inverse fields of relational fields in order
to make the cache of those inverse fields consistent.

For instance, when creating a new record like

    user = model.new({'group_ids': [Command.link(group.id)]})

The inverse of field 'group_ids' on the new record having 'group' as
origin is patched so that its value includes record.  A side effect of
this mechanism is that it fetches group.user_ids in order to patch the
value of new_group.user_ids, where 'new_group' is the new record having
'group' as origin.

The side effect described above is problematic when that inverse field
has huge cardinality, like hundreds of thousands of records, and this
performance overhead is unacceptable when the inverse field is actually
not used at all.

We address this performance issue by patching the value of x2many fields
only when they are used.  If the value of the field is not in cache yet,
the patch is applied once a value is put in cache.  If the field is not
used, the patch is simply never applied.

Part-of: odoo/odoo#149624

odoo/addons/test_new_api/tests/test_new_fields.py

@@ -2195,6 +2195,35 @@ class TestFields(TransactionCaseWithUserDemo):
         self.assertEqual(new_move.line_ids._origin, line)
         self.assertEqual(new_move.line_ids.move_id, new_move)
 
+    def test_41_new_many2many(self):
+        group = self.env['test_new_api.group'].create({})
+        user0 = self.env['test_new_api.user'].create({'group_ids': [Command.link(group.id)]})
+        new_user0 = user0.new(origin=user0)
+        new_group = group.new(origin=group)
+
+        self.env.invalidate_all()
+
+        # creating new_user1 shoud not fetch new_group.user_ids, which is the
+        # inverse of field new_user1.group_ids
+        with self.assertQueryCount(0):
+            new_user1 = self.env['test_new_api.user'].new({'group_ids': [Command.link(group.id)]})
+            self.assertEqual(new_user1.group_ids, new_group)
+
+        # accessing new_group.user_ids should fetch group.user_ids and patch
+        # new_group.user_ids
+        with self.assertQueryCount(1):
+            self.assertEqual(new_group.user_ids, new_user0 + new_user1)
+
+        # creating new_user2 should patch new_group.user_ids immediately, since
+        # it is in cache
+        with self.assertQueryCount(0):
+            new_user2 = self.env['test_new_api.user'].new({'group_ids': [Command.link(group.id)]})
+            self.assertEqual(new_user2.group_ids, new_group)
+            self.assertEqual(new_group.user_ids, new_user0 + new_user1 + new_user2)
+
+        # the patches on new_group.user_ids should not have changed group.user_ids
+        self.assertEqual(group.user_ids, user0)
+
     @mute_logger('odoo.addons.base.models.ir_model')
     def test_41_new_related(self):
         """ test the behavior of related fields starting on new records. """

odoo/api.py

@@ -910,6 +910,10 @@ class Cache(object):
         # in `_data`
         self._dirty = defaultdict(OrderedSet)
 
+        # {field: {record_id: ids}} record ids to be added to the values of
+        # x2many fields if they are not in cache yet
+        self._patches = defaultdict(lambda: defaultdict(list))
+
     def __repr__(self):
         # for debugging: show the cache content and dirty flags as stars
         data = {}
@@ -1099,6 +1103,34 @@ class Cache(object):
             for id_, val in zip(records._ids, values):
                 field_cache.setdefault(id_, val)
 
+    def patch(self, records, field, new_id):
+        """ Apply a patch to an x2many field on new records. The patch consists
+        in adding new_id to its value in cache. If the value is not in cache
+        yet, it will be applied once the value is put in cache with method
+        :meth:`patch_and_set`.
+        """
+        assert not new_id, "Cache.patch can only be called with a new id"
+        field_cache = self._set_field_cache(records, field)
+        for id_ in records._ids:
+            assert not id_, "Cache.patch can only be called with new records"
+            if id_ in field_cache:
+                field_cache[id_] = tuple(dict.fromkeys(field_cache[id_] + (new_id,)))
+            else:
+                self._patches[field][id_].append(new_id)
+
+    def patch_and_set(self, record, field, value):
+        """ Set the value of ``field`` for ``record``, like :meth:`set`, but
+        apply pending patches to ``value`` and return the value actually put
+        in cache.
+        """
+        field_patches = self._patches.get(field)
+        if field_patches:
+            ids = field_patches.pop(record.id, ())
+            if ids:
+                value = tuple(dict.fromkeys(value + tuple(ids)))
+        self.set(record, field, value)
+        return value
+
     def remove(self, record, field):
         """ Remove the value of ``field`` for ``record``. """
         assert record.id not in self._dirty.get(field, ())
@@ -1257,6 +1289,7 @@ class Cache(object):
         """ Invalidate the cache and its dirty flags. """
         self._data.clear()
         self._dirty.clear()
+        self._patches.clear()
 
     def check(self, env):
         """ Check the consistency of the cache for the given environment. """

odoo/fields.py

@@ -1194,7 +1194,7 @@ class Field(MetaField('DummyField', (object,), {})):
         elif self.store and record._origin and not (self.compute and self.readonly):
             # new record with origin: fetch from origin
             value = self.convert_to_cache(record._origin[self.name], record, validate=False)
-            env.cache.set(record, self, value)
+            value = env.cache.patch_and_set(record, self, value)
 
         elif self.compute: #pylint: disable=using-constant-test
             # non-stored field or new record without origin: compute
@@ -1240,7 +1240,7 @@ class Field(MetaField('DummyField', (object,), {})):
         else:
             # non-stored field or stored field on new record: default value
             value = self.convert_to_cache(False, record, validate=False)
-            env.cache.set(record, self, value)
+            value = env.cache.patch_and_set(record, self, value)
             defaults = record.default_get([self.name])
             if self.name in defaults:
                 # The null value above is necessary to convert x2many field
@@ -4167,10 +4167,7 @@ class _RelationalMulti(_Relational):
 
     def _update(self, records, value):
         """ Update the cached value of ``self`` for ``records`` with ``value``. """
-        cache = records.env.cache
-        for record in records.with_context(active_test=False):
-            val = self.convert_to_cache(record[self.name] | value, record, validate=False)
-            cache.set(record, self, val)
+        records.env.cache.patch(records, self, value.id)
         records.modified([self.name])
 
     def convert_to_cache(self, value, record, validate=True):