From 726fcfdb796407e71c7a459a255b201417ef8c87 Mon Sep 17 00:00:00 2001 From: Faris Ansari Date: Sun, 25 Dec 2022 23:19:11 +0530 Subject: [PATCH 01/23] refactor: qb.engine - simplify - qb.engine.get_query -> qb.get_query - qb.engine.build_conditions -> qb.get_query --- frappe/__init__.py | 4 +- frappe/database/database.py | 22 +- frappe/database/query.py | 707 +++++++----------- .../desk/doctype/number_card/number_card.py | 2 +- frappe/desk/listview.py | 2 +- frappe/query_builder/__init__.py | 2 +- frappe/query_builder/functions.py | 2 +- frappe/query_builder/utils.py | 5 +- frappe/tests/test_db_query.py | 4 +- frappe/tests/test_query.py | 88 +-- frappe/utils/goal.py | 2 +- 11 files changed, 347 insertions(+), 493 deletions(-) diff --git a/frappe/__init__.py b/frappe/__init__.py index 2d491ca068..6b9d157003 100644 --- a/frappe/__init__.py +++ b/frappe/__init__.py @@ -23,7 +23,7 @@ import click from werkzeug.local import Local, release_local from frappe.query_builder import ( - get_qb_engine, + get_query, get_query_builder, patch_query_aggregation, patch_query_execute, @@ -244,7 +244,7 @@ def init(site: str, sites_path: str = ".", new_site: bool = False) -> None: local.session = _dict() local.dev_server = _dev_server local.qb = get_query_builder(local.conf.db_type or "mariadb") - local.qb.engine = get_qb_engine() + local.qb.get_query = get_query setup_module_map() if not _qb_patched.get(local.conf.db_type): diff --git a/frappe/database/database.py b/frappe/database/database.py index dfcc9dfe58..acbd28c9d7 100644 --- a/frappe/database/database.py +++ b/frappe/database/database.py @@ -620,7 +620,7 @@ class Database: return [map(values.get, fields)] else: - r = frappe.qb.engine.get_query( + r = frappe.qb.get_query( "Singles", filters={"field": ("in", tuple(fields)), "doctype": doctype}, fields=["field", "value"], @@ -653,7 +653,7 @@ class Database: # Get coulmn and value of the single doctype Accounts Settings account_settings = frappe.db.get_singles_dict("Accounts Settings") """ - queried_result = frappe.qb.engine.get_query( + queried_result = frappe.qb.get_query( "Singles", filters={"doctype": doctype}, fields=["field", "value"], @@ -726,7 +726,7 @@ class Database: if cache and fieldname in self.value_cache[doctype]: return self.value_cache[doctype][fieldname] - val = frappe.qb.engine.get_query( + val = frappe.qb.get_query( table="Singles", filters={"doctype": doctype, "field": fieldname}, fields="value", @@ -766,10 +766,10 @@ class Database: distinct=False, limit=None, ): - query = frappe.qb.engine.get_query( + query = frappe.qb.get_query( table=doctype, filters=filters, - orderby=order_by, + order_by=order_by, for_update=for_update, fields=fields, distinct=distinct, @@ -795,7 +795,7 @@ class Database: as_dict=False, ): if names := list(filter(None, names)): - return frappe.qb.engine.get_query( + return frappe.qb.get_query( doctype, fields=field, filters=names, @@ -852,7 +852,7 @@ class Database: frappe.clear_document_cache(dt, dt) else: - query = frappe.qb.engine.build_conditions(table=dt, filters=dn, update=True) + query = frappe.qb.get_query(table=dt, filters=dn, update=True) if isinstance(dn, str): frappe.clear_document_cache(dt, dn) @@ -1017,9 +1017,9 @@ class Database: cache_count = frappe.cache().get_value(f"doctype:count:{dt}") if cache_count is not None: return cache_count - count = frappe.qb.engine.get_query( - table=dt, filters=filters, fields=Count("*"), distinct=distinct - ).run(debug=debug)[0][0] + count = frappe.qb.get_query(table=dt, filters=filters, fields=Count("*"), distinct=distinct).run( + debug=debug + )[0][0] if not filters and cache: frappe.cache().set_value(f"doctype:count:{dt}", count, expires_in_sec=86400) return count @@ -1160,7 +1160,7 @@ class Database: Doctype name can be passed directly, it will be pre-pended with `tab`. """ filters = filters or kwargs.get("conditions") - query = frappe.qb.engine.build_conditions(table=doctype, filters=filters).delete() + query = frappe.qb.get_query(table=doctype, filters=filters).delete() if "debug" not in kwargs: kwargs["debug"] = debug return query.run(**kwargs) diff --git a/frappe/database/query.py b/frappe/database/query.py index a9dab02744..88de7f7088 100644 --- a/frappe/database/query.py +++ b/frappe/database/query.py @@ -8,6 +8,7 @@ from typing import TYPE_CHECKING, Callable import sqlparse from pypika.dialects import MySQLQueryBuilder, PostgreSQLQueryBuilder +from pypika.queries import QueryBuilder import frappe from frappe import _ @@ -171,18 +172,16 @@ def table_from_string(table: str) -> "DocType": return frappe.qb.DocType(table_name=table_name) -def get_nested_set_hierarchy_result(hierarchy: str, field: str, table: str): - ref_doctype = table +def get_nested_set_hierarchy_result(doctype: str, name: str, hierarchy: str): + table = frappe.qb.DocType(doctype) try: - lft, rgt = ( - frappe.qb.from_(ref_doctype).select("lft", "rgt").where(Field("name") == field).run()[0] - ) + lft, rgt = frappe.qb.from_(table).select("lft", "rgt").where(table.name == name).run()[0] except IndexError: lft, rgt = None, None if hierarchy in ("descendants of", "not descendants of"): result = ( - frappe.qb.from_(ref_doctype) + frappe.qb.from_(table) .select(Field("name")) .where(Field("lft") > lft) .where(Field("rgt") < rgt) @@ -192,7 +191,7 @@ def get_nested_set_hierarchy_result(hierarchy: str, field: str, table: str): else: # Get ancestor elements of a DocType with a tree structure result = ( - frappe.qb.from_(ref_doctype) + frappe.qb.from_(table) .select(Field("name")) .where(Field("lft") < lft) .where(Field("rgt") > rgt) @@ -232,37 +231,67 @@ OPERATOR_MAP: dict[str, Callable] = { class Engine: tables: dict[str, str] = {} - @cached_property - def OPERATOR_MAP(self): - # default operators - all_operators = OPERATOR_MAP.copy() + def get_query( + self, + table: str, + fields: list | tuple | None = None, + filters: dict[str, str | int] | str | int | list[list | str | int] | None = None, + pluck: str | None = None, + order_by: str | None = None, + group_by: str | None = None, + limit: int | None = None, + offset: int | None = None, + distinct: bool = False, + for_update: bool = False, + update: bool = False, + into: bool = False, + ) -> MySQLQueryBuilder | PostgreSQLQueryBuilder: + # Clean up state before each query + self.is_mariadb = frappe.db.db_type == "mariadb" + self.is_postgres = frappe.db.db_type == "postgres" + self.tables = {} + self.implicit_joins = set() - # TODO: update with site-specific custom operators / removed previous buggy implementation - if frappe.get_hooks("filters_config"): - from frappe.utils.commands import warn + self.doctype = table + self.table = self.get_table(table) - warn( - "The 'filters_config' hook used to add custom operators is not yet implemented" - " in frappe.db.query engine. Use db_query (frappe.get_list) instead." - ) + if update: + self.query = frappe.qb.update(self.table) + elif into: + self.query = frappe.qb.into(self.table) + else: + self.query = frappe.qb.from_(self.table) - return all_operators + self.fields = self.parse_fields(fields) + if not self.fields: + self.fields = [getattr(self.table, pluck or "name")] - def get_condition(self, table: str | Table, **kwargs) -> frappe.qb: - """Get initial table object + for field in self.fields: + if isinstance(field, DynamicTableField): + self.query = field.apply(self.query) + else: + self.query = self.query.select(field) - Args: - table (str): DocType + self.apply_filters(filters) + self.apply_implicit_joins() + self.apply_order_by(order_by) - Returns: - frappe.qb: DocType with initial condition - """ - table_object = self.get_table(table) - if kwargs.get("update"): - return frappe.qb.update(table_object) - if kwargs.get("into"): - return frappe.qb.into(table_object) - return frappe.qb.from_(table_object) + if limit: + self.query = self.query.limit(limit) + + if offset: + self.query = self.query.offset(offset) + + if distinct: + self.query = self.query.distinct() + + if for_update: + self.query = self.query.for_update() + + if group_by: + self.query = self.query.groupby(group_by) + + return self.query def get_table(self, table_name: str | Table) -> Table: if isinstance(table_name, Table): @@ -272,178 +301,93 @@ class Engine: self.tables[table_name] = frappe.qb.DocType(table_name) return self.tables[table_name] - def criterion_query(self, table: str, criterion: Criterion, **kwargs) -> frappe.qb: - """Generate filters from Criterion objects - - Args: - table (str): DocType - criterion (Criterion): Filters - - Returns: - frappe.qb: condition object - """ - condition = self.add_conditions(self.get_condition(table, **kwargs), **kwargs) - return condition.where(criterion) - - def add_conditions(self, conditions: frappe.qb, **kwargs): - """Adding additional conditions - - Args: - conditions (frappe.qb): built conditions - - Returns: - conditions (frappe.qb): frappe.qb object - """ - if kwargs.get("orderby") and kwargs.get("orderby") != "KEEP_DEFAULT_ORDERING": - orderby = kwargs.get("orderby") - if isinstance(orderby, str) and len(orderby.split()) > 1: - for ordby in orderby.split(","): - if ordby := ordby.strip(): - orderby, order = change_orderby(ordby) - conditions = conditions.orderby(orderby, order=order) - else: - conditions = conditions.orderby(orderby, order=kwargs.get("order") or Order.desc) - - if kwargs.get("limit"): - conditions = conditions.limit(kwargs.get("limit")) - conditions = conditions.offset(kwargs.get("offset", 0)) - - if kwargs.get("distinct"): - conditions = conditions.distinct() - - if kwargs.get("for_update"): - conditions = conditions.for_update() - - if kwargs.get("groupby"): - conditions = conditions.groupby(kwargs.get("groupby")) - - return conditions - - def misc_query(self, table: str, filters: list | tuple = None, **kwargs): - """Build conditions using the given Lists or Tuple filters - - Args: - table (str): DocType - filters (Union[List, Tuple], optional): Filters. Defaults to None. - """ - conditions = self.get_condition(table, **kwargs) + def apply_filters( + self, filters: dict[str, str | int | list] | str | int | list[list] | None = None + ): if not filters: - return conditions - if isinstance(filters, list): - for f in filters: - if isinstance(f, (list, tuple)): - _operator = self.OPERATOR_MAP[f[-2].casefold()] - if len(f) == 4: - table_object = self.get_table(f[0]) - _field = table_object[f[1]] - else: - _field = Field(f[0]) - conditions = conditions.where(_operator(_field, f[-1])) - elif isinstance(f, dict): - conditions = self.dict_query(table, f, **kwargs) - else: - _operator = self.OPERATOR_MAP[filters[1].casefold()] - if not isinstance(filters[0], str): - conditions = self.make_function_for_filters(filters[0], filters[2]) - break - conditions = conditions.where(_operator(Field(filters[0]), filters[2])) - break + return - return self.add_conditions(conditions, **kwargs) - - def dict_query(self, table: str, filters: dict[str, str | int] = None, **kwargs) -> frappe.qb: - """Build conditions using the given dictionary filters - - Args: - table (str): DocType - filters (Dict[str, Union[str, int]], optional): Filters. Defaults to None. - - Returns: - frappe.qb: conditions object - """ - conditions = self.get_condition(table, **kwargs) - if isinstance(table, str): - table = frappe.qb.DocType(table) - if not filters: - conditions = self.add_conditions(conditions, **kwargs) - return conditions - - for key, value in filters.items(): - if isinstance(value, bool): - filters.update({key: str(int(value))}) - - filters = { - (self.get_function_object(k) if has_function(k) else k): v for k, v in filters.items() - } - for key in filters: - value = filters.get(key) - _operator = self.OPERATOR_MAP["="] - - if not isinstance(key, str): - conditions = conditions.where(self.make_function_for_filters(key, value)) - continue - # Nested set support - if isinstance(value, (list, tuple)): - if value[0] in self.OPERATOR_MAP["nested_set"]: - hierarchy, _field = value - result = get_nested_set_hierarchy_result(hierarchy, _field, table) - _operator = ( - self.OPERATOR_MAP["not in"] - if hierarchy in ("not ancestors of", "not descendants of") - else self.OPERATOR_MAP["in"] - ) - if result: - result = list(itertools.chain.from_iterable(result)) - conditions = conditions.where(_operator(getattr(table, key), result)) - else: - conditions = conditions.where(_operator(getattr(table, key), ("",))) - # Allow additional conditions - break - - _operator = self.OPERATOR_MAP[value[0].casefold()] - _value = value[1] if value[1] else ("",) - conditions = conditions.where(_operator(getattr(table, key), _value)) - else: - if value is not None: - conditions = conditions.where(_operator(getattr(table, key), value)) - else: - _table = conditions._from[0] - field = getattr(_table, key) - conditions = conditions.where(field.isnull()) - - return self.add_conditions(conditions, **kwargs) - - def build_conditions( - self, table: str, filters: dict[str, str | int] | str | int = None, **kwargs - ) -> frappe.qb: - """Build conditions for sql query - - Args: - filters (Union[Dict[str, Union[str, int]], str, int]): conditions in Dict - table (str): DocType - - Returns: - frappe.qb: frappe.qb conditions object - """ - if isinstance(filters, int) or isinstance(filters, str): + if isinstance(filters, (str, int)): filters = {"name": str(filters)} if isinstance(filters, Criterion): - criterion = self.criterion_query(table, filters, **kwargs) + self.query = self.query.where(filters) + + elif isinstance(filters, dict): + self.apply_dict_filters(filters) elif isinstance(filters, (list, tuple)): - criterion = self.misc_query(table, filters, **kwargs) + self.apply_list_filters(filters) + def apply_list_filters(self, filters: list[list]): + for filter in filters: + if len(filter) == 2: + field, value = filter + self._apply_filter(field, value) + elif len(filter) == 3: + field, operator, value = filter + self._apply_filter(field, value, operator) + elif len(filter) == 4: + doctype, field, operator, value = filter + self._apply_filter(field, value, operator, doctype) + + def apply_dict_filters(self, filters: dict[str, str | int | list]): + for key in filters: + value = filters.get(key) + self._apply_filter(key, value) + + def _apply_filter( + self, field: str, value: str | int | list | None, operator: str = "=", doctype: str | None = None + ): + _field = field + _value = value + _operator = operator + + if has_function(field): + _field = self.get_function_object(field) + elif not doctype or doctype == self.doctype: + _field = self.table[field] + elif doctype: + _field = self.get_table(doctype)[field] + + # keep track of implicit join if child table is referenced + if doctype and doctype != self.doctype: + meta = frappe.get_meta(doctype) + if meta.istable: + self.implicit_joins.add((doctype, "child")) + + if isinstance(_value, (str, int)): + _value = str(_value) + elif isinstance(_value, (list, tuple)): + _operator, _value = _value + elif isinstance(_value, bool): + _value = int(_value) + + if isinstance(_value, str) and has_function(_value): + _value = self.get_function_object(_value) + + # Nested set + if _operator in self.OPERATOR_MAP["nested_set"]: + hierarchy = _operator + docname = _value + result = get_nested_set_hierarchy_result(self.doctype, docname, hierarchy) + operator_fn = ( + self.OPERATOR_MAP["not in"] + if hierarchy in ("not ancestors of", "not descendants of") + else self.OPERATOR_MAP["in"] + ) + if result: + result = list(itertools.chain.from_iterable(result)) + self.query = self.query.where(operator_fn(_field, result)) + else: + self.query = self.query.where(operator_fn(_field, ("",))) + return + + operator_fn = self.OPERATOR_MAP[_operator.casefold()] + if _value is None and isinstance(_field, Field): + self.query = self.query.where(_field.isnull()) else: - criterion = self.dict_query(filters=filters, table=table, **kwargs) - - return criterion - - def make_function_for_filters(self, key, value: int | str): - value = list(value) - if isinstance(value[1], str) and has_function(value[1]): - value[1] = self.get_function_object(value[1]) - return OPERATOR_MAP[value[0].casefold()](key, value[1]) + self.query = self.query.where(operator_fn(_field, _value)) def get_function_object(self, field: str) -> "Function": """Expects field to look like 'SUM(*)' or 'name' or something similar. Returns PyPika Function object""" @@ -495,84 +439,12 @@ class Engine: # Fall back for functions not present in `SqlFunctions`` return Function(func, *_args, alias=alias or None) - def function_objects_from_string(self, fields): - fields = list(map(lambda str: str.strip(), COMMA_PATTERN.split(fields))) - return self.function_objects_from_list(fields=fields) - - def function_objects_from_list(self, fields): - functions = [] - for field in fields: - field = field.casefold() if (isinstance(field, str) and "`" not in field) else field - if not issubclass(type(field), Criterion): - if any([f"{func}(" in field for func in SQL_FUNCTIONS]) or "(" in field: - functions.append(field) - - return [self.get_function_object(function) for function in functions] - - def remove_string_functions(self, fields, function_objects): - """Remove string functions from fields which have already been converted to function objects""" - - def _remove_string_aliasing(function, fields: list | str): - if function.alias: - to_replace = " as " + function.alias.casefold() - if to_replace in fields: - fields = fields.replace(to_replace, "") - elif " as " + f"`{function.alias.casefold()}" in fields: - fields = fields.replace(" as " + f"`{function.alias.casefold()}`", "") - return fields - - for function in function_objects: - if isinstance(fields, str): - fields = _remove_string_aliasing(function, fields) - fields = BRACKETS_PATTERN.sub("", re.sub(function.name, "", fields, flags=re.IGNORECASE)) - # Check if only comma is left in fields after stripping functions. - if "," in fields and (len(fields.strip()) == 1): - fields = "" - else: - updated_fields = [] - for field in fields: - if isinstance(field, str): - field = _remove_string_aliasing(function, field) - substituted_string = ( - BRACKETS_PATTERN.sub("", field).strip().casefold() - if "`" not in field - else BRACKETS_PATTERN.sub("", field).strip() - ) - # This is done to avoid casefold of table name. - if substituted_string.casefold() == function.name.casefold(): - replaced_string = substituted_string.casefold().replace(function.name.casefold(), "") - else: - replaced_string = substituted_string.replace(function.name.casefold(), "") - updated_fields.append(replaced_string) - fields = [field for field in updated_fields if field] - return fields - - def get_fieldnames_from_child_table(self, doctype, fields): - # Hacky and flaky implementation of implicit joins. - # convert child_table.fieldname to `tabChild DocType`.`fieldname` - _fields = [] - for field in fields: - if "." in field and "tab" not in field: - alias = None - if " as " in field: - field, alias = field.split(" as ") - fieldname, linked_fieldname = field.split(".") - linked_doctype = frappe.get_meta(doctype).get_field(fieldname).options - - field = f"`tab{linked_doctype}`.`{linked_fieldname}`" - if alias: - field = f"{field} {alias}" - _fields.append(field) - return _fields - def sanitize_fields(self, fields: str | list | tuple): - is_mariadb = frappe.db.db_type == "mariadb" - def _sanitize_field(field: str): if not isinstance(field, str): return field stripped_field = sqlparse.format(field, strip_comments=True, keyword_case="lower") - if is_mariadb: + if self.is_mariadb: return MARIADB_SPECIFIC_COMMENT.sub("", stripped_field) return stripped_field @@ -583,174 +455,88 @@ class Engine: return fields - def get_list_fields(self, table: str, fields: list) -> list: - updated_fields = [] - if issubclass(type(fields), Criterion) or "*" in fields: - return fields - fields = self.get_fieldnames_from_child_table(doctype=table, fields=fields) - for field in fields: - if not isinstance(field, Criterion) and field: - if " as " in field: - field, reference = field.split(" as ") - if "`" in field: - updated_fields.append(PseudoColumnMapper(f"{field} {reference}")) - else: - updated_fields.append(Field(field.strip()).as_(reference)) - elif "`" in str(field): - updated_fields.append(PseudoColumnMapper(field.strip())) - else: - updated_fields.append(Field(field)) - return updated_fields + def parse_string_field(self, field: str): + if field == "*": + return self.table.star + alias = None + if " as " in field: + field, alias = field.split(" as ") + if "`" in field: + if alias: + return PseudoColumnMapper(f"{field} {alias}") + return PseudoColumnMapper(field) + if alias: + return self.table[field].as_(alias) + return self.table[field] - def get_string_fields(self, fields: str) -> Field: - if fields == "*": - return fields - if "`" in fields: - fields = PseudoColumnMapper(fields) - if " as " in str(fields): - fields, reference = str(fields).split(" as ") - if "`" in str(fields): - fields = PseudoColumnMapper(f"{fields} {reference}") - else: - fields = Field(fields).as_(reference) - return fields - - def set_fields(self, table: str, fields, **kwargs) -> list: - fields = kwargs.get("pluck") if kwargs.get("pluck") else fields or "name" + def parse_fields(self, fields: str | list | tuple | None) -> list: + if not fields: + return [] fields = self.sanitize_fields(fields) - if isinstance(fields, list) and None in fields and Field not in fields: - return None - function_objects = [] - is_list = isinstance(fields, (list, tuple, set)) - if is_list and len(fields) == 1: - fields = fields[0] - is_list = False + if isinstance(fields, (list, tuple, set)) and None in fields and Field not in fields: + return [] - if is_list: - function_objects += self.function_objects_from_list(fields=fields) + if not isinstance(fields, (list, tuple)): + fields = [fields] - is_str = isinstance(fields, str) - if is_str: - fields = fields.casefold() if "`" not in fields else fields - function_objects += self.function_objects_from_string(fields=fields) - - fields = self.remove_string_functions(fields, function_objects) - - if is_str and "," in fields: - fields = [field.replace(" ", "") if "as" not in field else field for field in fields.split(",")] - is_list, is_str = True, False - - if is_str: - fields = self.get_string_fields(fields) - if not is_str and fields: - fields = self.get_list_fields(table, fields) - - # Need to check instance again since fields modified. - if not isinstance(fields, (list, tuple, set)): - fields = [fields] if fields else [] - - fields.extend(function_objects) - return fields - - def join_child_tables( - self, - criterion: Criterion, - join_type: str, - child_table: Table, - parent_table: Table, - ) -> Criterion: - if self.joined_tables.get(join_type) != child_table: - criterion = getattr(criterion, join_type)(child_table).on( - (child_table.parent == parent_table.name) - & (child_table.parenttype == TAB_PATTERN.sub("", parent_table._table_name)) - ) - self.joined_tables[join_type] = child_table - return criterion - - def join(self, criterion, fields, table, join_type): - """Handles all join operations on criterion objects""" - has_join = False - table_pattern = ( - re.compile(r"`\btab\w+") if frappe.db.db_type == "mariadb" else re.compile(r'"\btab\w+') - ) - - def _update_pypika_fields(field): - if not is_pypika_function_object(field): - field = field if isinstance(field, (str, PseudoColumnMapper)) else field.get_sql() - if not table_pattern.search(str(field)): - if isinstance(field, PseudoColumnMapper): - field = field.get_sql() - return getattr(frappe.qb.DocType(table), field) - else: - return field + def parse_field(field: str): + if has_function(field): + return self.get_function_object(field) + elif parsed := DynamicTableField.parse(field, self.doctype): + return parsed else: - field.args = [getattr(frappe.qb.DocType(table), arg.get_sql()) for arg in field.args] - return field + return self.parse_string_field(field) - if not isinstance(fields, Criterion): - for field in fields: - # Only perform this bit if foreign doctype in fields - if ( - not is_pypika_function_object(field) - and (str(field).startswith('"tab') or str(field).startswith("`tab")) - and (f"`tab{table}`" not in str(field) and f'tab{table}"' not in str(field)) - ): - has_join = True - child_table = table_from_string(str(field)) - parent_table = frappe.qb.DocType(table) if not isinstance(table, Table) else table - criterion = self.join_child_tables( - criterion=criterion, - join_type=join_type, - child_table=child_table, - parent_table=parent_table, - ) + _fields = [] + for field in fields: + if isinstance(field, Criterion): + _fields.append(field) + elif isinstance(field, str): + if "," in field: + field = field.casefold() if "`" not in field else field + field_list = COMMA_PATTERN.split(field) + for field in field_list: + if _field := field.strip(): + _fields.append(parse_field(_field)) + else: + _fields.append(parse_field(field)) - if has_join: - fields = [_update_pypika_fields(field) for field in fields] + return _fields - if len(self.tables) > 1: - parent_table = self.tables[table] - child_tables = list(self.tables.values())[1:] - for child_table in child_tables: - criterion = self.join_child_tables( - criterion, - join_type=join_type, - child_table=child_table, - parent_table=parent_table, + def apply_implicit_joins(self): + for d in self.implicit_joins: + doctype, join_type = d + table = self.get_table(doctype) + if join_type == "child": + self.query = self.query.left_join(table).on( + (table.parent == self.table.name) & (table.parenttype == self.doctype) ) - return criterion, fields + def apply_order_by(self, order_by: str | None): + if not order_by or order_by == "KEEP_DEFAULT_ORDERING": + return + for declaration in order_by.split(","): + if _order_by := declaration.strip(): + parts = _order_by.split(" ") + order_field, order_direction = parts[0], parts[1] if len(parts) > 1 else "asc" + order_direction = Order.asc if order_direction.lower() == "asc" else Order.desc + self.query = self.query.orderby(order_field, order=order_direction) - def get_query( - self, - table: str, - fields: list | tuple, - filters: dict[str, str | int] | str | int | list[list | str | int] = None, - **kwargs, - ) -> MySQLQueryBuilder | PostgreSQLQueryBuilder: - # Clean up state before each query - self.tables = {} - self.joined_tables = {} - self.linked_doctype = None - self.fieldname = None + @cached_property + def OPERATOR_MAP(self): + # default operators + all_operators = OPERATOR_MAP.copy() - criterion = self.build_conditions(table, filters, **kwargs) - fields = self.set_fields(table, fields, **kwargs) - join_type = kwargs.get("join").replace(" ", "_") if kwargs.get("join") else "left_join" - criterion, fields = self.join( - criterion=criterion, fields=fields, table=table, join_type=join_type - ) + # TODO: update with site-specific custom operators / removed previous buggy implementation + if frappe.get_hooks("filters_config"): + from frappe.utils.commands import warn - if isinstance(fields, (list, tuple)): - query = criterion.select(*fields) + warn( + "The 'filters_config' hook used to add custom operators is not yet implemented" + " in frappe.db.query engine. Use db_query (frappe.get_list) instead." + ) - elif isinstance(fields, Criterion): - query = criterion.select(fields) - - else: - query = criterion.select(fields) - - return query + return all_operators class Permission: @@ -781,3 +567,80 @@ class Permission: @staticmethod def get_tables_from_query(query: str): return [table for table in WORDS_PATTERN.findall(query) if table.startswith("tab")] + + +class DynamicTableField: + def __init__( + self, + doctype: str, + fieldname: str, + parent_doctype: str, + alias: str | None = None, + ) -> None: + self.doctype = doctype + self.fieldname = fieldname + self.alias = alias + self.parent_doctype = parent_doctype + + def __str__(self) -> str: + table_name = f"`tab{self.doctype}`" + fieldname = f"`{self.fieldname}`" + if frappe.db.db_type == "postgres": + table_name = table_name.replace("`", '"') + fieldname = fieldname.replace("`", '"') + alias = f"AS {self.alias}" if self.alias else "" + return f"{table_name}.{fieldname} {alias}".strip() + + @staticmethod + def parse(field: str, doctype: str): + if "." in field: + alias = None + if " as " in field: + field, alias = field.split(" as ") + if field.startswith("`tab") or field.startswith('"tab'): + _, child_doctype, child_field = re.search(r'([`"])tab(.+?)\1.\1(.+)\1', field).groups() + if child_doctype == doctype: + return + return ChildTableField(child_doctype, child_field, doctype, alias=alias) + else: + linked_fieldname, fieldname = field.split(".") + linked_field = frappe.get_meta(doctype).get_field(linked_fieldname) + linked_doctype = linked_field.options + if linked_field.fieldtype == "Link": + return LinkTableField(linked_doctype, fieldname, doctype, linked_fieldname, alias=alias) + elif linked_field.fieldtype in frappe.model.table_fields: + return ChildTableField(linked_doctype, fieldname, doctype, alias=alias) + + def apply(self, query: QueryBuilder) -> QueryBuilder: + raise NotImplementedError + + +class ChildTableField(DynamicTableField): + def apply(self, query: QueryBuilder) -> QueryBuilder: + table = frappe.qb.DocType(self.doctype) + main_table = frappe.qb.DocType(self.parent_doctype) + if not query.is_joined(table): + query = query.left_join(table).on( + (table.parent == main_table.name) & (table.parenttype == self.parent_doctype) + ) + return query.select(getattr(table, self.fieldname).as_(self.alias or None)) + + +class LinkTableField(DynamicTableField): + def __init__( + self, + doctype: str, + fieldname: str, + parent_doctype: str, + link_fieldname: str, + alias: str | None = None, + ) -> None: + super().__init__(doctype, fieldname, parent_doctype, alias=alias) + self.link_fieldname = link_fieldname + + def apply(self, query: QueryBuilder) -> QueryBuilder: + table = frappe.qb.DocType(self.doctype) + main_table = frappe.qb.DocType(self.parent_doctype) + if not query.is_joined(table): + query = query.left_join(table).on(table.name == getattr(main_table, self.link_fieldname)) + return query.select(getattr(table, self.fieldname).as_(self.alias or None)) diff --git a/frappe/desk/doctype/number_card/number_card.py b/frappe/desk/doctype/number_card/number_card.py index 8e808ff635..30ec99644a 100644 --- a/frappe/desk/doctype/number_card/number_card.py +++ b/frappe/desk/doctype/number_card/number_card.py @@ -200,7 +200,7 @@ def get_cards_for_user(doctype, txt, searchfield, start, page_len, filters): if txt: search_conditions = [numberCard[field].like(f"%{txt}%") for field in searchfields] - condition_query = frappe.qb.engine.build_conditions(doctype, filters) + condition_query = frappe.qb.get_query(doctype, filters) return ( condition_query.select(numberCard.name, numberCard.label, numberCard.document_type) diff --git a/frappe/desk/listview.py b/frappe/desk/listview.py index ea6eb6259c..8b514444df 100644 --- a/frappe/desk/listview.py +++ b/frappe/desk/listview.py @@ -36,7 +36,7 @@ def get_group_by_count(doctype: str, current_filters: str, field: str) -> list[d ToDo = DocType("ToDo") User = DocType("User") count = Count("*").as_("count") - filtered_records = frappe.qb.engine.build_conditions(doctype, current_filters).select("name") + filtered_records = frappe.qb.get_query(doctype, filters=current_filters).select("name") return ( frappe.qb.from_(ToDo) diff --git a/frappe/query_builder/__init__.py b/frappe/query_builder/__init__.py index eb1d9df08f..b1f242f78c 100644 --- a/frappe/query_builder/__init__.py +++ b/frappe/query_builder/__init__.py @@ -7,7 +7,7 @@ from frappe.query_builder.terms import ParameterizedFunction, ParameterizedValue from frappe.query_builder.utils import ( Column, DocType, - get_qb_engine, + get_query, get_query_builder, patch_query_aggregation, patch_query_execute, diff --git a/frappe/query_builder/functions.py b/frappe/query_builder/functions.py index 24e2ee0e5f..b1e4e7eff1 100644 --- a/frappe/query_builder/functions.py +++ b/frappe/query_builder/functions.py @@ -103,7 +103,7 @@ class Cast_(Function): def _aggregate(function, dt, fieldname, filters, **kwargs): return ( - frappe.qb.engine.build_conditions(dt, filters) + frappe.qb.get_query(dt, filters=filters) .select(function(PseudoColumn(fieldname))) .run(**kwargs)[0][0] or 0 diff --git a/frappe/query_builder/utils.py b/frappe/query_builder/utils.py index be0403a291..f80dd4fc33 100644 --- a/frappe/query_builder/utils.py +++ b/frappe/query_builder/utils.py @@ -3,6 +3,7 @@ from importlib import import_module from typing import Any, Callable, get_type_hints from pypika import Query +from pypika.dialects import MySQLQueryBuilder, PostgreSQLQueryBuilder from pypika.queries import Column from pypika.terms import PseudoColumn @@ -55,10 +56,10 @@ def get_query_builder(type_of_db: str) -> Postgres | MariaDB: return picks[db] -def get_qb_engine(): +def get_query(*args, **kwargs) -> MySQLQueryBuilder | PostgreSQLQueryBuilder: from frappe.database.query import Engine - return Engine() + return Engine().get_query(*args, **kwargs) def get_attr(method_string): diff --git a/frappe/tests/test_db_query.py b/frappe/tests/test_db_query.py index 162d3e9d8a..1a3e8735dc 100644 --- a/frappe/tests/test_db_query.py +++ b/frappe/tests/test_db_query.py @@ -229,9 +229,7 @@ class TestReportview(FrappeTestCase): ) def test_none_filter(self): - query = frappe.qb.engine.get_query( - "DocType", fields="name", filters={"restrict_to_domain": None} - ) + query = frappe.qb.get_query("DocType", fields="name", filters={"restrict_to_domain": None}) sql = str(query).replace("`", "").replace('"', "") condition = "restrict_to_domain IS NULL" self.assertIn(condition, sql) diff --git a/frappe/tests/test_query.py b/frappe/tests/test_query.py index 3f48882345..957d76d022 100644 --- a/frappe/tests/test_query.py +++ b/frappe/tests/test_query.py @@ -56,30 +56,28 @@ class TestQuery(FrappeTestCase): @run_only_if(db_type_is.MARIADB) def test_multiple_tables_in_filters(self): self.assertEqual( - frappe.qb.engine.get_query( + frappe.qb.get_query( "DocType", ["*"], [ - ["BOM Update Log", "name", "like", "f%"], + ["DocField", "name", "like", "f%"], ["DocType", "parent", "=", "something"], ], ).get_sql(), - "SELECT * FROM `tabDocType` LEFT JOIN `tabBOM Update Log` ON `tabBOM Update Log`.`parent`=`tabDocType`.`name` AND `tabBOM Update Log`.`parenttype`='DocType' WHERE `tabBOM Update Log`.`name` LIKE 'f%' AND `tabDocType`.`parent`='something'", + "SELECT `tabDocType`.* FROM `tabDocType` LEFT JOIN `tabDocField` ON `tabDocField`.`parent`=`tabDocType`.`name` AND `tabDocField`.`parenttype`='DocType' WHERE `tabDocField`.`name` LIKE 'f%' AND `tabDocType`.`parent`='something'", ) @run_only_if(db_type_is.MARIADB) def test_string_fields(self): self.assertEqual( - frappe.qb.engine.get_query( - "User", fields="name, email", filters={"name": "Administrator"} - ).get_sql(), + frappe.qb.get_query("User", fields="name, email", filters={"name": "Administrator"}).get_sql(), frappe.qb.from_("User") .select(Field("name"), Field("email")) .where(Field("name") == "Administrator") .get_sql(), ) self.assertEqual( - frappe.qb.engine.get_query( + frappe.qb.get_query( "User", fields=["`name`, `email`"], filters={"name": "Administrator"} ).get_sql(), frappe.qb.from_("User") @@ -89,7 +87,7 @@ class TestQuery(FrappeTestCase): ) self.assertEqual( - frappe.qb.engine.get_query( + frappe.qb.get_query( "User", fields=["`tabUser`.`name`", "`tabUser`.`email`"], filters={"name": "Administrator"} ).run(), frappe.qb.from_("User") @@ -99,7 +97,7 @@ class TestQuery(FrappeTestCase): ) self.assertEqual( - frappe.qb.engine.get_query( + frappe.qb.get_query( "User", fields=["`tabUser`.`name` as owner", "`tabUser`.`email`"], filters={"name": "Administrator"}, @@ -111,7 +109,7 @@ class TestQuery(FrappeTestCase): ) self.assertEqual( - frappe.qb.engine.get_query( + frappe.qb.get_query( "User", fields=["`tabUser`.`name`, Count(`name`) as count"], filters={"name": "Administrator"} ).run(), frappe.qb.from_("User") @@ -121,7 +119,7 @@ class TestQuery(FrappeTestCase): ) self.assertEqual( - frappe.qb.engine.get_query( + frappe.qb.get_query( "User", fields=["`tabUser`.`name`, Count(`name`) as `count`"], filters={"name": "Administrator"}, @@ -133,7 +131,7 @@ class TestQuery(FrappeTestCase): ) self.assertEqual( - frappe.qb.engine.get_query( + frappe.qb.get_query( "User", fields="`tabUser`.`name`, Count(`name`) as `count`", filters={"name": "Administrator"} ).run(), frappe.qb.from_("User") @@ -144,38 +142,34 @@ class TestQuery(FrappeTestCase): def test_functions_fields(self): self.assertEqual( - frappe.qb.engine.get_query("User", fields="Count(name)", filters={}).get_sql(), + frappe.qb.get_query("User", fields="Count(name)", filters={}).get_sql(), frappe.qb.from_("User").select(Count(Field("name"))).get_sql(), ) self.assertEqual( - frappe.qb.engine.get_query("User", fields=["Count(name)", "Max(name)"], filters={}).get_sql(), + frappe.qb.get_query("User", fields=["Count(name)", "Max(name)"], filters={}).get_sql(), frappe.qb.from_("User").select(Count(Field("name")), Max(Field("name"))).get_sql(), ) self.assertEqual( - frappe.qb.engine.get_query( - "User", fields=["abs(name-email)", "Count(name)"], filters={} - ).get_sql(), + frappe.qb.get_query("User", fields=["abs(name-email)", "Count(name)"], filters={}).get_sql(), frappe.qb.from_("User") .select(Abs(Field("name") - Field("email")), Count(Field("name"))) .get_sql(), ) self.assertEqual( - frappe.qb.engine.get_query("User", fields=[Count("*")], filters={}).get_sql(), + frappe.qb.get_query("User", fields=[Count("*")], filters={}).get_sql(), frappe.qb.from_("User").select(Count("*")).get_sql(), ) self.assertEqual( - frappe.qb.engine.get_query( - "User", fields="timestamp(creation, modified)", filters={} - ).get_sql(), + frappe.qb.get_query("User", fields="timestamp(creation, modified)", filters={}).get_sql(), frappe.qb.from_("User").select(Timestamp(Field("creation"), Field("modified"))).get_sql(), ) self.assertEqual( - frappe.qb.engine.get_query( + frappe.qb.get_query( "User", fields="Count(name) as count, Max(email) as max_email", filters={} ).get_sql(), frappe.qb.from_("User") @@ -186,85 +180,83 @@ class TestQuery(FrappeTestCase): def test_qb_fields(self): user_doctype = frappe.qb.DocType("User") self.assertEqual( - frappe.qb.engine.get_query( + frappe.qb.get_query( user_doctype, fields=[user_doctype.name, user_doctype.email], filters={} ).get_sql(), frappe.qb.from_(user_doctype).select(user_doctype.name, user_doctype.email).get_sql(), ) self.assertEqual( - frappe.qb.engine.get_query(user_doctype, fields=user_doctype.email, filters={}).get_sql(), + frappe.qb.get_query(user_doctype, fields=user_doctype.email, filters={}).get_sql(), frappe.qb.from_(user_doctype).select(user_doctype.email).get_sql(), ) def test_aliasing(self): user_doctype = frappe.qb.DocType("User") self.assertEqual( - frappe.qb.engine.get_query( - user_doctype, fields=["name as owner", "email as id"], filters={} - ).get_sql(), + frappe.qb.get_query("User", fields=["name as owner", "email as id"], filters={}).get_sql(), frappe.qb.from_(user_doctype) .select(user_doctype.name.as_("owner"), user_doctype.email.as_("id")) .get_sql(), ) self.assertEqual( - frappe.qb.engine.get_query( - user_doctype, fields="name as owner, email as id", filters={} - ).get_sql(), + frappe.qb.get_query(user_doctype, fields="name as owner, email as id", filters={}).get_sql(), frappe.qb.from_(user_doctype) .select(user_doctype.name.as_("owner"), user_doctype.email.as_("id")) .get_sql(), ) self.assertEqual( - frappe.qb.engine.get_query( + frappe.qb.get_query( user_doctype, fields=["Count(name) as count", "email as id"], filters={} ).get_sql(), frappe.qb.from_(user_doctype) - .select(user_doctype.email.as_("id"), Count(Field("name")).as_("count")) + .select(Count(Field("name")).as_("count"), user_doctype.email.as_("id")) .get_sql(), ) @run_only_if(db_type_is.MARIADB) def test_filters(self): self.assertEqual( - frappe.qb.engine.get_query( + frappe.qb.get_query( "User", filters={"IfNull(name, " ")": ("<", Now())}, fields=["Max(name)"] ).run(), frappe.qb.from_("User").select(Max(Field("name"))).where(Ifnull("name", "") < Now()).run(), ) def test_implicit_join_query(self): + self.maxDiff = None + self.assertEqual( - frappe.qb.engine.get_query( + frappe.qb.get_query( "Note", filters={"name": "Test Note Title"}, fields=["name", "`tabNote Seen By`.`user` as seen_by"], ).get_sql(), - "SELECT `tabNote`.`name`,`tabNote Seen By`.`user` seen_by FROM `tabNote` LEFT JOIN `tabNote Seen By` ON `tabNote Seen By`.`parent`=`tabNote`.`name` AND `tabNote Seen By`.`parenttype`='Note' WHERE `tabNote`.`name`='Test Note Title'".replace( + "SELECT `tabNote`.`name`,`tabNote Seen By`.`user` `seen_by` FROM `tabNote` LEFT JOIN `tabNote Seen By` ON `tabNote Seen By`.`parent`=`tabNote`.`name` AND `tabNote Seen By`.`parenttype`='Note' WHERE `tabNote`.`name`='Test Note Title'".replace( "`", '"' if frappe.db.db_type == "postgres" else "`" ), ) self.assertEqual( - frappe.qb.engine.get_query( + frappe.qb.get_query( "Note", filters={"name": "Test Note Title"}, fields=["name", "`tabNote Seen By`.`user` as seen_by", "`tabNote Seen By`.`idx` as idx"], ).get_sql(), - "SELECT `tabNote`.`name`,`tabNote Seen By`.`user` seen_by,`tabNote Seen By`.`idx` idx FROM `tabNote` LEFT JOIN `tabNote Seen By` ON `tabNote Seen By`.`parent`=`tabNote`.`name` AND `tabNote Seen By`.`parenttype`='Note' WHERE `tabNote`.`name`='Test Note Title'".replace( + "SELECT `tabNote`.`name`,`tabNote Seen By`.`user` `seen_by`,`tabNote Seen By`.`idx` `idx` FROM `tabNote` LEFT JOIN `tabNote Seen By` ON `tabNote Seen By`.`parent`=`tabNote`.`name` AND `tabNote Seen By`.`parenttype`='Note' WHERE `tabNote`.`name`='Test Note Title'".replace( "`", '"' if frappe.db.db_type == "postgres" else "`" ), ) self.assertEqual( - frappe.qb.engine.get_query( + frappe.qb.get_query( "Note", filters={"name": "Test Note Title"}, fields=["name", "seen_by.user as seen_by", "`tabNote Seen By`.`idx` as idx"], ).get_sql(), - "SELECT `tabNote`.`name`,`tabNote Seen By`.`user` seen_by,`tabNote Seen By`.`idx` idx FROM `tabNote` LEFT JOIN `tabNote Seen By` ON `tabNote Seen By`.`parent`=`tabNote`.`name` AND `tabNote Seen By`.`parenttype`='Note' WHERE `tabNote`.`name`='Test Note Title'".replace( + "SELECT `tabNote`.`name`,`tabNote Seen By`.`user` `seen_by`,`tabNote Seen By`.`idx` `idx` FROM `tabNote` LEFT JOIN `tabNote Seen By` ON `tabNote Seen By`.`parent`=`tabNote`.`name` AND `tabNote Seen By`.`parenttype`='Note' WHERE `tabNote`.`name`='Test Note Title'".replace( "`", '"' if frappe.db.db_type == "postgres" else "`" ), ) @@ -272,40 +264,40 @@ class TestQuery(FrappeTestCase): @run_only_if(db_type_is.MARIADB) def test_comment_stripping(self): self.assertNotIn( - "email", frappe.qb.engine.get_query("User", fields=["name", "#email"], filters={}).get_sql() + "email", frappe.qb.get_query("User", fields=["name", "#email"], filters={}).get_sql() ) def test_nestedset(self): frappe.db.sql("delete from `tabDocType` where `name` = 'Test Tree DocType'") frappe.db.sql_ddl("drop table if exists `tabTest Tree DocType`") create_tree_docs() - descendants_result = frappe.qb.engine.get_query( + descendants_result = frappe.qb.get_query( "Test Tree DocType", fields=["name"], filters={"name": ("descendants of", "Parent 1")}, - orderby="modified", + order_by="modified", ).run(as_list=1) # Format decendants result descendants_result = list(itertools.chain.from_iterable(descendants_result)) self.assertListEqual(descendants_result, get_descendants_of("Test Tree DocType", "Parent 1")) - ancestors_result = frappe.qb.engine.get_query( + ancestors_result = frappe.qb.get_query( "Test Tree DocType", fields=["name"], filters={"name": ("ancestors of", "Child 2")}, - orderby="modified", + order_by="modified", ).run(as_list=1) # Format ancestors result ancestors_result = list(itertools.chain.from_iterable(ancestors_result)) self.assertListEqual(ancestors_result, get_ancestors_of("Test Tree DocType", "Child 2")) - not_descendants_result = frappe.qb.engine.get_query( + not_descendants_result = frappe.qb.get_query( "Test Tree DocType", fields=["name"], filters={"name": ("not descendants of", "Parent 1")}, - orderby="modified", + order_by="modified", ).run(as_dict=1) self.assertListEqual( @@ -317,11 +309,11 @@ class TestQuery(FrappeTestCase): ), ) - not_ancestors_result = frappe.qb.engine.get_query( + not_ancestors_result = frappe.qb.get_query( "Test Tree DocType", fields=["name"], filters={"name": ("not ancestors of", "Child 2")}, - orderby="modified", + order_by="modified", ).run(as_dict=1) self.assertListEqual( diff --git a/frappe/utils/goal.py b/frappe/utils/goal.py index 13c4633031..0dcadc5ec6 100644 --- a/frappe/utils/goal.py +++ b/frappe/utils/goal.py @@ -24,7 +24,7 @@ def get_monthly_results( date_format = "%m-%Y" if frappe.db.db_type != "postgres" else "MM-YYYY" return dict( - frappe.qb.engine.build_conditions(table=goal_doctype, filters=filters) + frappe.qb.get_query(table=goal_doctype, filters=filters) .select( DateFormat(Table[date_col], date_format).as_("month_year"), Function(aggregation, goal_field), From 847206222f9033ae3e1212a94b76c1df94319d48 Mon Sep 17 00:00:00 2001 From: Faris Ansari Date: Sat, 31 Dec 2022 22:17:20 +0530 Subject: [PATCH 02/23] fix: delete option --- frappe/database/database.py | 2 +- frappe/database/query.py | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/frappe/database/database.py b/frappe/database/database.py index acbd28c9d7..9bb4bfc8e6 100644 --- a/frappe/database/database.py +++ b/frappe/database/database.py @@ -1160,7 +1160,7 @@ class Database: Doctype name can be passed directly, it will be pre-pended with `tab`. """ filters = filters or kwargs.get("conditions") - query = frappe.qb.get_query(table=doctype, filters=filters).delete() + query = frappe.qb.get_query(table=doctype, filters=filters, delete=True) if "debug" not in kwargs: kwargs["debug"] = debug return query.run(**kwargs) diff --git a/frappe/database/query.py b/frappe/database/query.py index 88de7f7088..7119819a40 100644 --- a/frappe/database/query.py +++ b/frappe/database/query.py @@ -245,6 +245,7 @@ class Engine: for_update: bool = False, update: bool = False, into: bool = False, + delete: bool = False, ) -> MySQLQueryBuilder | PostgreSQLQueryBuilder: # Clean up state before each query self.is_mariadb = frappe.db.db_type == "mariadb" @@ -259,6 +260,8 @@ class Engine: self.query = frappe.qb.update(self.table) elif into: self.query = frappe.qb.into(self.table) + elif delete: + self.query = frappe.qb.from_(self.table).delete() else: self.query = frappe.qb.from_(self.table) From e272adb0b13a4e0642af1e9af6e758299eee7c7d Mon Sep 17 00:00:00 2001 From: Faris Ansari Date: Sat, 31 Dec 2022 22:17:39 +0530 Subject: [PATCH 03/23] fix: use table.field instead Field('field') --- frappe/database/query.py | 44 ++++++++-------------------------------- 1 file changed, 8 insertions(+), 36 deletions(-) diff --git a/frappe/database/query.py b/frappe/database/query.py index 7119819a40..b9e60043a0 100644 --- a/frappe/database/query.py +++ b/frappe/database/query.py @@ -130,26 +130,6 @@ def func_timespan(key: Field, value: str) -> frappe.qb: return func_between(key, get_timespan_date_range(value)) -def change_orderby(order: str): - """Convert orderby to standart Order object - - Args: - order (str): Field, order - - Returns: - tuple: field, order - """ - order = order.split() - - try: - if order[1].lower() == "asc": - return order[0], Order.asc - except IndexError: - pass - - return order[0], Order.desc - - def literal_eval_(literal): try: return literal_eval(literal) @@ -164,14 +144,6 @@ def has_function(field): return True -def table_from_string(table: str) -> "DocType": - if frappe.db.db_type == "postgres": - table_name = table.split('"', maxsplit=1)[1].split(".")[0][3:].replace('"', "") - else: - table_name = table.split("`", maxsplit=1)[1].split(".")[0][3:].replace("`", "") - return frappe.qb.DocType(table_name=table_name) - - def get_nested_set_hierarchy_result(doctype: str, name: str, hierarchy: str): table = frappe.qb.DocType(doctype) try: @@ -182,20 +154,20 @@ def get_nested_set_hierarchy_result(doctype: str, name: str, hierarchy: str): if hierarchy in ("descendants of", "not descendants of"): result = ( frappe.qb.from_(table) - .select(Field("name")) - .where(Field("lft") > lft) - .where(Field("rgt") < rgt) - .orderby(Field("lft"), order=Order.asc) + .select(table.name) + .where(table.lft > lft) + .where(table.rgt < rgt) + .orderby(table.lft, order=Order.asc) .run() ) else: # Get ancestor elements of a DocType with a tree structure result = ( frappe.qb.from_(table) - .select(Field("name")) - .where(Field("lft") < lft) - .where(Field("rgt") > rgt) - .orderby(Field("lft"), order=Order.desc) + .select(table.name) + .where(table.lft < lft) + .where(table.rgt > rgt) + .orderby(table.lft, order=Order.desc) .run() ) return result From 99160cbe1703d047c36766e07a6e6fa82064b476 Mon Sep 17 00:00:00 2001 From: Faris Ansari Date: Sat, 31 Dec 2022 22:17:54 +0530 Subject: [PATCH 04/23] fix: refactor usage --- frappe/desk/doctype/number_card/number_card.py | 2 +- frappe/desk/listview.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/frappe/desk/doctype/number_card/number_card.py b/frappe/desk/doctype/number_card/number_card.py index 30ec99644a..d940448cb1 100644 --- a/frappe/desk/doctype/number_card/number_card.py +++ b/frappe/desk/doctype/number_card/number_card.py @@ -200,7 +200,7 @@ def get_cards_for_user(doctype, txt, searchfield, start, page_len, filters): if txt: search_conditions = [numberCard[field].like(f"%{txt}%") for field in searchfields] - condition_query = frappe.qb.get_query(doctype, filters) + condition_query = frappe.qb.get_query(doctype, filters=filters) return ( condition_query.select(numberCard.name, numberCard.label, numberCard.document_type) diff --git a/frappe/desk/listview.py b/frappe/desk/listview.py index 8b514444df..05d45ad9ac 100644 --- a/frappe/desk/listview.py +++ b/frappe/desk/listview.py @@ -36,7 +36,7 @@ def get_group_by_count(doctype: str, current_filters: str, field: str) -> list[d ToDo = DocType("ToDo") User = DocType("User") count = Count("*").as_("count") - filtered_records = frappe.qb.get_query(doctype, filters=current_filters).select("name") + filtered_records = frappe.qb.get_query(doctype, filters=current_filters, fields=["name"]) return ( frappe.qb.from_(ToDo) From 7ca39a81bfbd9a79d85ae6db1e3cf4a99716ce71 Mon Sep 17 00:00:00 2001 From: Faris Ansari Date: Sat, 31 Dec 2022 22:18:07 +0530 Subject: [PATCH 05/23] fix: explicitly specifiy order --- frappe/tests/test_query.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/frappe/tests/test_query.py b/frappe/tests/test_query.py index 957d76d022..486bf9fe49 100644 --- a/frappe/tests/test_query.py +++ b/frappe/tests/test_query.py @@ -275,7 +275,7 @@ class TestQuery(FrappeTestCase): "Test Tree DocType", fields=["name"], filters={"name": ("descendants of", "Parent 1")}, - order_by="modified", + order_by="modified desc", ).run(as_list=1) # Format decendants result @@ -286,7 +286,7 @@ class TestQuery(FrappeTestCase): "Test Tree DocType", fields=["name"], filters={"name": ("ancestors of", "Child 2")}, - order_by="modified", + order_by="modified desc", ).run(as_list=1) # Format ancestors result @@ -297,7 +297,7 @@ class TestQuery(FrappeTestCase): "Test Tree DocType", fields=["name"], filters={"name": ("not descendants of", "Parent 1")}, - order_by="modified", + order_by="modified desc", ).run(as_dict=1) self.assertListEqual( @@ -313,7 +313,7 @@ class TestQuery(FrappeTestCase): "Test Tree DocType", fields=["name"], filters={"name": ("not ancestors of", "Child 2")}, - order_by="modified", + order_by="modified desc", ).run(as_dict=1) self.assertListEqual( From b7c0ba1beaa2cadc5f6bca52ec4e0943c47681d0 Mon Sep 17 00:00:00 2001 From: Faris Ansari Date: Sat, 31 Dec 2022 22:55:00 +0530 Subject: [PATCH 06/23] fix: allow dynamic fields in filters e.g., `filters={'link.field': 'value'}` `filters={'child.field': 'value'}` --- frappe/database/query.py | 53 +++++++++++++++++++++++++++++++------- frappe/tests/test_query.py | 43 +++++++++++++++++++++++++++++++ 2 files changed, 86 insertions(+), 10 deletions(-) diff --git a/frappe/database/query.py b/frappe/database/query.py index b9e60043a0..3593de7c74 100644 --- a/frappe/database/query.py +++ b/frappe/database/query.py @@ -243,7 +243,7 @@ class Engine: for field in self.fields: if isinstance(field, DynamicTableField): - self.query = field.apply(self.query) + self.query = field.apply_select(self.query) else: self.query = self.query.select(field) @@ -318,18 +318,25 @@ class Engine: _value = value _operator = operator - if has_function(field): + if dynamic_field := DynamicTableField.parse(field, self.doctype): + # apply implicit join if link field's field is referenced + self.query = dynamic_field.apply_join(self.query) + _field = dynamic_field.field + elif has_function(field): _field = self.get_function_object(field) elif not doctype or doctype == self.doctype: _field = self.table[field] elif doctype: _field = self.get_table(doctype)[field] - # keep track of implicit join if child table is referenced + # apply implicit join if child table is referenced if doctype and doctype != self.doctype: meta = frappe.get_meta(doctype) - if meta.istable: - self.implicit_joins.add((doctype, "child")) + table = self.get_table(doctype) + if meta.istable and not self.query.is_joined(table): + self.query = self.query.left_join(table).on( + (table.parent == self.table.name) & (table.parenttype == self.doctype) + ) if isinstance(_value, (str, int)): _value = str(_value) @@ -586,19 +593,38 @@ class DynamicTableField: elif linked_field.fieldtype in frappe.model.table_fields: return ChildTableField(linked_doctype, fieldname, doctype, alias=alias) - def apply(self, query: QueryBuilder) -> QueryBuilder: + def apply_select(self, query: QueryBuilder) -> QueryBuilder: raise NotImplementedError class ChildTableField(DynamicTableField): - def apply(self, query: QueryBuilder) -> QueryBuilder: + def __init__( + self, + doctype: str, + fieldname: str, + parent_doctype: str, + alias: str | None = None, + ) -> None: + self.doctype = doctype + self.fieldname = fieldname + self.alias = alias + self.parent_doctype = parent_doctype + self.table = frappe.qb.DocType(self.doctype) + self.field = self.table[self.fieldname] + + def apply_select(self, query: QueryBuilder) -> QueryBuilder: + table = frappe.qb.DocType(self.doctype) + query = self.apply_join(query) + return query.select(getattr(table, self.fieldname).as_(self.alias or None)) + + def apply_join(self, query: QueryBuilder) -> QueryBuilder: table = frappe.qb.DocType(self.doctype) main_table = frappe.qb.DocType(self.parent_doctype) if not query.is_joined(table): query = query.left_join(table).on( (table.parent == main_table.name) & (table.parenttype == self.parent_doctype) ) - return query.select(getattr(table, self.fieldname).as_(self.alias or None)) + return query class LinkTableField(DynamicTableField): @@ -612,10 +638,17 @@ class LinkTableField(DynamicTableField): ) -> None: super().__init__(doctype, fieldname, parent_doctype, alias=alias) self.link_fieldname = link_fieldname + self.table = frappe.qb.DocType(self.doctype) + self.field = self.table[self.fieldname] - def apply(self, query: QueryBuilder) -> QueryBuilder: + def apply_select(self, query: QueryBuilder) -> QueryBuilder: + table = frappe.qb.DocType(self.doctype) + query = self.apply_join(query) + return query.select(getattr(table, self.fieldname).as_(self.alias or None)) + + def apply_join(self, query: QueryBuilder) -> QueryBuilder: table = frappe.qb.DocType(self.doctype) main_table = frappe.qb.DocType(self.parent_doctype) if not query.is_joined(table): query = query.left_join(table).on(table.name == getattr(main_table, self.link_fieldname)) - return query.select(getattr(table, self.fieldname).as_(self.alias or None)) + return query diff --git a/frappe/tests/test_query.py b/frappe/tests/test_query.py index 486bf9fe49..12cb6446d2 100644 --- a/frappe/tests/test_query.py +++ b/frappe/tests/test_query.py @@ -225,6 +225,39 @@ class TestQuery(FrappeTestCase): frappe.qb.from_("User").select(Max(Field("name"))).where(Ifnull("name", "") < Now()).run(), ) + self.assertEqual( + frappe.qb.get_query( + "DocType", + fields=["name"], + filters={"module.app_name": "frappe"}, + ).get_sql(), + "SELECT `tabDocType`.`name` FROM `tabDocType` LEFT JOIN `tabModule Def` ON `tabModule Def`.`name`=`tabDocType`.`module` WHERE `tabModule Def`.`app_name`='frappe'".replace( + "`", '"' if frappe.db.db_type == "postgres" else "`" + ), + ) + + self.assertEqual( + frappe.qb.get_query( + "DocType", + fields=["name"], + filters={"module.app_name": ("like", "frap%")}, + ).get_sql(), + "SELECT `tabDocType`.`name` FROM `tabDocType` LEFT JOIN `tabModule Def` ON `tabModule Def`.`name`=`tabDocType`.`module` WHERE `tabModule Def`.`app_name` LIKE 'frap%'".replace( + "`", '"' if frappe.db.db_type == "postgres" else "`" + ), + ) + + self.assertEqual( + frappe.qb.get_query( + "DocType", + fields=["name"], + filters={"permissions.role": "System Manager"}, + ).get_sql(), + "SELECT `tabDocType`.`name` FROM `tabDocType` LEFT JOIN `tabDocPerm` ON `tabDocPerm`.`parent`=`tabDocType`.`name` AND `tabDocPerm`.`parenttype`='DocType' WHERE `tabDocPerm`.`role`='System Manager'".replace( + "`", '"' if frappe.db.db_type == "postgres" else "`" + ), + ) + def test_implicit_join_query(self): self.maxDiff = None @@ -261,6 +294,16 @@ class TestQuery(FrappeTestCase): ), ) + self.assertEqual( + frappe.qb.get_query( + "DocType", + fields=["name", "module.app_name as app_name"], + ).get_sql(), + "SELECT `tabDocType`.`name`,`tabModule Def`.`app_name` `app_name` FROM `tabDocType` LEFT JOIN `tabModule Def` ON `tabModule Def`.`name`=`tabDocType`.`module`".replace( + "`", '"' if frappe.db.db_type == "postgres" else "`" + ), + ) + @run_only_if(db_type_is.MARIADB) def test_comment_stripping(self): self.assertNotIn( From e4ac91a035faf88560911057b0e8cf926a619ad9 Mon Sep 17 00:00:00 2001 From: Faris Ansari Date: Mon, 9 Jan 2023 15:20:30 +0530 Subject: [PATCH 07/23] fix: ignore string with parenthesis if it is not an sql function --- 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 3593de7c74..ac23097226 100644 --- a/frappe/database/query.py +++ b/frappe/database/query.py @@ -140,7 +140,7 @@ def literal_eval_(literal): def has_function(field): _field = field.casefold() if (isinstance(field, str) and "`" not in field) else field if not issubclass(type(_field), Criterion): - if any([f"{func}(" in _field for func in SQL_FUNCTIONS]) or "(" in _field: + if any([f"{func}(" in _field for func in SQL_FUNCTIONS]): return True From 35c2654f005a60495978f6588e2eb4be64f87872 Mon Sep 17 00:00:00 2001 From: Faris Ansari Date: Mon, 9 Jan 2023 15:34:50 +0530 Subject: [PATCH 08/23] chore: indentation fix --- frappe/database/database.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/database/database.py b/frappe/database/database.py index c9c37fda30..c104c9cf9c 100644 --- a/frappe/database/database.py +++ b/frappe/database/database.py @@ -887,7 +887,7 @@ class Database: field, val, modified=modified, modified_by=modified_by, update_modified=update_modified ) - query = frappe.qb.get_query(table=dt, filters=dn, update=True) + query = frappe.qb.get_query(table=dt, filters=dn, update=True) if isinstance(dn, str): frappe.clear_document_cache(dt, dn) From f982439eb9a86561cdb0b9d449ab83a12c56e0a0 Mon Sep 17 00:00:00 2001 From: Faris Ansari Date: Mon, 9 Jan 2023 16:43:44 +0530 Subject: [PATCH 09/23] fix: pass fields explicitly - to prevent addition of default `name` field - also, add fields only if it is a select query --- frappe/database/query.py | 18 +++++++++--------- frappe/query_builder/functions.py | 6 +++--- frappe/utils/goal.py | 11 +++++++---- 3 files changed, 19 insertions(+), 16 deletions(-) diff --git a/frappe/database/query.py b/frappe/database/query.py index ac23097226..b9dfa33217 100644 --- a/frappe/database/query.py +++ b/frappe/database/query.py @@ -236,16 +236,16 @@ class Engine: self.query = frappe.qb.from_(self.table).delete() else: self.query = frappe.qb.from_(self.table) + # add fields + self.fields = self.parse_fields(fields) + if not self.fields: + self.fields = [getattr(self.table, pluck or "name")] - self.fields = self.parse_fields(fields) - if not self.fields: - self.fields = [getattr(self.table, pluck or "name")] - - for field in self.fields: - if isinstance(field, DynamicTableField): - self.query = field.apply_select(self.query) - else: - self.query = self.query.select(field) + for field in self.fields: + if isinstance(field, DynamicTableField): + self.query = field.apply_select(self.query) + else: + self.query = self.query.select(field) self.apply_filters(filters) self.apply_implicit_joins() diff --git a/frappe/query_builder/functions.py b/frappe/query_builder/functions.py index e1ab182553..512df8835c 100644 --- a/frappe/query_builder/functions.py +++ b/frappe/query_builder/functions.py @@ -119,9 +119,9 @@ class Cast_(Function): def _aggregate(function, dt, fieldname, filters, **kwargs): return ( - frappe.qb.get_query(dt, filters=filters) - .select(function(PseudoColumn(fieldname))) - .run(**kwargs)[0][0] + frappe.qb.get_query(dt, filters=filters, fields=[function(PseudoColumn(fieldname))]).run( + **kwargs + )[0][0] or 0 ) diff --git a/frappe/utils/goal.py b/frappe/utils/goal.py index 0dcadc5ec6..f60aec4d2b 100644 --- a/frappe/utils/goal.py +++ b/frappe/utils/goal.py @@ -24,10 +24,13 @@ def get_monthly_results( date_format = "%m-%Y" if frappe.db.db_type != "postgres" else "MM-YYYY" return dict( - frappe.qb.get_query(table=goal_doctype, filters=filters) - .select( - DateFormat(Table[date_col], date_format).as_("month_year"), - Function(aggregation, goal_field), + frappe.qb.get_query( + table=goal_doctype, + fields=[ + DateFormat(Table[date_col], date_format).as_("month_year"), + Function(aggregation, goal_field), + ], + filters=filters, ) .groupby("month_year") .run() From 6192a9285a561b0e7d219ca04a36251aaec864a7 Mon Sep 17 00:00:00 2001 From: Faris Ansari Date: Mon, 9 Jan 2023 17:51:55 +0530 Subject: [PATCH 10/23] fix: use Field objects as is in apply_filter --- frappe/database/query.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/frappe/database/query.py b/frappe/database/query.py index b9dfa33217..dc6511bb95 100644 --- a/frappe/database/query.py +++ b/frappe/database/query.py @@ -318,7 +318,9 @@ class Engine: _value = value _operator = operator - if dynamic_field := DynamicTableField.parse(field, self.doctype): + if isinstance(_field, Field): + pass + elif dynamic_field := DynamicTableField.parse(field, self.doctype): # apply implicit join if link field's field is referenced self.query = dynamic_field.apply_join(self.query) _field = dynamic_field.field From 9e9de7053cf7c852daa3d631f2f25239d90c2cf9 Mon Sep 17 00:00:00 2001 From: Faris Ansari Date: Mon, 9 Jan 2023 18:19:31 +0530 Subject: [PATCH 11/23] fix: set default order_by direction to desc --- 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 dc6511bb95..eb092c9d0d 100644 --- a/frappe/database/query.py +++ b/frappe/database/query.py @@ -502,7 +502,7 @@ class Engine: for declaration in order_by.split(","): if _order_by := declaration.strip(): parts = _order_by.split(" ") - order_field, order_direction = parts[0], parts[1] if len(parts) > 1 else "asc" + order_field, order_direction = parts[0], parts[1] if len(parts) > 1 else "desc" order_direction = Order.asc if order_direction.lower() == "asc" else Order.desc self.query = self.query.orderby(order_field, order=order_direction) From 60febc9799e0c45590ed966967c6ec03377cefed Mon Sep 17 00:00:00 2001 From: Faris Ansari Date: Mon, 9 Jan 2023 19:43:48 +0530 Subject: [PATCH 12/23] fix: list filter filters as list must always be list of list --- frappe/tests/test_db.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/tests/test_db.py b/frappe/tests/test_db.py index 3962cc746d..ed01af655c 100644 --- a/frappe/tests/test_db.py +++ b/frappe/tests/test_db.py @@ -545,7 +545,7 @@ class TestDB(FrappeTestCase): self.assertEqual((frappe.db.count("Note")), 2) # simple filters - self.assertEqual((frappe.db.count("Note", ["title", "=", "note1"])), 1) + self.assertEqual((frappe.db.count("Note", [["title", "=", "note1"]])), 1) frappe.get_doc(doctype="Note", title="note3", content="something other").insert() From 08fc5b5c90368d66b5374d7478227b8b8dab3290 Mon Sep 17 00:00:00 2001 From: Faris Ansari Date: Mon, 9 Jan 2023 19:54:26 +0530 Subject: [PATCH 13/23] fix: allow list of dict in filters --- frappe/database/query.py | 27 +++++++++++++++------------ 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/frappe/database/query.py b/frappe/database/query.py index eb092c9d0d..532e03f2c6 100644 --- a/frappe/database/query.py +++ b/frappe/database/query.py @@ -292,19 +292,22 @@ class Engine: self.apply_dict_filters(filters) elif isinstance(filters, (list, tuple)): - self.apply_list_filters(filters) + for filter in filters: + if isinstance(filter, (str, int, Criterion, dict)): + self.apply_filters(filter) + elif isinstance(filter, (list, tuple)): + self.apply_list_filters(filter) - def apply_list_filters(self, filters: list[list]): - for filter in filters: - if len(filter) == 2: - field, value = filter - self._apply_filter(field, value) - elif len(filter) == 3: - field, operator, value = filter - self._apply_filter(field, value, operator) - elif len(filter) == 4: - doctype, field, operator, value = filter - self._apply_filter(field, value, operator, doctype) + def apply_list_filters(self, filter: list): + if len(filter) == 2: + field, value = filter + self._apply_filter(field, value) + elif len(filter) == 3: + field, operator, value = filter + self._apply_filter(field, value, operator) + elif len(filter) == 4: + doctype, field, operator, value = filter + self._apply_filter(field, value, operator, doctype) def apply_dict_filters(self, filters: dict[str, str | int | list]): for key in filters: From fe13108eecc246c9e0aa91987e3b235ba1098ac2 Mon Sep 17 00:00:00 2001 From: Faris Ansari Date: Tue, 10 Jan 2023 16:15:08 +0530 Subject: [PATCH 14/23] fix: refactor - move operator map in separate file - remove unnecessary code - organize functions --- frappe/database/operator_map.py | 138 +++++++++++++++ frappe/database/query.py | 305 +++++++------------------------- frappe/query_builder/utils.py | 6 +- 3 files changed, 208 insertions(+), 241 deletions(-) create mode 100644 frappe/database/operator_map.py diff --git a/frappe/database/operator_map.py b/frappe/database/operator_map.py new file mode 100644 index 0000000000..2c8b53dae3 --- /dev/null +++ b/frappe/database/operator_map.py @@ -0,0 +1,138 @@ +# Copyright (c) 2023, Frappe Technologies Pvt. Ltd. and Contributors +# MIT License. See license.txt + +import operator +from typing import Callable + +import frappe +from frappe.database.utils import NestedSetHierarchy +from frappe.model.db_query import get_timespan_date_range +from frappe.query_builder import Field + + +def like(key: Field, value: str) -> frappe.qb: + """Wrapper method for `LIKE` + + Args: + key (str): field + value (str): criterion + + Returns: + frappe.qb: `frappe.qb object with `LIKE` + """ + return key.like(value) + + +def func_in(key: Field, value: list | tuple) -> frappe.qb: + """Wrapper method for `IN` + + Args: + key (str): field + value (Union[int, str]): criterion + + Returns: + frappe.qb: `frappe.qb object with `IN` + """ + if isinstance(value, str): + value = value.split(",") + return key.isin(value) + + +def not_like(key: Field, value: str) -> frappe.qb: + """Wrapper method for `NOT LIKE` + + Args: + key (str): field + value (str): criterion + + Returns: + frappe.qb: `frappe.qb object with `NOT LIKE` + """ + return key.not_like(value) + + +def func_not_in(key: Field, value: list | tuple | str): + """Wrapper method for `NOT IN` + + Args: + key (str): field + value (Union[int, str]): criterion + + Returns: + frappe.qb: `frappe.qb object with `NOT IN` + """ + if isinstance(value, str): + value = value.split(",") + return key.notin(value) + + +def func_regex(key: Field, value: str) -> frappe.qb: + """Wrapper method for `REGEX` + + Args: + key (str): field + value (str): criterion + + Returns: + frappe.qb: `frappe.qb object with `REGEX` + """ + return key.regex(value) + + +def func_between(key: Field, value: list | tuple) -> frappe.qb: + """Wrapper method for `BETWEEN` + + Args: + key (str): field + value (Union[int, str]): criterion + + Returns: + frappe.qb: `frappe.qb object with `BETWEEN` + """ + return key[slice(*value)] + + +def func_is(key, value): + "Wrapper for IS" + return key.isnotnull() if value.lower() == "set" else key.isnull() + + +def func_timespan(key: Field, value: str) -> frappe.qb: + """Wrapper method for `TIMESPAN` + + Args: + key (str): field + value (str): criterion + + Returns: + frappe.qb: `frappe.qb object with `TIMESPAN` + """ + + return func_between(key, get_timespan_date_range(value)) + + +# default operators +OPERATOR_MAP: dict[str, Callable] = { + "+": operator.add, + "=": operator.eq, + "-": operator.sub, + "!=": operator.ne, + "<": operator.lt, + ">": operator.gt, + "<=": operator.le, + "=<": operator.le, + ">=": operator.ge, + "=>": operator.ge, + "/": operator.truediv, + "*": operator.mul, + "in": func_in, + "not in": func_not_in, + "like": like, + "not like": not_like, + "regex": func_regex, + "between": func_between, + "is": func_is, + "timespan": func_timespan, + "nested_set": NestedSetHierarchy, + # TODO: Add support for custom operators (WIP) - via filters_config hooks +} diff --git a/frappe/database/query.py b/frappe/database/query.py index 532e03f2c6..4852f80739 100644 --- a/frappe/database/query.py +++ b/frappe/database/query.py @@ -1,20 +1,16 @@ import itertools -import operator import re from ast import literal_eval -from functools import cached_property from types import BuiltinFunctionType -from typing import TYPE_CHECKING, Callable +from typing import TYPE_CHECKING import sqlparse -from pypika.dialects import MySQLQueryBuilder, PostgreSQLQueryBuilder from pypika.queries import QueryBuilder import frappe from frappe import _ -from frappe.database.utils import NestedSetHierarchy, is_pypika_function_object -from frappe.model.db_query import get_timespan_date_range -from frappe.query_builder import Criterion, Field, Order, Table, functions +from frappe.database.operator_map import OPERATOR_MAP +from frappe.query_builder import Criterion, Field, Order, functions from frappe.query_builder.functions import Function, SqlFunctions from frappe.query_builder.utils import PseudoColumnMapper from frappe.utils.data import MARIADB_SPECIFIC_COMMENT @@ -29,186 +25,12 @@ SQL_FUNCTIONS = [sql_function.value for sql_function in SqlFunctions] COMMA_PATTERN = re.compile(r",\s*(?![^()]*\))") -def like(key: Field, value: str) -> frappe.qb: - """Wrapper method for `LIKE` - - Args: - key (str): field - value (str): criterion - - Returns: - frappe.qb: `frappe.qb object with `LIKE` - """ - return key.like(value) - - -def func_in(key: Field, value: list | tuple) -> frappe.qb: - """Wrapper method for `IN` - - Args: - key (str): field - value (Union[int, str]): criterion - - Returns: - frappe.qb: `frappe.qb object with `IN` - """ - if isinstance(value, str): - value = value.split(",") - return key.isin(value) - - -def not_like(key: Field, value: str) -> frappe.qb: - """Wrapper method for `NOT LIKE` - - Args: - key (str): field - value (str): criterion - - Returns: - frappe.qb: `frappe.qb object with `NOT LIKE` - """ - return key.not_like(value) - - -def func_not_in(key: Field, value: list | tuple | str): - """Wrapper method for `NOT IN` - - Args: - key (str): field - value (Union[int, str]): criterion - - Returns: - frappe.qb: `frappe.qb object with `NOT IN` - """ - if isinstance(value, str): - value = value.split(",") - return key.notin(value) - - -def func_regex(key: Field, value: str) -> frappe.qb: - """Wrapper method for `REGEX` - - Args: - key (str): field - value (str): criterion - - Returns: - frappe.qb: `frappe.qb object with `REGEX` - """ - return key.regex(value) - - -def func_between(key: Field, value: list | tuple) -> frappe.qb: - """Wrapper method for `BETWEEN` - - Args: - key (str): field - value (Union[int, str]): criterion - - Returns: - frappe.qb: `frappe.qb object with `BETWEEN` - """ - return key[slice(*value)] - - -def func_is(key, value): - "Wrapper for IS" - return key.isnotnull() if value.lower() == "set" else key.isnull() - - -def func_timespan(key: Field, value: str) -> frappe.qb: - """Wrapper method for `TIMESPAN` - - Args: - key (str): field - value (str): criterion - - Returns: - frappe.qb: `frappe.qb object with `TIMESPAN` - """ - - return func_between(key, get_timespan_date_range(value)) - - -def literal_eval_(literal): - try: - return literal_eval(literal) - except (ValueError, SyntaxError): - return literal - - -def has_function(field): - _field = field.casefold() if (isinstance(field, str) and "`" not in field) else field - if not issubclass(type(_field), Criterion): - if any([f"{func}(" in _field for func in SQL_FUNCTIONS]): - return True - - -def get_nested_set_hierarchy_result(doctype: str, name: str, hierarchy: str): - table = frappe.qb.DocType(doctype) - try: - lft, rgt = frappe.qb.from_(table).select("lft", "rgt").where(table.name == name).run()[0] - except IndexError: - lft, rgt = None, None - - if hierarchy in ("descendants of", "not descendants of"): - result = ( - frappe.qb.from_(table) - .select(table.name) - .where(table.lft > lft) - .where(table.rgt < rgt) - .orderby(table.lft, order=Order.asc) - .run() - ) - else: - # Get ancestor elements of a DocType with a tree structure - result = ( - frappe.qb.from_(table) - .select(table.name) - .where(table.lft < lft) - .where(table.rgt > rgt) - .orderby(table.lft, order=Order.desc) - .run() - ) - return result - - -# default operators -OPERATOR_MAP: dict[str, Callable] = { - "+": operator.add, - "=": operator.eq, - "-": operator.sub, - "!=": operator.ne, - "<": operator.lt, - ">": operator.gt, - "<=": operator.le, - "=<": operator.le, - ">=": operator.ge, - "=>": operator.ge, - "/": operator.truediv, - "*": operator.mul, - "in": func_in, - "not in": func_not_in, - "like": like, - "not like": not_like, - "regex": func_regex, - "between": func_between, - "is": func_is, - "timespan": func_timespan, - "nested_set": NestedSetHierarchy, - # TODO: Add support for custom operators (WIP) - via filters_config hooks -} - - class Engine: - tables: dict[str, str] = {} - def get_query( self, table: str, fields: list | tuple | None = None, filters: dict[str, str | int] | str | int | list[list | str | int] | None = None, - pluck: str | None = None, order_by: str | None = None, group_by: str | None = None, limit: int | None = None, @@ -218,15 +40,11 @@ class Engine: update: bool = False, into: bool = False, delete: bool = False, - ) -> MySQLQueryBuilder | PostgreSQLQueryBuilder: - # Clean up state before each query + ) -> QueryBuilder: self.is_mariadb = frappe.db.db_type == "mariadb" self.is_postgres = frappe.db.db_type == "postgres" - self.tables = {} - self.implicit_joins = set() - self.doctype = table - self.table = self.get_table(table) + self.table = frappe.qb.DocType(table) if update: self.query = frappe.qb.update(self.table) @@ -236,19 +54,9 @@ class Engine: self.query = frappe.qb.from_(self.table).delete() else: self.query = frappe.qb.from_(self.table) - # add fields - self.fields = self.parse_fields(fields) - if not self.fields: - self.fields = [getattr(self.table, pluck or "name")] - - for field in self.fields: - if isinstance(field, DynamicTableField): - self.query = field.apply_select(self.query) - else: - self.query = self.query.select(field) + self.apply_fields(fields) self.apply_filters(filters) - self.apply_implicit_joins() self.apply_order_by(order_by) if limit: @@ -268,16 +76,21 @@ class Engine: return self.query - def get_table(self, table_name: str | Table) -> Table: - if isinstance(table_name, Table): - return table_name - table_name = table_name.strip('"').strip("'") - if table_name not in self.tables: - self.tables[table_name] = frappe.qb.DocType(table_name) - return self.tables[table_name] + def apply_fields(self, fields): + # add fields + self.fields = self.parse_fields(fields) + if not self.fields: + self.fields = [getattr(self.table, "name")] + + for field in self.fields: + if isinstance(field, DynamicTableField): + self.query = field.apply_select(self.query) + else: + self.query = self.query.select(field) def apply_filters( - self, filters: dict[str, str | int | list] | str | int | list[list] | None = None + self, + filters: dict[str, str | int] | str | int | list[list | str | int] | None = None, ): if not filters: return @@ -332,12 +145,12 @@ class Engine: elif not doctype or doctype == self.doctype: _field = self.table[field] elif doctype: - _field = self.get_table(doctype)[field] + _field = frappe.qb.DocType(doctype)[field] # apply implicit join if child table is referenced if doctype and doctype != self.doctype: meta = frappe.get_meta(doctype) - table = self.get_table(doctype) + table = frappe.qb.DocType(doctype) if meta.istable and not self.query.is_joined(table): self.query = self.query.left_join(table).on( (table.parent == self.table.name) & (table.parenttype == self.doctype) @@ -354,14 +167,14 @@ class Engine: _value = self.get_function_object(_value) # Nested set - if _operator in self.OPERATOR_MAP["nested_set"]: + if _operator in OPERATOR_MAP["nested_set"]: hierarchy = _operator docname = _value result = get_nested_set_hierarchy_result(self.doctype, docname, hierarchy) operator_fn = ( - self.OPERATOR_MAP["not in"] + OPERATOR_MAP["not in"] if hierarchy in ("not ancestors of", "not descendants of") - else self.OPERATOR_MAP["in"] + else OPERATOR_MAP["in"] ) if result: result = list(itertools.chain.from_iterable(result)) @@ -370,7 +183,7 @@ class Engine: self.query = self.query.where(operator_fn(_field, ("",))) return - operator_fn = self.OPERATOR_MAP[_operator.casefold()] + operator_fn = OPERATOR_MAP[_operator.casefold()] if _value is None and isinstance(_field, Field): self.query = self.query.where(_field.isnull()) else: @@ -490,15 +303,6 @@ class Engine: return _fields - def apply_implicit_joins(self): - for d in self.implicit_joins: - doctype, join_type = d - table = self.get_table(doctype) - if join_type == "child": - self.query = self.query.left_join(table).on( - (table.parent == self.table.name) & (table.parenttype == self.doctype) - ) - def apply_order_by(self, order_by: str | None): if not order_by or order_by == "KEEP_DEFAULT_ORDERING": return @@ -509,22 +313,6 @@ class Engine: order_direction = Order.asc if order_direction.lower() == "asc" else Order.desc self.query = self.query.orderby(order_field, order=order_direction) - @cached_property - def OPERATOR_MAP(self): - # default operators - all_operators = OPERATOR_MAP.copy() - - # TODO: update with site-specific custom operators / removed previous buggy implementation - if frappe.get_hooks("filters_config"): - from frappe.utils.commands import warn - - warn( - "The 'filters_config' hook used to add custom operators is not yet implemented" - " in frappe.db.query engine. Use db_query (frappe.get_list) instead." - ) - - return all_operators - class Permission: @classmethod @@ -657,3 +445,46 @@ class LinkTableField(DynamicTableField): if not query.is_joined(table): query = query.left_join(table).on(table.name == getattr(main_table, self.link_fieldname)) return query + + +def literal_eval_(literal): + try: + return literal_eval(literal) + except (ValueError, SyntaxError): + return literal + + +def has_function(field): + _field = field.casefold() if (isinstance(field, str) and "`" not in field) else field + if not issubclass(type(_field), Criterion): + if any([f"{func}(" in _field for func in SQL_FUNCTIONS]): + return True + + +def get_nested_set_hierarchy_result(doctype: str, name: str, hierarchy: str): + table = frappe.qb.DocType(doctype) + try: + lft, rgt = frappe.qb.from_(table).select("lft", "rgt").where(table.name == name).run()[0] + except IndexError: + lft, rgt = None, None + + if hierarchy in ("descendants of", "not descendants of"): + result = ( + frappe.qb.from_(table) + .select(table.name) + .where(table.lft > lft) + .where(table.rgt < rgt) + .orderby(table.lft, order=Order.asc) + .run() + ) + else: + # Get ancestor elements of a DocType with a tree structure + result = ( + frappe.qb.from_(table) + .select(table.name) + .where(table.lft < lft) + .where(table.rgt > rgt) + .orderby(table.lft, order=Order.desc) + .run() + ) + return result diff --git a/frappe/query_builder/utils.py b/frappe/query_builder/utils.py index 004a226036..bfc2c49b8e 100644 --- a/frappe/query_builder/utils.py +++ b/frappe/query_builder/utils.py @@ -2,9 +2,7 @@ from enum import Enum from importlib import import_module from typing import Any, Callable, get_type_hints -from pypika import Query -from pypika.dialects import MySQLQueryBuilder, PostgreSQLQueryBuilder -from pypika.queries import Column +from pypika.queries import Column, QueryBuilder from pypika.terms import PseudoColumn import frappe @@ -56,7 +54,7 @@ def get_query_builder(type_of_db: str) -> Postgres | MariaDB: return picks[db] -def get_query(*args, **kwargs) -> MySQLQueryBuilder | PostgreSQLQueryBuilder: +def get_query(*args, **kwargs) -> QueryBuilder: from frappe.database.query import Engine return Engine().get_query(*args, **kwargs) From 95d8a0f919c68f5f828256b4957045bb786e05de Mon Sep 17 00:00:00 2001 From: Faris Ansari Date: Tue, 10 Jan 2023 16:48:38 +0530 Subject: [PATCH 15/23] fix: allow Table instance --- frappe/database/query.py | 14 ++++++++++---- frappe/database/utils.py | 8 ++++++++ 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/frappe/database/query.py b/frappe/database/query.py index 4852f80739..cdca28601d 100644 --- a/frappe/database/query.py +++ b/frappe/database/query.py @@ -5,11 +5,12 @@ from types import BuiltinFunctionType from typing import TYPE_CHECKING import sqlparse -from pypika.queries import QueryBuilder +from pypika.queries import QueryBuilder, Table import frappe from frappe import _ from frappe.database.operator_map import OPERATOR_MAP +from frappe.database.utils import get_doctype_name from frappe.query_builder import Criterion, Field, Order, functions from frappe.query_builder.functions import Function, SqlFunctions from frappe.query_builder.utils import PseudoColumnMapper @@ -28,7 +29,7 @@ COMMA_PATTERN = re.compile(r",\s*(?![^()]*\))") class Engine: def get_query( self, - table: str, + table: str | Table, fields: list | tuple | None = None, filters: dict[str, str | int] | str | int | list[list | str | int] | None = None, order_by: str | None = None, @@ -43,8 +44,13 @@ class Engine: ) -> QueryBuilder: self.is_mariadb = frappe.db.db_type == "mariadb" self.is_postgres = frappe.db.db_type == "postgres" - self.doctype = table - self.table = frappe.qb.DocType(table) + + if isinstance(table, Table): + self.table = table + self.doctype = get_doctype_name(table.get_sql()) + else: + self.doctype = table + self.table = frappe.qb.DocType(table) if update: self.query = frappe.qb.update(self.table) diff --git a/frappe/database/utils.py b/frappe/database/utils.py index 4ea039e5a7..f89ba3c737 100644 --- a/frappe/database/utils.py +++ b/frappe/database/utils.py @@ -34,6 +34,14 @@ def is_pypika_function_object(field: str) -> bool: return getattr(field, "__module__", None) == "pypika.functions" or isinstance(field, Function) +def get_doctype_name(table_name: str) -> str: + if "tab" in table_name: + table_name = table_name.replace("tab", "") + table_name = table_name.replace("`", "") + table_name = table_name.replace('"', "") + return table_name + + class LazyString: def _setup(self) -> None: raise NotImplementedError From a0f6a5ff46068dd9b9665bb1bd2e16564b84a2ce Mon Sep 17 00:00:00 2001 From: Faris Ansari Date: Tue, 10 Jan 2023 18:21:50 +0530 Subject: [PATCH 16/23] fix: move pluck to run --- frappe/database/database.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/frappe/database/database.py b/frappe/database/database.py index c104c9cf9c..2c0718a48a 100644 --- a/frappe/database/database.py +++ b/frappe/database/database.py @@ -835,10 +835,9 @@ class Database: fields=field, filters=names, order_by=order_by, - pluck=pluck, distinct=distinct, limit=limit, - ).run(debug=debug, run=run, as_dict=as_dict) + ).run(debug=debug, run=run, as_dict=as_dict, pluck=pluck) return {} def set_value( From 76deeb531cd6a8d022426658dd3e488dff0ce127 Mon Sep 17 00:00:00 2001 From: Faris Ansari Date: Tue, 10 Jan 2023 18:22:05 +0530 Subject: [PATCH 17/23] fix: support list of str or int in filters --- frappe/database/query.py | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/frappe/database/query.py b/frappe/database/query.py index cdca28601d..726f930b19 100644 --- a/frappe/database/query.py +++ b/frappe/database/query.py @@ -111,11 +111,14 @@ class Engine: self.apply_dict_filters(filters) elif isinstance(filters, (list, tuple)): - for filter in filters: - if isinstance(filter, (str, int, Criterion, dict)): - self.apply_filters(filter) - elif isinstance(filter, (list, tuple)): - self.apply_list_filters(filter) + if all(isinstance(d, (str, int)) for d in filters): + self.apply_dict_filters({"name": ("in", filters)}) + else: + for filter in filters: + if isinstance(filter, (str, int, Criterion, dict)): + self.apply_filters(filter) + elif isinstance(filter, (list, tuple)): + self.apply_list_filters(filter) def apply_list_filters(self, filter: list): if len(filter) == 2: From 5340efd1567759c6fc532094beb453989b5c81d2 Mon Sep 17 00:00:00 2001 From: Faris Ansari Date: Fri, 13 Jan 2023 16:21:51 +0530 Subject: [PATCH 18/23] fix: don't cast integer value in filter --- frappe/database/query.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/frappe/database/query.py b/frappe/database/query.py index 726f930b19..7152b77dab 100644 --- a/frappe/database/query.py +++ b/frappe/database/query.py @@ -165,9 +165,7 @@ class Engine: (table.parent == self.table.name) & (table.parenttype == self.doctype) ) - if isinstance(_value, (str, int)): - _value = str(_value) - elif isinstance(_value, (list, tuple)): + if isinstance(_value, (list, tuple)): _operator, _value = _value elif isinstance(_value, bool): _value = int(_value) From a93380ac9c55135443f4c0ad5d4ac24efa6e75f2 Mon Sep 17 00:00:00 2001 From: Faris Ansari Date: Fri, 13 Jan 2023 16:22:25 +0530 Subject: [PATCH 19/23] fix: handle empty list for "in" and "not in" --- frappe/database/query.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/frappe/database/query.py b/frappe/database/query.py index 7152b77dab..ea5f2b8760 100644 --- a/frappe/database/query.py +++ b/frappe/database/query.py @@ -173,6 +173,9 @@ class Engine: if isinstance(_value, str) and has_function(_value): _value = self.get_function_object(_value) + if isinstance(_value, (list, tuple)) and not _value: + _value = ("",) + # Nested set if _operator in OPERATOR_MAP["nested_set"]: hierarchy = _operator From 52e3d8d58b7fd62619bcd689b3599d501702cf57 Mon Sep 17 00:00:00 2001 From: Faris Ansari Date: Mon, 16 Jan 2023 14:11:37 +0530 Subject: [PATCH 20/23] fix: handle empty string passed to filters --- 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 ea5f2b8760..072f1eed4f 100644 --- a/frappe/database/query.py +++ b/frappe/database/query.py @@ -98,7 +98,7 @@ class Engine: self, filters: dict[str, str | int] | str | int | list[list | str | int] | None = None, ): - if not filters: + if filters is None: return if isinstance(filters, (str, int)): From 5bc5ff100bffc0210ddddab8c8513b6e8aa02dd6 Mon Sep 17 00:00:00 2001 From: Faris Ansari Date: Mon, 16 Jan 2023 14:12:04 +0530 Subject: [PATCH 21/23] test: tests for various filter options --- frappe/tests/test_query.py | 41 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/frappe/tests/test_query.py b/frappe/tests/test_query.py index 12cb6446d2..42d3670f2d 100644 --- a/frappe/tests/test_query.py +++ b/frappe/tests/test_query.py @@ -258,6 +258,47 @@ class TestQuery(FrappeTestCase): ), ) + self.assertEqual( + frappe.qb.get_query( + "DocType", + fields=["module"], + filters="", + ).get_sql(), + "SELECT `module` FROM `tabDocType` WHERE `name`=''".replace( + "`", '"' if frappe.db.db_type == "postgres" else "`" + ), + ) + + self.assertEqual( + frappe.qb.get_query( + "DocType", + filters=["ToDo", "Note"], + ).get_sql(), + "SELECT `name` FROM `tabDocType` WHERE `name` IN ('ToDo','Note')".replace( + "`", '"' if frappe.db.db_type == "postgres" else "`" + ), + ) + + self.assertEqual( + frappe.qb.get_query( + "DocType", + filters={"name": ("in", [])}, + ).get_sql(), + "SELECT `name` FROM `tabDocType` WHERE `name` IN ('')".replace( + "`", '"' if frappe.db.db_type == "postgres" else "`" + ), + ) + + self.assertEqual( + frappe.qb.get_query( + "DocType", + filters=[1, 2, 3], + ).get_sql(), + "SELECT `name` FROM `tabDocType` WHERE `name` IN (1,2,3)".replace( + "`", '"' if frappe.db.db_type == "postgres" else "`" + ), + ) + def test_implicit_join_query(self): self.maxDiff = None From 543458b473491db2e37683044ca8d01abe41290b Mon Sep 17 00:00:00 2001 From: Faris Ansari Date: Mon, 16 Jan 2023 15:38:15 +0530 Subject: [PATCH 22/23] fix: handle empty list as filters --- frappe/database/query.py | 2 +- frappe/tests/test_query.py | 8 ++++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/frappe/database/query.py b/frappe/database/query.py index 072f1eed4f..10423f9ca4 100644 --- a/frappe/database/query.py +++ b/frappe/database/query.py @@ -111,7 +111,7 @@ class Engine: self.apply_dict_filters(filters) elif isinstance(filters, (list, tuple)): - if all(isinstance(d, (str, int)) for d in filters): + if all(isinstance(d, (str, int)) for d in filters) and len(filters) > 0: self.apply_dict_filters({"name": ("in", filters)}) else: for filter in filters: diff --git a/frappe/tests/test_query.py b/frappe/tests/test_query.py index 42d3670f2d..82218e5952 100644 --- a/frappe/tests/test_query.py +++ b/frappe/tests/test_query.py @@ -299,6 +299,14 @@ class TestQuery(FrappeTestCase): ), ) + self.assertEqual( + frappe.qb.get_query( + "DocType", + filters=[], + ).get_sql(), + "SELECT `name` FROM `tabDocType`".replace("`", '"' if frappe.db.db_type == "postgres" else "`"), + ) + def test_implicit_join_query(self): self.maxDiff = None From ee17b221103907bcdfd194d48ba8fbbfea40c1c7 Mon Sep 17 00:00:00 2001 From: Faris Ansari Date: Tue, 17 Jan 2023 14:04:31 +0530 Subject: [PATCH 23/23] fix: only replace "tab" at the beginning --- frappe/database/utils.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frappe/database/utils.py b/frappe/database/utils.py index f89ba3c737..304fd72be6 100644 --- a/frappe/database/utils.py +++ b/frappe/database/utils.py @@ -35,8 +35,8 @@ def is_pypika_function_object(field: str) -> bool: def get_doctype_name(table_name: str) -> str: - if "tab" in table_name: - table_name = table_name.replace("tab", "") + if table_name.startswith(("tab", "`tab", '"tab')): + table_name = table_name.replace("tab", "", 1) table_name = table_name.replace("`", "") table_name = table_name.replace('"', "") return table_name