From f3eb6f395a8fe5abb8b976c0f09c2b7faeeaec65 Mon Sep 17 00:00:00 2001 From: Alvaro Fuentes Date: Fri, 9 Feb 2024 08:17:40 +0100 Subject: [PATCH] [FIX] core: ensure SQL formatter can handle large domains In previous versions the max size of a domain was bounded by psycopg memory limits. With the new SQL formatting mechanism the limit is bound by the maximum recursion limit in Python side. The purpose of this patch is to restore previous behavior. In 16.0: ``` >>> def make_dom(N): ... return [*('|' for x in range(N-1)), *(('login', '=', 'admin') for x in range(N))] ... >>> u.search(make_dom(9984)) res.users(2,) >>> u.search(make_dom(9985)) Traceback (most recent call last): File "", line 1, in u.search(make_dom(9985)) File "/home/odoo/src/odoo/16.0/odoo/models.py", line 1520, in search return res if count else self.browse(res) File "/home/odoo/src/odoo/16.0/odoo/models.py", line 5140, in browse if not ids: File "/home/odoo/src/odoo/16.0/odoo/tools/query.py", line 217, in __bool__ return bool(self._result) File "/home/odoo/src/odoo/16.0/odoo/tools/func.py", line 28, in __get__ value = self.fget(obj) File "/home/odoo/src/odoo/16.0/odoo/tools/query.py", line 210, in _result self._cr.execute(query_str, params) File "/home/odoo/src/odoo/16.0/odoo/sql_db.py", line 321, in execute res = self._obj.execute(query, params) psycopg2.errors.SyntaxError: memory exhausted at or near ""login"" LINE 1: ...((("res_users"."login" = 'admin') OR ("res_users"."login" = ... ``` in 17.0 without this patch ``` >>> u.search(make_dom(1480)) res.users(2,) >>> u.search(make_dom(1481)) File "/home/odoo/src/odoo/17.0/odoo/tools/sql.py", line 85, in code child = stack[-1].send(child) File "/home/odoo/src/odoo/17.0/odoo/tools/sql.py", line 86, in if isinstance(child, SQL): File "/home/odoo/src/odoo/17.0/odoo/tools/sql.py", line 85, in code child = stack[-1].send(child) File "/home/odoo/src/odoo/17.0/odoo/tools/sql.py", line 86, in if isinstance(child, SQL): File "/home/odoo/src/odoo/17.0/odoo/tools/sql.py", line 85, in code child = stack[-1].send(child) File "/home/odoo/src/odoo/17.0/odoo/tools/sql.py", line 86, in if isinstance(child, SQL): File "/home/odoo/src/odoo/17.0/odoo/tools/sql.py", line 85, in code child = stack[-1].send(child) RecursionError: maximum recursion depth exceeded ``` This issue was observed in upgrades in multiple instances. Example: MRP produces an OR domain with 2K terms for warehouse sub-locations that fail. closes odoo/odoo#153394 Signed-off-by: Raphael Collet --- odoo/addons/base/tests/test_search.py | 6 ++++++ odoo/tools/sql.py | 30 ++++++++++++++++++--------- 2 files changed, 26 insertions(+), 10 deletions(-) diff --git a/odoo/addons/base/tests/test_search.py b/odoo/addons/base/tests/test_search.py index 758176376ac..3524dea07e3 100644 --- a/odoo/addons/base/tests/test_search.py +++ b/odoo/addons/base/tests/test_search.py @@ -278,3 +278,9 @@ class test_search(TransactionCase): ]) self.assertEqual(len(partners) + count_partner_before, Partner.search_count([])) self.assertEqual(3, Partner.search_count([], limit=3)) + + def test_22_large_domain(self): + """ Ensure search and its unerlying SQL mechanism is able to handle large domains""" + N = 9500 + domain = ['|'] * (N - 1) + [('login', '=', 'admin')] * N + self.env['res.users'].search(domain) diff --git a/odoo/tools/sql.py b/odoo/tools/sql.py index 0403e315601..9011f957f01 100644 --- a/odoo/tools/sql.py +++ b/odoo/tools/sql.py @@ -82,21 +82,31 @@ class SQL: @property def code(self) -> str: """ Return the combined SQL code string. """ - return self.__code % tuple( - arg.code if isinstance(arg, SQL) else "%s" - for arg in self.__args - ) if self.__args else self.__code + stack = [] # stack of intermediate results + for node in self.__postfix(): + if not isinstance(node, SQL): + stack.append("%s") + elif arity := len(node.__args): + stack[-arity:] = [node.__code % tuple(stack[-arity:])] + else: + stack.append(node.__code) + return stack[0] @property def params(self) -> list: """ Return the combined SQL code params as a list of values. """ - result = [] - for arg in self.__args: - if isinstance(arg, SQL): - result.extend(arg.params) + return [node for node in self.__postfix() if not isinstance(node, SQL)] + + def __postfix(self): + """ Return a postfix iterator for the SQL tree ``self``. """ + stack = [(self, False)] + while stack: + node, ispostfix = stack.pop() + if ispostfix or not isinstance(node, SQL): + yield node else: - result.append(arg) - return result + stack.append((node, True)) + stack.extend((arg, False) for arg in reversed(node.__args)) def __repr__(self): return f"SQL({', '.join(map(repr, [self.code, *self.params]))})"