[REF] core: improve the API and implementation of Query

Introduce better methods for introducing tables, join clauses and where
clauses to the Query object, and other methods to generate a complete
SELECT query.  The Query object can also behave as a tuple of ids, where
the query is made on demand, and the result is memoized.

2 files changed, 164 insertions, 219 deletions

diff --git a/odoo/osv/expression.py b/odoo/osv/expression.py
index 90dd394537c..e5733164567 100644
--- a/odoo/osv/expression.py
+++ b/odoo/osv/expression.py
@@ -119,7 +119,6 @@ import warnings
 import logging
 import traceback
 from functools import partial
-from zlib import crc32
 
 from datetime import date, datetime, time
 import odoo.modules
@@ -339,55 +338,6 @@ def _quote(to_quote):
     return to_quote
 
 
-def generate_table_alias(src_table_alias, joined_tables=[]):
-    """ Generate a standard table alias name. An alias is generated as following:
-        - the base is the source table name (that can already be an alias)
-        - then, each joined table is added in the alias using a 'link field name'
-          that is used to render unique aliases for a given path
-        - returns a tuple composed of the alias, and the full table alias to be
-          added in a from condition with quoting done
-        Examples:
-        - src_table_alias='res_users', join_tables=[]:
-            alias = ('res_users','"res_users"')
-        - src_model='res_users', join_tables=[(res.partner, 'parent_id')]
-            alias = ('res_users__parent_id', '"res_partner" as "res_users__parent_id"')
-
-        :param model src_table_alias: model source of the alias
-        :param list joined_tables: list of tuples
-                                   (dst_model, link_field)
-
-        :return tuple: (table_alias, alias statement for from clause with quotes added)
-    """
-    alias = src_table_alias
-    if not joined_tables:
-        return '%s' % alias, '%s' % _quote(alias)
-    for link in joined_tables:
-        alias += '__' + link[1]
-    # Use an alternate alias scheme if length exceeds the PostgreSQL limit
-    # of 63 characters.
-    if len(alias) >= 64:
-        # We have to fit a crc32 hash and one underscore
-        # into a 63 character alias. The remaining space we can use to add
-        # a human readable prefix.
-        alias_hash = hex(crc32(alias.encode('utf-8')))[2:]
-        ALIAS_PREFIX_LENGTH = 63 - len(alias_hash) - 1
-        alias = "%s_%s" % (
-            alias[:ALIAS_PREFIX_LENGTH], alias_hash)
-    return '%s' % alias, '%s as %s' % (_quote(joined_tables[-1][0]), _quote(alias))
-
-
-def get_alias_from_query(from_query):
-    """ :param string from_query: is something like :
-        - '"res_partner"' OR
-        - '"res_partner" as "res_users__partner_id"''
-    """
-    from_splitted = from_query.split(' as ')
-    if len(from_splitted) > 1:
-        return from_splitted[0].replace('"', ''), from_splitted[1].replace('"', '')
-    else:
-        return from_splitted[0].replace('"', ''), from_splitted[0].replace('"', '')
-
-
 def normalize_leaf(element):
     """ Change a term's operator to some canonical form, simplifying later
         processing. """
@@ -482,7 +432,7 @@ class expression(object):
         self.expression = distribute_not(normalize_domain(domain))
 
         # this object handles all the joins
-        self.query = Query(['"%s"' % model._table]) if query is None else query
+        self.query = Query(model.env.cr, model._table) if query is None else query
 
         # parse the domain expression
         self.parse()
@@ -692,9 +642,8 @@ class expression(object):
             elif field.inherited:
                 parent_model = model.env[field.related_field.model_name]
                 parent_fname = model._inherits[parent_model._name]
-                parent_alias, _ = self.query.add_join(
-                    (alias, parent_model._table, parent_fname, 'id', parent_fname),
-                    implicit=False, outer=True,
+                parent_alias = self.query.left_join(
+                    alias, parent_fname, parent_model._table, 'id', parent_fname,
                 )
                 push(leaf, parent_model, parent_alias)
 
@@ -719,9 +668,8 @@ class expression(object):
 
             elif len(path) > 1 and field.store and field.type == 'many2one' and field.auto_join:
                 # res_partner.state_id = res_partner__state_id.id
-                coalias, _ = self.query.add_join(
-                    (alias, comodel._table, path[0], 'id', path[0]),
-                    implicit=False, outer=True,
+                coalias = self.query.left_join(
+                    alias, path[0], comodel._table, 'id', path[0],
                 )
                 push((path[1], operator, right), comodel, coalias)
 
@@ -729,10 +677,7 @@ class expression(object):
                 # 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,
-                )
+                subquery, subparams = query.select('"%s"."%s"' % (comodel._table, field.inverse_name))
                 push(('id', 'inselect', (subquery, subparams)), model, alias, internal=True)
 
             elif len(path) > 1 and field.store and field.auto_join:
@@ -968,8 +913,7 @@ class expression(object):
 
         [self.result] = result_stack
         where_clause, where_params = self.result
-        self.query.where_clause.append(where_clause)
-        self.query.where_clause_params.extend(where_params)
+        self.query.add_where(where_clause, where_params)
 
     def __leaf_to_sql(self, leaf, model, alias):
         left, operator, right = leaf
diff --git a/odoo/osv/query.py b/odoo/osv/query.py
index 79b7a8424ff..18302fa1d70 100644
--- a/odoo/osv/query.py
+++ b/odoo/osv/query.py
@@ -2,6 +2,9 @@
 # Part of Odoo. See LICENSE file for full copyright and licensing details.
 
 import warnings
+from zlib import crc32
+
+from odoo.tools import lazy_property
 
 
 def _quote(to_quote):
@@ -10,177 +13,175 @@ def _quote(to_quote):
     return to_quote
 
 
-class Query(object):
+def _generate_table_alias(src_table_alias, link):
+    """ Generate a standard table alias name. An alias is generated as following:
+        - the base is the source table name (that can already be an alias)
+        - then, the joined table is added in the alias using a 'link field name'
+          that is used to render unique aliases for a given path
+        - the name is shortcut if it goes beyond PostgreSQL's identifier limits
+
+        Examples:
+        - src_table_alias='res_users', link='parent_id'
+            alias = 'res_users__parent_id'
+
+        :param str src_table_alias: alias of the source table
+        :param str link: field name
+        :return str: alias
     """
-     Dumb implementation of a Query object, using 3 string lists so far
-     for backwards compatibility with the (table, where_clause, where_params) previously used.
-
-     TODO: To be improved after v6.0 to rewrite part of the ORM and add support for:
-      - auto-generated multiple table aliases
-      - multiple joins to the same table with different conditions
-      - dynamic right-hand-side values in domains  (e.g. a.name = a.description)
-      - etc.
+    alias = "%s__%s" % (src_table_alias, link)
+    # Use an alternate alias scheme if length exceeds the PostgreSQL limit
+    # of 63 characters.
+    if len(alias) >= 64:
+        # We have to fit a crc32 hash and one underscore into a 63 character
+        # alias. The remaining space we can use to add a human readable prefix.
+        alias_hash = hex(crc32(alias.encode('utf-8')))[2:]
+        ALIAS_PREFIX_LENGTH = 63 - len(alias_hash) - 1
+        alias = "%s_%s" % (alias[:ALIAS_PREFIX_LENGTH], alias_hash)
+    return alias
+
+
+class Query(object):
+    """ Simple implementation of a query object, managing tables with aliases,
+    join clauses (with aliases, condition and parameters), where clauses (with
+    parameters), order, limit and offset.
     """
 
-    def __init__(self, tables=None, where_clause=None, where_clause_params=None, joins=None, extras=None):
+    def __init__(self, cr, table, where_clause=None, where_params=()):
+        # database cursor
+        self._cr = cr
 
-        # holds the list of tables joined using default JOIN.
-        # the table names are stored double-quoted (backwards compatibility)
-        self._tables = tables or []
+        # tables {alias: table}
+        self._tables = {table: table}
+
+        # joins {alias: (kind, table, condition, condition_params)}
+        self._joins = {}
 
         # holds the list of WHERE clause elements, to be joined with
         # 'AND' when generating the final query
-        self.where_clause = where_clause or []
+        self._where_clauses = [where_clause] if where_clause else []
 
         # holds the parameters for the formatting of `where_clause`, to be
         # passed to psycopg's execute method.
-        self.where_clause_params = where_clause_params or []
-
-        # holds table joins done explicitly, supporting outer joins. The JOIN
-        # condition should not be in `where_clause`. The dict is used as follows:
-        #   self.joins = {
-        #                    'table_a': [
-        #                                  ('table_b', 'table_a_col1', 'table_b_col', 'LEFT JOIN'),
-        #                                  ('table_c', 'table_a_col2', 'table_c_col', 'LEFT JOIN'),
-        #                                  ('table_d', 'table_a_col3', 'table_d_col', 'JOIN'),
-        #                               ]
-        #                 }
-        #   which should lead to the following SQL:
-        #       SELECT ... FROM "table_a" LEFT JOIN "table_b" ON ("table_a"."table_a_col1" = "table_b"."table_b_col")
-        #                                 LEFT JOIN "table_c" ON ("table_a"."table_a_col2" = "table_c"."table_c_col")
-        self.joins = joins or {}
-
-        # holds extra conditions for table joins that should not be in the where
-        # clause but in the join condition itself. The dict is used as follows:
-        #
-        #   self.extras = {
-        #       ('table_a', ('table_b', 'table_a_col1', 'table_b_col', 'LEFT JOIN')):
-        #           ('"table_b"."table_b_col3" = %s', [42])
-        #   }
-        #
-        # which should lead to the following SQL:
-        #
-        #   SELECT ... FROM "table_a"
-        #   LEFT JOIN "table_b" ON ("table_a"."table_a_col1" = "table_b"."table_b_col" AND "table_b"."table_b_col3" = 42)
-        #   ...
-        self.extras = extras or {}
-
-    def add_table(self, table):
-        table = _quote(table)
-        assert table not in self._tables, '%s already in %s' % (table, self)
-        self._tables.append(table)
+        self._where_params = list(where_params)
+
+        # order, limit, offset
+        self.order = None
+        self.limit = None
+        self.offset = None
+
+    def add_table(self, table, alias=None):
+        """ Add a table with a given alias to the from clause. """
+        alias = alias or table
+        assert alias not in self._tables and alias not in self._joins, "Alias %r already in %s" % (alias, str(self))
+        self._tables[alias] = table
+
+    def add_where(self, where_clause, where_params=()):
+        """ Add a condition to the where clause. """
+        self._where_clauses.append(where_clause)
+        self._where_params.extend(where_params)
+
+    def join(self, lhs_alias, lhs_column, rhs_table, rhs_column, link, extra=None, extra_params=()):
+        """ Add an INNER JOIN to the current table (if necessary), and return
+        the alias corresponding to ``rhs_table``.
+        """
+        return self._join('JOIN', lhs_alias, lhs_column, rhs_table, rhs_column, link, extra, extra_params)
 
-    @property
-    def tables(self):
-        warnings.warn("deprecated Query.tables, use Query.get_sql() instead",
-                      DeprecationWarning)
-        return self._tables
-
-    def _get_table_aliases(self):
-        from odoo.osv.expression import get_alias_from_query
-        return [get_alias_from_query(from_statement)[1] for from_statement in self._tables]
-
-    def _get_alias_mapping(self):
-        from odoo.osv.expression import get_alias_from_query
-        mapping = {}
-        for table in self._tables:
-            alias, statement = get_alias_from_query(table)
-            mapping[statement] = table
-        return mapping
-
-    def add_join(self, connection, implicit=True, outer=False, extra=None, extra_params=[]):
-        """ Join a destination table to the current table.
-
-            :param implicit: False if the join is an explicit join. This allows
-                to fall back on the previous implementation of ``join`` before
-                OpenERP 7.0. It therefore adds the JOIN specified in ``connection``
-                If True, the join is done implicitely, by adding the table alias
-                in the from clause and the join condition in the where clause
-                of the query. Implicit joins do not handle outer, extra, extra_params parameters.
-            :param connection: a tuple ``(lhs, table, lhs_col, col, link)``.
-                The join corresponds to the SQL equivalent of::
-
-                (lhs.lhs_col = table.col)
-
-                Note that all connection elements are strings. Please refer to expression.py for more details about joins.
-
-            :param outer: True if a LEFT OUTER JOIN should be used, if possible
-                      (no promotion to OUTER JOIN is supported in case the JOIN
-                      was already present in the query, as for the moment
-                      implicit INNER JOINs are only connected from NON-NULL
-                      columns so it would not be correct (e.g. for
-                      ``_inherits`` or when a domain criterion explicitly
-                      adds filtering)
-
-            :param extra: A string with the extra join condition (SQL), or None.
-                This is used to provide an additional condition to the join
-                clause that cannot be added in the where clause (e.g., for LEFT
-                JOIN concerns). The condition string should refer to the table
-                aliases as "{lhs}" and "{rhs}".
-
-            :param extra_params: a list of parameters for the `extra` condition.
+    def left_join(self, lhs_alias, lhs_column, rhs_table, rhs_column, link, extra=None, extra_params=()):
+        """ Add a LEFT JOIN to the current table (if necessary), and return the
+        alias corresponding to ``rhs_table``.
         """
-        from odoo.osv.expression import generate_table_alias
-        (lhs, table, lhs_col, col, link) = connection
-        alias, alias_statement = generate_table_alias(lhs, [(table, link)])
-
-        if implicit:
-            if alias_statement not in self._tables:
-                self._tables.append(alias_statement)
-                condition = '("%s"."%s" = "%s"."%s")' % (lhs, lhs_col, alias, col)
-                self.where_clause.append(condition)
-            else:
-                # already joined
-                pass
-            return alias, alias_statement
-        else:
-            aliases = self._get_table_aliases()
-            assert lhs in aliases, "Left-hand-side table %s must already be part of the query tables %s!" % (lhs, str(self._tables))
-            if alias_statement in self._tables:
-                # already joined, must ignore (promotion to outer and multiple joins not supported yet)
-                pass
+        return self._join('LEFT JOIN', lhs_alias, lhs_column, rhs_table, rhs_column, link, extra, extra_params)
+
+    def _join(self, kind, lhs_alias, lhs_column, rhs_table, rhs_column, link, extra=None, extra_params=()):
+        assert lhs_alias in self._tables or lhs_alias in self._joins, "Alias %r not in %s" % (lhs_alias, str(self))
+
+        rhs_alias = _generate_table_alias(lhs_alias, link)
+        assert rhs_alias not in self._tables, "Alias %r already in %s" % (rhs_alias, str(self))
+
+        if rhs_alias not in self._joins:
+            condition = f'"{lhs_alias}"."{lhs_column}" = "{rhs_alias}"."{rhs_column}"'
+            condition_params = []
+            if extra:
+                condition = condition + " AND " + extra.format(lhs=lhs_alias, rhs=rhs_alias)
+                condition_params = list(extra_params)
+            if kind:
+                self._joins[rhs_alias] = (kind, rhs_table, condition, condition_params)
             else:
-                # add JOIN
-                self._tables.append(alias_statement)
-                join_tuple = (alias, lhs_col, col, outer and 'LEFT JOIN' or 'JOIN')
-                self.joins.setdefault(lhs, []).append(join_tuple)
-                if extra or extra_params:
-                    extra = (extra or '').format(lhs=lhs, rhs=alias)
-                    self.extras[(lhs, join_tuple)] = (extra, extra_params)
-            return alias, alias_statement
+                self._tables[rhs_alias] = rhs_table
+                self.add_where(condition, condition_params)
+
+        return rhs_alias
+
+    def select(self, *args):
+        """ Return the SELECT query as a pair ``(query_string, query_params)``. """
+        from_clause, where_clause, params = self.get_sql()
+        query_str = 'SELECT {} FROM {} WHERE {}{}{}{}'.format(
+            ", ".join(args or [_quote(next(iter(self._tables))) + ".id"]),
+            from_clause,
+            where_clause or "TRUE",
+            (" ORDER BY %s" % self.order) if self.order else "",
+            (" LIMIT %d" % self.limit) if self.limit else "",
+            (" OFFSET %d" % self.offset) if self.offset else "",
+        )
+        return query_str, params
 
     def get_sql(self):
         """ Returns (query_from, query_where, query_params). """
-        from odoo.osv.expression import get_alias_from_query
-        tables_to_process = list(self._tables)
-        alias_mapping = self._get_alias_mapping()
-        from_clause = []
-        from_params = []
-
-        def add_joins_for_table(lhs):
-            for (rhs, lhs_col, rhs_col, join) in self.joins.get(lhs, []):
-                tables_to_process.remove(alias_mapping[rhs])
-                from_clause.append(' %s %s ON ("%s"."%s" = "%s"."%s"' % \
-                    (join, alias_mapping[rhs], lhs, lhs_col, rhs, rhs_col))
-                extra = self.extras.get((lhs, (rhs, lhs_col, rhs_col, join)))
-                if extra:
-                    if extra[0]:
-                        from_clause.append(' AND ')
-                        from_clause.append(extra[0])
-                    if extra[1]:
-                        from_params.extend(extra[1])
-                from_clause.append(')')
-                add_joins_for_table(rhs)
-
-        for pos, table in enumerate(tables_to_process):
-            if pos > 0:
-                from_clause.append(',')
-            from_clause.append(table)
-            table_alias = get_alias_from_query(table)[1]
-            if table_alias in self.joins:
-                add_joins_for_table(table_alias)
-
-        return "".join(from_clause), " AND ".join(self.where_clause), from_params + self.where_clause_params
+        tables = []
+        joins = []
+        params = []
+        for alias, table in self._tables.items():
+            tables.append(f'"{table}"' if alias == table else f'"{table}" AS "{alias}"')
+        for alias, (kind, table, condition, condition_params) in self._joins.items():
+            joins.append(f'{kind} "{table}" AS "{alias}" ON ({condition})')
+            params.extend(condition_params)
+
+        from_clause = " ".join([", ".join(tables)] + joins)
+        where_clause = " AND ".join(self._where_clauses)
+        return from_clause, where_clause, params + self._where_params
+
+    @lazy_property
+    def _result(self):
+        query_str, params = self.select()
+        self._cr.execute(query_str, params)
+        return [row[0] for row in self._cr.fetchall()]
 
     def __str__(self):
-        return '<osv.Query: "SELECT ... FROM %s WHERE %s" with params: %r>' % self.get_sql()
+        return '<osv.Query: %r with params: %r>' % self.select()
+
+    def __bool__(self):
+        return bool(self._result)
+
+    def __len__(self):
+        return len(self._result)
+
+    def __iter__(self):
+        return iter(self._result)
+
+    #
+    # deprecated attributes and methods
+    #
+    @property
+    def tables(self):
+        warnings.warn("deprecated Query.tables, use Query.get_sql() instead",
+                      DeprecationWarning)
+        return tuple(
+            f'"{table}"' if alias == table else f'"{table}" AS "{alias}"'
+            for alias, table in self._tables
+        )
+
+    @property
+    def where_clause(self):
+        return tuple(self._where_clauses)
+
+    @property
+    def where_clause_params(self):
+        return tuple(self._where_params)
+
+    def add_join(self, connection, implicit=True, outer=False, extra=None, extra_params=()):
+        warnings.warn("deprecated Query.add_join, use Query.join() or Query.left_join() instead",
+                      DeprecationWarning)
+        lhs_alias, rhs_table, lhs_column, rhs_column, link = connection
+        kind = '' if implicit else ('LEFT JOIN' if outer else 'JOIN')
+        rhs_alias = self._join(kind, lhs_alias, lhs_column, rhs_table, rhs_column, link, extra, extra_params)
+        return rhs_alias, f'"{rhs_table}" AS "{rhs_alias}"'