[IMP] core: bulk insert

The `_create` don't batch the SQL request to insert of data which it
isn't efficient we there is a several record to create.
Batch the SQL insertion and measure performance
depending of batch size to avoid any regression (see result in the
PR: https://github.com/odoo/odoo/pull/80961).

task-2299314

closes odoo/odoo#80961

Signed-off-by: Raphael Collet <rco@odoo.com>
Co-authored-by: rco-odoo <rco@odoo.com>

odoo/addons/test_new_api/tests/test_new_fields.py

@@ -3399,11 +3399,12 @@ class TestFieldParametersValidation(common.TransactionCase):
         ))
 
 
-def insert(model, *fnames):
+def insert(model, *fnames, rowcount=1):
     """ Return the expected query string to INSERT the given columns. """
     columns = sorted(fnames + ('create_uid', 'create_date', 'write_uid', 'write_date'))
     header = ", ".join(f'"{column}"' for column in columns)
-    return f'INSERT INTO "{model._table}" ({header}) VALUES %s RETURNING id'
+    template = ", ".join("%s" for _index in range(rowcount))
+    return f'INSERT INTO "{model._table}" ({header}) VALUES {template} RETURNING "id"'
 
 
 def update(model, *fnames):
@@ -3500,6 +3501,23 @@ class TestComputeQueries(common.TransactionCase):
         self.assertEqual(record.foo, 'Bar')
         self.assertEqual(record.bar, 'Bar')
 
+    def test_multi_create(self):
+        model = self.env['test_new_api.foo']
+        model.create({})
+
+        with self.assertQueries([insert(model, 'name', 'value1', 'value2', rowcount=4)]):
+            create_values = [
+                {'name': 'Foo1', 'value1': 10},
+                {'name': 'Foo2', 'value2': 12},
+                {'name': 'Foo3'},
+                {},
+            ]
+            records = model.create(create_values)
+        self.assertEqual(records.mapped('name'), ['Foo1', 'Foo2', 'Foo3', False])
+        self.assertEqual(records.mapped('value1'), [10, 0, 0, 0])
+        self.assertEqual(records.mapped('value2'), [0, 12, 0, 0])
+
+
 class test_shared_cache(TransactionCaseWithUserDemo):
     def test_shared_cache_computed_field(self):
         # Test case: Check that the shared cache is not used if a compute_sudo stored field

odoo/models.py

@@ -59,7 +59,7 @@ from .tools import frozendict, lazy_classproperty, ormcache, \
                    groupby, discardattr, partition
 from .tools.config import config
 from .tools.func import frame_codeinfo
-from .tools.misc import CountingStream, clean_context, DEFAULT_SERVER_DATETIME_FORMAT, DEFAULT_SERVER_DATE_FORMAT, get_lang
+from .tools.misc import CountingStream, clean_context, DEFAULT_SERVER_DATETIME_FORMAT, DEFAULT_SERVER_DATE_FORMAT, get_lang, split_every
 from .tools.translate import _
 from .tools import date_utils
 from .tools import populate
@@ -76,6 +76,9 @@ regex_field_agg = re.compile(r'(\w+)(?::(\w+)(?:\((\w+)\))?)?')
 
 AUTOINIT_RECALCULATE_STORED_FIELDS = 1000
 
+INSERT_BATCH_SIZE = 100
+SQL_DEFAULT = psycopg2.extensions.AsIs("DEFAULT")
+
 def check_object_name(name):
     """ Check if the given name is a valid model name.
 
@@ -4186,57 +4189,45 @@ Fields:
         """ Create records from the stored field values in ``data_list``. """
         assert data_list
         cr = self.env.cr
-        quote = '"{}"'.format
 
-        # insert rows
+        # insert rows in batches of maximum INSERT_BATCH_SIZE
         ids = []                                # ids of created records
         other_fields = OrderedSet()             # non-column fields
         translated_fields = OrderedSet()        # translated fields
 
-        for data in data_list:
-            # determine column values
-            stored = data['stored']
-            columns = {}
-            for name, val in sorted(stored.items()):
-                field = self._fields[name]
-                assert field.store
+        for data_sublist in split_every(INSERT_BATCH_SIZE, data_list):
+            stored_list = [data['stored'] for data in data_sublist]
+            fnames = sorted({name for stored in stored_list for name in stored})
 
+            columns = []
+            rows = [[] for _ in stored_list]
+            for fname in fnames:
+                field = self._fields[fname]
                 if field.column_type:
-                    col_val = field.convert_to_column(val, self, stored)
-                    columns[name] = col_val
                     if field.translate is True:
                         translated_fields.add(field)
+                    columns.append(fname)
+                    for stored, row in zip(stored_list, rows):
+                        if fname in stored:
+                            row.append(field.convert_to_column(stored[fname], self, stored))
+                        else:
+                            row.append(SQL_DEFAULT)
                 else:
                     other_fields.add(field)
 
-            # Insert rows one by one
-            # - as records don't all specify the same columns, code building batch-insert query
-            #   was very complex
-            # - and the gains were low, so not worth spending so much complexity
-            #
-            # It also seems that we have to be careful with INSERTs in batch, because they have the
-            # same problem as SELECTs:
-            # If we inject a lot of data in a single query, we fall into pathological perfs in
-            # terms of SQL parser and the execution of the query itself.
-            # In SELECT queries, we inject max 1000 ids (integers) when we can, because we know
-            # that this limit is well managed by PostgreSQL.
-            # In INSERT queries, we inject integers (small) and larger data (TEXT blocks for
-            # example).
-            #
-            # The problem then becomes: how to "estimate" the right size of the batch to have
-            # good performance?
-            #
-            # This requires extensive testing, and it was preferred not to introduce INSERTs in
-            # batch, to avoid regressions as much as possible.
-            #
-            # That said, we haven't closed the door completely.
-            if not columns:  # Manage the case where we create empty record
-                columns = {'id': psycopg2.extensions.AsIs('DEFAULT')}
+            if not columns:
+                # manage the case where we create empty records
+                columns = ['id']
+                for row in rows:
+                    row.append(SQL_DEFAULT)
 
-            header = ", ".join(quote(name) for name in columns)
-            query = f'INSERT INTO "{self._table}" ({header}) VALUES %s RETURNING id'
-            cr.execute(query, [tuple(columns.values())])
-            ids.append(cr.fetchone()[0])
+            header = ", ".join(f'"{column}"' for column in columns)
+            template = ", ".join("%s" for _ in rows)
+            cr.execute(
+                f'INSERT INTO "{self._table}" ({header}) VALUES {template} RETURNING "id"',
+                [tuple(row) for row in rows],
+            )
+            ids.extend(id_ for id_, in cr.fetchall())
 
         # put the new records in cache, and update inverse fields, for many2one
         #
@@ -4244,11 +4235,12 @@ Fields:
         cachetoclear = []
         records = self.browse(ids)
         inverses_update = defaultdict(list)     # {(field, value): ids}
+        common_set_vals = set(LOG_ACCESS_COLUMNS + [self.CONCURRENCY_CHECK_FIELD, 'id', 'parent_path'])
         for data, record in zip(data_list, records):
             data['record'] = record
             # DLE P104: test_inherit.py, test_50_search_one2many
             vals = dict({k: v for d in data['inherited'].values() for k, v in d.items()}, **data['stored'])
-            set_vals = list(vals) + LOG_ACCESS_COLUMNS + [self.CONCURRENCY_CHECK_FIELD, 'id', 'parent_path']
+            set_vals = common_set_vals.union(vals)
             for field in self._fields.values():
                 if field.type in ('one2many', 'many2many'):
                     self.env.cache.set(record, field, ())