diff --git a/addons/account/models/account_move_line.py b/addons/account/models/account_move_line.py index 4ffe25c6a23..1ec61c3c954 100644 --- a/addons/account/models/account_move_line.py +++ b/addons/account/models/account_move_line.py @@ -686,7 +686,8 @@ class AccountMoveLine(models.Model): # get the where clause query = self._where_calc(list(self.env.context.get('domain_cumulated_balance') or [])) - order_string = ", ".join(self._generate_order_by_inner(self._table, self.env.context.get('order_cumulated_balance'), query, reverse_direction=True)) + sql_order = self._order_to_sql(self.env.context.get('order_cumulated_balance'), query, reverse=True) + order_string = self.env.cr.mogrify(sql_order).decode() from_clause, where_clause, where_clause_params = query.get_sql() sql = """ SELECT account_move_line.id, SUM(account_move_line.balance) OVER ( diff --git a/addons/project/models/project_project.py b/addons/project/models/project_project.py index 5d2b15366c6..24f728700d4 100644 --- a/addons/project/models/project_project.py +++ b/addons/project/models/project_project.py @@ -8,11 +8,12 @@ from datetime import timedelta from odoo import api, Command, fields, models, _, _lt from odoo.addons.rating.models import rating_data -from odoo.tools.misc import get_lang from odoo.exceptions import UserError +from odoo.tools import get_lang, SQL from .project_update import STATUS_COLOR from .project_task import CLOSED_STATES + class Project(models.Model): _name = "project.project" _description = "Project" @@ -552,29 +553,15 @@ class Project(models.Model): analytic_accounts_to_delete.unlink() return result - @api.model - def _search(self, domain, offset=0, limit=None, order=None, access_rights_uid=None): - if not order: - return super()._search(domain, offset, limit, order, access_rights_uid) - new_order, item_index, desc = [], -1, False - for index, order_item in enumerate(order.split(',')): - order_item_list = order_item.strip().lower().split(' ') - if order_item_list[0] == 'is_favorite': - item_index = index - desc = order_item_list[-1] == 'desc' - else: - new_order.append(order_item) - query = super()._search(domain, offset, limit, ', '.join(new_order), access_rights_uid) - if item_index != -1: - query_order_list = query.order.split(',') if query.order else [] - query_order_list.insert(item_index, f""" - "project_project"."id" IN ( - SELECT project_id - FROM project_favorite_user_rel - WHERE user_id = {self.env.uid} - ){" DESC" * desc}""") - query.order = ', '.join(query_order_list) - return query + def _order_field_to_sql(self, alias, field_name, direction, nulls, query): + if field_name == 'is_favorite': + sql_field = SQL( + "%s IN (SELECT project_id FROM project_favorite_user_rel WHERE user_id = %s)", + SQL.identifier(alias, 'id'), self.env.uid, + ) + return SQL("%s %s %s", sql_field, direction, nulls) + + return super()._order_field_to_sql(alias, field_name, direction, nulls, query) def message_subscribe(self, partner_ids=None, subtype_ids=None): """ diff --git a/addons/website_forum/controllers/website_forum.py b/addons/website_forum/controllers/website_forum.py index 0ce1ff935ed..2b377bc0516 100644 --- a/addons/website_forum/controllers/website_forum.py +++ b/addons/website_forum/controllers/website_forum.py @@ -123,7 +123,7 @@ class WebsiteForum(WebsiteProfile): # retro-compatibility for V8 and google links try: sorting = werkzeug.urls.url_unquote_plus(sorting) - Post._generate_order_by(sorting, None) + Post._order_to_sql(sorting, None) except (UserError, ValueError): sorting = False diff --git a/odoo/addons/base/tests/test_expression.py b/odoo/addons/base/tests/test_expression.py index b8511d4bfd7..3c6536d3c09 100644 --- a/odoo/addons/base/tests/test_expression.py +++ b/odoo/addons/base/tests/test_expression.py @@ -1281,7 +1281,7 @@ class TestQueries(TransactionCase): SELECT "res_partner_title"."id" FROM "res_partner_title" WHERE (COALESCE("res_partner_title"."name"->>%s, "res_partner_title"."name"->>'en_US') like %s) - ORDER BY COALESCE("res_partner_title"."name"->>'fr_FR', "res_partner_title"."name"->>'en_US') + ORDER BY COALESCE("res_partner_title"."name"->>%s, "res_partner_title"."name"->>'en_US') ''']): Model.search([('name', 'like', 'foo')]) diff --git a/odoo/fields.py b/odoo/fields.py index b421b2984b1..9a3e7dfb46a 100644 --- a/odoo/fields.py +++ b/odoo/fields.py @@ -4766,7 +4766,7 @@ class Many2many(_RelationalMulti): comodel._flush_search(domain, order=comodel._order) query = comodel._where_calc(domain) comodel._apply_ir_rules(query, 'read') - query.order = comodel._generate_order_by(comodel._order, query).replace(' ORDER BY ', '') + query.order = comodel._order_to_sql(comodel._order, query) # join with many2many relation table sql_id1 = SQL.identifier(self.relation, self.column1) diff --git a/odoo/models.py b/odoo/models.py index 739feb9ae7c..f747955df0a 100644 --- a/odoo/models.py +++ b/odoo/models.py @@ -5103,98 +5103,122 @@ class BaseModel(metaclass=MetaModel): if domain: expression.expression(domain, self.sudo(), self._table, query) - @api.model - def _generate_m2o_order_by(self, alias, order_field, query, reverse_direction, seen): + def _order_to_sql(self, order: str, query: Query, alias: (str | None) = None, + reverse: bool = False) -> SQL: + """ Return an :class:`SQL` object that represents the given ORDER BY + clause, without the ORDER BY keyword. """ - Add possibly missing JOIN to ``query`` and generate the ORDER BY clause for m2o fields, - either native m2o fields or function/related fields that are stored, including - intermediate JOINs for inheritance if required. - - :return: the qualified field name to use in an ORDER BY clause to sort by ``order_field`` - """ - field = self._fields[order_field] - if field.inherited: - # also add missing joins for reaching the table containing the m2o field - qualified_field = self._inherits_join_calc(alias, order_field, query) - alias, order_field = qualified_field.replace('"', '').split('.', 1) - field = field.base_field - - assert field.type == 'many2one', 'Invalid field passed to _generate_m2o_order_by()' - if not field.store: - _logger.debug("Many2one function/related fields must be stored " - "to be used as ordering fields! Ignoring sorting for %s.%s", - self._name, order_field) + order = order or self._order + if not order: return [] + self._check_qorder(order) - # figure out the applicable order_by for the m2o - dest_model = self.env[field.comodel_name] - m2o_order = dest_model._order - if not regex_order.match(m2o_order): - # _order is complex, can't use it here, so we default to _rec_name - m2o_order = dest_model._rec_name + alias = alias or self._table - # Join the dest m2o table if it's not joined yet. We use [LEFT] OUTER join here - # as we don't want to exclude results that have NULL values for the m2o - dest_alias = query.left_join(alias, order_field, dest_model._table, 'id', order_field) - return dest_model._generate_order_by_inner(dest_alias, m2o_order, query, - reverse_direction, seen) - - @api.model - def _generate_order_by_inner(self, alias, order_spec, query, reverse_direction=False, seen=None): - if seen is None: - seen = set() - self._check_qorder(order_spec) - - order_by_elements = [] - for order_part in order_spec.split(','): + terms = [] + for order_part in order.split(','): order_match = regex_order.match(order_part) - order_field = order_match['field'] - order_direction = (order_match['direction'] or '').upper() - order_nulls = (order_match['nulls'] or '').upper() - if reverse_direction: - order_direction = 'ASC' if order_direction == 'DESC' else 'DESC' - if order_nulls: - order_nulls = 'NULLS LAST' if order_nulls == 'NULLS FIRST' else 'NULLS FIRST' - do_reverse = order_direction == 'DESC' - - field = self._fields.get(order_field) - if not field: - raise ValueError("Invalid field %r on model %r" % (order_field, self._name)) + field_name = order_match['field'] property_name = order_match['property'] if property_name: - check_property_field_value_name(property_name) - if field.type != 'properties': - raise ValueError(f'Order a property ({property_name!r}) on a non-properties field ({field.name!r})') + field_name = f"{field_name}.{property_name}" - if order_field == 'id': - order_by_elements.append(f'"{alias}"."{order_field}" {order_direction} {order_nulls}') - else: - if field.inherited: - field = field.base_field - if field.store and field.type == 'many2one': - key = (field.model_name, field.comodel_name, order_field) - if order_nulls: - qname = self._inherits_join_calc(alias, order_field, query) - if order_nulls == 'NULLS LAST': - order_by_elements.append(f"{qname} IS NULL") - elif order_nulls == 'NULLS FIRST': - order_by_elements.append(f"{qname} IS NOT NULL") - if key not in seen: - seen.add(key) - order_by_elements += self._generate_m2o_order_by(alias, order_field, query, do_reverse, seen) - elif field.store and field.column_type: - qualifield_name = self._inherits_join_calc(alias, order_field, query) - if field.type == 'boolean': - qualifield_name = "COALESCE(%s, false)" % qualifield_name - elif field.type == 'properties' and property_name: - qualifield_name = f"({qualifield_name} -> '{property_name}')" - order_by_elements.append(f"{qualifield_name} {order_direction} {order_nulls}") - else: - _logger.warning("Model %r cannot be sorted on field %r (not a column)", self._name, order_field) - continue # ignore non-readable or "non-joinable" fields + direction = (order_match['direction'] or '').upper() + nulls = (order_match['nulls'] or '').upper() + if reverse: + direction = 'ASC' if direction == 'DESC' else 'DESC' + if nulls: + nulls = 'NULLS LAST' if nulls == 'NULLS FIRST' else 'NULLS FIRST' - return order_by_elements + sql_direction = SQL(direction) if direction in ('ASC', 'DESC') else SQL() + sql_nulls = SQL(nulls) if nulls in ('NULLS FIRST', 'NULLS LAST') else SQL() + + term = self._order_field_to_sql(alias, field_name, sql_direction, sql_nulls, query) + if term: + terms.append(term) + + return SQL(", ").join(terms) + + def _order_field_to_sql(self, alias: str, field_name: str, direction: SQL, + nulls: SQL, query: Query) -> SQL: + """ Return an :class:`SQL` object that represents the ordering by the + given field. + + :param direction: one of ``SQL("ASC")``, ``SQL("DESC")``, ``SQL()`` + :param nulls: one of ``SQL("NULLS FIRST")``, ``SQL("NULLS LAST")``, ``SQL()`` + """ + full_name = field_name + property_name = None + if '.' in field_name: + field_name, property_name = field_name.split('.', 1) + + field = self._fields.get(field_name) + if not field: + raise ValueError(f"Invalid field {field_name!r} on model {self._name!r}") + + if property_name and field.type != 'properties': + raise ValueError(f'Order a property ({property_name!r}) on a non-properties field ({field_name!r})') + + if field.inherited: + # delegate to the parent model via a join + parent_model = self.env[field.related_field.model_name] + parent_fname = field.related.split('.')[0] + parent_alias = query.make_alias(alias, parent_fname) + query.add_join('LEFT JOIN', parent_alias, parent_model._table, SQL( + "%s = %s", + self._field_to_sql(alias, parent_fname, query), + SQL.identifier(parent_alias, 'id'), + )) + return parent_model._order_field_to_sql(parent_alias, full_name, direction, nulls, query) + + if not (field.store and field.column_type): + _logger.warning("Model %r cannot be sorted on field %r (not a column)", self._name, field_name) + return + + if field.type == 'many2one': + seen = self.env.context.get('__m2o_order_seen', ()) + if field in seen: + return + self = self.with_context(__m2o_order_seen=frozenset((field, *seen))) + + # instead of ordering by the field's raw value, use the comodel's + # order on many2one values + terms = [] + if nulls.code == 'NULLS FIRST': + terms.append(SQL("%s IS NOT NULL", self._field_to_sql(alias, field_name, query))) + elif nulls.code == 'NULLS LAST': + terms.append(SQL("%s IS NULL", self._field_to_sql(alias, field_name, query))) + + # LEFT JOIN the comodel table, in order to include NULL values, too + comodel = self.env[field.comodel_name] + coalias = query.make_alias(alias, field_name) + query.add_join('LEFT JOIN', coalias, comodel._table, SQL( + "%s = %s", + self._field_to_sql(alias, field_name, query), + SQL.identifier(coalias, 'id'), + )) + + # figure out the applicable order_by for the m2o + coorder = comodel._order + if not regex_order.match(coorder): + # _order is complex, can't use it here, so we default to _rec_name + coorder = comodel._rec_name + + # delegate the order to the comodel + reverse = direction.code == 'DESC' + term = comodel._order_to_sql(coorder, query, alias=coalias, reverse=reverse) + if term: + terms.append(term) + return SQL(", ").join(terms) + + sql_field = self._field_to_sql(alias, field_name, query) + if field.type == 'boolean': + sql_field = SQL("COALESCE(%s, FALSE)", sql_field) + elif field.type == 'properties' and property_name: + sql_field = SQL("(%s -> %s)", sql_field, property_name) + + return SQL("%s %s %s", sql_field, direction, nulls) @api.model def _generate_order_by(self, order_spec, query): @@ -5203,14 +5227,13 @@ class BaseModel(metaclass=MetaModel): a comma-separated list of valid field names, optionally followed by an ASC or DESC direction. :raise ValueError in case order_spec is malformed - """ - order_by_clause = '' - order_spec = order_spec or self._order - if order_spec: - order_by_elements = self._generate_order_by_inner(self._table, order_spec, query) - if order_by_elements: - order_by_clause = ",".join(order_by_elements) + .. deprecated:: 17.0 + Deprecated method, use _order_to_sql() instead + """ + warnings.warn("Deprecated method _generate_order_by(), _order_to_sql() instead", DeprecationWarning, 2) + sql = self._order_to_sql(order_spec, query) + order_by_clause = self.env.cr.mogrify(sql).decode() return order_by_clause and (' ORDER BY %s ' % order_by_clause) or '' @api.model @@ -5339,7 +5362,7 @@ class BaseModel(metaclass=MetaModel): self._apply_ir_rules(query, 'read') if order: - query.order = self._generate_order_by(order, query).replace('ORDER BY ', '') + query.order = self._order_to_sql(order, query) query.limit = limit query.offset = offset diff --git a/odoo/tools/query.py b/odoo/tools/query.py index f859975b68e..0a2ccb3d55e 100644 --- a/odoo/tools/query.py +++ b/odoo/tools/query.py @@ -73,7 +73,7 @@ class Query(object): self._where_clauses = [] # order, limit, offset - self.order = None + self._order = None self.limit = None self.offset = None @@ -140,6 +140,14 @@ class Query(object): self.add_join('LEFT JOIN', rhs_alias, rhs_table, condition) return rhs_alias + @property + def order(self) -> SQL | None: + return self._order + + @order.setter + def order(self, value: SQL | str | None): + self._order = SQL(value) if value is not None else None + @property def table(self) -> str: """ Return the query's main table, i.e., the first one in the FROM clause. """ @@ -176,7 +184,7 @@ class Query(object): SQL("SELECT %s", SQL(", ").join(sql_args)), SQL(" FROM %s", self.from_clause), SQL(" WHERE %s", self.where_clause) if self._where_clauses else SQL(), - SQL(" ORDER BY %s", SQL(self.order)) if self.order else SQL(), + SQL(" ORDER BY %s", self._order) if self._order else SQL(), SQL(" LIMIT %s", self.limit) if self.limit else SQL(), SQL(" OFFSET %s", self.offset) if self.offset else SQL(), )