From 340fe279b3dd3396636bd558d4063f1bc735170a Mon Sep 17 00:00:00 2001 From: Akhil Narang Date: Thu, 25 Sep 2025 18:34:24 +0530 Subject: [PATCH 01/38] feat: add in initial version of DatabaseQuery using query builder Signed-off-by: Akhil Narang --- frappe/database/query.py | 205 +++++++++++++++++++++------ frappe/database/utils.py | 4 + frappe/model/qb_query.py | 259 ++++++++++++++++++++++++++++++++++ frappe/tests/test_query.py | 278 ++++++++++++++++++++++++++++++++++++- 4 files changed, 702 insertions(+), 44 deletions(-) create mode 100644 frappe/model/qb_query.py diff --git a/frappe/database/query.py b/frappe/database/query.py index 41e3e9cd05..555746f185 100644 --- a/frappe/database/query.py +++ b/frappe/database/query.py @@ -3,8 +3,9 @@ from functools import lru_cache from typing import TYPE_CHECKING, Any import sqlparse -from pypika.queries import QueryBuilder, Table -from pypika.terms import AggregateFunction, Term +from pypika.enums import Arithmetic +from pypika.queries import Column, QueryBuilder, Table +from pypika.terms import AggregateFunction, ArithmeticExpression, Term, ValueWrapper import frappe from frappe import _ @@ -53,6 +54,17 @@ FUNCTION_MAPPING = { "NOW": functions.Now, } +# Mapping from operator names to pypika Arithmetic enum values +# Operators use dict format: {"ADD": [left, right], "as": "alias"} +# Supported: ADD (+), SUB (-), MUL (*), DIV (/) +# Can be nested with functions: {"DIV": [1, {"NULLIF": ["value", 0]}]} +OPERATOR_MAPPING = { + "ADD": Arithmetic.add, + "SUB": Arithmetic.sub, + "MUL": Arithmetic.mul, + "DIV": Arithmetic.div, +} + class Engine: def get_query( @@ -60,6 +72,7 @@ class Engine: table: str | Table, fields: str | list | tuple | None = None, filters: dict[str, FilterValue] | FilterValue | list[list | FilterValue] | None = None, + or_filters: dict[str, FilterValue] | FilterValue | list[list | FilterValue] | None = None, order_by: str | None = None, group_by: str | None = None, limit: int | None = None, @@ -76,6 +89,7 @@ class Engine: ignore_permissions: bool = True, user: str | None = None, parent_doctype: str | None = None, + reference_doctype: str | None = None, ) -> QueryBuilder: qb = frappe.local.qb db_type = frappe.local.db.db_type @@ -86,6 +100,7 @@ class Engine: self.validate_filters = validate_filters self.user = user or frappe.session.user self.parent_doctype = parent_doctype + self.reference_doctype = reference_doctype self.apply_permissions = not ignore_permissions if isinstance(table, Table): @@ -110,6 +125,7 @@ class Engine: self.apply_fields(fields) self.apply_filters(filters) + self.apply_or_filters(or_filters) if limit: if not isinstance(limit, int) or limit < 0: @@ -163,6 +179,7 @@ class Engine: def apply_filters( self, filters: dict[str, FilterValue] | FilterValue | list[list | FilterValue] | None = None, + collect: list | None = None, ): if filters is None: return @@ -175,7 +192,7 @@ class Engine: return if isinstance(filters, dict): - self.apply_dict_filters(filters) + self.apply_dict_filters(filters, collect=collect) return if isinstance(filters, list | tuple): @@ -184,7 +201,9 @@ class Engine: # 1. Handle special case: list of names -> name IN (...) if all(isinstance(d, FilterValue) for d in filters): - self.apply_dict_filters({"name": ("in", tuple(convert_to_value(f) for f in filters))}) + self.apply_dict_filters( + {"name": ("in", tuple(convert_to_value(f) for f in filters))}, collect=collect + ) return # 2. Check for nested logic format [cond, op, cond, ...] or [[cond, op, cond]] @@ -233,9 +252,11 @@ class Engine: else: # Not a nested structure, assume it's a list of simple filters (implicitly ANDed) for filter_item in filters: if isinstance(filter_item, list | tuple): - self.apply_list_filters(filter_item) # Handles simple [field, op, value] lists + self.apply_list_filters( + filter_item, collect=collect + ) # Handles simple [field, op, value] lists elif isinstance(filter_item, dict | Criterion): - self.apply_filters(filter_item) # Recursive call for dict/criterion + self.apply_filters(filter_item, collect=collect) # Recursive call for dict/criterion else: # Disallow single values (strings, numbers, etc.) directly in the list # unless it's the name IN (...) case handled above. @@ -247,26 +268,53 @@ class Engine: # If filters type is none of the above raise ValueError(f"Unsupported filters type: {type(filters).__name__}") - def apply_list_filters(self, filter: list): + def apply_or_filters( + self, + or_filters: dict[str, FilterValue] | FilterValue | list[list | FilterValue] | None = None, + ): + """Apply OR filters - all conditions are combined with OR operator. + + Example: + or_filters={"name": "User", "module": "Core"} + → Collects: [Criterion(name='User'), Criterion(module='Core')] + → Combines: Criterion(name='User') | Criterion(module='Core') + → Result: WHERE name='User' OR module='Core' + """ + if or_filters is None: + return + + # Collect criteria instead of applying immediately + criteria = [] + self.apply_filters(or_filters, collect=criteria) + + # Combine all criteria with OR operator (|) + if criteria: + from functools import reduce + + # Reduce combines: [Criterion(name='User'), Criterion(module='Core')] → Criterion(name='User') | Criterion(module='Core') + combined = reduce(lambda a, b: a | b, criteria) + self.query = self.query.where(combined) + + def apply_list_filters(self, filter: list, collect: list | None = None): if len(filter) == 2: field, value = filter - self._apply_filter(field, value) + self._apply_filter(field, value, collect=collect) elif len(filter) == 3: field, operator, value = filter - self._apply_filter(field, value, operator) + self._apply_filter(field, value, operator, collect=collect) elif len(filter) == 4: doctype, field, operator, value = filter - self._apply_filter(field, value, operator, doctype) + self._apply_filter(field, value, operator, doctype, collect=collect) else: raise ValueError(f"Unknown filter format: {filter}") - def apply_dict_filters(self, filters: dict[str, FilterValue | list]): + def apply_dict_filters(self, filters: dict[str, FilterValue | list], collect: list | None = None): for field, value in filters.items(): operator = "=" if isinstance(value, list | tuple): operator, value = value - self._apply_filter(field, value, operator) + self._apply_filter(field, value, operator, collect=collect) def _apply_filter( self, @@ -274,16 +322,20 @@ class Engine: value: FilterValue | list | set | None, operator: str = "=", doctype: str | None = None, + collect: list | None = None, ): """Applies a simple filter condition to the query.""" criterion = self._build_criterion_for_simple_filter(field, value, operator, doctype) if criterion: - self.query = self.query.where(criterion) + if collect is not None: + collect.append(criterion) + else: + self.query = self.query.where(criterion) def _build_criterion_for_simple_filter( self, field: str | Field, - value: FilterValue | list | set | None, + value: FilterValue | Column | list | set | None, operator: str = "=", doctype: str | None = None, ) -> "Criterion | None": @@ -291,7 +343,12 @@ class Engine: import operator as builtin_operator _field = self._validate_and_prepare_filter_field(field, doctype) - _value = convert_to_value(value) + + if isinstance(value, Column): + _value = self._validate_and_prepare_filter_field(value.name, doctype) + else: + # Regular value processing for literal comparisons like: table.field = 'value' + _value = convert_to_value(value) _operator = operator if not _value and isinstance(_value, list | tuple | set): @@ -641,18 +698,20 @@ class Engine: if isinstance(field, Criterion | Field): return field elif isinstance(field, dict): - # Check if it's a SQL function dictionary + # Check if it's a SQL function or operator dictionary function_parser = SQLFunctionParser(engine=self) if function_parser.is_function_dict(field): return function_parser.parse_function(field) + elif function_parser.is_operator_dict(field): + return function_parser.parse_operator(field) else: # Handle child queries defined as dicts {fieldname: [child_fields]} _parsed_fields = [] for child_field, child_fields_list in field.items(): - # Skip uppercase keys as they might be unsupported SQL functions + # Skip uppercase keys as they might be unsupported SQL functions or operators if child_field.isupper(): frappe.throw( - _("Unsupported function or invalid field name: {0}").format(child_field), + _("Unsupported function or operator: {0}").format(child_field), frappe.ValidationError, ) @@ -1368,37 +1427,45 @@ class SQLFunctionParser: function_keys = [k for k in field_dict.keys() if k != "as"] return len(function_keys) == 1 and function_keys[0] in FUNCTION_MAPPING - def parse_function(self, function_dict: dict) -> Field: - """Parse a SQL function dictionary into a pypika function call.""" - function_name = None - alias = None - function_args = None + def is_operator_dict(self, field_dict: dict) -> bool: + """Check if a dictionary represents an arithmetic operator expression. - for key, value in function_dict.items(): + Example: {"ADD": [1, 2], "as": "sum"} or {"DIV": ["total", "count"]} + """ + operator_keys = [k for k in field_dict.keys() if k != "as"] + return len(operator_keys) == 1 and operator_keys[0] in OPERATOR_MAPPING + + def _extract_dict_components(self, d: dict, valid_keys: dict, error_msg: str) -> tuple: + """Extract name, alias, and args from function/operator dict.""" + name = None + alias = None + args = None + + for key, value in d.items(): if key == "as": alias = value else: - function_name = key - function_args = value + name = key + args = value - if not function_name: - frappe.throw(_("Invalid function dictionary format"), frappe.ValidationError) + if not name: + frappe.throw(_("Invalid {0} dictionary format").format(error_msg), frappe.ValidationError) - if function_name not in FUNCTION_MAPPING: - frappe.throw( - _("Unsupported function or invalid field name: {0}").format(function_name), - frappe.ValidationError, - ) + if name not in valid_keys: + frappe.throw(_("Unsupported {0}: {1}").format(error_msg, name), frappe.ValidationError) if alias: self._validate_alias(alias) - func_class = FUNCTION_MAPPING.get(function_name) - if not func_class: - frappe.throw( - _("Unsupported function or invalid field name: {0}").format(function_name), - frappe.ValidationError, - ) + return name, alias, args + + def parse_function(self, function_dict: dict) -> Field: + """Parse a SQL function dictionary into a pypika function call.""" + function_name, alias, function_args = self._extract_dict_components( + function_dict, FUNCTION_MAPPING, "function or invalid field name" + ) + + func_class = FUNCTION_MAPPING[function_name] if isinstance(function_args, str): parsed_arg = self._parse_and_validate_argument(function_args) @@ -1432,18 +1499,74 @@ class SQLFunctionParser: else: return function_call + def parse_operator(self, operator_dict: dict) -> ArithmeticExpression: + """Parse an arithmetic operator dictionary into a pypika ArithmeticExpression. + + Operators require exactly 2 arguments (left and right operands). + Arguments can be: numbers, field names, nested functions, or nested operators. + Example: {"DIV": [1, {"NULLIF": [{"LOCATE": ["'test'", "name"]}, 0]}]} + """ + operator_name, alias, operator_args = self._extract_dict_components( + operator_dict, OPERATOR_MAPPING, "operator" + ) + + operator = OPERATOR_MAPPING[operator_name] + + # Operators require exactly 2 arguments (left and right operands) + if not isinstance(operator_args, list) or len(operator_args) != 2: + frappe.throw( + _("Operator {0} requires exactly 2 arguments (left and right operands)").format( + operator_name + ), + frappe.ValidationError, + ) + + # Parse and validate both operands (supports nested functions/operators) + left = self._parse_and_validate_argument(operator_args[0]) + right = self._parse_and_validate_argument(operator_args[1]) + + # Wrap raw values (numbers, strings) in ValueWrapper so pypika can process them + if not isinstance(left, Term): + left = ValueWrapper(left) + if not isinstance(right, Term): + right = ValueWrapper(right) + + expression = ArithmeticExpression(operator=operator, left=left, right=right) + + if alias: + return expression.as_(alias) + else: + return expression + def _parse_and_validate_argument(self, arg): - """Parse and validate a single function argument against SQL injection.""" + """Parse and validate a single function/operator argument against SQL injection. + + Supports: + - Numbers: 1, 2.5, etc. + - Strings: field names or quoted literals + - Nested dicts: functions {"COUNT": "name"} or operators {"ADD": [1, 2]} + """ if isinstance(arg, (int | float)): return arg elif isinstance(arg, str): return self._validate_string_argument(arg) + elif isinstance(arg, dict): + # Recursively handle nested functions and operators + if self.is_function_dict(arg): + return self.parse_function(arg) + elif self.is_operator_dict(arg): + return self.parse_operator(arg) + else: + frappe.throw( + _("Invalid nested expression: dictionary must represent a function or operator"), + frappe.ValidationError, + ) elif arg is None: # None is allowed for some functions return arg else: frappe.throw( - _("Invalid argument type: {0}. Only strings, numbers, and None are allowed.").format( + _("Invalid argument type: {0}. Only strings, numbers, dicts, and None are allowed.").format( type(arg).__name__ ), frappe.ValidationError, diff --git a/frappe/database/utils.py b/frappe/database/utils.py index 5685b5ad00..969cc480a0 100644 --- a/frappe/database/utils.py +++ b/frappe/database/utils.py @@ -31,6 +31,10 @@ QUERY_TYPE_PATTERN = re.compile(r"\s*([A-Za-z]*)") def convert_to_value(o: FilterValue): if isinstance(o, bool): return int(o) + elif isinstance(o, dict): + return frappe.as_json(o) + elif isinstance(o, (list, tuple, set)): + return tuple(convert_to_value(item) for item in o) return o diff --git a/frappe/model/qb_query.py b/frappe/model/qb_query.py new file mode 100644 index 0000000000..af376bf6d6 --- /dev/null +++ b/frappe/model/qb_query.py @@ -0,0 +1,259 @@ +# Copyright (c) 2015, Frappe Technologies Pvt. Ltd. and Contributors +# License: MIT. See LICENSE +"""Query implementation using frappe's query builder""" + +import copy +import json +from typing import Any + +import frappe +from frappe.database.utils import DefaultOrderBy, FilterValue +from frappe.deprecation_dumpster import deprecation_warning +from frappe.model.utils import is_virtual_doctype +from frappe.model.utils.user_settings import get_user_settings, update_user_settings +from frappe.query_builder.utils import Column + + +class DatabaseQuery: + """ + Copy of db_query.py DatabaseQuery, using query builder instead. + """ + + def __init__(self, doctype: str) -> None: + self.doctype = doctype + + def execute( + self, + fields: list[str] | tuple[str, ...] | str | None = None, + filters: dict[str, FilterValue] | FilterValue | list[list | FilterValue] | None = None, + or_filters: dict[str, FilterValue] | FilterValue | list[list | FilterValue] | None = None, + group_by: str | None = None, + order_by: str = DefaultOrderBy, + limit: int | None = None, + offset: int | None = None, + limit_start: int = 0, + limit_page_length: int | None = None, + as_list: bool = False, + with_childnames: bool = False, + debug: bool = False, + ignore_permissions: bool = False, + user: str | None = None, + with_comment_count: bool = False, + join: str = "left join", + distinct: bool = False, + start: int | None = None, + page_length: int | None = None, + ignore_ifnull: bool = False, + save_user_settings: bool = False, + save_user_settings_fields: bool = False, + update: dict[str, Any] | None = None, + user_settings: str | dict[str, Any] | None = None, + reference_doctype: str | None = None, + run: bool = True, + strict: bool = True, + pluck: str | None = None, + ignore_ddl: bool = False, + *, + parent_doctype: str | None = None, + ) -> list: + """Execute a database query using the Query Builder engine. + + Args: + fields: Fields to select. Can be a list, tuple, or comma-separated string. + filters: Main filter conditions. Supports dicts, lists, and operator tuples. + or_filters: Additional filter conditions to be combined with OR. + group_by: Fields to group results by. + order_by: Fields to order results by. + limit: Maximum number of records to return. + offset: Number of records to skip for pagination. + limit_start: Legacy pagination start (deprecated, use offset). + limit_page_length: Legacy pagination length (deprecated, use limit). + as_list: Return results as list of lists instead of list of dicts. + with_childnames: Include child document names (not implemented). + debug: Enable debug mode for query inspection. + ignore_permissions: Skip permission checks for the query. + user: Execute query as specific user. + with_comment_count: Add comment count to results (_comment_count field). + join: Type of join for related tables (QB engine auto-determines optimal joins). + distinct: Return only distinct results. + start: Legacy alias for limit_start (deprecated). + page_length: Legacy alias for limit_page_length (deprecated). + ignore_ifnull: Skip IFNULL wrapping (QB engine handles NULL optimization automatically). + save_user_settings: Save current query settings for user. + save_user_settings_fields: Save field selection in user settings. + update: Dictionary to merge into each result when as_list=False. + user_settings: Custom user settings as JSON string or dict. + reference_doctype: Reference doctype for contextual user permissions. + run: Execute query immediately (True) or return query object (False). + strict: Enable strict mode for query validation (legacy compatibility). + pluck: Extract single field values as a simple list. + ignore_ddl: Ignore DDL operations during query execution (legacy compatibility). + parent_doctype: Parent doctype for child table queries. + + Returns: + Query results as list of dicts (default) or list of lists (as_list=True). + If pluck is specified, returns list of field values. + If run=False, returns query object instead of results. + + Raises: + ValidationError: For invalid parameters or query structure. + PermissionError: When user lacks required permissions. + """ + + # Check if table exists before running query (matching db_query behavior) + from frappe.model.meta import get_table_columns + + try: + get_table_columns(self.doctype) + except frappe.db.TableMissingError: + if ignore_ddl: + return [] + else: + raise + + # Handle deprecated parameters + if limit_start: + deprecation_warning( + "2024-01-01", "v17", "The 'limit_start' parameter is deprecated. Use 'offset' instead." + ) + if offset is None: + offset = limit_start + + if limit_page_length: + deprecation_warning( + "2024-01-01", "v17", "The 'limit_page_length' parameter is deprecated. Use 'limit' instead." + ) + if limit is None: + limit = limit_page_length + + if start: + deprecation_warning( + "2024-01-01", "v17", "The 'start' parameter is deprecated. Use 'offset' instead." + ) + if offset is None: + offset = start + + if page_length: + deprecation_warning( + "2024-01-01", "v17", "The 'page_length' parameter is deprecated. Use 'limit' instead." + ) + if limit is None: + limit = page_length + + # filters and fields swappable + # its hard to remember what comes first + if isinstance(fields, dict) or (fields and isinstance(fields, list) and isinstance(fields[0], list)): + # if fields is given as dict/list of list, its probably filters + filters, fields = fields, filters + + elif fields and isinstance(filters, list) and len(filters) > 1 and isinstance(filters[0], str): + # if `filters` is a list of strings, its probably fields + filters, fields = fields, filters + + # Set fields to the requested field or `name` if none specified + if not fields: + fields = [pluck or "name"] + + # Build query using QB engine with converted syntax + kwargs = { + "table": self.doctype, + "fields": fields, + "filters": filters, + "or_filters": or_filters, + "group_by": group_by, + "order_by": order_by, + "limit": limit, + "offset": offset, + "distinct": distinct, + "ignore_permissions": ignore_permissions, + "user": user, + "parent_doctype": parent_doctype, + "reference_doctype": reference_doctype, + } + + query = frappe.qb.get_query(**kwargs) + + if not run: + # Return the SQL query string instead of executing + return str(query.get_sql()) + + # Run the query + if pluck: + result = query.run(debug=debug, as_dict=True, pluck=pluck) + else: + result = query.run(debug=debug, as_dict=not as_list, update=update) + + # Add comment count if requested and not as_list + if with_comment_count and not as_list and self.doctype: + self._add_comment_count(result) + + # Save user settings if requested + if save_user_settings: + user_settings_fields = copy.deepcopy(fields) if save_user_settings_fields else None + + if user_settings and isinstance(user_settings, str): + user_settings = json.loads(user_settings) + + self._save_user_settings(user_settings, user_settings_fields, save_user_settings_fields) + + return result + + def _add_comment_count(self, result: list[Any]) -> None: + """Add comment count to each result row by parsing _comments field. + + This method adds a _comment_count field to each row based on the _comments field content. + It parses the JSON structure to count the number of comments. + + Args: + result: List of result dictionaries to modify + """ + if not result: + return + + for row in result: + if isinstance(row, dict) and "_comments" in row: + try: + comments_data = json.loads(row["_comments"] or "[]") + row["_comment_count"] = len(comments_data) if isinstance(comments_data, list) else 0 + except (json.JSONDecodeError, TypeError): + row["_comment_count"] = 0 + elif isinstance(row, dict): + row["_comment_count"] = 0 + + def _save_user_settings( + self, + user_settings: dict[str, Any] | None, + user_settings_fields: list[str] | None, + save_user_settings_fields: bool, + ) -> None: + """Save user settings for the current query. + + This method stores user preferences for field selections and other query parameters + to provide a personalized experience for repeated queries. + + Args: + user_settings: Custom user settings to save + user_settings_fields: Field list to save if save_user_settings_fields is True + save_user_settings_fields: Whether to save the field selection + """ + if not self.doctype: + return + + try: + current_settings = get_user_settings(self.doctype) or {} + + # Update with custom user settings if provided + if user_settings: + current_settings.update(user_settings) + + # Save field selection if requested + if save_user_settings_fields and user_settings_fields: + current_settings["fields"] = user_settings_fields + + # Only save if there are actual settings to save + if current_settings: + update_user_settings(self.doctype, current_settings) + + except Exception: + # Don't let user settings errors break the query + pass diff --git a/frappe/tests/test_query.py b/frappe/tests/test_query.py index d99bc68b1d..0feaeb63df 100644 --- a/frappe/tests/test_query.py +++ b/frappe/tests/test_query.py @@ -419,6 +419,158 @@ class TestQuery(IntegrationTestCase): "SELECT `name` FROM `tabDocType`", ) + def test_or_filters(self): + """Test OR filter conditions.""" + # Test 1: Basic dict or_filters + self.assertEqual( + frappe.qb.get_query( + "DocType", + fields=["name"], + or_filters={"name": "User", "module": "Core"}, + ).get_sql(), + "SELECT `name` FROM `tabDocType` WHERE `name`='User' OR `module`='Core'".replace( + "`", '"' if frappe.db.db_type == "postgres" else "`" + ), + ) + + # Test 2: List format or_filters + self.assertEqual( + frappe.qb.get_query( + "DocType", + fields=["name"], + or_filters=[["name", "=", "User"], ["module", "=", "Core"]], + ).get_sql(), + "SELECT `name` FROM `tabDocType` WHERE `name`='User' OR `module`='Core'".replace( + "`", '"' if frappe.db.db_type == "postgres" else "`" + ), + ) + + # Test 3: OR filters with operators + self.assertEqual( + frappe.qb.get_query( + "DocType", + fields=["name"], + or_filters={"name": ("like", "User%"), "module": ("in", ["Core", "Custom"])}, + ).get_sql(), + "SELECT `name` FROM `tabDocType` WHERE `name` LIKE 'User%' OR `module` IN ('Core','Custom')".replace( + "`", '"' if frappe.db.db_type == "postgres" else "`" + ), + ) + + # Test 4: Combining filters (AND) with or_filters (OR) + self.assertEqual( + frappe.qb.get_query( + "DocType", + fields=["name"], + filters={"issingle": 0}, + or_filters={"name": "User", "module": "Core"}, + ).get_sql(), + "SELECT `name` FROM `tabDocType` WHERE `issingle`=0 AND (`name`='User' OR `module`='Core')".replace( + "`", '"' if frappe.db.db_type == "postgres" else "`" + ), + ) + + # Test 5: Multiple AND filters with OR filters + self.assertEqual( + frappe.qb.get_query( + "DocType", + fields=["name"], + filters={"issingle": 0, "custom": 0}, + or_filters={"name": "User", "module": "Core"}, + ).get_sql(), + "SELECT `name` FROM `tabDocType` WHERE `issingle`=0 AND `custom`=0 AND (`name`='User' OR `module`='Core')".replace( + "`", '"' if frappe.db.db_type == "postgres" else "`" + ), + ) + + # Test 6: OR filters with simple list (name IN) + self.assertEqual( + frappe.qb.get_query( + "DocType", + or_filters=["User", "Role", "Note"], + ).get_sql(), + "SELECT `name` FROM `tabDocType` WHERE `name` IN ('User','Role','Note')".replace( + "`", '"' if frappe.db.db_type == "postgres" else "`" + ), + ) + + # Test 7: OR filters with greater than and less than + self.assertEqual( + frappe.qb.get_query( + "DocType", + fields=["name"], + or_filters={"idx": (">", 5), "issingle": ("=", 1)}, + ).get_sql(), + "SELECT `name` FROM `tabDocType` WHERE `idx`>5 OR `issingle`=1".replace( + "`", '"' if frappe.db.db_type == "postgres" else "`" + ), + ) + + # Test 8: OR filters with list including doctype + self.assertEqual( + frappe.qb.get_query( + "DocType", + fields=["name"], + or_filters=[["DocType", "name", "=", "User"], ["DocType", "name", "=", "Role"]], + ).get_sql(), + "SELECT `name` FROM `tabDocType` WHERE `name`='User' OR `name`='Role'".replace( + "`", '"' if frappe.db.db_type == "postgres" else "`" + ), + ) + + # Test 9: OR filters with != operator + self.assertEqual( + frappe.qb.get_query( + "DocType", + fields=["name"], + or_filters={"name": ("!=", "User"), "module": ("!=", "Core")}, + ).get_sql(), + "SELECT `name` FROM `tabDocType` WHERE `name`<>'User' OR `module`<>'Core'".replace( + "`", '"' if frappe.db.db_type == "postgres" else "`" + ), + ) + + # Test 10: Empty or_filters should return query without OR conditions + self.assertEqual( + frappe.qb.get_query( + "DocType", + fields=["name"], + filters={"custom": 0}, + or_filters={}, + ).get_sql(), + "SELECT `name` FROM `tabDocType` WHERE `custom`=0".replace( + "`", '"' if frappe.db.db_type == "postgres" else "`" + ), + ) + + # Test 11: OR filters with not in operator + self.assertEqual( + frappe.qb.get_query( + "DocType", + fields=["name"], + or_filters={"name": ("not in", ["User", "Role"]), "module": ("=", "Core")}, + ).get_sql(), + "SELECT `name` FROM `tabDocType` WHERE `name` NOT IN ('User','Role') OR `module`='Core'".replace( + "`", '"' if frappe.db.db_type == "postgres" else "`" + ), + ) + + # Test 12: OR filters with mixed field types + self.assertEqual( + frappe.qb.get_query( + "DocType", + fields=["name", "module"], + or_filters=[ + ["name", "like", "User%"], + ["issingle", "=", 1], + ["custom", "=", 0], + ], + ).get_sql(), + "SELECT `name`,`module` FROM `tabDocType` WHERE `name` LIKE 'User%' OR `issingle`=1 OR `custom`=0".replace( + "`", '"' if frappe.db.db_type == "postgres" else "`" + ), + ) + def test_nested_filters(self): """Test nested filter conditions with AND/OR logic.""" User = frappe.qb.DocType("User") @@ -1585,17 +1737,137 @@ class TestQuery(IntegrationTestCase): # Test unsupported function validation with self.assertRaises(frappe.ValidationError) as cm: frappe.qb.get_query("User", fields=[{"UNSUPPORTED_FUNC": "name"}]).get_sql() - self.assertIn("Unsupported function or invalid field name: UNSUPPORTED_FUNC", str(cm.exception)) + self.assertIn("Unsupported function or operator: UNSUPPORTED_FUNC", str(cm.exception)) # Test unsupported function that might be confused with child field with self.assertRaises(frappe.ValidationError) as cm: frappe.qb.get_query("User", fields=[{"UPPER": ["first_name"]}]).get_sql() - self.assertIn("Unsupported function or invalid field name: UPPER", str(cm.exception)) + self.assertIn("Unsupported function or operator: UPPER", str(cm.exception)) # Test SQL injection attempt with self.assertRaises(frappe.ValidationError) as cm: frappe.qb.get_query("User", fields=[{"DROP": "TABLE users"}]).get_sql() - self.assertIn("Unsupported function or invalid field name: DROP", str(cm.exception)) + self.assertIn("Unsupported function or operator: DROP", str(cm.exception)) + + def test_arithmetic_operators_in_fields(self): + """Test arithmetic operator support in fields.""" + + # Test simple addition + query = frappe.qb.get_query("User", fields=[{"ADD": [1, 2], "as": "sum_result"}]) + sql = query.get_sql() + self.assertIn("1+2 `sum_result`", sql) + + # Test simple subtraction + query = frappe.qb.get_query("User", fields=[{"SUB": [10, 5], "as": "diff_result"}]) + sql = query.get_sql() + self.assertIn("10-5 `diff_result`", sql) + + # Test simple multiplication + query = frappe.qb.get_query("User", fields=[{"MUL": [3, 4], "as": "prod_result"}]) + sql = query.get_sql() + self.assertIn("3*4 `prod_result`", sql) + + # Test simple division + query = frappe.qb.get_query("User", fields=[{"DIV": [10, 2], "as": "div_result"}]) + sql = query.get_sql() + self.assertIn("10/2 `div_result`", sql) + + # Test operator with field names + query = frappe.qb.get_query("User", fields=[{"ADD": ["enabled", "login_after"], "as": "field_sum"}]) + sql = query.get_sql() + self.assertIn("`enabled`+`login_after` `field_sum`", sql) + + # Test nested operators + query = frappe.qb.get_query("User", fields=[{"ADD": [{"MUL": [2, 3]}, 4], "as": "nested_result"}]) + sql = query.get_sql() + self.assertIn("2*3+4 `nested_result`", sql) + + # Test operator with function - NULLIF + query = frappe.qb.get_query( + "User", fields=[{"DIV": [1, {"NULLIF": ["enabled", 0]}], "as": "safe_div"}] + ) + sql = query.get_sql() + self.assertIn("1/NULLIF(`enabled`,0) `safe_div`", sql) + + # Test complex nested expression: (1 / NULLIF(value, 0)) + query = frappe.qb.get_query( + "User", + fields=[ + "name", + {"DIV": [1, {"NULLIF": ["enabled", 0]}], "as": "inverse"}, + ], + ) + sql = query.get_sql() + self.assertIn("`name`", sql) + self.assertIn("1/NULLIF(`enabled`,0) `inverse`", sql) + + # Test operator with LOCATE function (search relevance pattern) + query = frappe.qb.get_query( + "User", + fields=[ + "name", + {"DIV": [1, {"NULLIF": [{"LOCATE": ["'test'", "name"]}, 0]}], "as": "relevance"}, + ], + ) + sql = query.get_sql() + self.assertIn("1/NULLIF(LOCATE('test',`name`),0) `relevance`", sql) + + # Test multiple operators in fields + query = frappe.qb.get_query( + "User", + fields=[ + "name", + {"ADD": ["enabled", 1], "as": "enabled_plus_one"}, + {"MUL": ["enabled", 2], "as": "enabled_times_two"}, + ], + ) + sql = query.get_sql() + self.assertIn("`name`", sql) + self.assertIn("`enabled`+1 `enabled_plus_one`", sql) + self.assertIn("`enabled`*2 `enabled_times_two`", sql) + + # Test operator without alias + query = frappe.qb.get_query("User", fields=[{"ADD": [1, 1]}]) + sql = query.get_sql() + self.assertIn("1+1", sql) + + # Test validation: operator requires exactly 2 arguments + with self.assertRaises(frappe.ValidationError) as cm: + frappe.qb.get_query("User", fields=[{"ADD": [1, 2, 3]}]).get_sql() + self.assertIn("requires exactly 2 arguments", str(cm.exception)) + + # Test validation: operator with only 1 argument + with self.assertRaises(frappe.ValidationError) as cm: + frappe.qb.get_query("User", fields=[{"DIV": [10]}]).get_sql() + self.assertIn("requires exactly 2 arguments", str(cm.exception)) + + # Test validation: operator with non-list arguments + with self.assertRaises(frappe.ValidationError) as cm: + frappe.qb.get_query("User", fields=[{"MUL": "invalid"}]).get_sql() + self.assertIn("requires exactly 2 arguments", str(cm.exception)) + + # Test validation: unsupported operator + with self.assertRaises(frappe.ValidationError) as cm: + frappe.qb.get_query("User", fields=[{"XOR": [1, 2]}]).get_sql() + self.assertIn("Unsupported function or operator: XOR", str(cm.exception)) + + # Test deeply nested expression + query = frappe.qb.get_query( + "User", + fields=[ + { + "DIV": [ + {"ADD": [{"MUL": [2, 3]}, 4]}, + {"SUB": [10, 5]}, + ], + "as": "complex_expr", + } + ], + ) + sql = query.get_sql() + # PyPika adds parentheses for clarity in complex expressions + self.assertIn("complex_expr", sql) + self.assertIn("/", sql) def test_not_equal_condition_on_none(self): self.assertQueryEqual( From 5d3f7aaab942691fc990709f7e0ff5422fd24d3e Mon Sep 17 00:00:00 2001 From: Akhil Narang Date: Mon, 29 Sep 2025 18:49:56 +0530 Subject: [PATCH 02/38] refactor: use qb_query for get_list Signed-off-by: Akhil Narang --- frappe/__init__.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frappe/__init__.py b/frappe/__init__.py index 63d0e8a0e9..10400ed48f 100644 --- a/frappe/__init__.py +++ b/frappe/__init__.py @@ -1355,9 +1355,9 @@ def get_list(doctype, *args, **kwargs): # filter as a list of lists frappe.get_list("ToDo", fields="*", filters = [["modified", ">", "2014-01-01"]]) """ - import frappe.model.db_query + import frappe.model.qb_query - return frappe.model.db_query.DatabaseQuery(doctype).execute(*args, **kwargs) + return frappe.model.qb_query.DatabaseQuery(doctype).execute(*args, **kwargs) def get_all(doctype, *args, **kwargs): From 90ed0502fa59e4e79ecdec454fc9260dce2f57f0 Mon Sep 17 00:00:00 2001 From: Akhil Narang Date: Thu, 6 Nov 2025 18:27:43 +0530 Subject: [PATCH 03/38] refactor: support new function style - Migrate all SQL function usage from string format to dict format - Old: fields=['count(*) as count'] - New: fields=[{'COUNT': '*', 'as': 'count'}] - Add `NULLIF` Signed-off-by: Akhil Narang --- frappe/core/doctype/data_import/data_import.py | 2 +- frappe/core/notifications.py | 2 +- frappe/database/query.py | 7 +++++++ .../desk/doctype/dashboard_chart/dashboard_chart.py | 6 +++--- frappe/desk/doctype/route_history/route_history.py | 2 +- frappe/desk/listview.py | 2 +- frappe/desk/notifications.py | 2 +- frappe/desk/reportview.py | 8 ++++---- frappe/desk/search.py | 4 ++-- frappe/email/doctype/email_account/email_account.py | 2 +- frappe/tests/test_db.py | 2 +- frappe/tests/test_db_query.py | 12 +++++------- frappe/tests/test_frappe_client.py | 4 ++-- frappe/workflow/doctype/workflow/workflow.py | 2 +- 14 files changed, 31 insertions(+), 26 deletions(-) diff --git a/frappe/core/doctype/data_import/data_import.py b/frappe/core/doctype/data_import/data_import.py index 8bf62c6c9f..cc105f813a 100644 --- a/frappe/core/doctype/data_import/data_import.py +++ b/frappe/core/doctype/data_import/data_import.py @@ -236,7 +236,7 @@ def get_import_status(data_import_name: str): import_status = {"status": data_import.status} logs = frappe.get_all( "Data Import Log", - fields=["count(*) as count", "success"], + fields=[{"COUNT": "*", "as": "count"}, "success"], filters={"data_import": data_import_name}, group_by="success", ) diff --git a/frappe/core/notifications.py b/frappe/core/notifications.py index b4d903d7ca..17828b84f4 100644 --- a/frappe/core/notifications.py +++ b/frappe/core/notifications.py @@ -20,7 +20,7 @@ def get_things_todo(as_list=False): """Return a count of incomplete ToDos.""" data = frappe.get_list( "ToDo", - fields=["name", "description"] if as_list else "count(*)", + fields=["name", "description"] if as_list else [{"COUNT": "*"}], filters=[["ToDo", "status", "=", "Open"]], or_filters=[ ["ToDo", "allocated_to", "=", frappe.session.user], diff --git a/frappe/database/query.py b/frappe/database/query.py index 555746f185..a34474f532 100644 --- a/frappe/database/query.py +++ b/frappe/database/query.py @@ -52,6 +52,7 @@ FUNCTION_MAPPING = { "IFNULL": functions.IfNull, "CONCAT": functions.Concat, "NOW": functions.Now, + "NULLIF": functions.NullIf, } # Mapping from operator names to pypika Arithmetic enum values @@ -1579,6 +1580,12 @@ class SQLFunctionParser: if not arg: frappe.throw(_("Empty string arguments are not allowed"), frappe.ValidationError) + # Special case: allow '*' for COUNT(*) and similar aggregate functions + if arg == "*": + # Return as-is for SQL star expansion (COUNT(*), etc.) + # pypika will handle this correctly when used with aggregate functions + return Column("*") + # Check for string literals (quoted strings) if self._is_string_literal(arg): return self._validate_string_literal(arg) diff --git a/frappe/desk/doctype/dashboard_chart/dashboard_chart.py b/frappe/desk/doctype/dashboard_chart/dashboard_chart.py index b91b338ddb..44355b943f 100644 --- a/frappe/desk/doctype/dashboard_chart/dashboard_chart.py +++ b/frappe/desk/doctype/dashboard_chart/dashboard_chart.py @@ -201,7 +201,7 @@ def get_chart_config(chart, filters, timespan, timegrain, from_date, to_date): data = frappe.get_list( doctype, - fields=[datefield, f"SUM({value_field})", "COUNT(*)"], + fields=[datefield, {"SUM": value_field}, {"COUNT": "*"}], filters=filters, group_by=datefield, order_by=datefield, @@ -244,7 +244,7 @@ def get_heatmap_chart_config(chart, filters, heatmap_year): doctype, fields=[ timestamp_field, - f"{aggregate_function}({value_field})", + {aggregate_function: value_field}, ], filters=filters, group_by=f"date({datefield})", @@ -270,7 +270,7 @@ def get_group_by_chart_config(chart, filters) -> dict | None: doctype, fields=[ f"{group_by_field} as name", - f"{aggregate_function}({value_field}) as count", + {aggregate_function: value_field, "as": "count"}, ], filters=filters, parent_doctype=chart.parent_document_type, diff --git a/frappe/desk/doctype/route_history/route_history.py b/frappe/desk/doctype/route_history/route_history.py index ff64636b71..52e885e583 100644 --- a/frappe/desk/doctype/route_history/route_history.py +++ b/frappe/desk/doctype/route_history/route_history.py @@ -46,7 +46,7 @@ def deferred_insert(routes): def frequently_visited_links(): return frappe.get_all( "Route History", - fields=["route", "count(name) as count"], + fields=["route", {"COUNT": "name", "as": "count"}], filters={"user": frappe.session.user}, group_by="route", order_by="count desc", diff --git a/frappe/desk/listview.py b/frappe/desk/listview.py index 94af0a06aa..8de906648f 100644 --- a/frappe/desk/listview.py +++ b/frappe/desk/listview.py @@ -69,7 +69,7 @@ def get_group_by_count(doctype: str, current_filters: str, field: str) -> list[d doctype, filters=current_filters, group_by=f"`tab{doctype}`.{field}", - fields=["count(*) as count", f"`{field}` as name"], + fields=[{"COUNT": "*", "as": "count"}, f"`{field}` as name"], order_by="count desc", limit=1000, ) diff --git a/frappe/desk/notifications.py b/frappe/desk/notifications.py index 5aa9f0a40e..11b2b979ae 100644 --- a/frappe/desk/notifications.py +++ b/frappe/desk/notifications.py @@ -65,7 +65,7 @@ def get_notifications_for_doctypes(config, notification_count): try: if isinstance(condition, dict): result = frappe.get_list( - d, fields=["count(*) as count"], filters=condition, ignore_ifnull=True + d, fields=[{"COUNT": "*", "as": "count"}], filters=condition, ignore_ifnull=True )[0].count else: result = frappe.get_attr(condition)() diff --git a/frappe/desk/reportview.py b/frappe/desk/reportview.py index 97fa71b271..757a502128 100644 --- a/frappe/desk/reportview.py +++ b/frappe/desk/reportview.py @@ -686,7 +686,7 @@ def get_stats(stats, doctype, filters=None): try: tag_count = frappe.get_list( doctype, - fields=[column, "count(*)"], + fields=[column, {"COUNT": "*"}], filters=[*filters, [column, "!=", ""]], group_by=column, as_list=True, @@ -697,7 +697,7 @@ def get_stats(stats, doctype, filters=None): results[column] = scrub_user_tags(tag_count) no_tag_count = frappe.get_list( doctype, - fields=[column, "count(*)"], + fields=[column, {"COUNT": "1"}], filters=[*filters, [column, "in", ("", ",")]], as_list=True, group_by=column, @@ -736,7 +736,7 @@ def get_filter_dashboard_data(stats, doctype, filters=None): if tag["type"] not in ["Date", "Datetime"]: tagcount = frappe.get_list( doctype, - fields=[tag["name"], "count(*)"], + fields=[tag["name"], {"COUNT": "*"}], filters=[*filters, "ifnull(`{}`,'')!=''".format(tag["name"])], group_by=tag["name"], as_list=True, @@ -758,7 +758,7 @@ def get_filter_dashboard_data(stats, doctype, filters=None): "No Data", frappe.get_list( doctype, - fields=[tag["name"], "count(*)"], + fields=[tag["name"], {"COUNT": "*"}], filters=[*filters, "({0} = '' or {0} is null)".format(tag["name"])], as_list=True, )[0][1], diff --git a/frappe/desk/search.py b/frappe/desk/search.py index 3bafdcb2bd..1282a9b97a 100644 --- a/frappe/desk/search.py +++ b/frappe/desk/search.py @@ -176,8 +176,8 @@ def search_widget( if not meta.translated_doctype: _txt = frappe.db.escape((txt or "").replace("%", "").replace("@", "")) # locate returns 0 if string is not found, convert 0 to null and then sort null to end in order by - _relevance = f"(1 / nullif(locate({_txt}, `tab{doctype}`.`name`), 0))" - formatted_fields.append(f"""{_relevance} as `_relevance`""") + _relevance = {"DIV": [1, {"NULLIF": [{"LOCATE": [_txt, "name"]}, 0]}], "as": "_relevance"} + formatted_fields.append(f"{_relevance} as _relevance") # Since we are sorting by alias postgres needs to know number of column we are sorting if frappe.db.db_type == "mariadb": order_by = f"ifnull(_relevance, -9999) desc, {order_by}" diff --git a/frappe/email/doctype/email_account/email_account.py b/frappe/email/doctype/email_account/email_account.py index c41f1b8454..67559fe8d6 100755 --- a/frappe/email/doctype/email_account/email_account.py +++ b/frappe/email/doctype/email_account/email_account.py @@ -950,7 +950,7 @@ def get_max_email_uid(email_account): "sent_or_received": "Received", "email_account": email_account, }, - fields=["max(uid) as uid"], + fields=[{"MAX": "uid", "as": "uid"}], ): return cint(result[0].get("uid", 0)) + 1 return 1 diff --git a/frappe/tests/test_db.py b/frappe/tests/test_db.py index 286c492140..9481ca3fa3 100644 --- a/frappe/tests/test_db.py +++ b/frappe/tests/test_db.py @@ -403,7 +403,7 @@ class TestDB(IntegrationTestCase): random_field, ) self.assertEqual( - next(iter(frappe.get_all("ToDo", fields=[f"count(`{random_field}`)"], limit=1)[0])), + next(iter(frappe.get_all("ToDo", fields=[{"COUNT": f"`{random_field}`"}], limit=1)[0])), "count" if frappe.conf.db_type == "postgres" else f"count(`{random_field}`)", ) diff --git a/frappe/tests/test_db_query.py b/frappe/tests/test_db_query.py index 03ef9bfb65..df7f99e0b4 100644 --- a/frappe/tests/test_db_query.py +++ b/frappe/tests/test_db_query.py @@ -483,15 +483,13 @@ class TestDBQuery(IntegrationTestCase): self.assertTrue("count" in data[0]) data = DatabaseQuery("DocType").execute( - fields=["name", "issingle", "locate('', name) as _relevance"], - limit_start=0, - limit_page_length=1, + fields=["name", "issingle", "locate('','name') as _relevance"], limit_start=0, limit_page_length=1 ) self.assertTrue("_relevance" in data[0]) # Test that fields with keywords in strings are allowed data = DatabaseQuery("DocType").execute( - fields=["name", "locate('select', name)"], + fields=["name", "locate('select', 'name')"], limit_start=0, limit_page_length=1, ) @@ -818,7 +816,7 @@ class TestDBQuery(IntegrationTestCase): frappe.db.get_list( "Web Form", filters=[["Web Form Field", "reqd", "=", 1]], - fields=["count(*) as count"], + fields=[{"COUNT": "*", "as": "count"}], order_by="count desc", limit=50, ) @@ -846,7 +844,7 @@ class TestDBQuery(IntegrationTestCase): "DocType", filters={"docstatus": 0, "document_type": ("!=", "")}, group_by="document_type", - fields=["document_type", "sum(is_submittable) as is_submittable"], + fields=["document_type", {"SUM": "is_submittable", "as": "is_submittable"}], limit=1, as_list=True, ) @@ -1222,7 +1220,7 @@ class TestDBQuery(IntegrationTestCase): self.assertEqual(len(data["values"]), 1) def test_select_star_expansion(self): - count = frappe.get_list("Language", ["SUM(1)", "COUNT(*)"], as_list=1, order_by=None)[0] + count = frappe.get_list("Language", [{"SUM": 1}, {"COUNT": "*"}], as_list=1, order_by=None)[0] self.assertEqual(count[0], frappe.db.count("Language")) self.assertEqual(count[1], frappe.db.count("Language")) diff --git a/frappe/tests/test_frappe_client.py b/frappe/tests/test_frappe_client.py index 61a1754319..5f8c491f10 100644 --- a/frappe/tests/test_frappe_client.py +++ b/frappe/tests/test_frappe_client.py @@ -70,13 +70,13 @@ class TestFrappeClient(IntegrationTestCase): getlist_users = server.get_list( "User", - fields=["count(name) as user_count"], + fields=[{"COUNT": "name", "as": "user_count"}], filters={"user_type": "System User"}, group_by="user_type", ) getall_users = frappe.db.get_all( "User", - fields=["count(name) as system_user_count"], + fields=[{"COUNT": "name", "as": "system_user_count"}], filters={"user_type": "System User"}, group_by="user_type", ) diff --git a/frappe/workflow/doctype/workflow/workflow.py b/frappe/workflow/doctype/workflow/workflow.py index 04efba4177..951bb5f075 100644 --- a/frappe/workflow/doctype/workflow/workflow.py +++ b/frappe/workflow/doctype/workflow/workflow.py @@ -127,7 +127,7 @@ def get_workflow_state_count(doctype, workflow_state_field, states): if workflow_state_field in frappe.get_meta(doctype).get_valid_columns(): result = frappe.get_all( doctype, - fields=[workflow_state_field, "count(*) as count"], + fields=[workflow_state_field, {"COUNT": "*", "as": "count"}], filters={workflow_state_field: ["not in", states]}, group_by=workflow_state_field, ) From 977aee5ab37539ae6186b9b0e2b7d54250c5f036 Mon Sep 17 00:00:00 2001 From: Akhil Narang Date: Fri, 7 Nov 2025 16:47:29 +0530 Subject: [PATCH 04/38] refactor: backticks aren't allowed in order_by or group_by Signed-off-by: Akhil Narang --- frappe/contacts/doctype/address/address.py | 2 +- frappe/contacts/doctype/contact/contact.py | 2 +- frappe/core/doctype/activity_log/test_activity_log.py | 2 +- frappe/core/doctype/data_export/exporter.py | 9 +++++---- frappe/core/doctype/data_import/exporter.py | 4 ++-- frappe/core/doctype/report/report.py | 2 +- frappe/core/doctype/user_type/user_type.py | 2 +- frappe/desk/form/utils.py | 2 +- frappe/desk/listview.py | 4 ++-- frappe/desk/search.py | 2 +- frappe/model/db_query.py | 8 ++++---- 11 files changed, 20 insertions(+), 19 deletions(-) diff --git a/frappe/contacts/doctype/address/address.py b/frappe/contacts/doctype/address/address.py index 65622a9c4f..93bbcc6431 100644 --- a/frappe/contacts/doctype/address/address.py +++ b/frappe/contacts/doctype/address/address.py @@ -310,7 +310,7 @@ def get_address_display_list(doctype: str, name: str) -> list[dict]: ["Dynamic Link", "parenttype", "=", "Address"], ], fields=["*"], - order_by="is_primary_address DESC, `tabAddress`.creation ASC", + order_by="is_primary_address DESC, creation ASC", ) for a in address_list: a["display"] = get_address_display(a) diff --git a/frappe/contacts/doctype/contact/contact.py b/frappe/contacts/doctype/contact/contact.py index 6d90533be6..3ad5e54c62 100644 --- a/frappe/contacts/doctype/contact/contact.py +++ b/frappe/contacts/doctype/contact/contact.py @@ -486,7 +486,7 @@ def get_contact_display_list(doctype: str, name: str) -> list[dict]: ["Dynamic Link", "parenttype", "=", "Contact"], ], fields=["*"], - order_by="is_primary_contact DESC, `tabContact`.creation ASC", + order_by="is_primary_contact DESC, creation ASC", ) for contact in contact_list: diff --git a/frappe/core/doctype/activity_log/test_activity_log.py b/frappe/core/doctype/activity_log/test_activity_log.py index d78f29449b..330b1bd8c7 100644 --- a/frappe/core/doctype/activity_log/test_activity_log.py +++ b/frappe/core/doctype/activity_log/test_activity_log.py @@ -50,7 +50,7 @@ class TestActivityLog(IntegrationTestCase): "user": "Administrator", "operation": operation, }, - order_by="`creation` DESC", + order_by="creation DESC", ) name = names[0] diff --git a/frappe/core/doctype/data_export/exporter.py b/frappe/core/doctype/data_export/exporter.py index a0e2f934da..5ad7a7c132 100644 --- a/frappe/core/doctype/data_export/exporter.py +++ b/frappe/core/doctype/data_export/exporter.py @@ -370,11 +370,12 @@ class DataExporter: order_by = None table_columns = frappe.db.get_table_columns(self.parent_doctype) if "lft" in table_columns and "rgt" in table_columns: - order_by = f"`tab{self.parent_doctype}`.`lft` asc" + order_by = DocType(self.parent_doctype).lft + # get permitted data only - self.data = frappe.get_list( - self.doctype, fields=["*"], filters=self.filters, limit_page_length=None, order_by=order_by - ) + self.data = frappe.qb.get_query( + self.doctype, fields=["*"], filters=self.filters, order_by=order_by + ).run(as_dict=True) for doc in self.data: op = self.docs_to_export.get("op") diff --git a/frappe/core/doctype/data_import/exporter.py b/frappe/core/doctype/data_import/exporter.py index 3300693e26..f49a23b634 100644 --- a/frappe/core/doctype/data_import/exporter.py +++ b/frappe/core/doctype/data_import/exporter.py @@ -165,9 +165,9 @@ class Exporter: filters = self.export_filters if self.meta.is_nested_set(): - order_by = f"`tab{self.doctype}`.`lft` ASC" + order_by = "lft ASC" else: - order_by = f"`tab{self.doctype}`.`creation` DESC" + order_by = "creation DESC" parent_fields = [format_column_name(df) for df in self.fields if df.parent == self.doctype] parent_data = frappe.db.get_list( diff --git a/frappe/core/doctype/report/report.py b/frappe/core/doctype/report/report.py index 2defabc21a..c27280a78d 100644 --- a/frappe/core/doctype/report/report.py +++ b/frappe/core/doctype/report/report.py @@ -291,7 +291,7 @@ class Report(Document): @staticmethod def _format(parts): # sort by is saved as DocType.fieldname, covert it to sql - return "`tab{}`.`{}`".format(*parts) + return parts[1] def get_standard_report_columns(self, params): if params.get("fields"): diff --git a/frappe/core/doctype/user_type/user_type.py b/frappe/core/doctype/user_type/user_type.py index 0104b6119e..046e3203f9 100644 --- a/frappe/core/doctype/user_type/user_type.py +++ b/frappe/core/doctype/user_type/user_type.py @@ -234,7 +234,7 @@ def get_user_linked_doctypes(doctype, txt, searchfield, start, page_len, filters "DocType", fields=["`tabDocType`.`name`"], filters=filters, - order_by="`tabDocType`.`idx` desc", + order_by="idx desc", limit_start=start, limit_page_length=page_len, as_list=1, diff --git a/frappe/desk/form/utils.py b/frappe/desk/form/utils.py index 8bfa3cac69..f956428c91 100644 --- a/frappe/desk/form/utils.py +++ b/frappe/desk/form/utils.py @@ -101,7 +101,7 @@ def get_next(doctype, value, prev, filters=None, sort_order="desc", sort_field=" doctype, fields=["name"], filters=filters, - order_by=f"`tab{doctype}`.{sort_field}" + " " + sort_order, + order_by=f"{sort_field} {sort_order}", limit_start=0, limit_page_length=1, as_list=True, diff --git a/frappe/desk/listview.py b/frappe/desk/listview.py index 8de906648f..870cfe6199 100644 --- a/frappe/desk/listview.py +++ b/frappe/desk/listview.py @@ -68,8 +68,8 @@ def get_group_by_count(doctype: str, current_filters: str, field: str) -> list[d data = frappe.get_list( doctype, filters=current_filters, - group_by=f"`tab{doctype}`.{field}", - fields=[{"COUNT": "*", "as": "count"}, f"`{field}` as name"], + group_by=field, + fields=[{"COUNT": "*", "as": "count"}, f"{field} as name"], order_by="count desc", limit=1000, ) diff --git a/frappe/desk/search.py b/frappe/desk/search.py index 1282a9b97a..4232ca80f7 100644 --- a/frappe/desk/search.py +++ b/frappe/desk/search.py @@ -171,7 +171,7 @@ def search_widget( order_by_based_on_meta = get_order_by(doctype, meta) # `idx` is number of times a document is referred, check link_count.py - order_by = f"`tab{doctype}`.idx desc, {order_by_based_on_meta}" + order_by = f"idx desc, {order_by_based_on_meta}" if not meta.translated_doctype: _txt = frappe.db.escape((txt or "").replace("%", "").replace("@", "")) diff --git a/frappe/model/db_query.py b/frappe/model/db_query.py index 6a0b358654..ae39b9913c 100644 --- a/frappe/model/db_query.py +++ b/frappe/model/db_query.py @@ -846,7 +846,7 @@ from {tables} nodes = frappe.get_all( ref_doctype, filters={"lft": [">", lft], "rgt": ["<", rgt]}, - order_by="`lft` ASC", + order_by="lft ASC", pluck="name", ) if f.operator.lower() == "descendants of (inclusive)": @@ -856,7 +856,7 @@ from {tables} nodes = frappe.get_all( ref_doctype, filters={"lft": ["<", lft], "rgt": [">", rgt]}, - order_by="`lft` DESC", + order_by="lft DESC", pluck="name", ) @@ -1377,7 +1377,7 @@ def get_order_by(doctype, meta): # will covert to # `tabItem`.`idx` desc, `tabItem`.`creation` desc order_by = ", ".join( - f"`tab{doctype}`.`{f_split[0].strip()}` {f_split[1].strip()}" + f"{f_split[0].strip()} {f_split[1].strip()}" for f in meta.sort_field.split(",") if (f_split := f.split(maxsplit=2)) ) @@ -1385,7 +1385,7 @@ def get_order_by(doctype, meta): else: sort_field = meta.sort_field or "creation" sort_order = (meta.sort_field and meta.sort_order) or "desc" - order_by = f"`tab{doctype}`.`{sort_field}` {sort_order}" + order_by = f"{sort_field} {sort_order}" return order_by From 5d992a5eb16d6dbd05273a4a3bddd88c8bcdb55a Mon Sep 17 00:00:00 2001 From: Akhil Narang Date: Mon, 10 Nov 2025 17:18:49 +0530 Subject: [PATCH 05/38] refactor: migrate a few queries to use query builder Signed-off-by: Akhil Narang --- frappe/core/doctype/doctype/test_doctype.py | 35 ++++++++-------- frappe/core/doctype/user/user.py | 22 ++++++---- .../permission_manager/permission_manager.py | 40 +++++++++++-------- .../desk/doctype/desktop_icon/desktop_icon.py | 20 ++++++---- frappe/desk/treeview.py | 30 +++++++------- 5 files changed, 85 insertions(+), 62 deletions(-) diff --git a/frappe/core/doctype/doctype/test_doctype.py b/frappe/core/doctype/doctype/test_doctype.py index 5eeb91a790..28ba53275f 100644 --- a/frappe/core/doctype/doctype/test_doctype.py +++ b/frappe/core/doctype/doctype/test_doctype.py @@ -153,24 +153,25 @@ class TestDocType(IntegrationTestCase): def test_all_depends_on_fields_conditions(self): import re - docfields = frappe.get_all( - "DocField", - or_filters={ - "ifnull(depends_on, '')": ("!=", ""), - "ifnull(collapsible_depends_on, '')": ("!=", ""), - "ifnull(mandatory_depends_on, '')": ("!=", ""), - "ifnull(read_only_depends_on, '')": ("!=", ""), - }, - fields=[ - "parent", - "depends_on", - "collapsible_depends_on", - "mandatory_depends_on", - "read_only_depends_on", - "fieldname", - "fieldtype", - ], + DocField = frappe.qb.DocType("DocField") + docfields_query = ( + frappe.qb.from_(DocField) + .select( + DocField.parent, + DocField.depends_on, + DocField.collapsible_depends_on, + DocField.mandatory_depends_on, + DocField.read_only_depends_on, + DocField.fieldname, + ) + .where( + (DocField.depends_on != "") + | (DocField.collapsible_depends_on != "") + | (DocField.mandatory_depends_on != "") + | (DocField.read_only_depends_on != "") + ) ) + docfields = docfields_query.run(as_dict=True) pattern = r'[\w\.:_]+\s*={1}\s*[\w\.@\'"]+' for field in docfields: diff --git a/frappe/core/doctype/user/user.py b/frappe/core/doctype/user/user.py index 9536a57f56..210737e4d5 100644 --- a/frappe/core/doctype/user/user.py +++ b/frappe/core/doctype/user/user.py @@ -876,14 +876,20 @@ def get_all_roles(): """return all roles""" active_domains = frappe.get_active_domains() - roles = frappe.get_all( - "Role", - filters={ - "name": ("not in", frappe.permissions.AUTOMATIC_ROLES), - "disabled": 0, - }, - or_filters={"ifnull(restrict_to_domain, '')": "", "restrict_to_domain": ("in", active_domains)}, - order_by="name", + Role = frappe.qb.DocType("Role") + + domain_condition = (Role.restrict_to_domain.isnull()) | (Role.restrict_to_domain == "") + if active_domains: + domain_condition = domain_condition | Role.restrict_to_domain.isin(active_domains) + + roles = ( + frappe.qb.from_(Role) + .select(Role.name) + .where( + (Role.name.notin(frappe.permissions.AUTOMATIC_ROLES)) & (Role.disabled == 0) & domain_condition + ) + .orderby(Role.name) + .run(as_dict=True) ) return sorted([role.get("name") for role in roles]) diff --git a/frappe/core/page/permission_manager/permission_manager.py b/frappe/core/page/permission_manager/permission_manager.py index b27edb47f6..68a1fd8e3c 100644 --- a/frappe/core/page/permission_manager/permission_manager.py +++ b/frappe/core/page/permission_manager/permission_manager.py @@ -32,14 +32,20 @@ def get_roles_and_doctypes(): active_domains = frappe.get_active_domains() - doctypes = frappe.get_all( - "DocType", - filters={ - "istable": 0, - "name": ("not in", ",".join(not_allowed_in_permission_manager)), - }, - or_filters={"ifnull(restrict_to_domain, '')": "", "restrict_to_domain": ("in", active_domains)}, - fields=["name"], + DocType = frappe.qb.DocType("DocType") + doctype_domain_condition = (DocType.restrict_to_domain.isnull()) | (DocType.restrict_to_domain == "") + if active_domains: + doctype_domain_condition = doctype_domain_condition | DocType.restrict_to_domain.isin(active_domains) + + doctypes = ( + frappe.qb.from_(DocType) + .select(DocType.name) + .where( + (DocType.istable == 0) + & (DocType.name.notin(not_allowed_in_permission_manager)) + & doctype_domain_condition + ) + .run(as_dict=True) ) restricted_roles = ["Administrator"] @@ -48,14 +54,16 @@ def get_roles_and_doctypes(): restricted_roles.extend(row.role for row in custom_user_type_roles) restricted_roles.extend(AUTOMATIC_ROLES) - roles = frappe.get_all( - "Role", - filters={ - "name": ("not in", restricted_roles), - "disabled": 0, - }, - or_filters={"ifnull(restrict_to_domain, '')": "", "restrict_to_domain": ("in", active_domains)}, - fields=["name"], + Role = frappe.qb.DocType("Role") + role_domain_condition = (Role.restrict_to_domain.isnull()) | (Role.restrict_to_domain == "") + if active_domains: + role_domain_condition = role_domain_condition | Role.restrict_to_domain.isin(active_domains) + + roles = ( + frappe.qb.from_(Role) + .select(Role.name) + .where((Role.name.notin(restricted_roles)) & (Role.disabled == 0) & role_domain_condition) + .run(as_dict=True) ) doctypes_list = [{"label": _(d.get("name")), "value": d.get("name")} for d in doctypes] diff --git a/frappe/desk/doctype/desktop_icon/desktop_icon.py b/frappe/desk/doctype/desktop_icon/desktop_icon.py index 4a8366927f..14a256878c 100644 --- a/frappe/desk/doctype/desktop_icon/desktop_icon.py +++ b/frappe/desk/doctype/desktop_icon/desktop_icon.py @@ -152,13 +152,19 @@ def get_desktop_icons(user=None, bootinfo=None): active_domains = frappe.get_active_domains() - blocked_doctypes = frappe.get_all( - "DocType", - filters={"ifnull(restrict_to_domain, '')": ("not in", ",".join(active_domains))}, - fields=["name"], - ) - - blocked_doctypes = [d.get("name") for d in blocked_doctypes] + DocType = frappe.qb.DocType("DocType") + if active_domains: + blocked_condition = ( + (DocType.restrict_to_domain.isnull()) + | (DocType.restrict_to_domain == "") + | (DocType.restrict_to_domain.notin(active_domains)) + ) + else: + blocked_condition = (DocType.restrict_to_domain.isnull()) | (DocType.restrict_to_domain == "") + blocked_doctypes = [ + d.get("name") + for d in frappe.qb.from_(DocType).select(DocType.name).where(blocked_condition).run(as_dict=True) + ] standard_icons = frappe.get_all("Desktop Icon", fields=fields, filters={"standard": 1}) diff --git a/frappe/desk/treeview.py b/frappe/desk/treeview.py index 3e73db2806..238557a4c9 100644 --- a/frappe/desk/treeview.py +++ b/frappe/desk/treeview.py @@ -3,6 +3,7 @@ import frappe from frappe import _ +from frappe.query_builder import Field, functions @frappe.whitelist() @@ -42,24 +43,25 @@ def get_children(doctype, parent="", include_disabled=False, **filters): def _get_children(doctype, parent="", ignore_permissions=False, include_disabled=False): parent_field = "parent_" + frappe.scrub(doctype) - filters = [[f"ifnull(`{parent_field}`,'')", "=", parent], ["docstatus", "<", 2]] - if frappe.db.has_column(doctype, "disabled") and not include_disabled: - filters.append(["disabled", "=", False]) - meta = frappe.get_meta(doctype) - return frappe.get_list( - doctype, - fields=[ - "name as value", - "{} as title".format(meta.get("title_field") or "name"), - "is_group as expandable", - ], - filters=filters, - order_by="name", - ignore_permissions=ignore_permissions, + qb = ( + frappe.qb.from_(doctype) + .select( + Field("name").as_("value"), + Field(meta.get("title_field") or "name").as_("title"), + Field("is_group").as_("expandable"), + ) + .where(functions.IfNull(Field(parent_field), "").eq(parent)) + .where(Field("docstatus") < 2) ) + if frappe.db.has_column(doctype, "disabled") and not include_disabled: + qb = qb.where(Field("disabled").eq(False)) + + # Order by name and execute + return qb.orderby("name").run(as_dict=True) + @frappe.whitelist() def add_node(): From 7183caf871863462dd957019ae22d9cfaea4cb3c Mon Sep 17 00:00:00 2001 From: Akhil Narang Date: Mon, 10 Nov 2025 18:53:30 +0530 Subject: [PATCH 06/38] fix(query_builder): default sorting based on doctype meta Signed-off-by: Akhil Narang --- frappe/core/doctype/doctype/doctype.py | 9 +++++ .../property_setter/property_setter.py | 10 ++++++ frappe/database/database.py | 14 ++++++-- frappe/database/query.py | 31 ++++++++++++++-- frappe/database/utils.py | 35 +++++++++++++++++++ frappe/model/meta.py | 4 +++ 6 files changed, 99 insertions(+), 4 deletions(-) diff --git a/frappe/core/doctype/doctype/doctype.py b/frappe/core/doctype/doctype/doctype.py index 2e2f28349b..0723981a0f 100644 --- a/frappe/core/doctype/doctype/doctype.py +++ b/frappe/core/doctype/doctype/doctype.py @@ -550,6 +550,8 @@ class DocType(Document): self.sync_doctype_layouts() delete_notification_count_for(doctype=self.name) + self._clear_sort_cache_if_needed() + frappe.clear_cache(doctype=self.name) # clear user cache so that on the next reload this doctype is included in boot @@ -560,6 +562,13 @@ class DocType(Document): clear_linked_doctype_cache() + def _clear_sort_cache_if_needed(self): + """Clear sort cache only if sort_field or sort_order changed.""" + if self.has_value_changed("sort_field") or self.has_value_changed("sort_order"): + from frappe.model.meta import clear_doctype_sort_cache + + clear_doctype_sort_cache(self.name) + @savepoint(catch=Exception) def sync_doctype_layouts(self): """Sync Doctype Layout""" diff --git a/frappe/custom/doctype/property_setter/property_setter.py b/frappe/custom/doctype/property_setter/property_setter.py index ba191af542..1a5053aa54 100644 --- a/frappe/custom/doctype/property_setter/property_setter.py +++ b/frappe/custom/doctype/property_setter/property_setter.py @@ -41,9 +41,12 @@ class PropertySetter(Document): if self.is_new(): delete_property_setter(self.doc_type, self.property, self.field_name, self.row_name) + + self._clear_sort_cache_if_needed() frappe.clear_cache(doctype=self.doc_type) def on_trash(self): + self._clear_sort_cache_if_needed() frappe.clear_cache(doctype=self.doc_type) def validate_fieldtype_change(self): @@ -59,6 +62,13 @@ class PropertySetter(Document): validate_fields_for_doctype(self.doc_type) + def _clear_sort_cache_if_needed(self): + """Clear sort cache if this property setter modifies sort_field or sort_order.""" + if self.property in ("sort_field", "sort_order") and self.doctype_or_field == "DocType": + from frappe.model.meta import clear_doctype_sort_cache + + clear_doctype_sort_cache(self.doc_type) + def get_permission_log_options(self, event=None): if self.property in ("ignore_user_permissions", "permlevel"): return { diff --git a/frappe/database/database.py b/frappe/database/database.py index 9b8de4cb51..6497ee86b2 100644 --- a/frappe/database/database.py +++ b/frappe/database/database.py @@ -27,6 +27,7 @@ from frappe.database.utils import ( Query, QueryValues, convert_to_value, + get_doctype_sort_info, get_query_type, is_query_type, ) @@ -582,6 +583,14 @@ class Database: # single field is requested, send it without wrapping in containers return row[0] + def _convert_default_order_by(self, doctype: str, order_by: str) -> str: + """Convert DefaultOrderBy sentinel to explicit order string to avoid meta loading.""" + if order_by != DefaultOrderBy: + return order_by + + sort_field, sort_order = get_doctype_sort_info(doctype) + return f"{sort_field} {sort_order.lower()}" + def get_values( self, doctype: str, @@ -630,6 +639,8 @@ class Database: if isinstance(filters, list): if filters := list(f for f in filters if f is not None): + order_by = self._convert_default_order_by(doctype, order_by) + out = frappe.qb.get_query( table=doctype, fields=fieldname, @@ -647,8 +658,7 @@ class Database: else: if (filters is not None) and (filters != doctype or doctype == "DocType"): try: - if order_by: - order_by = "creation" if order_by == DefaultOrderBy else order_by + order_by = self._convert_default_order_by(doctype, order_by) query = frappe.qb.get_query( table=doctype, diff --git a/frappe/database/query.py b/frappe/database/query.py index a34474f532..3e77806697 100644 --- a/frappe/database/query.py +++ b/frappe/database/query.py @@ -10,7 +10,13 @@ from pypika.terms import AggregateFunction, ArithmeticExpression, Term, ValueWra import frappe from frappe import _ from frappe.database.operator_map import NESTED_SET_OPERATORS, OPERATOR_MAP -from frappe.database.utils import DefaultOrderBy, FilterValue, convert_to_value, get_doctype_name +from frappe.database.utils import ( + DefaultOrderBy, + FilterValue, + convert_to_value, + get_doctype_name, + get_doctype_sort_info, +) from frappe.model import get_permitted_fields from frappe.query_builder import Criterion, Field, Order, functions from frappe.query_builder.utils import PseudoColumnMapper @@ -743,12 +749,33 @@ class Engine: def apply_order_by(self, order_by: str | None): if not order_by or order_by == DefaultOrderBy: + self._apply_default_order_by() return parsed_order_fields = self._validate_order_by(order_by) for order_field, order_direction in parsed_order_fields: self.query = self.query.orderby(order_field, order=order_direction) + def _apply_default_order_by(self): + """Apply default ordering based on configured DocType metadata""" + from pypika.enums import Order + + sort_field, sort_order = get_doctype_sort_info(self.doctype) + + # Handle multiple sort fields + if "," in sort_field: + for sort_spec in sort_field.split(","): + if parts := sort_spec.strip().split(maxsplit=1): + field_name = parts[0] + spec_order = parts[1].lower() if len(parts) > 1 else sort_order.lower() + field = self.table[field_name] + order_direction = Order.desc if spec_order == "desc" else Order.asc + self.query = self.query.orderby(field, order=order_direction) + else: + field = self.table[sort_field] + order_direction = Order.desc if sort_order.lower() == "desc" else Order.asc + self.query = self.query.orderby(field, order=order_direction) + def _validate_and_parse_field_for_clause(self, field_name: str, clause_name: str) -> Field: """ Common helper to validate and parse field names for GROUP BY and ORDER BY clauses. @@ -839,7 +866,7 @@ class Engine: if len(parts) > 1: direction = parts[1].lower() - order_direction = Order.asc if direction == "asc" else Order.desc + order_direction = Order.desc if direction == "desc" else Order.asc parsed_field = self._validate_and_parse_field_for_clause(field_name, "Order By") parsed_order_fields.append((parsed_field, order_direction)) diff --git a/frappe/database/utils.py b/frappe/database/utils.py index 969cc480a0..f54608f02f 100644 --- a/frappe/database/utils.py +++ b/frappe/database/utils.py @@ -57,6 +57,41 @@ def get_doctype_name(table_name: str) -> str: return table_name.replace('"', "") +def get_doctype_sort_info(doctype: str) -> tuple[str, str]: + """ + Get sort_field and sort_order for a DocType from cache or database. + + This is separate from regular meta to avoid recursive calls. + Caches for a day since sort order won't change often (invalidated on doctype update). + + Args: + doctype: The DocType name + + Returns: + Tuple of (sort_field, sort_order) with defaults ("creation", "DESC") if not found + """ + + cache_key = f"doctype_sort_info::{doctype}" + + if cached := frappe.cache.get_value(cache_key): + sort_field, sort_order = cached + else: + sort_field, sort_order = None, None + if result := frappe.db.sql( + "SELECT sort_field, sort_order FROM tabDocType WHERE name = %s", + (doctype,), + ): + sort_field, sort_order = result[0] + + if not sort_field: + sort_field = "creation" + if not sort_order: + sort_order = "DESC" + frappe.cache.set_value(cache_key, (sort_field, sort_order), expires_in_sec=86400) + + return sort_field, sort_order + + class LazyString: def _setup(self) -> str: raise NotImplementedError diff --git a/frappe/model/meta.py b/frappe/model/meta.py index 99c884b16c..dd59cecd7b 100644 --- a/frappe/model/meta.py +++ b/frappe/model/meta.py @@ -101,6 +101,10 @@ def clear_meta_cache(doctype: str = "*"): frappe.client_cache.delete_value(key) +def clear_doctype_sort_cache(doctype: str): + frappe.cache.delete_value(f"doctype_sort_info::{doctype}") + + def load_meta(doctype): return Meta(doctype) From 1d2a73f659c36f895e4d30f51d041898c5e8ae6a Mon Sep 17 00:00:00 2001 From: Akhil Narang Date: Tue, 11 Nov 2025 16:25:05 +0530 Subject: [PATCH 07/38] fix: support virtual doctype handling Signed-off-by: Akhil Narang --- frappe/model/qb_query.py | 124 +++++++++++++++++++++++++++++++++------ 1 file changed, 105 insertions(+), 19 deletions(-) diff --git a/frappe/model/qb_query.py b/frappe/model/qb_query.py index af376bf6d6..a187e9b4d5 100644 --- a/frappe/model/qb_query.py +++ b/frappe/model/qb_query.py @@ -100,16 +100,35 @@ class DatabaseQuery: PermissionError: When user lacks required permissions. """ - # Check if table exists before running query (matching db_query behavior) - from frappe.model.meta import get_table_columns + # filters and fields swappable + # its hard to remember what comes first + if isinstance(fields, dict) or (fields and isinstance(fields, list) and isinstance(fields[0], list)): + # if fields is given as dict/list of list, its probably filters + filters, fields = fields, filters - try: - get_table_columns(self.doctype) - except frappe.db.TableMissingError: - if ignore_ddl: - return [] - else: - raise + elif fields and isinstance(filters, list) and len(filters) > 1 and isinstance(filters[0], str): + # if `filters` is a list of strings, its probably fields + filters, fields = fields, filters + + # Handle virtual doctypes before any other processing + if is_virtual_doctype(self.doctype): + return self._handle_virtual_doctype( + filters, + or_filters, + start, + offset, + limit_start, + page_length, + limit, + limit_page_length, + order_by, + as_list, + with_comment_count, + save_user_settings, + save_user_settings_fields, + pluck, + parent_doctype, + ) # Handle deprecated parameters if limit_start: @@ -140,20 +159,21 @@ class DatabaseQuery: if limit is None: limit = page_length - # filters and fields swappable - # its hard to remember what comes first - if isinstance(fields, dict) or (fields and isinstance(fields, list) and isinstance(fields[0], list)): - # if fields is given as dict/list of list, its probably filters - filters, fields = fields, filters - - elif fields and isinstance(filters, list) and len(filters) > 1 and isinstance(filters[0], str): - # if `filters` is a list of strings, its probably fields - filters, fields = fields, filters - # Set fields to the requested field or `name` if none specified if not fields: fields = [pluck or "name"] + # Check if table exists before running query + from frappe.model.meta import get_table_columns + + try: + get_table_columns(self.doctype) + except frappe.db.TableMissingError: + if ignore_ddl: + return [] + else: + raise + # Build query using QB engine with converted syntax kwargs = { "table": self.doctype, @@ -257,3 +277,69 @@ class DatabaseQuery: except Exception: # Don't let user settings errors break the query pass + + def _handle_virtual_doctype( + self, + filters: dict[str, FilterValue] | FilterValue | list[list | FilterValue] | None, + or_filters: dict[str, FilterValue] | FilterValue | list[list | FilterValue] | None, + start: int | None, + offset: int | None, + limit_start: int, + page_length: int | None, + limit: int | None, + limit_page_length: int | None, + order_by: str, + as_list: bool, + with_comment_count: bool, + save_user_settings: bool, + save_user_settings_fields: bool, + pluck: str | None, + parent_doctype: str | None, + ) -> list: + """Handle virtual doctype queries by delegating to controller.get_list(). + + Virtual doctypes don't have database tables and use controller methods + to generate data dynamically. Converts filters to Filters objects and + calls the doctype controller's get_list method. + + Returns: + List of results from controller.get_list() + """ + from frappe.model.base_document import get_controller + from frappe.types.filter import Filters + + controller = get_controller(self.doctype) + if not hasattr(controller, "get_list"): + return [] + + filters = filters or Filters() + if isinstance(filters, str): + filters = json.loads(filters) + if not isinstance(filters, Filters): + filters = Filters(filters, doctype=self.doctype) + + or_filters = or_filters or Filters() + if isinstance(or_filters, str): + or_filters = json.loads(or_filters) + if not isinstance(or_filters, Filters): + or_filters = Filters(or_filters, doctype=self.doctype) + + _page_length = page_length or limit or limit_page_length or 20 + kwargs = { + "filters": filters, + "or_filters": or_filters, + "start": start or offset or limit_start or 0, + "page_length": _page_length, + "limit_page_length": _page_length, + "order_by": order_by, + "as_list": as_list, + "with_comment_count": with_comment_count, + "save_user_settings": save_user_settings, + "save_user_settings_fields": save_user_settings_fields, + "pluck": pluck, + "parent_doctype": parent_doctype, + "doctype": self.doctype, + } + + # Use frappe.call to filter kwargs and call controller + return frappe.call(controller.get_list, args=kwargs, **kwargs) From 8e039243567788929a3d724ff370536ca30ef3a8 Mon Sep 17 00:00:00 2001 From: Akhil Narang Date: Tue, 11 Nov 2025 22:12:01 +0530 Subject: [PATCH 08/38] fix(query): allow `AggregateFunction` as well in `apply_field_permissions` Without this `fields=[{"COUNT": "name"}]` didn't work, although fields=[{"COUNT": "NAME"}] did. Signed-off-by: Akhil Narang --- frappe/database/query.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/database/query.py b/frappe/database/query.py index 3e77806697..69275ba5aa 100644 --- a/frappe/database/query.py +++ b/frappe/database/query.py @@ -973,7 +973,7 @@ class Engine: elif hasattr(field, "alias") and field.alias and field.name in permitted_fields_set: allowed_fields.append(field) - elif isinstance(field, PseudoColumnMapper): + elif isinstance(field, AggregateFunction | PseudoColumnMapper): # Typically functions or complex terms allowed_fields.append(field) From e15ec47ba1cc5dfd88415bec0e22075dff75eebc Mon Sep 17 00:00:00 2001 From: Akhil Narang Date: Tue, 11 Nov 2025 22:13:12 +0530 Subject: [PATCH 09/38] fix(query): allow passing `as` in any case Signed-off-by: Akhil Narang --- frappe/database/query.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/frappe/database/query.py b/frappe/database/query.py index 69275ba5aa..3742234b69 100644 --- a/frappe/database/query.py +++ b/frappe/database/query.py @@ -1452,7 +1452,7 @@ class SQLFunctionParser: def is_function_dict(self, field_dict: dict) -> bool: """Check if a dictionary represents a SQL function definition.""" - function_keys = [k for k in field_dict.keys() if k != "as"] + function_keys = [k for k in field_dict.keys() if k.lower() != "as"] return len(function_keys) == 1 and function_keys[0] in FUNCTION_MAPPING def is_operator_dict(self, field_dict: dict) -> bool: @@ -1460,7 +1460,7 @@ class SQLFunctionParser: Example: {"ADD": [1, 2], "as": "sum"} or {"DIV": ["total", "count"]} """ - operator_keys = [k for k in field_dict.keys() if k != "as"] + operator_keys = [k for k in field_dict.keys() if k.lower() != "as"] return len(operator_keys) == 1 and operator_keys[0] in OPERATOR_MAPPING def _extract_dict_components(self, d: dict, valid_keys: dict, error_msg: str) -> tuple: @@ -1470,7 +1470,7 @@ class SQLFunctionParser: args = None for key, value in d.items(): - if key == "as": + if key.lower() == "as": alias = value else: name = key From b4cf69732b250c9ab22e34aacf9c8314ff8f242b Mon Sep 17 00:00:00 2001 From: Akhil Narang Date: Tue, 11 Nov 2025 22:14:50 +0530 Subject: [PATCH 10/38] fix(query): allow numeric strings For things like `COUNT(1)` Signed-off-by: Akhil Narang --- frappe/database/query.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/frappe/database/query.py b/frappe/database/query.py index 3742234b69..fa9a24d4a9 100644 --- a/frappe/database/query.py +++ b/frappe/database/query.py @@ -1622,6 +1622,10 @@ class SQLFunctionParser: self._validate_function_field_arg(arg) return self.engine.table[arg] + # Check if it's a numeric string like "1" (for COUNT(1), etc.) + elif arg.isdigit(): + return int(arg) + else: frappe.throw( _( From e420e7646f62d604853ce668e6e668667f0c8f5c Mon Sep 17 00:00:00 2001 From: Akhil Narang Date: Wed, 12 Nov 2025 12:33:00 +0530 Subject: [PATCH 11/38] fix(query): match db_query, add parentheses around condition Signed-off-by: Akhil Narang --- frappe/database/query.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frappe/database/query.py b/frappe/database/query.py index fa9a24d4a9..d248256dbe 100644 --- a/frappe/database/query.py +++ b/frappe/database/query.py @@ -1079,13 +1079,13 @@ class Engine: for method in condition_methods: if c := frappe.call(frappe.get_attr(method), self.user, doctype=self.doctype): - conditions.append(RawCriterion(c)) + conditions.append(RawCriterion(f"({c})")) # Get conditions from server scripts if permission_script_name := get_server_script_map().get("permission_query", {}).get(self.doctype): script = frappe.get_doc("Server Script", permission_script_name) if condition := script.get_permission_query_conditions(self.user): - conditions.append(RawCriterion(condition)) + conditions.append(RawCriterion(f"({condition})")) return conditions From a9d0abaf7b57c3f34312cbef84f2149ca03ad5fa Mon Sep 17 00:00:00 2001 From: Akhil Narang Date: Wed, 12 Nov 2025 12:33:34 +0530 Subject: [PATCH 12/38] fix(test_server_script): loosen test a bit, allow any case Signed-off-by: Akhil Narang --- frappe/core/doctype/server_script/test_server_script.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/frappe/core/doctype/server_script/test_server_script.py b/frappe/core/doctype/server_script/test_server_script.py index aa2cfb8a94..72bd7e6988 100644 --- a/frappe/core/doctype/server_script/test_server_script.py +++ b/frappe/core/doctype/server_script/test_server_script.py @@ -157,10 +157,11 @@ class TestServerScript(IntegrationTestCase): self.assertEqual(frappe.get_doc("Server Script", "test_return_value").execute_method(), "hello") def test_permission_query(self): + sql = frappe.db.get_list("ToDo", run=False) if frappe.conf.db_type != "postgres": - self.assertTrue("where (1 = 1)" in frappe.db.get_list("ToDo", run=False)) + self.assertTrue("where (1 = 1)" in sql.lower()) else: - self.assertTrue("where (1 = '1')" in frappe.db.get_list("ToDo", run=False)) + self.assertTrue("where (1 = '1')" in sql.lower()) self.assertTrue(isinstance(frappe.db.get_list("ToDo"), list)) def test_attribute_error(self): From bd48f5df6554b1da2e9fdf0b80ca5f0d4fc6d691 Mon Sep 17 00:00:00 2001 From: Akhil Narang Date: Wed, 12 Nov 2025 12:33:53 +0530 Subject: [PATCH 13/38] fix(convert_to_value): convert dict_keys and dict_values to a tuple as well Signed-off-by: Akhil Narang --- frappe/database/utils.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/frappe/database/utils.py b/frappe/database/utils.py index f54608f02f..53bf14d011 100644 --- a/frappe/database/utils.py +++ b/frappe/database/utils.py @@ -3,6 +3,7 @@ import re import string +from collections.abc import KeysView, ValuesView from functools import cached_property, wraps import frappe @@ -33,7 +34,7 @@ def convert_to_value(o: FilterValue): return int(o) elif isinstance(o, dict): return frappe.as_json(o) - elif isinstance(o, (list, tuple, set)): + elif isinstance(o, (list, tuple, set, KeysView, ValuesView)): return tuple(convert_to_value(item) for item in o) return o From 0ea49a8f1e996c62c7b2f9c919c186df2b64268e Mon Sep 17 00:00:00 2001 From: Akhil Narang Date: Wed, 12 Nov 2025 12:34:18 +0530 Subject: [PATCH 14/38] fix: make sure limit is an integer Signed-off-by: Akhil Narang --- frappe/desk/doctype/notification_log/notification_log.py | 2 +- frappe/frappeclient.py | 6 +++--- frappe/model/qb_query.py | 4 ++-- frappe/utils/__init__.py | 2 +- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/frappe/desk/doctype/notification_log/notification_log.py b/frappe/desk/doctype/notification_log/notification_log.py index c192eccede..5b4f49526c 100644 --- a/frappe/desk/doctype/notification_log/notification_log.py +++ b/frappe/desk/doctype/notification_log/notification_log.py @@ -167,7 +167,7 @@ def format_email_header(header_map, language, docname): @frappe.whitelist() @http_cache(max_age=60, stale_while_revalidate=60 * 60) -def get_notification_logs(limit=20): +def get_notification_logs(limit: int = 20): notification_logs = frappe.db.get_list( "Notification Log", fields=["*"], limit=limit, order_by="creation desc" ) diff --git a/frappe/frappeclient.py b/frappe/frappeclient.py index f8c619fc6b..65c4b17fa3 100644 --- a/frappe/frappeclient.py +++ b/frappe/frappeclient.py @@ -109,11 +109,11 @@ class FrappeClient: def get_list( self, - doctype, + doctype: str, fields='["name"]', filters=None, - limit_start=0, - limit_page_length=None, + limit_start: int = 0, + limit_page_length: int | None = None, order_by=None, group_by=None, ): diff --git a/frappe/model/qb_query.py b/frappe/model/qb_query.py index a187e9b4d5..6e99dd50c4 100644 --- a/frappe/model/qb_query.py +++ b/frappe/model/qb_query.py @@ -182,8 +182,8 @@ class DatabaseQuery: "or_filters": or_filters, "group_by": group_by, "order_by": order_by, - "limit": limit, - "offset": offset, + "limit": frappe.cint(limit), + "offset": frappe.cint(offset), "distinct": distinct, "ignore_permissions": ignore_permissions, "user": user, diff --git a/frappe/utils/__init__.py b/frappe/utils/__init__.py index c9975d643c..b5bd7397b7 100644 --- a/frappe/utils/__init__.py +++ b/frappe/utils/__init__.py @@ -849,7 +849,7 @@ def get_site_info(): kwargs = { "fields": ["user", "creation", "full_name"], "filters": {"operation": "Login", "status": "Success"}, - "limit": "10", + "limit": 10, } site_info = { From df5080b45e4d20244ba9090586cac5d6e0cf733a Mon Sep 17 00:00:00 2001 From: Akhil Narang Date: Wed, 12 Nov 2025 14:08:59 +0530 Subject: [PATCH 15/38] fix(linked_with): don't add `tab` prefix query builder takes care of this Signed-off-by: Akhil Narang --- frappe/desk/form/linked_with.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/desk/form/linked_with.py b/frappe/desk/form/linked_with.py index 072dcc8433..fe2113302e 100644 --- a/frappe/desk/form/linked_with.py +++ b/frappe/desk/form/linked_with.py @@ -457,7 +457,7 @@ def get_linked_docs(doctype: str, name: str, linkinfo: dict | None = None) -> di if add_fields := link_context.get("add_fields"): fields += add_fields - fields = [f"`tab{linked_doctype}`.`{sf.strip()}`" for sf in fields if sf and "`tab" not in sf] + fields = [sf.strip() for sf in fields if sf] if filters_ctx := link_context.get("filters"): ret = frappe.get_list(doctype=linked_doctype, fields=fields, filters=filters_ctx, order_by=None) From 3040ab7eb269f4597b26d19fbc5e025c62a5c1c3 Mon Sep 17 00:00:00 2001 From: Akhil Narang Date: Thu, 13 Nov 2025 14:01:39 +0530 Subject: [PATCH 16/38] feat(query): add in IFNULL logic from db_query Use `IFNULL(var, "") == ""` instead of `isnull()` Signed-off-by: Akhil Narang --- frappe/database/query.py | 171 ++++++++++++++++++++++++++++++++++++- frappe/tests/test_query.py | 8 +- 2 files changed, 173 insertions(+), 6 deletions(-) diff --git a/frappe/database/query.py b/frappe/database/query.py index d248256dbe..53ba59b690 100644 --- a/frappe/database/query.py +++ b/frappe/database/query.py @@ -389,8 +389,58 @@ class Engine: operator_fn = OPERATOR_MAP[_operator.casefold()] if _value is None and isinstance(_field, Field): - return _field.isnotnull() if operator_fn == builtin_operator.ne else _field.isnull() + if operator_fn == builtin_operator.ne: + filter_field_name = ( + field + if isinstance(field, str) + else (_field.name if hasattr(_field, "name") else str(_field)) + ) + if "." in filter_field_name: + filter_field_name = filter_field_name.split(".")[-1] + + target_doctype = doctype or self.doctype + fallback_sql = self._get_ifnull_fallback(target_doctype, filter_field_name) + + if fallback_sql == "''": + fallback_value = "" + elif fallback_sql.startswith("'") and fallback_sql.endswith("'"): + fallback_value = fallback_sql[1:-1] + else: + try: + fallback_value = int(fallback_sql) + except (ValueError, TypeError): + fallback_value = fallback_sql + + return operator_fn(_field, ValueWrapper(fallback_value)) + else: + return _field.isnull() else: + filter_field_name = ( + field if isinstance(field, str) else (_field.name if hasattr(_field, "name") else str(_field)) + ) + + if "." in filter_field_name: + filter_field_name = filter_field_name.split(".")[-1] + + target_doctype = doctype or self.doctype + + # Skip applying ifnull if field already has null-handling function + if isinstance(_field, functions.IfNull | functions.Coalesce): + return operator_fn(_field, _value) + + if self._should_apply_ifnull(target_doctype, filter_field_name, _operator, _value): + fallback_sql = self._get_ifnull_fallback(target_doctype, filter_field_name) + if fallback_sql == "''": + fallback_value = "" + elif fallback_sql.startswith("'") and fallback_sql.endswith("'"): + fallback_value = fallback_sql[1:-1] + else: + try: + fallback_value = int(fallback_sql) + except (ValueError, TypeError): + fallback_value = fallback_sql + _field = functions.IfNull(_field, ValueWrapper(fallback_value)) + return operator_fn(_field, _value) def _parse_nested_filters(self, nested_list: list | tuple) -> "Criterion | None": @@ -1022,7 +1072,7 @@ class Engine: if strict_user_permissions: conditions.append(self.table[field_name].isin(docs)) else: - empty_value_condition = self.table[field_name].isnull() + empty_value_condition = functions.IfNull(self.table[field_name], "") == "" value_condition = self.table[field_name].isin(docs) conditions.append(empty_value_condition | value_condition) @@ -1113,6 +1163,123 @@ class Engine: # because either of those is required to perform a query return True + def _is_field_nullable(self, doctype: str, fieldname: str) -> bool: + """Check if a field can contain NULL values.""" + # primary key is never nullable, modified is usually indexed by default and always present + if fieldname in ("name", "modified", "creation"): + return False + + try: + # Use cached meta to avoid recursion when loading meta + if (meta := frappe.client_cache.get_value(f"doctype_meta::{doctype}")) is None: + return True + + if (df := meta.get_field(fieldname)) is None: + return True + + except Exception: + return True + + if df.fieldtype in ("Check", "Float", "Int", "Currency", "Percent"): + return False + + if getattr(df, "not_nullable", False): + return False + + return True + + def _get_ifnull_fallback(self, doctype: str, fieldname: str) -> str: + """Get type-appropriate fallback value for NULL comparisons.""" + try: + meta = frappe.get_meta(doctype) + df = meta.get_field(fieldname) + except Exception: + return "''" + + if df is None: + return "''" + + fieldtype = df.fieldtype + + if fieldtype in ("Link", "Data", "Dynamic Link"): + return "''" + + if fieldtype in ("Date", "Datetime"): + return "'0001-01-01'" + + if fieldtype == "Time": + return "'00:00:00'" + + if fieldtype in ("Float", "Int", "Currency", "Percent"): + return "0" + + try: + db_type_info = frappe.db.type_map.get(fieldtype, ("varchar",)) + if db_type_info: + db_type = db_type_info[0] if isinstance(db_type_info, (tuple, list)) else db_type_info + if db_type in ("varchar", "text", "longtext", "smalltext", "json"): + return "''" + except Exception: + pass + + return "''" + + def _should_apply_ifnull(self, doctype: str, fieldname: str, operator: str, value: Any) -> bool: + """Determine if IFNULL wrapping is needed for a filter condition.""" + if not self._is_field_nullable(doctype, fieldname): + return False + + if value is None: + return False + + if operator.lower() in ("=", "like", "is"): + return False + + try: + meta = frappe.get_meta(doctype) + df = meta.get_field(fieldname) + except Exception: + df = None + + is_datetime_field = df and df.fieldtype in ("Date", "Datetime") if df else False + is_creation_or_modified = fieldname in ("creation", "modified") + + if operator.lower() in (">", ">="): + # Null values can never be greater than any non-null value + if is_datetime_field or is_creation_or_modified: + return False + + if operator.lower() == "between": + # Between operator never needs to check for null + # Explanation: Consider SQL -> `COLUMN between X and Y` + # Actual computation: + # for row in rows: + # if Y > row.COLUMN > X: + # yield row + + # Since Y and X can't be null, null value in column will never match filter, so + # coalesce is extra cost that prevents index usage + if is_datetime_field or is_creation_or_modified: + return False + + if operator.lower() == "in": + if isinstance(value, (list, tuple)): + # if values contain '' or falsy values then only coalesce column + # for `in` query this is only required if values contain '' or values are empty. + has_null_or_empty = any(v is None or v == "" for v in value) + return has_null_or_empty + return False + + # for `not in` queries we can't be sure as column values might contain null. + if operator.lower() == "not in": + return True + + if operator.lower() == "<": + if is_datetime_field or is_creation_or_modified: + return True + + return True + class Permission: @classmethod diff --git a/frappe/tests/test_query.py b/frappe/tests/test_query.py index 0feaeb63df..2cc03c7293 100644 --- a/frappe/tests/test_query.py +++ b/frappe/tests/test_query.py @@ -501,7 +501,7 @@ class TestQuery(IntegrationTestCase): fields=["name"], or_filters={"idx": (">", 5), "issingle": ("=", 1)}, ).get_sql(), - "SELECT `name` FROM `tabDocType` WHERE `idx`>5 OR `issingle`=1".replace( + "SELECT `name` FROM `tabDocType` WHERE IFNULL(`idx`,'')>5 OR `issingle`=1".replace( "`", '"' if frappe.db.db_type == "postgres" else "`" ), ) @@ -525,7 +525,7 @@ class TestQuery(IntegrationTestCase): fields=["name"], or_filters={"name": ("!=", "User"), "module": ("!=", "Core")}, ).get_sql(), - "SELECT `name` FROM `tabDocType` WHERE `name`<>'User' OR `module`<>'Core'".replace( + "SELECT `name` FROM `tabDocType` WHERE `name`<>'User' OR IFNULL(`module`,'')<>'Core'".replace( "`", '"' if frappe.db.db_type == "postgres" else "`" ), ) @@ -876,7 +876,7 @@ class TestQuery(IntegrationTestCase): # Check for user permission condition in the query string if frappe.db.db_type == "mariadb": - self.assertIn("`name` IS NULL OR `name` IN ('_Test Blog Post 1','_Test Blog Post')", query) + self.assertIn("IFNULL(`name`,'')='' OR `name` IN ('_Test Blog Post 1','_Test Blog Post')", query) elif frappe.db.db_type == "postgres": self.assertIn("\"name\" IS NULL OR \"name\" IN ('_Test Blog Post 1','_Test Blog Post')", query) @@ -1879,7 +1879,7 @@ class TestQuery(IntegrationTestCase): ["DocType", "parent", "!=", None], ], ).get_sql(), - "SELECT `tabDocType`.* FROM `tabDocType` LEFT JOIN `tabDocField` ON `tabDocField`.`parent`=`tabDocType`.`name` AND `tabDocField`.`parenttype`='DocType' AND `tabDocField`.`parentfield`='fields' WHERE `tabDocField`.`name` IS NULL AND `tabDocType`.`parent` IS NOT NULL", + "SELECT `tabDocType`.* FROM `tabDocType` LEFT JOIN `tabDocField` ON `tabDocField`.`parent`=`tabDocType`.`name` AND `tabDocField`.`parenttype`='DocType' AND `tabDocField`.`parentfield`='fields' WHERE `tabDocField`.`name` IS NULL AND `tabDocType`.`parent`<>''", ) From b407fe80936aee474edcf258d1e166030c598a7d Mon Sep 17 00:00:00 2001 From: Akhil Narang Date: Thu, 13 Nov 2025 14:14:21 +0530 Subject: [PATCH 17/38] fix: allow function aliases Signed-off-by: Akhil Narang --- frappe/database/query.py | 6 ++++++ frappe/tests/test_db_query.py | 11 +++++++++++ 2 files changed, 17 insertions(+) diff --git a/frappe/database/query.py b/frappe/database/query.py index 53ba59b690..ff0ec3b9f7 100644 --- a/frappe/database/query.py +++ b/frappe/database/query.py @@ -109,6 +109,7 @@ class Engine: self.parent_doctype = parent_doctype self.reference_doctype = reference_doctype self.apply_permissions = not ignore_permissions + self.function_aliases = set() if isinstance(table, Table): self.table = table @@ -841,6 +842,10 @@ class Engine: # For numeric field references, return as-is (will be handled by caller) return field_name + # Allow function aliases - return as Field (no table prefix) + if field_name in self.function_aliases: + return Field(field_name) + # Reject backticks if "`" in field_name: frappe.throw( @@ -1690,6 +1695,7 @@ class SQLFunctionParser: ) if alias: + self.engine.function_aliases.add(alias) return function_call.as_(alias) else: return function_call diff --git a/frappe/tests/test_db_query.py b/frappe/tests/test_db_query.py index df7f99e0b4..c40d5e9cac 100644 --- a/frappe/tests/test_db_query.py +++ b/frappe/tests/test_db_query.py @@ -1126,6 +1126,17 @@ class TestDBQuery(IntegrationTestCase): ): frappe.get_all("Virtual DocType", filters={"name": "test"}, fields=["name"], limit=1) + def test_function_alias_in_clauses(self): + result = frappe.get_list( + "ToDo", + fields=["status", {"COUNT": "1", "as": "count"}], + group_by="status", + order_by="count desc", + limit=1, + ) + self.assertTrue(result) + self.assertIn("count", result[0]) + def test_coalesce_with_in_ops(self): self.assertNotIn("ifnull", frappe.get_all("User", {"first_name": ("in", ["a", "b"])}, run=0)) self.assertIn("ifnull", frappe.get_all("User", {"first_name": ("in", ["a", None])}, run=0)) From ddcda11d677bdc547f843c3f22dceaa8625d11b0 Mon Sep 17 00:00:00 2001 From: Akhil Narang Date: Thu, 13 Nov 2025 14:15:36 +0530 Subject: [PATCH 18/38] fix: function detection Signed-off-by: Akhil Narang --- frappe/database/query.py | 26 ++++++++++++++++++++++---- 1 file changed, 22 insertions(+), 4 deletions(-) diff --git a/frappe/database/query.py b/frappe/database/query.py index ff0ec3b9f7..ee44f90eee 100644 --- a/frappe/database/query.py +++ b/frappe/database/query.py @@ -33,6 +33,16 @@ COMMA_PATTERN = re.compile(r",\s*(?![^()]*\))") # to allow table names like __Auth TABLE_NAME_PATTERN = re.compile(r"^[\w -]*$", flags=re.ASCII) + +def _is_function_call(field_str: str) -> bool: + """Check if a string is a SQL function call using sqlparse.""" + parsed = sqlparse.parse(field_str.strip()) + if not parsed: + return False + + return any(isinstance(token, sqlparse.sql.Function) for token in parsed[0].tokens) + + # Pattern to validate field names in SELECT: # Allows: name, `name`, name as alias, `name` as alias, `table name`.`name`, `table name`.`name` as alias, table.name, table.name as alias ALLOWED_FIELD_PATTERN = re.compile(r"^(?:(`[\w\s-]+`|\w+)\.)?(`\w+`|\w+)(?:\s+as\s+\w+)?$", flags=re.ASCII) @@ -732,8 +742,7 @@ class Engine: for item in initial_field_list: if isinstance(item, str): # Sanitize and split potentially comma-separated strings within the list - sanitized_item = _sanitize_field(item.strip(), self.is_mariadb).strip() - if sanitized_item: + if sanitized_item := _sanitize_field(item.strip(), self.is_mariadb).strip(): parsed = self._parse_single_field_item(sanitized_item) if isinstance(parsed, list): # Result from parsing a child query dict _fields.extend(parsed) @@ -1028,8 +1037,8 @@ class Engine: elif hasattr(field, "alias") and field.alias and field.name in permitted_fields_set: allowed_fields.append(field) - elif isinstance(field, AggregateFunction | PseudoColumnMapper): - # Typically functions or complex terms + elif isinstance(field, Term): + # Allow any Term subclass, like LiteralValue (raw SQL expressions), AggregateFunction, PseudoColumnMapper (functions or complex terms) allowed_fields.append(field) return allowed_fields @@ -1555,6 +1564,15 @@ def _validate_select_field(field: str): if field.isdigit(): return + # Reject SQL functions + if _is_function_call(field): + frappe.throw( + _( + "SQL functions are not allowed in SELECT fields: {0}. Use the query builder API with functions instead." + ).format(field), + frappe.ValidationError, + ) + if ALLOWED_FIELD_PATTERN.match(field): return From 49c451068f0606b667e23303114600931bd36b39 Mon Sep 17 00:00:00 2001 From: Akhil Narang Date: Thu, 13 Nov 2025 14:17:31 +0530 Subject: [PATCH 19/38] fix: adjust tests for query builder permission Earlier you could use fields you didn't have access to for filtering, now you can't. Signed-off-by: Akhil Narang --- frappe/tests/test_db_query.py | 117 +++++----------------------------- 1 file changed, 16 insertions(+), 101 deletions(-) diff --git a/frappe/tests/test_db_query.py b/frappe/tests/test_db_query.py index c40d5e9cac..54123ee00d 100644 --- a/frappe/tests/test_db_query.py +++ b/frappe/tests/test_db_query.py @@ -874,128 +874,41 @@ class TestDBQuery(IntegrationTestCase): def test_permlevel_fields(self): with setup_patched_blog_post(), setup_test_user(set_user=True): - data = frappe.get_list( + self.assertRaises( + frappe.PermissionError, + frappe.get_list, "Test Blog Post", filters={"published": 1}, fields=["name", "published"], limit=1, ) - self.assertFalse("published" in data[0]) - self.assertTrue("name" in data[0]) - self.assertEqual(len(data[0]), 1) - data = frappe.get_list( + self.assertRaises( + frappe.PermissionError, + frappe.get_list, "Test Blog Post", filters={"published": 1}, fields=["name", "`published`"], limit=1, ) - self.assertFalse("published" in data[0]) - self.assertTrue("name" in data[0]) - self.assertEqual(len(data[0]), 1) - data = frappe.get_list( + self.assertRaises( + frappe.PermissionError, + frappe.get_list, "Test Blog Post", filters={"published": 1}, fields=["name", "`tabTest Blog Post`.`published`"], limit=1, ) - self.assertFalse("published" in data[0]) - self.assertTrue("name" in data[0]) - self.assertEqual(len(data[0]), 1) - data = frappe.get_list( + self.assertRaises( + frappe.PermissionError, + frappe.get_list, "Test Blog Post", filters={"published": 1}, fields=["name", "`tabTest Child`.`test_field`"], limit=1, ) - self.assertFalse("test_field" in data[0]) - self.assertTrue("name" in data[0]) - self.assertEqual(len(data[0]), 1) - - data = frappe.get_list( - "Test Blog Post", - filters={"published": 1}, - fields=["name", "MAX(`published`)"], - limit=1, - ) - self.assertTrue("name" in data[0]) - self.assertEqual(len(data[0]), 1) - - data = frappe.get_list( - "Test Blog Post", - filters={"published": 1}, - fields=["name", "LAST(published)"], - limit=1, - ) - self.assertTrue("name" in data[0]) - self.assertEqual(len(data[0]), 1) - - data = frappe.get_list( - "Test Blog Post", - filters={"published": 1}, - fields=["name", "MAX(`modified`)"], - limit=1, - order_by=None, - group_by="name", - ) - self.assertEqual(len(data[0]), 2) - - data = frappe.get_list( - "Test Blog Post", - filters={"published": 1}, - fields=["name", "now() abhi"], - limit=1, - ) - self.assertIsInstance(data[0]["abhi"], datetime.datetime) - self.assertEqual(len(data[0]), 2) - - data = frappe.get_list( - "Test Blog Post", - filters={"published": 1}, - fields=["name", "'LABEL'"], - limit=1, - ) - self.assertTrue("name" in data[0]) - self.assertTrue("LABEL" in data[0].values()) - self.assertEqual(len(data[0]), 2) - - data = frappe.get_list( - "Test Blog Post", - filters={"published": 1}, - fields=["name", "COUNT(*) as count"], - limit=1, - order_by=None, - group_by="name", - ) - self.assertTrue("count" in data[0]) - self.assertEqual(len(data[0]), 2) - - data = frappe.get_list( - "Test Blog Post", - filters={"published": 1}, - fields=["name", "COUNT(*) count"], - limit=1, - order_by=None, - group_by="name", - ) - self.assertTrue("count" in data[0]) - self.assertEqual(len(data[0]), 2) - - data = frappe.get_list( - "Test Blog Post", - fields=[ - "name", - "blogger.full_name as blogger_full_name", - "blog_category.title", - ], - limit=1, - ) - print(data[0]) - self.assertTrue("name" in data[0]) - self.assertTrue("blogger_full_name" in data[0]) - self.assertTrue("title" in data[0]) def test_cast_name(self): from frappe.core.doctype.doctype.test_doctype import new_doctype @@ -1433,8 +1346,10 @@ class TestReportView(IntegrationTestCase): response = execute_cmd("frappe.desk.reportview.get") self.assertNotIn("published", response["keys"]) - # If none of the fields are accessible then result should be empty - self.assertEqual(frappe.get_list("Test Blog Post", "published"), []) + data = frappe.get_list("Test Blog Post", "published") + self.assertTrue(len(data) > 0) + self.assertTrue(all("name" in row for row in data)) + self.assertTrue(all("published" not in row for row in data)) def test_reportview_get_admin(self): # Admin should be able to see access all fields From 08e7a72ba268fa7be271eb47e9e812c1f40f7a7b Mon Sep 17 00:00:00 2001 From: Akhil Narang Date: Thu, 13 Nov 2025 14:20:28 +0530 Subject: [PATCH 20/38] refactor: uppercase function assertions Signed-off-by: Akhil Narang --- frappe/tests/test_db.py | 4 ++-- frappe/tests/test_db_query.py | 30 +++++++++++++++--------------- 2 files changed, 17 insertions(+), 17 deletions(-) diff --git a/frappe/tests/test_db.py b/frappe/tests/test_db.py index 9481ca3fa3..10e3442cbd 100644 --- a/frappe/tests/test_db.py +++ b/frappe/tests/test_db.py @@ -403,8 +403,8 @@ class TestDB(IntegrationTestCase): random_field, ) self.assertEqual( - next(iter(frappe.get_all("ToDo", fields=[{"COUNT": f"`{random_field}`"}], limit=1)[0])), - "count" if frappe.conf.db_type == "postgres" else f"count(`{random_field}`)", + next(iter(frappe.get_all("ToDo", fields=[{"COUNT": random_field}], limit=1)[0])), + "COUNT" if frappe.conf.db_type == "postgres" else f"COUNT(`{random_field}`)", ) # Testing update diff --git a/frappe/tests/test_db_query.py b/frappe/tests/test_db_query.py index 54123ee00d..f7752223d3 100644 --- a/frappe/tests/test_db_query.py +++ b/frappe/tests/test_db_query.py @@ -1051,28 +1051,28 @@ class TestDBQuery(IntegrationTestCase): self.assertIn("count", result[0]) def test_coalesce_with_in_ops(self): - self.assertNotIn("ifnull", frappe.get_all("User", {"first_name": ("in", ["a", "b"])}, run=0)) - self.assertIn("ifnull", frappe.get_all("User", {"first_name": ("in", ["a", None])}, run=0)) - self.assertIn("ifnull", frappe.get_all("User", {"first_name": ("in", ["a", ""])}, run=0)) - self.assertIn("ifnull", frappe.get_all("User", {"first_name": ("in", [])}, run=0)) - self.assertIn("ifnull", frappe.get_all("User", {"first_name": ("not in", ["a"])}, run=0)) - self.assertIn("ifnull", frappe.get_all("User", {"first_name": ("not in", [])}, run=0)) - self.assertIn("ifnull", frappe.get_all("User", {"first_name": ("not in", [""])}, run=0)) + self.assertNotIn("IF", frappe.get_all("User", {"first_name": ("in", ["a", "b"])}, run=0)) + self.assertIn("IFNULL", frappe.get_all("User", {"first_name": ("in", ["a", None])}, run=0)) + self.assertIn("IFNULL", frappe.get_all("User", {"first_name": ("in", ["a", ""])}, run=0)) + self.assertIn("IFNULL", frappe.get_all("User", {"first_name": ("in", [])}, run=0)) + self.assertIn("IFNULL", frappe.get_all("User", {"first_name": ("not in", ["a"])}, run=0)) + self.assertIn("IFNULL", frappe.get_all("User", {"first_name": ("not in", [])}, run=0)) + self.assertIn("IFNULL", frappe.get_all("User", {"first_name": ("not in", [""])}, run=0)) # primary key is never nullable - self.assertNotIn("ifnull", frappe.get_all("User", {"name": ("in", ["a", None])}, run=0)) - self.assertNotIn("ifnull", frappe.get_all("User", {"name": ("in", ["a", ""])}, run=0)) - self.assertNotIn("ifnull", frappe.get_all("User", {"name": ("in", (""))}, run=0)) - self.assertNotIn("ifnull", frappe.get_all("User", {"name": ("in", ())}, run=0)) + self.assertNotIn("IFNULL", frappe.get_all("User", {"name": ("in", ["a", None])}, run=0)) + self.assertNotIn("IFNULL", frappe.get_all("User", {"name": ("in", ["a", ""])}, run=0)) + self.assertNotIn("IFNULL", frappe.get_all("User", {"name": ("in", (""))}, run=0)) + self.assertNotIn("IFNULL", frappe.get_all("User", {"name": ("in", ())}, run=0)) def test_coalesce_with_datetime_ops(self): - self.assertNotIn("ifnull", frappe.get_all("User", {"last_active": (">", "2022-01-01")}, run=0)) - self.assertNotIn("ifnull", frappe.get_all("User", {"creation": ("<", "2022-01-01")}, run=0)) + self.assertNotIn("IFNULL", frappe.get_all("User", {"last_active": (">", "2022-01-01")}, run=0)) + self.assertNotIn("IFNULL", frappe.get_all("User", {"creation": ("<", "2022-01-01")}, run=0)) self.assertNotIn( - "ifnull", + "IFNULL", frappe.get_all("User", {"last_active": ("between", ("2022-01-01", "2023-01-01"))}, run=0), ) - self.assertIn("ifnull", frappe.get_all("User", {"last_active": ("<", "2022-01-01")}, run=0)) + self.assertIn("IFNULL", frappe.get_all("User", {"last_active": ("<", "2022-01-01")}, run=0)) def test_ambiguous_linked_tables(self): from frappe.desk.reportview import get From ea926b0f3197cf1e618634ad7453adee75aced48 Mon Sep 17 00:00:00 2001 From: Akhil Narang Date: Sat, 8 Nov 2025 16:58:46 +0530 Subject: [PATCH 21/38] fix(search): adjust query formation to align with new restrictions Signed-off-by: Akhil Narang --- frappe/database/query.py | 1 + frappe/desk/search.py | 16 ++++++++++------ 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/frappe/database/query.py b/frappe/database/query.py index ee44f90eee..bb6b8dd949 100644 --- a/frappe/database/query.py +++ b/frappe/database/query.py @@ -1753,6 +1753,7 @@ class SQLFunctionParser: expression = ArithmeticExpression(operator=operator, left=left, right=right) if alias: + self.engine.function_aliases.add(alias) return expression.as_(alias) else: return expression diff --git a/frappe/desk/search.py b/frappe/desk/search.py index 4232ca80f7..b26c240d18 100644 --- a/frappe/desk/search.py +++ b/frappe/desk/search.py @@ -163,11 +163,11 @@ def search_widget( fields = get_std_fields_list(meta, searchfield or "name") if filter_fields: fields = list(set(fields + json.loads(filter_fields))) - formatted_fields = [f"`tab{meta.name}`.`{f.strip()}`" for f in fields] + formatted_fields = [f.strip() for f in fields] # Insert title field query after name if meta.show_title_field_in_link and meta.title_field: - formatted_fields.insert(1, f"`tab{meta.name}`.{meta.title_field} as `label`") + formatted_fields.insert(1, f"{meta.title_field} as label") order_by_based_on_meta = get_order_by(doctype, meta) # `idx` is number of times a document is referred, check link_count.py @@ -176,12 +176,16 @@ def search_widget( if not meta.translated_doctype: _txt = frappe.db.escape((txt or "").replace("%", "").replace("@", "")) # locate returns 0 if string is not found, convert 0 to null and then sort null to end in order by - _relevance = {"DIV": [1, {"NULLIF": [{"LOCATE": [_txt, "name"]}, 0]}], "as": "_relevance"} - formatted_fields.append(f"{_relevance} as _relevance") - # Since we are sorting by alias postgres needs to know number of column we are sorting + _relevance_expr = {"DIV": [1, {"NULLIF": [{"LOCATE": [_txt, "name"]}, 0]}]} + + # For MariaDB, wrap in IFNULL for sorting to push nulls to end if frappe.db.db_type == "mariadb": - order_by = f"ifnull(_relevance, -9999) desc, {order_by}" + _relevance = {"IFNULL": [_relevance_expr, -9999], "as": "_relevance"} + formatted_fields.append(_relevance) + order_by = f"_relevance desc, {order_by}" elif frappe.db.db_type == "postgres": + _relevance = {**_relevance_expr, "as": "_relevance"} + formatted_fields.append(_relevance) # Since we are sorting by alias postgres needs to know number of column we are sorting order_by = f"{len(formatted_fields)} desc nulls last, {order_by}" From 90e4ec29b7550e47f6fd0b48e70700e43619bbf6 Mon Sep 17 00:00:00 2001 From: Akhil Narang Date: Fri, 14 Nov 2025 00:48:01 +0530 Subject: [PATCH 22/38] fix(reportview): temporarily skip validation for dict fields Signed-off-by: Akhil Narang --- frappe/desk/reportview.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/frappe/desk/reportview.py b/frappe/desk/reportview.py index 757a502128..7af8008965 100644 --- a/frappe/desk/reportview.py +++ b/frappe/desk/reportview.py @@ -126,6 +126,10 @@ def validate_fields(data): wildcard = update_wildcard_field_param(data) for field in list(data.fields or []): + # TODO: extract_fieldnames needs to handle dict fields for qb_query aggregations + if isinstance(field, dict): + continue + fieldname = extract_fieldnames(field)[0] if not fieldname: raise_invalid_field(fieldname) From 30d6ebc6c6562bc1e96090a87c18e2a42042a253 Mon Sep 17 00:00:00 2001 From: Akhil Narang Date: Fri, 14 Nov 2025 00:50:24 +0530 Subject: [PATCH 23/38] fix: tests Signed-off-by: Akhil Narang --- frappe/core/doctype/rq_job/test_rq_job.py | 2 +- frappe/tests/test_api.py | 3 +-- frappe/tests/test_db.py | 2 +- frappe/tests/test_db_query.py | 2 +- 4 files changed, 4 insertions(+), 5 deletions(-) diff --git a/frappe/core/doctype/rq_job/test_rq_job.py b/frappe/core/doctype/rq_job/test_rq_job.py index cb9bfd6092..d616526fb1 100644 --- a/frappe/core/doctype/rq_job/test_rq_job.py +++ b/frappe/core/doctype/rq_job/test_rq_job.py @@ -191,7 +191,7 @@ class TestRQJob(IntegrationTestCase): jobs = [frappe.enqueue(method=self.BG_JOB, queue="short", fail=True) for _ in range(limit * 2)] self.check_status(jobs[-1], "failed") - self.assertLessEqual(RQJob.get_count(filters=[["RQ Job", "status", "=", "failed"]]), limit * 1.1) + self.assertLessEqual(RQJob.get_count(filters=[["RQ Job", "status", "=", "failed"]]), limit * 1.2) def test_func(fail=False, sleep=0): diff --git a/frappe/tests/test_api.py b/frappe/tests/test_api.py index eeae179a34..fea624e67d 100644 --- a/frappe/tests/test_api.py +++ b/frappe/tests/test_api.py @@ -230,8 +230,7 @@ class TestResourceAPI(FrappeAPITestCase): def test_get_list_debug(self): # test 5: fetch response with debug - with suppress_stdout(): - response = self.get(self.resource(self.DOCTYPE), {"sid": self.sid, "debug": True}) + response = self.get(self.resource(self.DOCTYPE), {"sid": self.sid, "debug": True}) self.assertEqual(response.status_code, 200) self.assertIn("_debug_messages", response.json) self.assertIsInstance(response.json["_debug_messages"], str) diff --git a/frappe/tests/test_db.py b/frappe/tests/test_db.py index 10e3442cbd..69acfb517a 100644 --- a/frappe/tests/test_db.py +++ b/frappe/tests/test_db.py @@ -132,7 +132,7 @@ class TestDB(IntegrationTestCase): # test multiple orderby's delimiter = '"' if frappe.db.db_type == "postgres" else "`" self.assertIn( - "ORDER BY {deli}creation{deli} DESC,{deli}modified{deli} ASC,{deli}name{deli} DESC".format( + "ORDER BY {deli}creation{deli} DESC,{deli}modified{deli} ASC,{deli}name{deli} ASC".format( deli=delimiter ), frappe.db.get_value("DocType", "DocField", order_by="creation desc, modified asc, name", run=0), diff --git a/frappe/tests/test_db_query.py b/frappe/tests/test_db_query.py index f7752223d3..2d4e84933b 100644 --- a/frappe/tests/test_db_query.py +++ b/frappe/tests/test_db_query.py @@ -1150,7 +1150,7 @@ class TestDBQuery(IntegrationTestCase): def test_ifnull_none(self): query = frappe.get_all("DocField", {"fieldname": None}, run=0) - self.assertIn("''", query) + self.assertIn("IS NULL", query) self.assertNotIn("\\'", query) self.assertNotIn("ifnull", query) self.assertFalse(frappe.get_all("DocField", {"name": None})) From a5e44c4c6ed26ae7b7096fbc023c549b9511f0d4 Mon Sep 17 00:00:00 2001 From: Akhil Narang Date: Fri, 14 Nov 2025 23:22:04 +0530 Subject: [PATCH 24/38] fix(query): check whether filter fields belong to child tables if not part of parent Signed-off-by: Akhil Narang --- frappe/database/query.py | 61 +++++++++++++++++++++++++++++++++++++++ frappe/tests/test_perf.py | 3 +- 2 files changed, 63 insertions(+), 1 deletion(-) diff --git a/frappe/database/query.py b/frappe/database/query.py index bb6b8dd949..24a333084f 100644 --- a/frappe/database/query.py +++ b/frappe/database/query.py @@ -83,6 +83,43 @@ OPERATOR_MAPPING = { } +def _get_table_fields(doctype: str) -> list[dict]: + """Try to get table fields from cached meta, queries DB if not cached.""" + + # Don't use cache during install/migrate/tests + if not (frappe.flags.in_install or frappe.flags.in_migrate or frappe.flags.in_test): + if meta := frappe.client_cache.get_value(f"doctype_meta::{doctype}"): + return [ + {"fieldname": df.fieldname, "options": df.options} + for df in meta.get_table_fields(include_computed=True) + ] + + return frappe.db.sql( + """ + SELECT fieldname, options + FROM tabDocField + WHERE parent = %s AND fieldtype = 'Table' + """, + doctype, + as_dict=True, + ) + + +def _field_exists_in_doctype(doctype: str, fieldname: str) -> bool: + """Check if field exists in doctype, using cache if available.""" + # Don't query client cache during install/migrate/tests + if not (frappe.flags.in_install or frappe.flags.in_migrate or frappe.flags.in_test): + if meta := frappe.client_cache.get_value(f"doctype_meta::{doctype}"): + return meta.has_field(fieldname) + + from frappe.model.meta import get_table_columns + + try: + return fieldname in get_table_columns(doctype) + except frappe.db.TableMissingError: + return False + + class Engine: def get_query( self, @@ -633,6 +670,30 @@ class Engine: return child_field_handler.field else: # Field belongs to the main doctype or doctype wasn't specified differently + # If doctype wasn't specified, and the field isn't a standard field and doesn't exist in main doctype, check child tables + from frappe.model import child_table_fields, default_fields, optional_fields + + if ( + not doctype + and target_fieldname not in default_fields + optional_fields + child_table_fields + and not _field_exists_in_doctype(self.doctype, target_fieldname) + ): + for df in _get_table_fields(self.doctype): + if _field_exists_in_doctype(df["options"], target_fieldname): + # Found in child table, create handler for it + child_field_handler = ChildTableField( + doctype=df["options"], + fieldname=target_fieldname, + parent_doctype=self.doctype, + parent_fieldname=df["fieldname"], + ) + parent_doctype_for_perm = self.doctype + self._check_field_permission( + df["options"], target_fieldname, parent_doctype_for_perm + ) + self.query = child_field_handler.apply_join(self.query) + return child_field_handler.field + self._check_field_permission(target_doctype, target_fieldname, parent_doctype_for_perm) # Convert string field name to pypika Field object for the specified/current doctype return frappe.qb.DocType(target_doctype)[target_fieldname] diff --git a/frappe/tests/test_perf.py b/frappe/tests/test_perf.py index a5575344a1..dc1276a357 100644 --- a/frappe/tests/test_perf.py +++ b/frappe/tests/test_perf.py @@ -80,7 +80,8 @@ class TestPerformance(IntegrationTestCase): with self.assertQueryCount(1): frappe.db.set_value("User", "Administrator", "interest", "Nothing") - with self.assertQueryCount(1): + # TODO: get this back down to one after fixing query builder meta access + with self.assertQueryCount(2): frappe.db.set_value("User", {"user_type": "System User"}, "interest", "Nothing") with self.assertQueryCount(1): From 7ad6f7e2c6498e5c6baf34bc9bc3c946952e5592 Mon Sep 17 00:00:00 2001 From: Akhil Narang Date: Mon, 17 Nov 2025 11:29:40 +0530 Subject: [PATCH 25/38] refactor: ensure no meta recursion Signed-off-by: Akhil Narang --- frappe/core/doctype/doctype/doctype.py | 9 --- .../property_setter/property_setter.py | 9 --- frappe/database/database.py | 19 +++-- frappe/database/query.py | 70 +++++++------------ frappe/database/utils.py | 31 +++----- frappe/model/meta.py | 5 -- frappe/tests/test_perf.py | 3 +- 7 files changed, 48 insertions(+), 98 deletions(-) diff --git a/frappe/core/doctype/doctype/doctype.py b/frappe/core/doctype/doctype/doctype.py index 0723981a0f..2e2f28349b 100644 --- a/frappe/core/doctype/doctype/doctype.py +++ b/frappe/core/doctype/doctype/doctype.py @@ -550,8 +550,6 @@ class DocType(Document): self.sync_doctype_layouts() delete_notification_count_for(doctype=self.name) - self._clear_sort_cache_if_needed() - frappe.clear_cache(doctype=self.name) # clear user cache so that on the next reload this doctype is included in boot @@ -562,13 +560,6 @@ class DocType(Document): clear_linked_doctype_cache() - def _clear_sort_cache_if_needed(self): - """Clear sort cache only if sort_field or sort_order changed.""" - if self.has_value_changed("sort_field") or self.has_value_changed("sort_order"): - from frappe.model.meta import clear_doctype_sort_cache - - clear_doctype_sort_cache(self.name) - @savepoint(catch=Exception) def sync_doctype_layouts(self): """Sync Doctype Layout""" diff --git a/frappe/custom/doctype/property_setter/property_setter.py b/frappe/custom/doctype/property_setter/property_setter.py index 1a5053aa54..0a38a16128 100644 --- a/frappe/custom/doctype/property_setter/property_setter.py +++ b/frappe/custom/doctype/property_setter/property_setter.py @@ -42,11 +42,9 @@ class PropertySetter(Document): if self.is_new(): delete_property_setter(self.doc_type, self.property, self.field_name, self.row_name) - self._clear_sort_cache_if_needed() frappe.clear_cache(doctype=self.doc_type) def on_trash(self): - self._clear_sort_cache_if_needed() frappe.clear_cache(doctype=self.doc_type) def validate_fieldtype_change(self): @@ -62,13 +60,6 @@ class PropertySetter(Document): validate_fields_for_doctype(self.doc_type) - def _clear_sort_cache_if_needed(self): - """Clear sort cache if this property setter modifies sort_field or sort_order.""" - if self.property in ("sort_field", "sort_order") and self.doctype_or_field == "DocType": - from frappe.model.meta import clear_doctype_sort_cache - - clear_doctype_sort_cache(self.doc_type) - def get_permission_log_options(self, event=None): if self.property in ("ignore_user_permissions", "permlevel"): return { diff --git a/frappe/database/database.py b/frappe/database/database.py index 6497ee86b2..d76388fbc8 100644 --- a/frappe/database/database.py +++ b/frappe/database/database.py @@ -11,9 +11,9 @@ import warnings from collections.abc import Iterable, Sequence from contextlib import contextmanager, suppress from time import time -from typing import TYPE_CHECKING, Any +from typing import TYPE_CHECKING, Any, Literal -from pypika.queries import QueryBuilder +from pypika.queries import QueryBuilder, Table import frappe import frappe.defaults @@ -583,11 +583,16 @@ class Database: # single field is requested, send it without wrapping in containers return row[0] - def _convert_default_order_by(self, doctype: str, order_by: str) -> str: + def _convert_default_order_by( + self, doctype: str | Table, order_by: str | Literal["KEEP_DEFAULT_ORDERING"] + ) -> str: """Convert DefaultOrderBy sentinel to explicit order string to avoid meta loading.""" if order_by != DefaultOrderBy: return order_by + if isinstance(doctype, Table): + doctype = doctype.get_table_name()[3:] + sort_field, sort_order = get_doctype_sort_info(doctype) return f"{sort_field} {sort_order.lower()}" @@ -1333,12 +1338,12 @@ class Database: from frappe.utils import now_datetime - Table = frappe.qb.DocType(doctype) + dt = frappe.qb.DocType(doctype) return ( - frappe.qb.from_(Table) - .select(Count(Table.name)) - .where(Table.creation >= now_datetime() - relativedelta(minutes=minutes)) + frappe.qb.from_(dt) + .select(Count(dt.name)) + .where(dt.creation >= now_datetime() - relativedelta(minutes=minutes)) .run()[0][0] ) diff --git a/frappe/database/query.py b/frappe/database/query.py index 24a333084f..c683cf58db 100644 --- a/frappe/database/query.py +++ b/frappe/database/query.py @@ -18,7 +18,12 @@ from frappe.database.utils import ( get_doctype_sort_info, ) from frappe.model import get_permitted_fields +from frappe.model.base_document import DOCTYPES_FOR_DOCTYPE from frappe.query_builder import Criterion, Field, Order, functions + +CORE_DOCTYPES = DOCTYPES_FOR_DOCTYPE | frozenset( + ("Custom Field", "Property Setter", "Module Def", "__Auth", "__global_search", "Singles") +) from frappe.query_builder.utils import PseudoColumnMapper from frappe.utils.data import MARIADB_SPECIFIC_COMMENT @@ -83,43 +88,6 @@ OPERATOR_MAPPING = { } -def _get_table_fields(doctype: str) -> list[dict]: - """Try to get table fields from cached meta, queries DB if not cached.""" - - # Don't use cache during install/migrate/tests - if not (frappe.flags.in_install or frappe.flags.in_migrate or frappe.flags.in_test): - if meta := frappe.client_cache.get_value(f"doctype_meta::{doctype}"): - return [ - {"fieldname": df.fieldname, "options": df.options} - for df in meta.get_table_fields(include_computed=True) - ] - - return frappe.db.sql( - """ - SELECT fieldname, options - FROM tabDocField - WHERE parent = %s AND fieldtype = 'Table' - """, - doctype, - as_dict=True, - ) - - -def _field_exists_in_doctype(doctype: str, fieldname: str) -> bool: - """Check if field exists in doctype, using cache if available.""" - # Don't query client cache during install/migrate/tests - if not (frappe.flags.in_install or frappe.flags.in_migrate or frappe.flags.in_test): - if meta := frappe.client_cache.get_value(f"doctype_meta::{doctype}"): - return meta.has_field(fieldname) - - from frappe.model.meta import get_table_columns - - try: - return fieldname in get_table_columns(doctype) - except frappe.db.TableMissingError: - return False - - class Engine: def get_query( self, @@ -673,23 +641,37 @@ class Engine: # If doctype wasn't specified, and the field isn't a standard field and doesn't exist in main doctype, check child tables from frappe.model import child_table_fields, default_fields, optional_fields + if self.doctype in CORE_DOCTYPES: + meta = None + else: + try: + meta = frappe.get_meta(self.doctype) + except frappe.DoesNotExistError: + meta = None + if ( - not doctype + meta + and not doctype and target_fieldname not in default_fields + optional_fields + child_table_fields - and not _field_exists_in_doctype(self.doctype, target_fieldname) + and not meta.has_field(target_fieldname) ): - for df in _get_table_fields(self.doctype): - if _field_exists_in_doctype(df["options"], target_fieldname): + for df in meta.get_table_fields(include_computed=True): + try: + child_meta = frappe.get_meta(df.options) + except frappe.DoesNotExistError: + continue + + if child_meta.has_field(target_fieldname): # Found in child table, create handler for it child_field_handler = ChildTableField( - doctype=df["options"], + doctype=df.options, fieldname=target_fieldname, parent_doctype=self.doctype, - parent_fieldname=df["fieldname"], + parent_fieldname=df.fieldname, ) parent_doctype_for_perm = self.doctype self._check_field_permission( - df["options"], target_fieldname, parent_doctype_for_perm + df.options, target_fieldname, parent_doctype_for_perm ) self.query = child_field_handler.apply_join(self.query) return child_field_handler.field diff --git a/frappe/database/utils.py b/frappe/database/utils.py index 53bf14d011..478d64790a 100644 --- a/frappe/database/utils.py +++ b/frappe/database/utils.py @@ -60,10 +60,7 @@ def get_doctype_name(table_name: str) -> str: def get_doctype_sort_info(doctype: str) -> tuple[str, str]: """ - Get sort_field and sort_order for a DocType from cache or database. - - This is separate from regular meta to avoid recursive calls. - Caches for a day since sort order won't change often (invalidated on doctype update). + Get sort_field and sort_order for a DocType from meta. Args: doctype: The DocType name @@ -71,26 +68,16 @@ def get_doctype_sort_info(doctype: str) -> tuple[str, str]: Returns: Tuple of (sort_field, sort_order) with defaults ("creation", "DESC") if not found """ + from frappe.database.query import CORE_DOCTYPES - cache_key = f"doctype_sort_info::{doctype}" + if doctype in CORE_DOCTYPES: + return "creation", "DESC" - if cached := frappe.cache.get_value(cache_key): - sort_field, sort_order = cached - else: - sort_field, sort_order = None, None - if result := frappe.db.sql( - "SELECT sort_field, sort_order FROM tabDocType WHERE name = %s", - (doctype,), - ): - sort_field, sort_order = result[0] - - if not sort_field: - sort_field = "creation" - if not sort_order: - sort_order = "DESC" - frappe.cache.set_value(cache_key, (sort_field, sort_order), expires_in_sec=86400) - - return sort_field, sort_order + try: + meta = frappe.get_meta(doctype) + return meta.sort_field or "creation", meta.sort_order or "DESC" + except frappe.DoesNotExistError: + return "creation", "DESC" class LazyString: diff --git a/frappe/model/meta.py b/frappe/model/meta.py index dd59cecd7b..28c2e134f0 100644 --- a/frappe/model/meta.py +++ b/frappe/model/meta.py @@ -424,11 +424,6 @@ class Meta(Document): self.extend("fields", custom_fields) def apply_property_setters(self): - """ - Property Setters are set via Customize Form. They override standard properties - of the doctype or its child properties like fields, links etc. This method - applies the customized properties over the standard meta object - """ if not frappe.db.table_exists("Property Setter"): return diff --git a/frappe/tests/test_perf.py b/frappe/tests/test_perf.py index dc1276a357..a5575344a1 100644 --- a/frappe/tests/test_perf.py +++ b/frappe/tests/test_perf.py @@ -80,8 +80,7 @@ class TestPerformance(IntegrationTestCase): with self.assertQueryCount(1): frappe.db.set_value("User", "Administrator", "interest", "Nothing") - # TODO: get this back down to one after fixing query builder meta access - with self.assertQueryCount(2): + with self.assertQueryCount(1): frappe.db.set_value("User", {"user_type": "System User"}, "interest", "Nothing") with self.assertQueryCount(1): From 943df998d68ce593c0bcb5631d407cc8d7d51155 Mon Sep 17 00:00:00 2001 From: Akhil Narang Date: Mon, 17 Nov 2025 16:52:19 +0530 Subject: [PATCH 26/38] feat: support certain backticked expressions Signed-off-by: Akhil Narang --- frappe/core/doctype/data_export/exporter.py | 9 +- frappe/core/doctype/report/report.py | 2 +- frappe/database/query.py | 137 ++++++++++++++++++-- frappe/tests/test_query.py | 24 ++-- 4 files changed, 144 insertions(+), 28 deletions(-) diff --git a/frappe/core/doctype/data_export/exporter.py b/frappe/core/doctype/data_export/exporter.py index 5ad7a7c132..a0e2f934da 100644 --- a/frappe/core/doctype/data_export/exporter.py +++ b/frappe/core/doctype/data_export/exporter.py @@ -370,12 +370,11 @@ class DataExporter: order_by = None table_columns = frappe.db.get_table_columns(self.parent_doctype) if "lft" in table_columns and "rgt" in table_columns: - order_by = DocType(self.parent_doctype).lft - + order_by = f"`tab{self.parent_doctype}`.`lft` asc" # get permitted data only - self.data = frappe.qb.get_query( - self.doctype, fields=["*"], filters=self.filters, order_by=order_by - ).run(as_dict=True) + self.data = frappe.get_list( + self.doctype, fields=["*"], filters=self.filters, limit_page_length=None, order_by=order_by + ) for doc in self.data: op = self.docs_to_export.get("op") diff --git a/frappe/core/doctype/report/report.py b/frappe/core/doctype/report/report.py index c27280a78d..2defabc21a 100644 --- a/frappe/core/doctype/report/report.py +++ b/frappe/core/doctype/report/report.py @@ -291,7 +291,7 @@ class Report(Document): @staticmethod def _format(parts): # sort by is saved as DocType.fieldname, covert it to sql - return parts[1] + return "`tab{}`.`{}`".format(*parts) def get_standard_report_columns(self, params): if params.get("fields"): diff --git a/frappe/database/query.py b/frappe/database/query.py index c683cf58db..50519f17fa 100644 --- a/frappe/database/query.py +++ b/frappe/database/query.py @@ -49,8 +49,15 @@ def _is_function_call(field_str: str) -> bool: # Pattern to validate field names in SELECT: -# Allows: name, `name`, name as alias, `name` as alias, `table name`.`name`, `table name`.`name` as alias, table.name, table.name as alias -ALLOWED_FIELD_PATTERN = re.compile(r"^(?:(`[\w\s-]+`|\w+)\.)?(`\w+`|\w+)(?:\s+as\s+\w+)?$", flags=re.ASCII) +# Allows: name, `name`, name as alias, `name` as alias, table.name, table.name as alias +# Also allows backtick-qualified identifiers with spaces/hyphens: +# - `tabTable`.`field` +# - `tabTable Name`.`field` (spaces in table name) +# - `tabTable-Field`.`field` (hyphens in table name) +# - Any of above with aliases: ... as alias +ALLOWED_FIELD_PATTERN = re.compile( + r"^(?:(`[\w\s-]+`|\w+)\.)?(`[\w\s-]+`|\w+)(?:\s+as\s+\w+)?$", flags=re.ASCII | re.IGNORECASE +) # Regex to parse field names: # Group 1: Optional quote for table name @@ -551,10 +558,17 @@ class Engine: # return if field is already a pypika Term return field - # Reject backticks + # Parse backtick table.field notation: `tabDocType`.`fieldname` if "`" in field: + if parsed := self._parse_backtick_field_notation(field): + table_name, field_name = parsed + + # Return query builder field reference + return frappe.qb.DocType(table_name)[field_name] + + # If parsing failed, fall through to error handling below frappe.throw( - _("Filter fields cannot contain backticks (`)."), + _("Filter fields have invalid backtick notation: {0}").format(field), frappe.ValidationError, title=_("Invalid Filter"), ) @@ -743,7 +757,9 @@ class Engine: # Ensure the extracted table name is valid before creating DocType object if not TABLE_NAME_PATTERN.match(table_name.lstrip("tab")): frappe.throw(_("Invalid characters in table name: {0}").format(table_name)) - table_obj = frappe.qb.DocType(table_name) + + doctype_name = table_name[3:] if table_name.startswith("tab") else table_name + table_obj = frappe.qb.DocType(doctype_name) pypika_field = table_obj[field_name] else: # Simple field name (e.g., `y` or y) - use the main table @@ -802,10 +818,11 @@ class Engine: return _fields def _parse_single_field_item( - self, field: str | Criterion | dict | Field + self, field: str | Criterion | dict | Field | Term ) -> "list | Criterion | Field | DynamicTableField | ChildQuery | None": """Parses a single item from the fields list/tuple. Assumes comma-separated strings have already been split.""" - if isinstance(field, Criterion | Field): + if isinstance(field, Term): + # Accept any pypika Term (Field, Criterion, ArithmeticExpression, AggregateFunction, etc.) return field elif isinstance(field, dict): # Check if it's a SQL function or operator dictionary @@ -879,6 +896,76 @@ class Engine: order_direction = Order.desc if sort_order.lower() == "desc" else Order.asc self.query = self.query.orderby(field, order=order_direction) + def _parse_backtick_field_notation(self, field_name: str) -> tuple[str, str] | None: + """ + Parse backtick field notation like `tabDocType`.`fieldname` or `tabDocType`.fieldname and return (table_name, field_name). + Uses sqlparse for robust SQL parsing with Identifier support. + Returns None if the notation is invalid. + """ + import sqlparse + from sqlparse.sql import Identifier + + # Parse the field name as SQL + parsed = sqlparse.parse(field_name.strip()) + if not parsed or not parsed[0].tokens: + return None + + tokens = parsed[0].tokens + + # Filter out whitespace tokens + non_ws_tokens = [t for t in tokens if not t.is_whitespace] + + if len(non_ws_tokens) != 1: + return None + + # Check if it's an Identifier (which handles table.field notation) + first_token = non_ws_tokens[0] + if not isinstance(first_token, Identifier): + return None + + # Get the sub-tokens within the identifier + # Should have: `tabTable` (Name), `.` (Punctuation), `fieldname` (Name) + identifier_tokens = [t for t in first_token.tokens if not t.is_whitespace] + + if len(identifier_tokens) != 3: + return None + + table_token = identifier_tokens[0] + dot_token = identifier_tokens[1] + field_token = identifier_tokens[2] + + # Verify the dot + if str(dot_token).strip() != ".": + return None + + # Extract and validate table name (should be backtick-quoted) + table_str = str(table_token).strip() + if not (table_str.startswith("`") and table_str.endswith("`")): + return None + + # Extract field name (can be backtick-quoted or unquoted) + field_str = str(field_token).strip() + # Remove backticks if present + if field_str.startswith("`") and field_str.endswith("`"): + field_str = field_str[1:-1] + + # Remove backticks from table name + table_name = table_str[1:-1] + field_name = field_str + + # Validate table name starts with "tab" + if not table_name.startswith("tab"): + return None + + # Extract doctype name by stripping "tab" prefix + doctype_name = table_name[3:] + + # Validate doctype name is not empty and table actually exists + if not doctype_name or not frappe.db.table_exists(doctype_name): + return None + + return (doctype_name, field_name) + def _validate_and_parse_field_for_clause(self, field_name: str, clause_name: str) -> Field: """ Common helper to validate and parse field names for GROUP BY and ORDER BY clauses. @@ -898,10 +985,15 @@ class Engine: if field_name in self.function_aliases: return Field(field_name) - # Reject backticks + # Parse backtick table.field notation: `tabDocType`.`fieldname` if "`" in field_name: + if parsed := self._parse_backtick_field_notation(field_name): + table_name, field_name = parsed + return frappe.qb.DocType(table_name)[field_name] + + # If parsing failed, fall through to error handling below frappe.throw( - _("{0} fields cannot contain backticks (`): {1}").format(clause_name, field_name), + _("{0} has invalid backtick notation: {1}").format(clause_name, field_name), frappe.ValidationError, ) @@ -967,11 +1059,16 @@ class Engine: for declaration in order_by.split(","): if _order_by := declaration.strip(): + # Extract direction from end of declaration (handles backtick identifiers with spaces) + # Check if the last word is a valid direction parts = _order_by.split() - field_name = parts[0] direction = None - if len(parts) > 1: - direction = parts[1].lower() + field_name = _order_by + + if len(parts) > 1 and parts[-1].lower() in valid_directions: + # Last part is a direction, so field_name is everything before it + direction = parts[-1].lower() + field_name = " ".join(parts[:-1]) order_direction = Order.desc if direction == "desc" else Order.asc @@ -980,7 +1077,7 @@ class Engine: if direction and direction not in valid_directions: frappe.throw( - _("Invalid direction in Order By: {0}. Must be 'ASC' or 'DESC'.").format(parts[1]), + _("Invalid direction in Order By: {0}. Must be 'ASC' or 'DESC'.").format(direction), ValueError, ) @@ -1852,6 +1949,20 @@ class SQLFunctionParser: if self._is_string_literal(arg): return self._validate_string_literal(arg) + # Check for backtick notation: `tabDocType`.`fieldname` + # Parse and return as Field object to preserve field reference in operators + elif "`" in arg: + if parsed := self.engine._parse_backtick_field_notation(arg): + table_name, field_name = parsed + return Table(f"tab{table_name}")[field_name] + else: + frappe.throw( + _( + "Invalid argument format: {0}. Only quoted string literals or simple field names are allowed." + ).format(arg), + frappe.ValidationError, + ) + elif self._is_valid_field_name(arg): # Validate field name and check permissions self._validate_function_field_arg(arg) diff --git a/frappe/tests/test_query.py b/frappe/tests/test_query.py index 2cc03c7293..def1effe4d 100644 --- a/frappe/tests/test_query.py +++ b/frappe/tests/test_query.py @@ -197,11 +197,10 @@ class TestQuery(IntegrationTestCase): def test_field_validation_filters(self): """Test validation for fields used in filters (WHERE clause).""" - valid_fields = ["name", "creation", "language.name"] + valid_fields = ["name", "creation", "language.name", "`tabUser`.`name`"] # Filters should not allow aliases or functions directly as field names invalid_fields = [ "tabUser.name", - "`tabUser`.`name`", "name as alias", "`name` as alias", "tabUser.name as alias", @@ -248,6 +247,7 @@ class TestQuery(IntegrationTestCase): "1", # Allow numeric indices "name, email", "1, 2", + "`tabUser`.`name`", ] # GROUP BY should not allow aliases or functions invalid_fields = [ @@ -262,7 +262,6 @@ class TestQuery(IntegrationTestCase): "table.invalid-field", "tabUser.name", "`name`", - "`tabUser`.`name`", "`name`, `tabUser`.`email`", "`table`.`invalid-field`", "field with space", @@ -293,6 +292,8 @@ class TestQuery(IntegrationTestCase): "2 DESC", "name, email", "1 asc, 2 desc", + "`tabUser`.`name`", + "`tabUser`.`name` desc", ] # ORDER BY should not allow aliases or functions, or invalid directions invalid_fields = [ @@ -305,10 +306,8 @@ class TestQuery(IntegrationTestCase): "name /* comment */", "`name`", "tabUser.name", - "`tabUser`.`name`", "`name` DESC", "tabUser.name Asc", - "`tabUser`.`name` desc", "`name` asc, `tabUser`.`email` DESC", "invalid-field-name", "table.invalid-field", @@ -1629,18 +1628,25 @@ class TestQuery(IntegrationTestCase): frappe.qb.get_query("User", order_by=field).get_sql() def test_backtick_rejection_group_order(self): - """Test that backticks are properly rejected in GROUP BY and ORDER BY.""" + """Test that malformed backticks are properly rejected in GROUP BY and ORDER BY.""" + # Test single backtick (invalid notation - should be `tabTable`.`field`) with self.assertRaises(frappe.ValidationError) as cm: frappe.qb.get_query("User", group_by="`name`").get_sql() - self.assertIn("cannot contain backticks", str(cm.exception)) + self.assertIn("invalid backtick notation", str(cm.exception)) + # Test single backtick with direction (invalid notation) with self.assertRaises(frappe.ValidationError) as cm: frappe.qb.get_query("User", order_by="`name` ASC").get_sql() - self.assertIn("cannot contain backticks", str(cm.exception)) + self.assertIn("invalid backtick notation", str(cm.exception)) + # Test multiple single backticks (invalid notation) with self.assertRaises(frappe.ValidationError) as cm: frappe.qb.get_query("User", group_by="`name`, `email`").get_sql() - self.assertIn("cannot contain backticks", str(cm.exception)) + self.assertIn("invalid backtick notation", str(cm.exception)) + + # Valid backtick notation should work + frappe.qb.get_query("User", group_by="`tabUser`.`name`").get_sql() + frappe.qb.get_query("User", order_by="`tabUser`.`name` ASC").get_sql() def test_sql_functions_in_fields(self): """Test SQL function support in fields with various syntaxes.""" From 1ba9c14cd5166e7c4d7a528b26ebbdc60105792b Mon Sep 17 00:00:00 2001 From: Akhil Narang Date: Mon, 17 Nov 2025 20:36:52 +0530 Subject: [PATCH 27/38] fix: check for shared documents Signed-off-by: Akhil Narang --- frappe/database/query.py | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/frappe/database/query.py b/frappe/database/query.py index 50519f17fa..e2860a3fc3 100644 --- a/frappe/database/query.py +++ b/frappe/database/query.py @@ -1095,9 +1095,12 @@ class Engine: ) if not has_permission("select") and not has_permission("read"): - frappe.throw( - _("Insufficient Permission for {0}").format(frappe.bold(self.doctype)), frappe.PermissionError - ) + # Check for shared documents + if not frappe.share.get_shared(self.doctype, self.user): + frappe.throw( + _("Insufficient Permission for {0}").format(frappe.bold(self.doctype)), + frappe.PermissionError, + ) def apply_field_permissions(self): """Filter the list of fields based on permlevel.""" @@ -1253,6 +1256,17 @@ class Engine: user_perm_conditions, fetch_shared = self.get_user_permission_conditions(role_permissions) conditions.extend(user_perm_conditions) fetch_shared_docs = fetch_shared_docs or fetch_shared + else: + # No role permissions - check if user has any user permissions + # If not, must check for shared documents (like db_query does) + user_permissions = frappe.permissions.get_user_permissions(self.user) + doctype_user_permissions = user_permissions.get(self.doctype, []) + has_user_perm = any( + not perm.get("applicable_for") or perm.get("applicable_for") == self.reference_doctype + for perm in doctype_user_permissions + ) + if not has_user_perm: + fetch_shared_docs = True permission_query_conditions = self.get_permission_query_conditions() if permission_query_conditions: From 07e8c987cb082b4bb50fd79e8eab929f12eb13aa Mon Sep 17 00:00:00 2001 From: Akhil Narang Date: Tue, 18 Nov 2025 12:25:05 +0530 Subject: [PATCH 28/38] fix(check_field_permissions): allow if no permissions defined Signed-off-by: Akhil Narang --- frappe/database/query.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/frappe/database/query.py b/frappe/database/query.py index e2860a3fc3..b0d585db91 100644 --- a/frappe/database/query.py +++ b/frappe/database/query.py @@ -699,6 +699,11 @@ class Engine: if not self.apply_permissions: return + # Skip field permission check if doctype has no permissions defined + meta = frappe.get_meta(doctype) + if not meta.get_permissions(parenttype=parent_doctype): + return + permission_type = self.get_permission_type(doctype) permitted_fields = get_permitted_fields( doctype=doctype, From ccb675b0f24abc895102019524470fa8e4702d23 Mon Sep 17 00:00:00 2001 From: Akhil Narang Date: Tue, 18 Nov 2025 15:09:31 +0530 Subject: [PATCH 29/38] fix(list): fix `distinct` usage Signed-off-by: Akhil Narang --- frappe/www/list.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/frappe/www/list.py b/frappe/www/list.py index 80b548b347..39ec720011 100644 --- a/frappe/www/list.py +++ b/frappe/www/list.py @@ -239,8 +239,10 @@ def get_list( if not filters: filters = [] + distinct = False if not fields: - fields = "distinct *" + fields = "*" + distinct = True if or_filters is None: or_filters = [] @@ -267,4 +269,5 @@ def get_list( limit_page_length=limit_page_length, ignore_permissions=ignore_permissions, order_by=order_by, + distinct=distinct, ) From 0bb1b4477ced992a9c38df585e1782fadb314afc Mon Sep 17 00:00:00 2001 From: Akhil Narang Date: Tue, 18 Nov 2025 15:10:15 +0530 Subject: [PATCH 30/38] fix(query): case insensitive check Signed-off-by: Akhil Narang --- frappe/database/query.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/frappe/database/query.py b/frappe/database/query.py index b0d585db91..09fe6309de 100644 --- a/frappe/database/query.py +++ b/frappe/database/query.py @@ -535,10 +535,12 @@ class Engine: field, value, operator, doctype = None, None, None, None # Determine structure based on length and types - if len(condition) == 3 and isinstance(condition[1], str) and condition[1] in OPERATOR_MAP: + if len(condition) == 3 and isinstance(condition[1], str) and condition[1].lower() in OPERATOR_MAP: # [field, operator, value] field, operator, value = condition - elif len(condition) == 4 and isinstance(condition[2], str) and condition[2] in OPERATOR_MAP: + elif ( + len(condition) == 4 and isinstance(condition[2], str) and condition[2].lower() in OPERATOR_MAP + ): # [doctype, field, operator, value] doctype, field, operator, value = condition elif len(condition) == 2: From 978edeaa1d5e517db2595bdeef40e467b6b954f6 Mon Sep 17 00:00:00 2001 From: Akhil Narang Date: Tue, 18 Nov 2025 16:20:44 +0530 Subject: [PATCH 31/38] feat: add in some functions Signed-off-by: Akhil Narang --- frappe/database/query.py | 4 ++++ frappe/query_builder/custom.py | 17 ++++++++++++++++- frappe/query_builder/functions.py | 10 +++++++++- 3 files changed, 29 insertions(+), 2 deletions(-) diff --git a/frappe/database/query.py b/frappe/database/query.py index 09fe6309de..e211ca979e 100644 --- a/frappe/database/query.py +++ b/frappe/database/query.py @@ -20,6 +20,7 @@ from frappe.database.utils import ( from frappe.model import get_permitted_fields from frappe.model.base_document import DOCTYPES_FOR_DOCTYPE from frappe.query_builder import Criterion, Field, Order, functions +from frappe.query_builder.custom import Month, MonthName, Quarter CORE_DOCTYPES = DOCTYPES_FOR_DOCTYPE | frozenset( ("Custom Field", "Property Setter", "Module Def", "__Auth", "__global_search", "Singles") @@ -81,6 +82,9 @@ FUNCTION_MAPPING = { "CONCAT": functions.Concat, "NOW": functions.Now, "NULLIF": functions.NullIf, + "MONTHNAME": MonthName, + "QUARTER": Quarter, + "MONTH": Month, } # Mapping from operator names to pypika Arithmetic enum values diff --git a/frappe/query_builder/custom.py b/frappe/query_builder/custom.py index e209852891..faa1c4b37f 100644 --- a/frappe/query_builder/custom.py +++ b/frappe/query_builder/custom.py @@ -1,6 +1,6 @@ from typing import Any -from pypika.functions import DistinctOptionFunction +from pypika.functions import DistinctOptionFunction, Function from pypika.terms import Term from pypika.utils import builder, format_alias_sql, format_quotes @@ -98,3 +98,18 @@ class ConstantColumn(Term): quote_char=quote_char, **kwargs, ) + + +class MonthName(Function): + def __init__(self, field, alias=None): + super().__init__("MONTHNAME", field, alias=alias) + + +class Quarter(Function): + def __init__(self, field, alias=None): + super().__init__("QUARTER", field, alias=alias) + + +class Month(Function): + def __init__(self, field, alias=None): + super().__init__("MONTH", field, alias=alias) diff --git a/frappe/query_builder/functions.py b/frappe/query_builder/functions.py index 8ef3f7ddcb..7be63576a1 100644 --- a/frappe/query_builder/functions.py +++ b/frappe/query_builder/functions.py @@ -5,7 +5,15 @@ from pypika.functions import * from pypika.terms import Arithmetic, ArithmeticExpression, CustomFunction, Function import frappe -from frappe.query_builder.custom import GROUP_CONCAT, MATCH, STRING_AGG, TO_TSVECTOR +from frappe.query_builder.custom import ( + GROUP_CONCAT, + MATCH, + STRING_AGG, + TO_TSVECTOR, + Month, + MonthName, + Quarter, +) from frappe.query_builder.utils import ImportMapper, db_type_is from .utils import PseudoColumn From 9455721c776b0da325469e28f64ddfe7f1c2aa33 Mon Sep 17 00:00:00 2001 From: Akhil Narang Date: Tue, 18 Nov 2025 16:56:01 +0530 Subject: [PATCH 32/38] fix: skip group by permission check for certain columns Signed-off-by: Akhil Narang --- frappe/database/query.py | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/frappe/database/query.py b/frappe/database/query.py index e211ca979e..24edd01c3f 100644 --- a/frappe/database/query.py +++ b/frappe/database/query.py @@ -21,12 +21,14 @@ from frappe.model import get_permitted_fields from frappe.model.base_document import DOCTYPES_FOR_DOCTYPE from frappe.query_builder import Criterion, Field, Order, functions from frappe.query_builder.custom import Month, MonthName, Quarter +from frappe.query_builder.utils import PseudoColumnMapper +from frappe.utils.data import MARIADB_SPECIFIC_COMMENT CORE_DOCTYPES = DOCTYPES_FOR_DOCTYPE | frozenset( ("Custom Field", "Property Setter", "Module Def", "__Auth", "__global_search", "Singles") ) -from frappe.query_builder.utils import PseudoColumnMapper -from frappe.utils.data import MARIADB_SPECIFIC_COMMENT + +OPTIONAL_COLUMNS = frozenset(["_user_tags", "_comments", "_assign", "_liked_by", "_seen"]) if TYPE_CHECKING: from frappe.query_builder import DocType @@ -719,6 +721,9 @@ class Engine: user=self.user, ) + if fieldname in OPTIONAL_COLUMNS: + return + if fieldname not in permitted_fields: frappe.throw( _("You do not have permission to access field: {0}").format( @@ -1055,8 +1060,12 @@ class Engine: if not field_name: continue - parsed_field = self._validate_and_parse_field_for_clause(field_name, "Group By") - parsed_fields.append(parsed_field) + # Skip permission check for optional system columns in group_by + if field_name in OPTIONAL_COLUMNS: + parsed_fields.append(self.table[field_name]) + else: + parsed_field = self._validate_and_parse_field_for_clause(field_name, "Group By") + parsed_fields.append(parsed_field) return parsed_fields From 952b0d4500893361e0c810c4afcf5e6d6da2a20f Mon Sep 17 00:00:00 2001 From: Akhil Narang Date: Tue, 18 Nov 2025 17:34:47 +0530 Subject: [PATCH 33/38] chore: test Signed-off-by: Akhil Narang --- frappe/database/utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/database/utils.py b/frappe/database/utils.py index 478d64790a..0ceb9bd440 100644 --- a/frappe/database/utils.py +++ b/frappe/database/utils.py @@ -34,7 +34,7 @@ def convert_to_value(o: FilterValue): return int(o) elif isinstance(o, dict): return frappe.as_json(o) - elif isinstance(o, (list, tuple, set, KeysView, ValuesView)): + elif isinstance(o, (KeysView, ValuesView)): return tuple(convert_to_value(item) for item in o) return o From bd84d7a66ad92a39118256788e121ba2d90bece9 Mon Sep 17 00:00:00 2001 From: Akhil Narang Date: Tue, 18 Nov 2025 18:18:53 +0530 Subject: [PATCH 34/38] fix: match db_query behaviour for certain cases like `filters.append(["reports_to", "=", ""])` Earlier this generated: ``` ( `tabEmployee`.`reports_to` is NULL OR `tabEmployee`.`reports_to` = '' ) ``` Without this change, with qb it was ``` `reports_to`='' ``` Signed-off-by: Akhil Narang --- frappe/database/query.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/frappe/database/query.py b/frappe/database/query.py index 24edd01c3f..9c09bda25c 100644 --- a/frappe/database/query.py +++ b/frappe/database/query.py @@ -1420,7 +1420,12 @@ class Engine: if value is None: return False - if operator.lower() in ("=", "like", "is"): + if operator.lower() in ("like", "is"): + return False + + # For "=" operator, only skip IFNULL if value is truthy (non-empty string, non-zero, etc) + # When value is empty string "", we need to check for NULL values too + if operator.lower() == "=" and value: return False try: From 2c15bb4a5be585b9eee74dd07013490d58ce7d88 Mon Sep 17 00:00:00 2001 From: Akhil Narang Date: Tue, 18 Nov 2025 23:34:46 +0530 Subject: [PATCH 35/38] fix(query): extend regex for allow backticked aliases For example: ``` `tabSerial and Batch Entry`.`name` as `child_row` ``` Signed-off-by: Akhil Narang --- frappe/database/query.py | 3 ++- frappe/tests/test_query.py | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/frappe/database/query.py b/frappe/database/query.py index 9c09bda25c..a5de9c0064 100644 --- a/frappe/database/query.py +++ b/frappe/database/query.py @@ -59,7 +59,8 @@ def _is_function_call(field_str: str) -> bool: # - `tabTable-Field`.`field` (hyphens in table name) # - Any of above with aliases: ... as alias ALLOWED_FIELD_PATTERN = re.compile( - r"^(?:(`[\w\s-]+`|\w+)\.)?(`[\w\s-]+`|\w+)(?:\s+as\s+\w+)?$", flags=re.ASCII | re.IGNORECASE + r"^(?:(`[\w\s-]+`|\w+)\.)?(`[\w\s-]+`|\w+)(?:\s+as\s+(?:`[\w\s-]+`|\w+))?$", + flags=re.ASCII | re.IGNORECASE, ) # Regex to parse field names: diff --git a/frappe/tests/test_query.py b/frappe/tests/test_query.py index def1effe4d..02d5ed1f1e 100644 --- a/frappe/tests/test_query.py +++ b/frappe/tests/test_query.py @@ -153,7 +153,9 @@ class TestQuery(IntegrationTestCase): "`tabUser`.`name` as alias", "*", "`tabHas Role`.`name`", + "field as `alias with space`", ] + invalid_fields = [ "name; DROP TABLE users", "`name` ; SELECT * FROM secrets", @@ -166,7 +168,6 @@ class TestQuery(IntegrationTestCase): "field with space", "`field with space`", "field as alias with space", - "field as `alias with space`", "COUNT(*)", "COUNT(name)", "SUM(amount) as total", From 15588de6cd92534dafe4d27e161952ad0289c4d9 Mon Sep 17 00:00:00 2001 From: Akhil Narang Date: Tue, 18 Nov 2025 18:47:21 +0530 Subject: [PATCH 36/38] fix(query): ensure backwards compatibility for sorting, filtering If `db_query_compat=True` (set by `qb_query.py`), then we default to some `db_query.py` behaviour. Otherwise, we'll retail the previous query builder behaviour, this is to minimize breakage on either side. Signed-off-by: Akhil Narang --- frappe/database/database.py | 19 ++----------------- frappe/database/query.py | 35 +++++++++++++++++++++++++++++++---- frappe/model/qb_query.py | 1 + frappe/tests/test_db.py | 6 +++--- frappe/tests/test_query.py | 4 ++-- 5 files changed, 39 insertions(+), 26 deletions(-) diff --git a/frappe/database/database.py b/frappe/database/database.py index d76388fbc8..e5274d0174 100644 --- a/frappe/database/database.py +++ b/frappe/database/database.py @@ -583,19 +583,6 @@ class Database: # single field is requested, send it without wrapping in containers return row[0] - def _convert_default_order_by( - self, doctype: str | Table, order_by: str | Literal["KEEP_DEFAULT_ORDERING"] - ) -> str: - """Convert DefaultOrderBy sentinel to explicit order string to avoid meta loading.""" - if order_by != DefaultOrderBy: - return order_by - - if isinstance(doctype, Table): - doctype = doctype.get_table_name()[3:] - - sort_field, sort_order = get_doctype_sort_info(doctype) - return f"{sort_field} {sort_order.lower()}" - def get_values( self, doctype: str, @@ -644,8 +631,6 @@ class Database: if isinstance(filters, list): if filters := list(f for f in filters if f is not None): - order_by = self._convert_default_order_by(doctype, order_by) - out = frappe.qb.get_query( table=doctype, fields=fieldname, @@ -663,8 +648,8 @@ class Database: else: if (filters is not None) and (filters != doctype or doctype == "DocType"): try: - order_by = self._convert_default_order_by(doctype, order_by) - + if order_by: + order_by = "creation" if order_by == DefaultOrderBy else order_by query = frappe.qb.get_query( table=doctype, filters=filters, diff --git a/frappe/database/query.py b/frappe/database/query.py index a5de9c0064..a9ca8ae5f8 100644 --- a/frappe/database/query.py +++ b/frappe/database/query.py @@ -108,7 +108,6 @@ class Engine: table: str | Table, fields: str | list | tuple | None = None, filters: dict[str, FilterValue] | FilterValue | list[list | FilterValue] | None = None, - or_filters: dict[str, FilterValue] | FilterValue | list[list | FilterValue] | None = None, order_by: str | None = None, group_by: str | None = None, limit: int | None = None, @@ -126,7 +125,16 @@ class Engine: user: str | None = None, parent_doctype: str | None = None, reference_doctype: str | None = None, + or_filters: dict[str, FilterValue] | FilterValue | list[list | FilterValue] | None = None, + db_query_compat: bool = False, ) -> QueryBuilder: + """Build a query with optional compatibility mode for legacy db_query behavior. + + Args: + db_query_compat: When True, uses legacy db_query behavior for sorting and filtering. + This is kept optional to not break existing code that relies on the original query builder behaviour. + """ + qb = frappe.local.qb db_type = frappe.local.db.db_type @@ -139,6 +147,7 @@ class Engine: self.reference_doctype = reference_doctype self.apply_permissions = not ignore_permissions self.function_aliases = set() + self.db_query_compat = db_query_compat if isinstance(table, Table): self.table = table @@ -391,6 +400,11 @@ class Engine: if not _value and isinstance(_value, list | tuple | set): _value = ("",) + # db_query compatibility: handle None values for 'in' and 'not in' operators + # In db_query, None values are converted to empty tuples for these operators + if self.db_query_compat and _value is None and _operator.casefold() in ("in", "not in"): + _value = ("",) + if _operator in NESTED_SET_OPERATORS: hierarchy = _operator docname = _value @@ -906,11 +920,17 @@ class Engine: field_name = parts[0] spec_order = parts[1].lower() if len(parts) > 1 else sort_order.lower() field = self.table[field_name] - order_direction = Order.desc if spec_order == "desc" else Order.asc + if self.db_query_compat: + order_direction = Order.desc if spec_order == "desc" else Order.asc + else: + order_direction = Order.asc if spec_order == "asc" else Order.desc self.query = self.query.orderby(field, order=order_direction) else: field = self.table[sort_field] - order_direction = Order.desc if sort_order.lower() == "desc" else Order.asc + if self.db_query_compat: + order_direction = Order.desc if sort_order.lower() == "desc" else Order.asc + else: + order_direction = Order.asc if sort_order.lower() == "asc" else Order.desc self.query = self.query.orderby(field, order=order_direction) def _parse_backtick_field_notation(self, field_name: str) -> tuple[str, str] | None: @@ -1091,7 +1111,10 @@ class Engine: direction = parts[-1].lower() field_name = " ".join(parts[:-1]) - order_direction = Order.desc if direction == "desc" else Order.asc + if self.db_query_compat: + order_direction = Order.desc if direction == "desc" else Order.asc + else: + order_direction = Order.asc if direction == "asc" else Order.desc parsed_field = self._validate_and_parse_field_for_clause(field_name, "Order By") parsed_order_fields.append((parsed_field, order_direction)) @@ -1415,6 +1438,10 @@ class Engine: def _should_apply_ifnull(self, doctype: str, fieldname: str, operator: str, value: Any) -> bool: """Determine if IFNULL wrapping is needed for a filter condition.""" + # Skip this if we don't need db_query compatibility + if not self.db_query_compat: + return False + if not self._is_field_nullable(doctype, fieldname): return False diff --git a/frappe/model/qb_query.py b/frappe/model/qb_query.py index 6e99dd50c4..035526843a 100644 --- a/frappe/model/qb_query.py +++ b/frappe/model/qb_query.py @@ -189,6 +189,7 @@ class DatabaseQuery: "user": user, "parent_doctype": parent_doctype, "reference_doctype": reference_doctype, + "db_query_compat": True, } query = frappe.qb.get_query(**kwargs) diff --git a/frappe/tests/test_db.py b/frappe/tests/test_db.py index 69acfb517a..fc7553bcbb 100644 --- a/frappe/tests/test_db.py +++ b/frappe/tests/test_db.py @@ -93,12 +93,12 @@ class TestDB(IntegrationTestCase): ), ) self.assertEqual( - frappe.db.sql("""SELECT name FROM `tabUser` WHERE name > 's' ORDER BY MODIFIED DESC""")[0][0], + frappe.db.sql("""SELECT name FROM `tabUser` WHERE name > 's' ORDER BY creation DESC""")[0][0], frappe.db.get_value("User", {"name": [">", "s"]}), ) self.assertEqual( - frappe.db.sql("""SELECT name FROM `tabUser` WHERE name >= 't' ORDER BY MODIFIED DESC""")[0][0], + frappe.db.sql("""SELECT name FROM `tabUser` WHERE name >= 't' ORDER BY creation DESC""")[0][0], frappe.db.get_value("User", {"name": [">=", "t"]}), ) self.assertEqual( @@ -132,7 +132,7 @@ class TestDB(IntegrationTestCase): # test multiple orderby's delimiter = '"' if frappe.db.db_type == "postgres" else "`" self.assertIn( - "ORDER BY {deli}creation{deli} DESC,{deli}modified{deli} ASC,{deli}name{deli} ASC".format( + "ORDER BY {deli}creation{deli} DESC,{deli}modified{deli} ASC,{deli}name{deli} DESC".format( deli=delimiter ), frappe.db.get_value("DocType", "DocField", order_by="creation desc, modified asc, name", run=0), diff --git a/frappe/tests/test_query.py b/frappe/tests/test_query.py index 02d5ed1f1e..b2d739fa98 100644 --- a/frappe/tests/test_query.py +++ b/frappe/tests/test_query.py @@ -501,7 +501,7 @@ class TestQuery(IntegrationTestCase): fields=["name"], or_filters={"idx": (">", 5), "issingle": ("=", 1)}, ).get_sql(), - "SELECT `name` FROM `tabDocType` WHERE IFNULL(`idx`,'')>5 OR `issingle`=1".replace( + "SELECT `name` FROM `tabDocType` WHERE `idx`>5 OR `issingle`=1".replace( "`", '"' if frappe.db.db_type == "postgres" else "`" ), ) @@ -525,7 +525,7 @@ class TestQuery(IntegrationTestCase): fields=["name"], or_filters={"name": ("!=", "User"), "module": ("!=", "Core")}, ).get_sql(), - "SELECT `name` FROM `tabDocType` WHERE `name`<>'User' OR IFNULL(`module`,'')<>'Core'".replace( + "SELECT `name` FROM `tabDocType` WHERE `name`<>'User' OR `module`<>'Core'".replace( "`", '"' if frappe.db.db_type == "postgres" else "`" ), ) From 0f3fc00f007d09a1e9b403a93248cc0d326300cb Mon Sep 17 00:00:00 2001 From: Akhil Narang Date: Wed, 19 Nov 2025 00:44:31 +0530 Subject: [PATCH 37/38] fix: handle converting datetime -> date for fieldtype date Signed-off-by: Akhil Narang --- frappe/database/query.py | 58 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 58 insertions(+) diff --git a/frappe/database/query.py b/frappe/database/query.py index a9ca8ae5f8..d9ebbd9b0e 100644 --- a/frappe/database/query.py +++ b/frappe/database/query.py @@ -1,3 +1,4 @@ +import datetime import re from functools import lru_cache from typing import TYPE_CHECKING, Any @@ -28,6 +29,57 @@ CORE_DOCTYPES = DOCTYPES_FOR_DOCTYPE | frozenset( ("Custom Field", "Property Setter", "Module Def", "__Auth", "__global_search", "Singles") ) + +def _apply_date_field_filter_conversion(value, operator: str, doctype: str, field): + """Apply datetime to date conversion for Date fieldtype filters. + + This matches db_query behavior where datetime values are truncated to dates + when filtering on Date fields, for all operators (not just 'between'). + + Args: + value: The filter value (can be datetime, tuple of datetimes, or other) + operator: The operator being used (between, >, <, etc.) + doctype: The doctype to get field metadata from + field: The field name or pypika Field object + + Returns: + The converted value with datetimes converted to dates if field is Date type + """ + try: + # Extract field name + if "." in str(field): + field = field.split(".")[-1] + + # Skip querying meta for core doctypes to avoid recursion + if doctype in CORE_DOCTYPES: + meta = None + else: + meta = frappe.get_meta(doctype) + + if meta is None: + return value + + df = meta.get_field(field) + if df is None or df.fieldtype != "Date": + return value + + # Convert datetime to date if the fieldtype is date + if operator.lower() == "between" and isinstance(value, list | tuple) and len(value) == 2: + from_val, to_val = value + if isinstance(from_val, datetime.datetime): + from_val = from_val.date() + if isinstance(to_val, datetime.datetime): + to_val = to_val.date() + return (from_val, to_val) + elif isinstance(value, datetime.datetime): + return value.date() + + except (AttributeError, TypeError, KeyError): + pass + + return value + + OPTIONAL_COLUMNS = frozenset(["_user_tags", "_comments", "_assign", "_liked_by", "_seen"]) if TYPE_CHECKING: @@ -397,6 +449,12 @@ class Engine: _value = convert_to_value(value) _operator = operator + # For Date fields with datetime values, convert to date to match db_query behavior + if isinstance(_value, datetime.datetime) or ( + isinstance(_value, list | tuple) and any(isinstance(v, datetime.datetime) for v in _value) + ): + _value = _apply_date_field_filter_conversion(_value, _operator, doctype or self.doctype, field) + if not _value and isinstance(_value, list | tuple | set): _value = ("",) From 81bc61fe970ab1b89b6461f20243769c6336b2a9 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Wed, 19 Nov 2025 15:01:15 +0530 Subject: [PATCH 38/38] chore: remove dead code --- frappe/model/meta.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/frappe/model/meta.py b/frappe/model/meta.py index 28c2e134f0..8167bd145d 100644 --- a/frappe/model/meta.py +++ b/frappe/model/meta.py @@ -101,10 +101,6 @@ def clear_meta_cache(doctype: str = "*"): frappe.client_cache.delete_value(key) -def clear_doctype_sort_cache(doctype: str): - frappe.cache.delete_value(f"doctype_sort_info::{doctype}") - - def load_meta(doctype): return Meta(doctype)