[FIX] core: searching through o2m with auto_join

Replaces the auto_join scheme which is pretty fast but doesn't quite
work (it's *completely* broken for read_group) by a subquery which is
slower but has the same semantics as the ORM version.

Ideally we'd use `EXISTS (select 1 from ...)` but that has no support
inside the SQL compiler right now, so use `id in (select inverse_field
from ...)`.

Here are some measures of a `read_group` on a simple object with lines
filtered on line values, similar to

    read_group([('order_line.product_uom', '=', xxx)],
               fields=['total_amount'], groupby=['user_id'])

    +-------+-------+-------+--------+
    |       |   ORM |  join |subquery|
    +-------+-------+-------+--------+
    | large |   160 |    10 |     20 |
    +-------+-------+-------+--------+
    |  many | 36000 |    50 |    180 |
    +-------+-------+-------+--------+

* values of data fields (filtering, grouping and aggregating) randomly
picked between 10 different values
* "large" is 1000 objects with 1000~15000 lines each
* "many" is 1000000 objects with 1~40 lines each

closes odoo/odoo#48494

Signed-off-by: Raphael Collet (rco) <rco@openerp.com>

4 files changed, 62 insertions, 52 deletions

diff --git a/odoo/addons/base/tests/test_expression.py b/odoo/addons/base/tests/test_expression.py
index 7aeb57cac1d..1544f22c6e3 100644
--- a/odoo/addons/base/tests/test_expression.py
+++ b/odoo/addons/base/tests/test_expression.py
@@ -1338,22 +1338,27 @@ class TestOne2many(TransactionCase):
         with self.assertQueries(['''
             SELECT "res_partner".id
             FROM "res_partner"
-            LEFT JOIN "res_partner_bank" AS "res_partner__bank_ids" ON
-                ("res_partner"."id" = "res_partner__bank_ids"."partner_id")
-            WHERE ("res_partner__bank_ids"."sanitized_acc_number"::text LIKE %s)
+            WHERE ("res_partner"."id" IN (
+                SELECT "res_partner_bank"."partner_id"
+                FROM "res_partner_bank"
+                WHERE ("res_partner_bank"."sanitized_acc_number"::text LIKE %s)
+            ))
             ORDER BY "res_partner"."display_name"
         ''']):
             self.Partner.search([('bank_ids.sanitized_acc_number', 'like', '12')])
 
-        # WRONG QUERY: the domain semantics is having some line matching '12'
-        # and having some line matching '45', but not necessarily the same line!
         with self.assertQueries(['''
             SELECT "res_partner".id
             FROM "res_partner"
-            LEFT JOIN "res_partner_bank" AS "res_partner__bank_ids" ON
-                ("res_partner"."id" = "res_partner__bank_ids"."partner_id")
-            WHERE (("res_partner__bank_ids"."sanitized_acc_number"::text LIKE %s)
-            AND ("res_partner__bank_ids"."sanitized_acc_number"::text LIKE %s))
+            WHERE (("res_partner"."id" IN (
+                SELECT "res_partner_bank"."partner_id"
+                FROM "res_partner_bank"
+                WHERE ("res_partner_bank"."sanitized_acc_number"::text LIKE %s)
+            )) AND ("res_partner"."id" IN (
+                SELECT "res_partner_bank"."partner_id"
+                FROM "res_partner_bank"
+                WHERE ("res_partner_bank"."sanitized_acc_number"::text LIKE %s)
+            )))
             ORDER BY "res_partner"."display_name"
         ''']):
             self.Partner.search([
@@ -1364,12 +1369,15 @@ class TestOne2many(TransactionCase):
         with self.assertQueries(['''
             SELECT "res_partner".id
             FROM "res_partner"
-            LEFT JOIN "res_partner" AS "res_partner__child_ids" ON
-                ("res_partner"."id" = "res_partner__child_ids"."parent_id")
-            LEFT JOIN "res_partner_bank" AS "res_partner__child_ids__bank_ids" ON
-                ("res_partner__child_ids"."id" = "res_partner__child_ids__bank_ids"."partner_id")
-            WHERE (("res_partner__child_ids"."active" = %s)
-            AND ("res_partner__child_ids__bank_ids"."sanitized_acc_number"::text LIKE %s))
+            WHERE ("res_partner"."id" IN (
+                SELECT "res_partner"."parent_id"
+                FROM "res_partner"
+                WHERE (("res_partner"."id" IN (
+                    SELECT "res_partner_bank"."partner_id"
+                    FROM "res_partner_bank"
+                    WHERE ("res_partner_bank"."sanitized_acc_number"::text LIKE %s)
+                )) AND ("res_partner"."active" = %s))
+            ))
             ORDER BY "res_partner"."display_name"
         ''']):
             self.Partner.search([('child_ids.bank_ids.sanitized_acc_number', 'like', '12')])
@@ -1383,13 +1391,23 @@ class TestOne2many(TransactionCase):
         with self.assertQueries(['''
             SELECT "res_partner".id
             FROM "res_partner"
-            LEFT JOIN "res_partner" AS "res_partner__child_ids" ON
-                ("res_partner"."id" = "res_partner__child_ids"."parent_id")
-            LEFT JOIN "res_partner_bank" AS "res_partner__child_ids__bank_ids" ON
-                ("res_partner__child_ids"."id" = "res_partner__child_ids__bank_ids"."partner_id")
-            WHERE ((NOT (("res_partner__child_ids"."name" = %s)))
-            AND (("res_partner__child_ids__bank_ids"."sanitized_acc_number"::text LIKE %s)
-            AND ("res_partner__child_ids__bank_ids"."id" in (%s,%s,%s))))
+            WHERE ("res_partner"."id" IN (
+                SELECT "res_partner"."parent_id"
+                FROM "res_partner"
+                WHERE ((
+                    "res_partner"."id" IN (
+                        SELECT "res_partner_bank"."partner_id"
+                        FROM "res_partner_bank"
+                        WHERE ((
+                            "res_partner_bank"."id" IN (%s,%s,%s)
+                        ) AND (
+                            "res_partner_bank"."sanitized_acc_number"::text LIKE %s
+                        ))
+                    )
+                ) AND (
+                    ("res_partner"."name" != %s) OR "res_partner"."name" IS NULL
+                ))
+            ))
             ORDER BY "res_partner"."display_name"
         ''']):
             self.Partner.search([('child_ids.bank_ids.id', 'in', self.partner.bank_ids.ids)])
@@ -1403,14 +1421,19 @@ class TestOne2many(TransactionCase):
         with self.assertQueries(['''
             SELECT "res_partner".id
             FROM "res_partner"
-            LEFT JOIN "res_partner" AS "res_partner__child_ids" ON
-                ("res_partner"."id" = "res_partner__child_ids"."parent_id")
-            LEFT JOIN "res_country_state" AS "res_partner__child_ids__state_id" ON
-                ("res_partner__child_ids"."state_id" = "res_partner__child_ids__state_id"."id")
-            LEFT JOIN "res_country" AS "res_partner__child_ids__state_id__country_id" ON
-                ("res_partner__child_ids__state_id"."country_id" = "res_partner__child_ids__state_id__country_id"."id")
-            WHERE (("res_partner__child_ids"."active" = %s)
-            AND ("res_partner__child_ids__state_id__country_id"."code"::text LIKE %s))
+            WHERE ("res_partner"."id" IN (
+                SELECT "res_partner"."parent_id"
+                FROM "res_partner"
+                LEFT JOIN "res_country_state" AS "res_partner__state_id"
+                    ON ("res_partner"."state_id" = "res_partner__state_id"."id")
+                LEFT JOIN "res_country" AS "res_partner__state_id__country_id"
+                    ON ("res_partner__state_id"."country_id" = "res_partner__state_id__country_id"."id")
+                WHERE ((
+                    "res_partner__state_id__country_id"."code"::text LIKE %s
+                ) AND (
+                    "res_partner"."active" = %s
+                ))
+            ))
             ORDER BY "res_partner"."display_name"
         ''']):
             self.Partner.search([('child_ids.state_id.country_id.code', 'like', 'US')])
diff --git a/odoo/addons/test_read_group/tests/test_auto_join.py b/odoo/addons/test_read_group/tests/test_auto_join.py
index 6c5e7a88f77..7347645949c 100644
--- a/odoo/addons/test_read_group/tests/test_auto_join.py
+++ b/odoo/addons/test_read_group/tests/test_auto_join.py
@@ -38,7 +38,6 @@ class TestAutoJoin(common.TransactionCase):
         self.assertEqual(len(result1), 1)
         self.assertEqual(result1[0]['__count'], 2)
 
-        # TO FIX: the first record is counted twice!
         result2 = model.read_group(domain2, [], [])
         self.assertEqual(len(result2), 1)
-        self.assertEqual(result2[0]['__count'], 3)
+        self.assertEqual(result2[0]['__count'], 2)
diff --git a/odoo/models.py b/odoo/models.py
index 80d65935cc9..8af30b89bb5 100644
--- a/odoo/models.py
+++ b/odoo/models.py
@@ -4369,16 +4369,7 @@ Record ids: %(records)s
         offset_str = offset and ' offset %d' % offset or ''
         query_str = 'SELECT "%s".id FROM ' % self._table + from_clause + where_str + order_by + limit_str + offset_str
         self._cr.execute(query_str, where_clause_params)
-        res = self._cr.fetchall()
-
-        # TDE note: with auto_join, we could have several lines about the same result
-        # i.e. a lead with several unread messages; we uniquify the result using
-        # a fast way to do it while preserving order (http://www.peterbe.com/plog/uniqifiers-benchmark)
-        def _uniquify_list(seq):
-            seen = set()
-            return [x for x in seq if x not in seen and not seen.add(x)]
-
-        return _uniquify_list([x[0] for x in res])
+        return [row[0] for row in self._cr.fetchall()]
 
     @api.returns(None, lambda value: value[0])
     def copy_data(self, default=None):
diff --git a/odoo/osv/expression.py b/odoo/osv/expression.py
index eba82d9950c..732b49523ae 100644
--- a/odoo/osv/expression.py
+++ b/odoo/osv/expression.py
@@ -744,17 +744,14 @@ class expression(object):
                 push((path[1], operator, right), comodel, coalias)
 
             elif len(path) > 1 and field.store and field.type == 'one2many' and field.auto_join:
-                # res_partner.id = res_partner__bank_ids.partner_id
-                coalias, _ = self.query.add_join(
-                    (alias, comodel._table, 'id', field.inverse_name, path[0]),
-                    implicit=False, outer=True,
+                # use a subquery bypassing access rules and business logic
+                domain = [(path[1], operator, right)] + field.get_domain_list(model)
+                query = comodel.with_context(**field.context)._where_calc(domain)
+                subfrom, subwhere, subparams = query.get_sql()
+                subquery = 'SELECT "{}"."{}" FROM {} WHERE {}'.format(
+                    comodel._table, field.inverse_name, subfrom, subwhere,
                 )
-                domain = field.get_domain_list(model)
-                if domain:
-                    push(AND_OPERATOR, comodel, coalias)
-                    for elem in normalize_domain(domain):
-                        push(elem, comodel, coalias)
-                push((path[1], operator, right), comodel, coalias)
+                push(('id', 'inselect', (subquery, subparams)), model, alias, internal=True)
 
             elif len(path) > 1 and field.store and field.auto_join:
                 raise NotImplementedError('auto_join attribute not supported on field %s' % field)