From 36194ac5f56185b1bf989477ed62933706a955e8 Mon Sep 17 00:00:00 2001 From: Aradhya Date: Mon, 18 Jul 2022 21:09:42 +0530 Subject: [PATCH 01/43] feat: moving all get_all queries to frappe.qb.engine --- frappe/model/db_query.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/frappe/model/db_query.py b/frappe/model/db_query.py index 9cf831a8b9..0cd9430883 100644 --- a/frappe/model/db_query.py +++ b/frappe/model/db_query.py @@ -121,6 +121,9 @@ class DatabaseQuery: # if `filters` is a list of strings, its probably fields filters, fields = fields, filters + self.fields, self.filters = fields, filters + self.ignore_permissions = ignore_permissions + if fields: self.fields = fields else: @@ -207,6 +210,17 @@ class DatabaseQuery: % args ) + if self.ignore_permissions: + return frappe.qb.engine.get_query( + table=self.doctype, fields=self.fields, filters=self.filters + ).run( + as_dict=not self.as_list, + debug=self.debug, + update=self.update, + ignore_ddl=self.ignore_ddl, + run=self.run, + ) + return frappe.db.sql( query, as_dict=not self.as_list, From 2c3ef963a1c0d1a8e6d5285302e32691e9350fe4 Mon Sep 17 00:00:00 2001 From: Aradhya Date: Mon, 18 Jul 2022 21:29:01 +0530 Subject: [PATCH 02/43] fix: using new set of fields and filters --- frappe/model/db_query.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frappe/model/db_query.py b/frappe/model/db_query.py index 0cd9430883..1cf142e857 100644 --- a/frappe/model/db_query.py +++ b/frappe/model/db_query.py @@ -121,7 +121,7 @@ class DatabaseQuery: # if `filters` is a list of strings, its probably fields filters, fields = fields, filters - self.fields, self.filters = fields, filters + self.qb_fields, self.qb_filters = fields, filters self.ignore_permissions = ignore_permissions if fields: @@ -212,7 +212,7 @@ class DatabaseQuery: if self.ignore_permissions: return frappe.qb.engine.get_query( - table=self.doctype, fields=self.fields, filters=self.filters + table=self.doctype, fields=self.qb_fields, filters=self.qb_filters ).run( as_dict=not self.as_list, debug=self.debug, From f3f7baf185ba27790328495e42a334f31d566c5b Mon Sep 17 00:00:00 2001 From: Aradhya Date: Mon, 18 Jul 2022 21:51:38 +0530 Subject: [PATCH 03/43] feat: Added locals object to execute & pluck to qb engine --- frappe/model/db_query.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/frappe/model/db_query.py b/frappe/model/db_query.py index 1cf142e857..83ecc6bcf3 100644 --- a/frappe/model/db_query.py +++ b/frappe/model/db_query.py @@ -121,8 +121,8 @@ class DatabaseQuery: # if `filters` is a list of strings, its probably fields filters, fields = fields, filters + self.locals = locals() self.qb_fields, self.qb_filters = fields, filters - self.ignore_permissions = ignore_permissions if fields: self.fields = fields @@ -210,9 +210,12 @@ class DatabaseQuery: % args ) - if self.ignore_permissions: + if self.locals.get("ignore_permissions"): return frappe.qb.engine.get_query( - table=self.doctype, fields=self.qb_fields, filters=self.qb_filters + table=self.doctype, + fields=self.qb_fields, + filters=self.qb_filters, + pluck=self.locals.get("pluck"), ).run( as_dict=not self.as_list, debug=self.debug, From 4e0ec7919e765681e920a038d5674a686849a3b4 Mon Sep 17 00:00:00 2001 From: Aradhya Date: Mon, 18 Jul 2022 22:19:52 +0530 Subject: [PATCH 04/43] fix: removing additional "`" from fields --- frappe/database/query.py | 1 + 1 file changed, 1 insertion(+) diff --git a/frappe/database/query.py b/frappe/database/query.py index 9bb5383b24..8421d417f3 100644 --- a/frappe/database/query.py +++ b/frappe/database/query.py @@ -465,6 +465,7 @@ class Engine: is_list = False if is_list: + fields = [field.replace("""`""", "") for field in fields] function_objects += self.function_objects_from_list(fields=fields) is_str = isinstance(fields, str) From f2ada4630c36b18f8d25762551e2d5bb7a64330a Mon Sep 17 00:00:00 2001 From: Aradhya Date: Sat, 23 Jul 2022 17:18:43 +0530 Subject: [PATCH 05/43] fix: removed exessive quotes from query --- frappe/database/query.py | 8 +++++--- frappe/model/db_query.py | 2 +- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/frappe/database/query.py b/frappe/database/query.py index 8421d417f3..29838d9486 100644 --- a/frappe/database/query.py +++ b/frappe/database/query.py @@ -3,7 +3,7 @@ import re from ast import literal_eval from functools import cached_property from types import BuiltinFunctionType -from typing import TYPE_CHECKING, Any, Callable +from typing import Any, Callable import frappe from frappe import _ @@ -465,7 +465,6 @@ class Engine: is_list = False if is_list: - fields = [field.replace("""`""", "") for field in fields] function_objects += self.function_objects_from_list(fields=fields) is_str = isinstance(fields, str) @@ -514,6 +513,7 @@ class Engine: table: str, fields: list | tuple, filters: dict[str, str | int] | str | int | list[list | str | int] = None, + run: bool = False, **kwargs, ): # Clean up state before each query @@ -539,7 +539,9 @@ class Engine: else: query = criterion.select(fields) - + query = str(query).replace("``", "`") + if run: + return frappe.db.sql(query, **kwargs) return query diff --git a/frappe/model/db_query.py b/frappe/model/db_query.py index 83ecc6bcf3..2fff70cd6e 100644 --- a/frappe/model/db_query.py +++ b/frappe/model/db_query.py @@ -210,7 +210,7 @@ class DatabaseQuery: % args ) - if self.locals.get("ignore_permissions"): + if self.locals.get("ignore_permissions") and ("." not in self.qb_fields): return frappe.qb.engine.get_query( table=self.doctype, fields=self.qb_fields, From a3ae6794eced49d8837bd636fe4174b9f2f754f6 Mon Sep 17 00:00:00 2001 From: Aradhya Date: Sun, 24 Jul 2022 01:25:38 +0530 Subject: [PATCH 06/43] fix: fixed PseudoColumn fields --- frappe/database/query.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/frappe/database/query.py b/frappe/database/query.py index 29838d9486..ba0789f545 100644 --- a/frappe/database/query.py +++ b/frappe/database/query.py @@ -10,6 +10,7 @@ from frappe import _ from frappe.model.db_query import get_timespan_date_range from frappe.query_builder import Criterion, Field, Order, Table, functions from frappe.query_builder.functions import Function, SqlFunctions +from frappe.query_builder.utils import PseudoColumn TAB_PATTERN = re.compile("^tab") WORDS_PATTERN = re.compile(r"\w+") @@ -458,7 +459,6 @@ class Engine: return None function_objects = [] - is_list = isinstance(fields, (list, tuple, set)) if is_list and len(fields) == 1: fields = fields[0] @@ -496,6 +496,8 @@ class Engine: if " as " in field: field, reference = field.split(" as ") updated_fields.append(Field(field.strip()).as_(reference)) + elif "`.`" in str(field): + updated_fields.append(PseudoColumn(field.strip())) else: updated_fields.append(Field(field)) @@ -539,9 +541,7 @@ class Engine: else: query = criterion.select(fields) - query = str(query).replace("``", "`") - if run: - return frappe.db.sql(query, **kwargs) + return query From 9348e1cd980f4eda9009369fac04da5291679a70 Mon Sep 17 00:00:00 2001 From: Aradhya Date: Sun, 24 Jul 2022 01:30:44 +0530 Subject: [PATCH 07/43] fix: removed eccess "`" --- frappe/integrations/doctype/webhook/__init__.py | 2 +- frappe/model/db_query.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/frappe/integrations/doctype/webhook/__init__.py b/frappe/integrations/doctype/webhook/__init__.py index 192cd2fa12..b9c96190ca 100644 --- a/frappe/integrations/doctype/webhook/__init__.py +++ b/frappe/integrations/doctype/webhook/__init__.py @@ -25,7 +25,7 @@ def run_webhooks(doc, method): # query webhooks webhooks_list = frappe.get_all( "Webhook", - fields=["name", "`condition`", "webhook_docevent", "webhook_doctype"], + fields=["name", "condition", "webhook_docevent", "webhook_doctype"], filters={"enabled": True}, ) diff --git a/frappe/model/db_query.py b/frappe/model/db_query.py index 2fff70cd6e..83ecc6bcf3 100644 --- a/frappe/model/db_query.py +++ b/frappe/model/db_query.py @@ -210,7 +210,7 @@ class DatabaseQuery: % args ) - if self.locals.get("ignore_permissions") and ("." not in self.qb_fields): + if self.locals.get("ignore_permissions"): return frappe.qb.engine.get_query( table=self.doctype, fields=self.qb_fields, From 3802d83900cb803ca0e53dbfa75f765f3fe00090 Mon Sep 17 00:00:00 2001 From: Aradhya Date: Sun, 24 Jul 2022 01:39:53 +0530 Subject: [PATCH 08/43] test: Added test for "``" query --- frappe/tests/test_query.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/frappe/tests/test_query.py b/frappe/tests/test_query.py index 8afcaf07d0..5fae35623d 100644 --- a/frappe/tests/test_query.py +++ b/frappe/tests/test_query.py @@ -4,6 +4,7 @@ import frappe from frappe.query_builder import Field from frappe.query_builder.functions import Abs, Count, Max, Timestamp from frappe.tests.test_query_builder import db_type_is, run_only_if +from frappe.query_builder.utils import PseudoColumn class TestQuery(unittest.TestCase): @@ -42,6 +43,16 @@ class TestQuery(unittest.TestCase): .get_sql(), ) + self.assertEqual( + frappe.qb.engine.get_query( + "User", fields=["`tabUser`.`name`", "`tabUser`.`email`"], filters={"name": "Administrator"} + ).run(), + frappe.qb.from_("User") + .select(Field("name"), Field("email")) + .where(Field("name") == "Administrator") + .run() + ) + def test_functions_fields(self): self.assertEqual( frappe.qb.engine.get_query("User", fields="Count(name)", filters={}).get_sql(), From 3d6247efbd0ddba8170386a59fe121086890cdb5 Mon Sep 17 00:00:00 2001 From: Aradhya Date: Sun, 24 Jul 2022 14:22:57 +0530 Subject: [PATCH 09/43] refactor: removed unnecesary pseudocolumns --- .../permitted_documents_for_user.py | 2 +- frappe/share.py | 12 ++++++------ frappe/tests/test_query.py | 3 +-- 3 files changed, 8 insertions(+), 9 deletions(-) diff --git a/frappe/core/report/permitted_documents_for_user/permitted_documents_for_user.py b/frappe/core/report/permitted_documents_for_user/permitted_documents_for_user.py index 362cc6b105..b8d7ca2a97 100644 --- a/frappe/core/report/permitted_documents_for_user/permitted_documents_for_user.py +++ b/frappe/core/report/permitted_documents_for_user/permitted_documents_for_user.py @@ -38,7 +38,7 @@ def validate(user, doctype): def get_columns_and_fields(doctype): columns = [f"Name:Link/{doctype}:200"] - fields = ["`name`"] + fields = ["name"] for df in frappe.get_meta(doctype).fields: if df.in_list_view and df.fieldtype in data_fieldtypes: fields.append(f"`{df.fieldname}`") diff --git a/frappe/share.py b/frappe/share.py index dfb4836995..7f7ca2033f 100644 --- a/frappe/share.py +++ b/frappe/share.py @@ -104,12 +104,12 @@ def get_users(doctype, name): return frappe.db.get_all( "DocShare", fields=[ - "`name`", - "`user`", - "`read`", - "`write`", - "`submit`", - "`share`", + "name", # Don't understant the need for pseudocolumns here, don't know why get_all supports it? + "user", + "read", + "write", + "submit", + "share", "everyone", "owner", "creation", diff --git a/frappe/tests/test_query.py b/frappe/tests/test_query.py index 5fae35623d..7b9a307e04 100644 --- a/frappe/tests/test_query.py +++ b/frappe/tests/test_query.py @@ -4,7 +4,6 @@ import frappe from frappe.query_builder import Field from frappe.query_builder.functions import Abs, Count, Max, Timestamp from frappe.tests.test_query_builder import db_type_is, run_only_if -from frappe.query_builder.utils import PseudoColumn class TestQuery(unittest.TestCase): @@ -50,7 +49,7 @@ class TestQuery(unittest.TestCase): frappe.qb.from_("User") .select(Field("name"), Field("email")) .where(Field("name") == "Administrator") - .run() + .run(), ) def test_functions_fields(self): From 69089d3fd523655467515bae5b7e4bcd6291634c Mon Sep 17 00:00:00 2001 From: Aradhya Date: Sun, 24 Jul 2022 14:47:51 +0530 Subject: [PATCH 10/43] refactor: removed unnecesary pseudocolumns --- frappe/workflow/doctype/workflow_action/workflow_action.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/workflow/doctype/workflow_action/workflow_action.py b/frappe/workflow/doctype/workflow_action/workflow_action.py index 038a3021d2..545ad6ec77 100644 --- a/frappe/workflow/doctype/workflow_action/workflow_action.py +++ b/frappe/workflow/doctype/workflow_action/workflow_action.py @@ -290,7 +290,7 @@ def update_completed_workflow_actions_using_user(doc, user=None): def get_next_possible_transitions(workflow_name, state, doc=None): transitions = frappe.get_all( "Workflow Transition", - fields=["allowed", "action", "state", "allow_self_approval", "next_state", "`condition`"], + fields=["allowed", "action", "state", "allow_self_approval", "next_state", "condition"], filters=[["parent", "=", workflow_name], ["state", "=", state]], ) From a7d74266d2b177512c833799c8f74e3e25a3198c Mon Sep 17 00:00:00 2001 From: Aradhya Date: Sun, 24 Jul 2022 15:40:48 +0530 Subject: [PATCH 11/43] feat: Flexible pseudocolumns --- frappe/database/query.py | 2 +- frappe/tests/test_query.py | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/frappe/database/query.py b/frappe/database/query.py index e4738d3a0c..e9e5521a9f 100644 --- a/frappe/database/query.py +++ b/frappe/database/query.py @@ -491,7 +491,7 @@ class Engine: if " as " in field: field, reference = field.split(" as ") updated_fields.append(Field(field.strip()).as_(reference)) - elif "`.`" in str(field): + elif "`" in str(field): updated_fields.append(PseudoColumn(field.strip())) else: updated_fields.append(Field(field)) diff --git a/frappe/tests/test_query.py b/frappe/tests/test_query.py index 7b9a307e04..be5fcc33ac 100644 --- a/frappe/tests/test_query.py +++ b/frappe/tests/test_query.py @@ -31,10 +31,9 @@ class TestQuery(unittest.TestCase): .where(Field("name") == "Administrator") .get_sql(), ) - self.assertEqual( frappe.qb.engine.get_query( - "User", fields=["name, email"], filters={"name": "Administrator"} + "User", fields=["`name`, `email`"], filters={"name": "Administrator"} ).get_sql(), frappe.qb.from_("User") .select(Field("name"), Field("email")) From 1a74d19bade10fba015daad2dad1a4293447e950 Mon Sep 17 00:00:00 2001 From: Aradhya Date: Sun, 24 Jul 2022 16:56:47 +0530 Subject: [PATCH 12/43] feat: Added support for pseudocolumns in functions --- 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 e9e5521a9f..846507896f 100644 --- a/frappe/database/query.py +++ b/frappe/database/query.py @@ -398,7 +398,11 @@ class Engine: *map(lambda field: Field(field.strip()), arg.split(_operator)), ) - field = Field(initial_fields) if not has_primitive_operator else field + field = ( + (Field(initial_fields) if "`" not in initial_fields else PseudoColumn(initial_fields)) + if not has_primitive_operator + else field + ) else: field = initial_fields @@ -416,7 +420,7 @@ class Engine: def function_objects_from_list(self, fields): functions = [] for field in fields: - field = field.casefold() if isinstance(field, str) else 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: functions.append(field) @@ -429,7 +433,7 @@ class Engine: if isinstance(fields, str): if function.alias: fields = fields.replace(" as " + function.alias.casefold(), "") - fields = BRACKETS_PATTERN.sub("", fields.replace(function.name.casefold(), "")) + fields = BRACKETS_PATTERN.sub("", fields.casefold().replace(function.name.casefold(), "")) # Check if only comma is left in fields after stripping functions. if "," in fields and (len(fields.strip()) == 1): fields = "" @@ -464,7 +468,7 @@ class Engine: is_str = isinstance(fields, str) if is_str: - fields = fields.casefold() + 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) @@ -510,7 +514,6 @@ class Engine: table: str, fields: list | tuple, filters: dict[str, str | int] | str | int | list[list | str | int] = None, - run: bool = False, **kwargs, ): # Clean up state before each query From 4779443466eb814f1ea4eee98dfa990e98d6e643 Mon Sep 17 00:00:00 2001 From: Aradhya Date: Sun, 24 Jul 2022 22:53:24 +0530 Subject: [PATCH 13/43] feat: Added support for aliasing in PseudoColumns --- frappe/database/query.py | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/frappe/database/query.py b/frappe/database/query.py index 846507896f..570c3e6a63 100644 --- a/frappe/database/query.py +++ b/frappe/database/query.py @@ -480,9 +480,14 @@ class Engine: if is_str: if fields == "*": return fields - if " as " in fields: - fields, reference = fields.split(" as ") - fields = Field(fields).as_(reference) + if "`" in fields: + fields = PseudoColumn(fields) + if " as " in str(fields): + fields, reference = str(fields).split(" as ") + if "`" in str(fields): + fields = PseudoColumn(f"{fields} as {reference}") + else: + fields = Field(fields).as_(reference) if not is_str and fields: if issubclass(type(fields), Criterion): From 41f62ba0d2594185051ec8d4ca4da0a0f407c9f8 Mon Sep 17 00:00:00 2001 From: Aradhya Date: Tue, 26 Jul 2022 19:33:45 +0530 Subject: [PATCH 14/43] fix: converting back to capitalized doctype names --- frappe/database/query.py | 28 +++++++++++++++++++++------- frappe/tests/test_query.py | 22 ++++++++++++++++++++++ 2 files changed, 43 insertions(+), 7 deletions(-) diff --git a/frappe/database/query.py b/frappe/database/query.py index 570c3e6a63..c4c22266e9 100644 --- a/frappe/database/query.py +++ b/frappe/database/query.py @@ -17,6 +17,7 @@ WORDS_PATTERN = re.compile(r"\w+") BRACKETS_PATTERN = re.compile(r"\(.*?\)|$") SQL_FUNCTIONS = [sql_function.value for sql_function in SqlFunctions] COMMA_PATTERN = re.compile(r",\s*(?![^()]*\))") +TABLE_PATTERN = re.compile(r"\btab\w+") def like(key: Field, value: str) -> frappe.qb: @@ -434,6 +435,11 @@ class Engine: if function.alias: fields = fields.replace(" as " + function.alias.casefold(), "") fields = BRACKETS_PATTERN.sub("", fields.casefold().replace(function.name.casefold(), "")) + # Converting back to capitalized doctype names. + if "tab" in fields: + fields = TABLE_PATTERN.sub( + lambda p: p.group(0)[:3] + p.group(0)[3].upper() + p.group(0)[3 + 1 :], fields + ) # Check if only comma is left in fields after stripping functions. if "," in fields and (len(fields.strip()) == 1): fields = "" @@ -443,20 +449,24 @@ class Engine: if isinstance(field, str): if function.alias: field = field.replace(" as " + function.alias.casefold(), "") - field = ( - BRACKETS_PATTERN.sub("", field).strip().casefold().replace(function.name.casefold(), "") + substituted_string = ( + BRACKETS_PATTERN.sub("", field).strip().casefold() + if "`" not in field + else BRACKETS_PATTERN.sub("", field).strip() ) - updated_fields.append(field) - + # 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 set_fields(self, fields, **kwargs): fields = kwargs.get("pluck") if kwargs.get("pluck") else fields or "name" 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: @@ -499,7 +509,11 @@ class Engine: if not isinstance(field, Criterion) and field: if " as " in field: field, reference = field.split(" as ") - updated_fields.append(Field(field.strip()).as_(reference)) + if "`" in field: + updated_fields.append(PseudoColumn(f"{field} as {reference}")) + else: + updated_fields.append(Field(field.strip()).as_(reference)) + elif "`" in str(field): updated_fields.append(PseudoColumn(field.strip())) else: diff --git a/frappe/tests/test_query.py b/frappe/tests/test_query.py index be5fcc33ac..9df7c3f6e0 100644 --- a/frappe/tests/test_query.py +++ b/frappe/tests/test_query.py @@ -51,6 +51,28 @@ class TestQuery(unittest.TestCase): .run(), ) + self.assertEqual( + frappe.qb.engine.get_query( + "User", + fields=["`tabUser`.`name` as owner", "`tabUser`.`email`"], + filters={"name": "Administrator"}, + ).run(as_dict=1), + frappe.qb.from_("User") + .select(Field("name").as_("owner"), Field("email")) + .where(Field("name") == "Administrator") + .run(as_dict=1), + ) + + self.assertEqual( + frappe.qb.engine.get_query( + "User", fields=["`tabUser`.`name`, Count(`name`) as count"], filters={"name": "Administrator"} + ).run(), + frappe.qb.from_("User") + .select(Field("name"), Count("name").as_("count")) + .where(Field("name") == "Administrator") + .run(), + ) + def test_functions_fields(self): self.assertEqual( frappe.qb.engine.get_query("User", fields="Count(name)", filters={}).get_sql(), From 90282d968e9fd7f3e82b483c2590562acbbf1731 Mon Sep 17 00:00:00 2001 From: Aradhya Date: Tue, 26 Jul 2022 20:01:17 +0530 Subject: [PATCH 15/43] fix: added support for non iterables in "in" and "not in" --- frappe/database/query.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/frappe/database/query.py b/frappe/database/query.py index c4c22266e9..a1942597af 100644 --- a/frappe/database/query.py +++ b/frappe/database/query.py @@ -3,7 +3,7 @@ import re from ast import literal_eval from functools import cached_property from types import BuiltinFunctionType -from typing import Any, Callable +from typing import Any, Callable, Iterable import frappe from frappe import _ @@ -43,6 +43,8 @@ def func_in(key: Field, value: list | tuple) -> frappe.qb: Returns: frappe.qb: `frappe.qb object with `IN` """ + if isinstance(value, str): + value = value.split(",") return key.isin(value) @@ -69,6 +71,8 @@ def func_not_in(key: Field, value: list | tuple): Returns: frappe.qb: `frappe.qb object with `NOT IN` """ + if isinstance(value, str): + value = value.split(",") return key.notin(value) From 65e0251e70958e01a985627664f296428b7845ae Mon Sep 17 00:00:00 2001 From: Aradhya Date: Wed, 27 Jul 2022 18:38:02 +0530 Subject: [PATCH 16/43] feat: Added support for "`" in alias name --- frappe/database/query.py | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/frappe/database/query.py b/frappe/database/query.py index a1942597af..0ebafc3960 100644 --- a/frappe/database/query.py +++ b/frappe/database/query.py @@ -412,6 +412,9 @@ class Engine: field = initial_fields _args.append(field) + + if alias and "`" in alias: + alias = alias.replace("`", "") try: return getattr(functions, func)(*_args, alias=alias or None) except AttributeError: @@ -437,7 +440,11 @@ class Engine: for function in function_objects: if isinstance(fields, str): if function.alias: - fields = fields.replace(" as " + function.alias.casefold(), "") + 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()}`", "") fields = BRACKETS_PATTERN.sub("", fields.casefold().replace(function.name.casefold(), "")) # Converting back to capitalized doctype names. if "tab" in fields: @@ -452,7 +459,11 @@ class Engine: for field in fields: if isinstance(field, str): if function.alias: - field = field.replace(" as " + function.alias.casefold(), "") + to_replace = " as " + function.alias.casefold() + if to_replace in field: + field = field.replace(to_replace, "") + elif " as " + f"`{function.alias.casefold()}" in field: + field = field.replace(" as " + f"`{function.alias.casefold()}`", "") substituted_string = ( BRACKETS_PATTERN.sub("", field).strip().casefold() if "`" not in field From 8624afdf6c4792ebd1d7d3c63f2a9ed87f9c7061 Mon Sep 17 00:00:00 2001 From: Aradhya Date: Wed, 27 Jul 2022 18:39:31 +0530 Subject: [PATCH 17/43] test: Added tests for "`" in alias name --- frappe/tests/test_query.py | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/frappe/tests/test_query.py b/frappe/tests/test_query.py index 9df7c3f6e0..47856f29e6 100644 --- a/frappe/tests/test_query.py +++ b/frappe/tests/test_query.py @@ -73,6 +73,26 @@ class TestQuery(unittest.TestCase): .run(), ) + self.assertEqual( + frappe.qb.engine.get_query( + "User", fields=["`tabUser`.`name`, Count(`name`) as `count`"], filters={"name": "Administrator"} + ).run(), + frappe.qb.from_("User") + .select(Field("name"), Count("name").as_("count")) + .where(Field("name") == "Administrator") + .run(), + ) + + self.assertEqual( + frappe.qb.engine.get_query( + "User", fields="`tabUser`.`name`, Count(`name`) as `count`", filters={"name": "Administrator"} + ).run(), + frappe.qb.from_("User") + .select(Field("name"), Count("name").as_("count")) + .where(Field("name") == "Administrator") + .run(), + ) + def test_functions_fields(self): self.assertEqual( frappe.qb.engine.get_query("User", fields="Count(name)", filters={}).get_sql(), From 1ae3b7f16b02f9ac4836ee84e343b3a495fca288 Mon Sep 17 00:00:00 2001 From: Aradhya Date: Wed, 27 Jul 2022 21:19:08 +0530 Subject: [PATCH 18/43] feat: Added support for fields in Locate --- frappe/query_builder/functions.py | 3 +++ frappe/tests/test_query.py | 4 +++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/frappe/query_builder/functions.py b/frappe/query_builder/functions.py index 824de7fbf5..1c48f8a402 100644 --- a/frappe/query_builder/functions.py +++ b/frappe/query_builder/functions.py @@ -17,6 +17,9 @@ class Concat_ws(Function): class Locate(Function): def __init__(self, *terms, **kwargs): + terms = list(terms) + if not isinstance(terms[0], str): + terms[0] = terms[0].get_sql() super().__init__("LOCATE", *terms, **kwargs) diff --git a/frappe/tests/test_query.py b/frappe/tests/test_query.py index 47856f29e6..c3b42061a9 100644 --- a/frappe/tests/test_query.py +++ b/frappe/tests/test_query.py @@ -75,7 +75,9 @@ class TestQuery(unittest.TestCase): self.assertEqual( frappe.qb.engine.get_query( - "User", fields=["`tabUser`.`name`, Count(`name`) as `count`"], filters={"name": "Administrator"} + "User", + fields=["`tabUser`.`name`, Count(`name`) as `count`"], + filters={"name": "Administrator"}, ).run(), frappe.qb.from_("User") .select(Field("name"), Count("name").as_("count")) From 6c905233c5f040869349c74761bcffc9779b408a Mon Sep 17 00:00:00 2001 From: Aradhya Date: Thu, 28 Jul 2022 23:55:58 +0530 Subject: [PATCH 19/43] feat: Added support for string filters in query --- frappe/database/query.py | 37 ++++++++++++++++++------------- frappe/query_builder/functions.py | 8 +++++++ frappe/tests/test_query.py | 10 ++++++++- 3 files changed, 38 insertions(+), 17 deletions(-) diff --git a/frappe/database/query.py b/frappe/database/query.py index 0ebafc3960..31c5d20ae3 100644 --- a/frappe/database/query.py +++ b/frappe/database/query.py @@ -2,6 +2,7 @@ import operator import re from ast import literal_eval from functools import cached_property +import sys from types import BuiltinFunctionType from typing import Any, Callable, Iterable @@ -120,20 +121,6 @@ def func_timespan(key: Field, value: str) -> frappe.qb: return func_between(key, get_timespan_date_range(value)) - -def make_function(key: Any, value: int | str): - """returns fucntion query - - Args: - key (Any): field - value (Union[int, str]): criterion - - Returns: - frappe.qb: frappe.qb object - """ - return OPERATOR_MAP[value[0].casefold()](key, value[1]) - - def change_orderby(order: str): """Convert orderby to standart Order object @@ -161,6 +148,13 @@ def literal_eval_(literal): 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]) or "(" in _field: + return True + + # default operators OPERATOR_MAP: dict[str, Callable] = { "+": operator.add, @@ -305,7 +299,7 @@ class Engine: else: _operator = self.OPERATOR_MAP[filters[1].casefold()] if not isinstance(filters[0], str): - conditions = make_function(filters[0], filters[2]) + conditions = self.make_function_for_filters(filters[0], filters[2]) break conditions = conditions.where(_operator(Field(filters[0]), filters[2])) break @@ -331,12 +325,15 @@ class Engine: 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(make_function(key, value)) + conditions = conditions.where(self.make_function_for_filters(key, value)) continue if isinstance(value, (list, tuple)): _operator = self.OPERATOR_MAP[value[0].casefold()] @@ -378,6 +375,12 @@ class Engine: return criterion + def make_function_for_filters(self, key: Any, 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]) + def get_function_object(self, field: str) -> "Function": """Expects field to look like 'SUM(*)' or 'name' or something similar. Returns PyPika Function object""" func = field.split("(", maxsplit=1)[0].capitalize() @@ -416,6 +419,8 @@ class Engine: if alias and "`" in alias: alias = alias.replace("`", "") try: + if func.casefold() == "now": + return getattr(functions, func)() return getattr(functions, func)(*_args, alias=alias or None) except AttributeError: # Fall back for functions not present in `SqlFunctions`` diff --git a/frappe/query_builder/functions.py b/frappe/query_builder/functions.py index 1c48f8a402..f355eb05bc 100644 --- a/frappe/query_builder/functions.py +++ b/frappe/query_builder/functions.py @@ -22,6 +22,13 @@ class Locate(Function): terms[0] = terms[0].get_sql() super().__init__("LOCATE", *terms, **kwargs) +class Ifnull(IfNull): + def __init__(self, condition, term, **kwargs): + if not isinstance(condition, str): + condition = condition.get_sql() + if not isinstance(term, str): + term = term.get_sql() + super().__init__(condition, term, **kwargs) class Timestamp(Function): def __init__(self, term: str, time=None, alias=None): @@ -108,6 +115,7 @@ class SqlFunctions(Enum): Min = "min" Abs = "abs" Timestamp = "timestamp" + IfNull = "ifnull" def _max(dt, fieldname, filters=None, **kwargs): diff --git a/frappe/tests/test_query.py b/frappe/tests/test_query.py index c3b42061a9..3322ab9e01 100644 --- a/frappe/tests/test_query.py +++ b/frappe/tests/test_query.py @@ -2,7 +2,7 @@ import unittest import frappe from frappe.query_builder import Field -from frappe.query_builder.functions import Abs, Count, Max, Timestamp +from frappe.query_builder.functions import Abs, Count, Ifnull, Max, Now, Timestamp from frappe.tests.test_query_builder import db_type_is, run_only_if @@ -178,3 +178,11 @@ class TestQuery(unittest.TestCase): .select(user_doctype.email.as_("id"), Count(Field("name")).as_("count")) .get_sql(), ) + + def test_filters(self): + self.assertEqual( + frappe.qb.engine.get_query( + "User", filters={"IfNull(name, " ")": ("<", Now())}, fields=["Max(name)"] + ).run(), + frappe.qb.from_("User").select(Max(Field("name"))).where(Ifnull("name", "") < Now()).run(), + ) \ No newline at end of file From 5792265f7713b52d7c6fcd343e442922501185b4 Mon Sep 17 00:00:00 2001 From: Aradhya Date: Wed, 3 Aug 2022 16:31:04 +0530 Subject: [PATCH 20/43] feat: Added support for fieldnames from child tables --- frappe/database/query.py | 36 +++++++++++++++++++++++++++++++----- 1 file changed, 31 insertions(+), 5 deletions(-) diff --git a/frappe/database/query.py b/frappe/database/query.py index 89e3857f9e..b873844ff6 100644 --- a/frappe/database/query.py +++ b/frappe/database/query.py @@ -4,7 +4,7 @@ from ast import literal_eval from functools import cached_property import sys from types import BuiltinFunctionType -from typing import Any, Callable, Iterable +from typing import Any, Callable import frappe from frappe import _ @@ -481,10 +481,30 @@ class Engine: fields = [field for field in updated_fields if field] return fields - def set_fields(self, fields, **kwargs): + def get_fieldnames_from_child_table(self, doctype, fields): + # convert child_table.fieldname to `tabChild DocType`.`fieldname` + linked_doctype, fieldname = None, None + for field in fields: + if "." in field and "tab" not in field: + original_field = field + alias = None + if " as " in field: + field, alias = field.split(" as ") + linked_fieldname, fieldname = field.split(".") + linked_field = frappe.get_meta(doctype).get_field(linked_fieldname) + linked_doctype = linked_field.options + field = f"`tab{linked_doctype}`.`{fieldname}`" + if alias: + field = f"{field} as {alias}" + fields[fields.index(original_field)] = field + + return fields, linked_doctype, fieldname + + def set_fields(self, table, fields, **kwargs): fields = kwargs.get("pluck") if kwargs.get("pluck") else fields or "name" if isinstance(fields, list) and None in fields and Field not in fields: return None + linked_doctype, linked_field = None, None function_objects = [] is_list = isinstance(fields, (list, tuple, set)) if is_list and len(fields) == 1: @@ -523,6 +543,7 @@ class Engine: updated_fields = [] if "*" in fields: return fields + fields, linked_doctype, linked_field = 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: @@ -538,13 +559,12 @@ class Engine: updated_fields.append(Field(field)) fields = updated_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 + return fields, linked_doctype, linked_field def get_query( self, @@ -556,7 +576,13 @@ class Engine: # Clean up state before each query self.tables = {} criterion = self.build_conditions(table, filters, **kwargs) - fields = self.set_fields(kwargs.get("field_objects") or fields, **kwargs) + fields, linked_doctype, linked_field = self.set_fields(table, kwargs.get("field_objects") or fields, **kwargs) + + if linked_doctype: + linked_doctype = frappe.qb.DocType(linked_doctype) + criterion = criterion.left_join(linked_doctype).on( + linked_doctype.name == frappe.qb.DocType(table).linked_field + ) join = kwargs.get("join").replace(" ", "_") if kwargs.get("join") else "left_join" From 4df8b18503012e02f9034b5ad6f0e1b70fcccdcf Mon Sep 17 00:00:00 2001 From: Aradhya Date: Thu, 4 Aug 2022 18:58:03 +0530 Subject: [PATCH 21/43] fix: fixed fields when getting field names from child tables --- frappe/database/query.py | 44 ++++++++++++++++++++++++---------------- 1 file changed, 27 insertions(+), 17 deletions(-) diff --git a/frappe/database/query.py b/frappe/database/query.py index b873844ff6..f1a6d3141e 100644 --- a/frappe/database/query.py +++ b/frappe/database/query.py @@ -1,8 +1,9 @@ import operator import re -from ast import literal_eval -from functools import cached_property import sys +from ast import literal_eval +from fileinput import filename +from functools import cached_property from types import BuiltinFunctionType from typing import Any, Callable @@ -121,6 +122,7 @@ 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 @@ -483,28 +485,26 @@ class Engine: def get_fieldnames_from_child_table(self, doctype, fields): # convert child_table.fieldname to `tabChild DocType`.`fieldname` - linked_doctype, fieldname = None, None for field in fields: if "." in field and "tab" not in field: original_field = field alias = None if " as " in field: field, alias = field.split(" as ") - linked_fieldname, fieldname = field.split(".") - linked_field = frappe.get_meta(doctype).get_field(linked_fieldname) - linked_doctype = linked_field.options - field = f"`tab{linked_doctype}`.`{fieldname}`" + self.fieldname, linked_fieldname = field.split(".") + linked_field = frappe.get_meta(doctype).get_field(self.fieldname) + self.linked_doctype = linked_field.options + field = f"`tab{self.linked_doctype}`.`{linked_fieldname}`" if alias: field = f"{field} as {alias}" fields[fields.index(original_field)] = field - return fields, linked_doctype, fieldname + return fields def set_fields(self, table, fields, **kwargs): fields = kwargs.get("pluck") if kwargs.get("pluck") else fields or "name" if isinstance(fields, list) and None in fields and Field not in fields: return None - linked_doctype, linked_field = None, None function_objects = [] is_list = isinstance(fields, (list, tuple, set)) if is_list and len(fields) == 1: @@ -543,7 +543,9 @@ class Engine: updated_fields = [] if "*" in fields: return fields - fields, linked_doctype, linked_field = self.get_fieldnames_from_child_table(doctype=table, fields=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: @@ -564,7 +566,7 @@ class Engine: fields = [fields] if fields else [] fields.extend(function_objects) - return fields, linked_doctype, linked_field + return fields def get_query( self, @@ -575,13 +577,21 @@ class Engine: ): # Clean up state before each query self.tables = {} - criterion = self.build_conditions(table, filters, **kwargs) - fields, linked_doctype, linked_field = self.set_fields(table, kwargs.get("field_objects") or fields, **kwargs) + self.linked_doctype = None + self.fieldname = None - if linked_doctype: - linked_doctype = frappe.qb.DocType(linked_doctype) - criterion = criterion.left_join(linked_doctype).on( - linked_doctype.name == frappe.qb.DocType(table).linked_field + criterion = self.build_conditions(table, filters, **kwargs) + fields = self.set_fields( + table, kwargs.get("field_objects") or fields, **kwargs + ) + if self.linked_doctype and self.fieldname: + for field in fields: + if "tab" not in str(field): + fields[fields.index(field)] = PseudoColumn(f"`tab{table}`.`{field.get_sql()}`") + + self.linked_doctype = frappe.qb.DocType(self.linked_doctype) + criterion = criterion.left_join(self.linked_doctype).on( + self.linked_doctype.name == getattr(frappe.qb.DocType(table), self.fieldname) ) join = kwargs.get("join").replace(" ", "_") if kwargs.get("join") else "left_join" From 2ed0e1e64822c336bcb59f62bb5971ca01839c75 Mon Sep 17 00:00:00 2001 From: Aradhya Date: Fri, 5 Aug 2022 14:51:44 +0530 Subject: [PATCH 22/43] fix: fixed filters when getting field names from child tables --- frappe/database/query.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/frappe/database/query.py b/frappe/database/query.py index f1a6d3141e..7ba1e9a162 100644 --- a/frappe/database/query.py +++ b/frappe/database/query.py @@ -317,6 +317,8 @@ class Engine: 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 @@ -338,10 +340,10 @@ class Engine: if isinstance(value, (list, tuple)): _operator = self.OPERATOR_MAP[value[0].casefold()] _value = value[1] if value[1] else ("",) - conditions = conditions.where(_operator(Field(key), _value)) + conditions = conditions.where(_operator(getattr(table, key), _value)) else: if value is not None: - conditions = conditions.where(_operator(Field(key), value)) + conditions = conditions.where(_operator(getattr(table, key), value)) else: _table = conditions._from[0] field = getattr(_table, key) @@ -580,10 +582,10 @@ class Engine: self.linked_doctype = None self.fieldname = None - criterion = self.build_conditions(table, filters, **kwargs) fields = self.set_fields( table, kwargs.get("field_objects") or fields, **kwargs ) + criterion = self.build_conditions(table, filters, **kwargs) if self.linked_doctype and self.fieldname: for field in fields: if "tab" not in str(field): From f4eaa4a481fdbba2e6428e7269059fb473e9f261 Mon Sep 17 00:00:00 2001 From: Aradhya Date: Wed, 10 Aug 2022 01:18:17 +0530 Subject: [PATCH 23/43] feat: joining on tables mentioned in fields --- frappe/database/query.py | 23 ++++++++++++++++++++--- frappe/database/utils.py | 10 ++++++++++ 2 files changed, 30 insertions(+), 3 deletions(-) diff --git a/frappe/database/query.py b/frappe/database/query.py index 7ba1e9a162..1e4d8c65ef 100644 --- a/frappe/database/query.py +++ b/frappe/database/query.py @@ -1,8 +1,6 @@ import operator import re -import sys from ast import literal_eval -from fileinput import filename from functools import cached_property from types import BuiltinFunctionType from typing import Any, Callable @@ -13,6 +11,8 @@ from frappe.model.db_query import get_timespan_date_range from frappe.query_builder import Criterion, Field, Order, Table, functions from frappe.query_builder.functions import Function, SqlFunctions from frappe.query_builder.utils import PseudoColumn +from frappe.database.utils import table_from_string +from pypika import functions as f TAB_PATTERN = re.compile("^tab") WORDS_PATTERN = re.compile(r"\w+") @@ -503,7 +503,7 @@ class Engine: return fields - def set_fields(self, table, fields, **kwargs): + def set_fields(self, table, fields, **kwargs) -> list: fields = kwargs.get("pluck") if kwargs.get("pluck") else fields or "name" if isinstance(fields, list) and None in fields and Field not in fields: return None @@ -586,6 +586,23 @@ class Engine: table, kwargs.get("field_objects") or fields, **kwargs ) criterion = self.build_conditions(table, filters, **kwargs) + joined = False + for field in fields: + if "tab" in str(field): + join_on = table_from_string(str(field)) + criterion = criterion.left_join(join_on).on(join_on.parent == getattr(frappe.qb.DocType(table), "name")) + joined = True + if joined: + # Converting all fields to avoid ambiguity. + for field in fields: + if not getattr(field, '__module__', None) == f.__name__: + field = field if isinstance(field, str) else field.get_sql() + fields[fields.index(field)] = getattr(frappe.qb.DocType(table), field) + else: + field.args = [getattr(frappe.qb.DocType(table), arg.get_sql()) for arg in field.args] + field.args[0] = getattr(frappe.qb.DocType(table), field.args[0].get_sql()) + fields[fields.index(field)] = field + if self.linked_doctype and self.fieldname: for field in fields: if "tab" not in str(field): diff --git a/frappe/database/utils.py b/frappe/database/utils.py index c4d8cb4953..531fbbd973 100644 --- a/frappe/database/utils.py +++ b/frappe/database/utils.py @@ -3,10 +3,14 @@ from functools import cached_property from types import NoneType +import typing import frappe from frappe.query_builder.builder import MariaDB, Postgres +if typing.TYPE_CHECKING: + from frappe.query_builder import DocType + Query = str | MariaDB | Postgres QueryValues = tuple | list | dict | NoneType @@ -17,6 +21,12 @@ FallBackDateTimeStr = "0001-01-01 00:00:00.000000" def is_query_type(query: str, query_type: str | tuple[str]) -> bool: return query.lstrip().split(maxsplit=1)[0].lower().startswith(query_type) +def table_from_string(table: str) -> "DocType": + table_name = table.split("`", maxsplit=1)[1].split(".")[0][3:] + if "`" in table_name: + return frappe.qb.DocType(table_name=table_name.replace("`", "")) + else: + return frappe.qb.DocType(table_name=table_name) class LazyString: def _setup(self) -> None: From 85d1e417382f504b2b09cf7fb4b6b76b8c1b289e Mon Sep 17 00:00:00 2001 From: Aradhya Date: Wed, 10 Aug 2022 15:54:12 +0530 Subject: [PATCH 24/43] fix: fixed join query from fields --- frappe/database/query.py | 81 ++++++++++++++++++------------- frappe/database/utils.py | 4 +- frappe/query_builder/functions.py | 2 + frappe/tests/test_query.py | 16 +++++- 4 files changed, 67 insertions(+), 36 deletions(-) diff --git a/frappe/database/query.py b/frappe/database/query.py index 1e4d8c65ef..5db6eb26b5 100644 --- a/frappe/database/query.py +++ b/frappe/database/query.py @@ -5,14 +5,15 @@ from functools import cached_property from types import BuiltinFunctionType from typing import Any, Callable +from pypika.dialects import MySQLQueryBuilder, PostgreSQLQueryBuilder + import frappe from frappe import _ +from frappe.database.utils import table_from_string from frappe.model.db_query import get_timespan_date_range from frappe.query_builder import Criterion, Field, Order, Table, functions from frappe.query_builder.functions import Function, SqlFunctions from frappe.query_builder.utils import PseudoColumn -from frappe.database.utils import table_from_string -from pypika import functions as f TAB_PATTERN = re.compile("^tab") WORDS_PATTERN = re.compile(r"\w+") @@ -494,8 +495,12 @@ class Engine: if " as " in field: field, alias = field.split(" as ") self.fieldname, linked_fieldname = field.split(".") - linked_field = frappe.get_meta(doctype).get_field(self.fieldname) - self.linked_doctype = linked_field.options + linked_field = frappe.get_meta(doctype, cached=True).get_field(self.fieldname) + try: + self.linked_doctype = linked_field.options + except AttributeError: + print("here") + return fields field = f"`tab{self.linked_doctype}`.`{linked_fieldname}`" if alias: field = f"{field} as {alias}" @@ -545,9 +550,7 @@ class Engine: updated_fields = [] if "*" in fields: return fields - fields = self.get_fieldnames_from_child_table( - doctype=table, fields=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: @@ -576,42 +579,51 @@ class Engine: 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.linked_doctype = None self.fieldname = None - fields = self.set_fields( - table, kwargs.get("field_objects") or fields, **kwargs - ) + fields = self.set_fields(table, kwargs.get("field_objects") or fields, **kwargs) criterion = self.build_conditions(table, filters, **kwargs) - joined = False - for field in fields: - if "tab" in str(field): - join_on = table_from_string(str(field)) - criterion = criterion.left_join(join_on).on(join_on.parent == getattr(frappe.qb.DocType(table), "name")) - joined = True - if joined: - # Converting all fields to avoid ambiguity. + join_query = False + if not isinstance(fields, Criterion): for field in fields: - if not getattr(field, '__module__', None) == f.__name__: - field = field if isinstance(field, str) else field.get_sql() - fields[fields.index(field)] = getattr(frappe.qb.DocType(table), field) - else: - field.args = [getattr(frappe.qb.DocType(table), arg.get_sql()) for arg in field.args] - field.args[0] = getattr(frappe.qb.DocType(table), field.args[0].get_sql()) - fields[fields.index(field)] = field + if ("tab" in str(field)) and (f"`tab{table}`" not in str(field)): + join_table = table_from_string(str(field)) + if self.fieldname: + criterion = criterion.left_join(join_table).on( + getattr(join_table, "name") == getattr(frappe.qb.DocType(table), self.fieldname) + ) + else: + criterion = criterion.left_join(join_table).on( + getattr(join_table, "parent") == getattr(frappe.qb.DocType(table), "name") + ) + join_query = True - if self.linked_doctype and self.fieldname: - for field in fields: - if "tab" not in str(field): - fields[fields.index(field)] = PseudoColumn(f"`tab{table}`.`{field.get_sql()}`") + if join_query: + for field in fields: + if not ( + getattr(field, "__module__", None) == "pypika.functions" or isinstance(field, Function) + ): + field = field if isinstance(field, str) else field.get_sql() + if "tab" not in str(field): + fields[fields.index(field)] = getattr(frappe.qb.DocType(table), field) + else: + field.args = [getattr(frappe.qb.DocType(table), arg.get_sql()) for arg in field.args] + field.args[0] = getattr(frappe.qb.DocType(table), field.args[0].get_sql()) + fields[fields.index(field)] = field - self.linked_doctype = frappe.qb.DocType(self.linked_doctype) - criterion = criterion.left_join(self.linked_doctype).on( - self.linked_doctype.name == getattr(frappe.qb.DocType(table), self.fieldname) - ) + # if self.linked_doctype and self.fieldname and not join_query: + # for field in fields: + # if "tab" not in str(field): + # fields[fields.index(field)] = PseudoColumn(f"`tab{table}`.`{field.get_sql()}`") + # self.linked_doctype = frappe.qb.DocType(self.linked_doctype) + # criterion = criterion.left_join(self.linked_doctype).on( + # getattr(self.linked_doctype, "name") == getattr(frappe.qb.DocType(table), self.fieldname) + # ) + # join_query = True join = kwargs.get("join").replace(" ", "_") if kwargs.get("join") else "left_join" @@ -622,6 +634,7 @@ class Engine: criterion = getattr(criterion, join)(table_object).on( table_object.parent == primary_table.name ) + join_query = True if isinstance(fields, (list, tuple)): query = criterion.select(*fields) diff --git a/frappe/database/utils.py b/frappe/database/utils.py index 531fbbd973..4e2ee70a48 100644 --- a/frappe/database/utils.py +++ b/frappe/database/utils.py @@ -1,9 +1,9 @@ # Copyright (c) 2022, Frappe Technologies Pvt. Ltd. and Contributors # License: MIT. See LICENSE +import typing from functools import cached_property from types import NoneType -import typing import frappe from frappe.query_builder.builder import MariaDB, Postgres @@ -21,6 +21,7 @@ FallBackDateTimeStr = "0001-01-01 00:00:00.000000" def is_query_type(query: str, query_type: str | tuple[str]) -> bool: return query.lstrip().split(maxsplit=1)[0].lower().startswith(query_type) + def table_from_string(table: str) -> "DocType": table_name = table.split("`", maxsplit=1)[1].split(".")[0][3:] if "`" in table_name: @@ -28,6 +29,7 @@ def table_from_string(table: str) -> "DocType": else: return frappe.qb.DocType(table_name=table_name) + class LazyString: def _setup(self) -> None: raise NotImplementedError diff --git a/frappe/query_builder/functions.py b/frappe/query_builder/functions.py index f355eb05bc..e725dff828 100644 --- a/frappe/query_builder/functions.py +++ b/frappe/query_builder/functions.py @@ -22,6 +22,7 @@ class Locate(Function): terms[0] = terms[0].get_sql() super().__init__("LOCATE", *terms, **kwargs) + class Ifnull(IfNull): def __init__(self, condition, term, **kwargs): if not isinstance(condition, str): @@ -30,6 +31,7 @@ class Ifnull(IfNull): term = term.get_sql() super().__init__(condition, term, **kwargs) + class Timestamp(Function): def __init__(self, term: str, time=None, alias=None): if time: diff --git a/frappe/tests/test_query.py b/frappe/tests/test_query.py index 3322ab9e01..0c8ec36ec4 100644 --- a/frappe/tests/test_query.py +++ b/frappe/tests/test_query.py @@ -185,4 +185,18 @@ class TestQuery(unittest.TestCase): "User", filters={"IfNull(name, " ")": ("<", Now())}, fields=["Max(name)"] ).run(), frappe.qb.from_("User").select(Max(Field("name"))).where(Ifnull("name", "") < Now()).run(), - ) \ No newline at end of file + ) + + def test_indirect_join_query(self): + self.assertEqual( + frappe.qb.engine.get_query( + "Note", + filters={"name": "Test Note Title"}, + fields=["name", "`tabNote Seen By`.`user` as seen_by"], + ).run(as_dict=1), + frappe.get_list( + "Note", + filters={"name": "Test Note Title"}, + fields=["name", "`tabNote Seen By`.`user` as seen_by"], + ), + ) From 306e259847a1256e68f595f507544e0079fea2fa Mon Sep 17 00:00:00 2001 From: Aradhya Date: Thu, 11 Aug 2022 00:13:07 +0530 Subject: [PATCH 25/43] fix: fixed join logic --- frappe/database/query.py | 24 ++++++++---------------- frappe/database/utils.py | 5 +++++ 2 files changed, 13 insertions(+), 16 deletions(-) diff --git a/frappe/database/query.py b/frappe/database/query.py index 5db6eb26b5..a279ea124d 100644 --- a/frappe/database/query.py +++ b/frappe/database/query.py @@ -9,7 +9,7 @@ from pypika.dialects import MySQLQueryBuilder, PostgreSQLQueryBuilder import frappe from frappe import _ -from frappe.database.utils import table_from_string +from frappe.database.utils import is_function_object, table_from_string from frappe.model.db_query import get_timespan_date_range from frappe.query_builder import Criterion, Field, Order, Table, functions from frappe.query_builder.functions import Function, SqlFunctions @@ -499,7 +499,6 @@ class Engine: try: self.linked_doctype = linked_field.options except AttributeError: - print("here") return fields field = f"`tab{self.linked_doctype}`.`{linked_fieldname}`" if alias: @@ -590,7 +589,12 @@ class Engine: join_query = False if not isinstance(fields, Criterion): for field in fields: - if ("tab" in str(field)) and (f"`tab{table}`" not in str(field)): + # Only perform this bit if foreign doctype in fields + if ( + not is_function_object(field) + and ("tab" in str(field)) + and (f"`tab{table}`" not in str(field)) + ): join_table = table_from_string(str(field)) if self.fieldname: criterion = criterion.left_join(join_table).on( @@ -604,9 +608,7 @@ class Engine: if join_query: for field in fields: - if not ( - getattr(field, "__module__", None) == "pypika.functions" or isinstance(field, Function) - ): + if not is_function_object(field): field = field if isinstance(field, str) else field.get_sql() if "tab" not in str(field): fields[fields.index(field)] = getattr(frappe.qb.DocType(table), field) @@ -615,16 +617,6 @@ class Engine: field.args[0] = getattr(frappe.qb.DocType(table), field.args[0].get_sql()) fields[fields.index(field)] = field - # if self.linked_doctype and self.fieldname and not join_query: - # for field in fields: - # if "tab" not in str(field): - # fields[fields.index(field)] = PseudoColumn(f"`tab{table}`.`{field.get_sql()}`") - # self.linked_doctype = frappe.qb.DocType(self.linked_doctype) - # criterion = criterion.left_join(self.linked_doctype).on( - # getattr(self.linked_doctype, "name") == getattr(frappe.qb.DocType(table), self.fieldname) - # ) - # join_query = True - join = kwargs.get("join").replace(" ", "_") if kwargs.get("join") else "left_join" if len(self.tables) > 1: diff --git a/frappe/database/utils.py b/frappe/database/utils.py index 4e2ee70a48..2a23ca485e 100644 --- a/frappe/database/utils.py +++ b/frappe/database/utils.py @@ -7,6 +7,7 @@ from types import NoneType import frappe from frappe.query_builder.builder import MariaDB, Postgres +from frappe.query_builder.functions import Function if typing.TYPE_CHECKING: from frappe.query_builder import DocType @@ -30,6 +31,10 @@ def table_from_string(table: str) -> "DocType": return frappe.qb.DocType(table_name=table_name) +def is_function_object(field: str) -> bool: + return getattr(field, "__module__", None) == "pypika.functions" or isinstance(field, Function) + + class LazyString: def _setup(self) -> None: raise NotImplementedError From caab7ff86303f195593de7ee02b3ea4351492488 Mon Sep 17 00:00:00 2001 From: Aradhya Date: Thu, 11 Aug 2022 18:24:02 +0530 Subject: [PATCH 26/43] fix: fixed foreign doctype parsing in get_query lint: removed redundant imports --- 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 a279ea124d..96697603a4 100644 --- a/frappe/database/query.py +++ b/frappe/database/query.py @@ -592,7 +592,7 @@ class Engine: # Only perform this bit if foreign doctype in fields if ( not is_function_object(field) - and ("tab" in str(field)) + and str(field).startswith("`tab") and (f"`tab{table}`" not in str(field)) ): join_table = table_from_string(str(field)) From 4cc7d1c49705056e39360b6bb104aee72bb449fa Mon Sep 17 00:00:00 2001 From: Aradhya Date: Sun, 14 Aug 2022 11:37:18 +0530 Subject: [PATCH 27/43] refactor: removed engine from execute --- frappe/model/db_query.py | 17 ----------------- 1 file changed, 17 deletions(-) diff --git a/frappe/model/db_query.py b/frappe/model/db_query.py index 561806634f..a29ede37bf 100644 --- a/frappe/model/db_query.py +++ b/frappe/model/db_query.py @@ -122,9 +122,6 @@ class DatabaseQuery: # if `filters` is a list of strings, its probably fields filters, fields = fields, filters - self.locals = locals() - self.qb_fields, self.qb_filters = fields, filters - if fields: self.fields = fields else: @@ -211,20 +208,6 @@ class DatabaseQuery: % args ) - if self.locals.get("ignore_permissions"): - return frappe.qb.engine.get_query( - table=self.doctype, - fields=self.qb_fields, - filters=self.qb_filters, - pluck=self.locals.get("pluck"), - ).run( - as_dict=not self.as_list, - debug=self.debug, - update=self.update, - ignore_ddl=self.ignore_ddl, - run=self.run, - ) - return frappe.db.sql( query, as_dict=not self.as_list, From ba2caf206bb8494e8a9e9dd2ed560a172fbe7061 Mon Sep 17 00:00:00 2001 From: Aradhya Date: Sun, 14 Aug 2022 12:52:00 +0530 Subject: [PATCH 28/43] feat: Added parameterization on queries going through database.py --- frappe/database/database.py | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/frappe/database/database.py b/frappe/database/database.py index 3b29fd522c..40eb7d8bbd 100644 --- a/frappe/database/database.py +++ b/frappe/database/database.py @@ -789,7 +789,7 @@ class Database: if fields == "*" and not isinstance(fields, (list, tuple)) and not isinstance(fields, Criterion): as_dict = True - return self.sql(query, as_dict=as_dict, debug=debug, update=update, run=run, pluck=pluck) + return query.run(as_dict=as_dict, debug=debug, update=update, run=run, pluck=pluck) def _get_value_for_many_names( self, @@ -806,18 +806,15 @@ class Database: as_dict=False, ): if names := list(filter(None, names)): - return self.get_all( + return frappe.qb.engine.get_query( doctype, fields=field, filters=names, order_by=order_by, pluck=pluck, - debug=debug, - as_list=not as_dict, - run=run, distinct=distinct, - limit_page_length=limit, - ) + limit=limit, + ).run(debug=debug, as_list=not as_dict, run=run) return {} def update(self, *args, **kwargs): @@ -1068,10 +1065,9 @@ class Database: cache_count = frappe.cache().get_value(f"doctype:count:{dt}") if cache_count is not None: return cache_count - query = frappe.qb.engine.get_query( + count = frappe.qb.engine.get_query( table=dt, filters=filters, fields=Count("*"), distinct=distinct - ) - count = self.sql(query, debug=debug)[0][0] + ).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 From 9bb39131c0ddfdbfb5370d625e81bbf8eb39e730 Mon Sep 17 00:00:00 2001 From: Aradhya Date: Sun, 14 Aug 2022 14:34:02 +0530 Subject: [PATCH 29/43] refactor: changed get_value_for_many_names to get tuple values --- 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 40eb7d8bbd..636ff73186 100644 --- a/frappe/database/database.py +++ b/frappe/database/database.py @@ -814,7 +814,7 @@ class Database: pluck=pluck, distinct=distinct, limit=limit, - ).run(debug=debug, as_list=not as_dict, run=run) + ).run(debug=debug, run=run, as_dict=as_dict) return {} def update(self, *args, **kwargs): From 4d41465dc38c7013c7cdbfd53dd095a810fdfc32 Mon Sep 17 00:00:00 2001 From: Aradhya Date: Sun, 14 Aug 2022 14:57:04 +0530 Subject: [PATCH 30/43] test: updated to run tests on maria db only --- frappe/tests/test_query.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/frappe/tests/test_query.py b/frappe/tests/test_query.py index 0c8ec36ec4..7ffd111901 100644 --- a/frappe/tests/test_query.py +++ b/frappe/tests/test_query.py @@ -21,6 +21,7 @@ class TestQuery(unittest.TestCase): "SELECT * FROM `tabDocType` LEFT JOIN `tabBOM Update Log` ON `tabBOM Update Log`.`parent`=`tabDocType`.`name` WHERE `tabBOM Update Log`.`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( @@ -179,6 +180,7 @@ class TestQuery(unittest.TestCase): .get_sql(), ) + @run_only_if(db_type_is.MARIADB) def test_filters(self): self.assertEqual( frappe.qb.engine.get_query( From 40bfad9aeb719296b5a4ce633b95a5a98dd5b40b Mon Sep 17 00:00:00 2001 From: Aradhya Date: Sun, 14 Aug 2022 15:24:06 +0530 Subject: [PATCH 31/43] refactor: moved join operations to function --- frappe/database/query.py | 45 ++++++++++++++++++++++------------------ 1 file changed, 25 insertions(+), 20 deletions(-) diff --git a/frappe/database/query.py b/frappe/database/query.py index 96697603a4..91163fb607 100644 --- a/frappe/database/query.py +++ b/frappe/database/query.py @@ -572,21 +572,9 @@ class Engine: fields.extend(function_objects) return fields - 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.linked_doctype = None - self.fieldname = None - - fields = self.set_fields(table, kwargs.get("field_objects") or fields, **kwargs) - criterion = self.build_conditions(table, filters, **kwargs) - join_query = False + def join_(self, criterion, fields, table, join): + """Handles all join operations on criterion objects""" + has_join = False if not isinstance(fields, Criterion): for field in fields: # Only perform this bit if foreign doctype in fields @@ -604,9 +592,9 @@ class Engine: criterion = criterion.left_join(join_table).on( getattr(join_table, "parent") == getattr(frappe.qb.DocType(table), "name") ) - join_query = True + has_join = True - if join_query: + if has_join: for field in fields: if not is_function_object(field): field = field if isinstance(field, str) else field.get_sql() @@ -617,8 +605,6 @@ class Engine: field.args[0] = getattr(frappe.qb.DocType(table), field.args[0].get_sql()) fields[fields.index(field)] = field - join = kwargs.get("join").replace(" ", "_") if kwargs.get("join") else "left_join" - if len(self.tables) > 1: primary_table = self.tables[table] del self.tables[table] @@ -626,7 +612,26 @@ class Engine: criterion = getattr(criterion, join)(table_object).on( table_object.parent == primary_table.name ) - join_query = True + has_join = True + + return criterion, fields + + 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.linked_doctype = None + self.fieldname = None + + fields = self.set_fields(table, kwargs.get("field_objects") or fields, **kwargs) + criterion = self.build_conditions(table, filters, **kwargs) + join = kwargs.get("join").replace(" ", "_") if kwargs.get("join") else "left_join" + criterion, fields = self.join_(criterion=criterion, fields=fields, table=table, join=join) if isinstance(fields, (list, tuple)): query = criterion.select(*fields) From 0addffafb949feb604e5afb01fc319986036f1d1 Mon Sep 17 00:00:00 2001 From: Aradhya Date: Wed, 17 Aug 2022 19:52:51 +0530 Subject: [PATCH 32/43] refactor: minor changes --- frappe/database/query.py | 43 ++++++++++++++++++++++++-------------- frappe/database/utils.py | 8 ------- frappe/share.py | 2 +- frappe/tests/test_query.py | 2 +- 4 files changed, 29 insertions(+), 26 deletions(-) diff --git a/frappe/database/query.py b/frappe/database/query.py index 91163fb607..12c878a567 100644 --- a/frappe/database/query.py +++ b/frappe/database/query.py @@ -3,18 +3,21 @@ import re from ast import literal_eval from functools import cached_property from types import BuiltinFunctionType -from typing import Any, Callable +from typing import TYPE_CHECKING, Any, Callable from pypika.dialects import MySQLQueryBuilder, PostgreSQLQueryBuilder import frappe from frappe import _ -from frappe.database.utils import is_function_object, table_from_string +from frappe.database.utils import is_function_object from frappe.model.db_query import get_timespan_date_range from frappe.query_builder import Criterion, Field, Order, Table, functions from frappe.query_builder.functions import Function, SqlFunctions from frappe.query_builder.utils import PseudoColumn +if TYPE_CHECKING: + from frappe.query_builder import DocType + TAB_PATTERN = re.compile("^tab") WORDS_PATTERN = re.compile(r"\w+") BRACKETS_PATTERN = re.compile(r"\(.*?\)|$") @@ -64,7 +67,7 @@ def not_like(key: Field, value: str) -> frappe.qb: return key.not_like(value) -def func_not_in(key: Field, value: list | tuple): +def func_not_in(key: Field, value: list | tuple | str): """Wrapper method for `NOT IN` Args: @@ -158,6 +161,14 @@ def has_function(field): return True +def table_from_string(table: str) -> "DocType": + table_name = table.split("`", maxsplit=1)[1].split(".")[0][3:] + if "`" in table_name: + return frappe.qb.DocType(table_name=table_name.replace("`", "")) + else: + return frappe.qb.DocType(table_name=table_name) + + # default operators OPERATOR_MAP: dict[str, Callable] = { "+": operator.add, @@ -378,7 +389,7 @@ class Engine: return criterion - def make_function_for_filters(self, key: Any, value: int | str): + 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]) @@ -445,14 +456,19 @@ class Engine: 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): - 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()}`", "") + fields = _remove_string_aliasing(function, fields) fields = BRACKETS_PATTERN.sub("", fields.casefold().replace(function.name.casefold(), "")) # Converting back to capitalized doctype names. if "tab" in fields: @@ -466,12 +482,7 @@ class Engine: updated_fields = [] for field in fields: if isinstance(field, str): - if function.alias: - to_replace = " as " + function.alias.casefold() - if to_replace in field: - field = field.replace(to_replace, "") - elif " as " + f"`{function.alias.casefold()}" in field: - field = field.replace(" as " + f"`{function.alias.casefold()}`", "") + field = _remove_string_aliasing(function, field) substituted_string = ( BRACKETS_PATTERN.sub("", field).strip().casefold() if "`" not in field diff --git a/frappe/database/utils.py b/frappe/database/utils.py index 2a23ca485e..1d44b358fe 100644 --- a/frappe/database/utils.py +++ b/frappe/database/utils.py @@ -23,14 +23,6 @@ def is_query_type(query: str, query_type: str | tuple[str]) -> bool: return query.lstrip().split(maxsplit=1)[0].lower().startswith(query_type) -def table_from_string(table: str) -> "DocType": - table_name = table.split("`", maxsplit=1)[1].split(".")[0][3:] - if "`" in table_name: - return frappe.qb.DocType(table_name=table_name.replace("`", "")) - else: - return frappe.qb.DocType(table_name=table_name) - - def is_function_object(field: str) -> bool: return getattr(field, "__module__", None) == "pypika.functions" or isinstance(field, Function) diff --git a/frappe/share.py b/frappe/share.py index 7f7ca2033f..97c9a472e6 100644 --- a/frappe/share.py +++ b/frappe/share.py @@ -104,7 +104,7 @@ def get_users(doctype, name): return frappe.db.get_all( "DocShare", fields=[ - "name", # Don't understant the need for pseudocolumns here, don't know why get_all supports it? + "name", "user", "read", "write", diff --git a/frappe/tests/test_query.py b/frappe/tests/test_query.py index 7ffd111901..ce819930aa 100644 --- a/frappe/tests/test_query.py +++ b/frappe/tests/test_query.py @@ -189,7 +189,7 @@ class TestQuery(unittest.TestCase): frappe.qb.from_("User").select(Max(Field("name"))).where(Ifnull("name", "") < Now()).run(), ) - def test_indirect_join_query(self): + def test_implicit_join_query(self): self.assertEqual( frappe.qb.engine.get_query( "Note", From e400df90ff9f7e1e48f5434ace679c5b5df61767 Mon Sep 17 00:00:00 2001 From: Aradhya Date: Fri, 19 Aug 2022 15:56:34 +0530 Subject: [PATCH 33/43] refactor: removed iterable from iteration ;/ --- frappe/database/query.py | 7 ++++--- frappe/model/db_query.py | 2 +- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/frappe/database/query.py b/frappe/database/query.py index 12c878a567..95ee46fd40 100644 --- a/frappe/database/query.py +++ b/frappe/database/query.py @@ -494,7 +494,7 @@ class Engine: else: replaced_string = substituted_string.replace(function.name.casefold(), "") updated_fields.append(replaced_string) - fields = [field for field in updated_fields if field] + fields = [field for field in updated_fields if field] return fields def get_fieldnames_from_child_table(self, doctype, fields): @@ -575,7 +575,8 @@ class Engine: else: updated_fields.append(Field(field)) - fields = updated_fields + fields = updated_fields + # Need to check instance again since fields modified. if not isinstance(fields, (list, tuple, set)): fields = [fields] if fields else [] @@ -626,7 +627,7 @@ class Engine: has_join = True return criterion, fields - + # try meta to validate fields and doctypes def get_query( self, table: str, diff --git a/frappe/model/db_query.py b/frappe/model/db_query.py index 9ce4cc4942..8c61bcc447 100644 --- a/frappe/model/db_query.py +++ b/frappe/model/db_query.py @@ -312,7 +312,7 @@ class DatabaseQuery: except ValueError: self.fields = [f.strip() for f in self.fields.split(",")] - # remove empty strings / nulls in fields + # remove empty st1rings / nulls in fields self.fields = [f for f in self.fields if f] # convert child_table.fieldname to `tabChild DocType`.`fieldname` From 0b50e6371b325afaf58dcbf89bdcd87df1dfca14 Mon Sep 17 00:00:00 2001 From: Aradhya Date: Fri, 19 Aug 2022 16:01:38 +0530 Subject: [PATCH 34/43] refactor(minor): lint --- frappe/database/query.py | 1 + 1 file changed, 1 insertion(+) diff --git a/frappe/database/query.py b/frappe/database/query.py index 95ee46fd40..6706087116 100644 --- a/frappe/database/query.py +++ b/frappe/database/query.py @@ -627,6 +627,7 @@ class Engine: has_join = True return criterion, fields + # try meta to validate fields and doctypes def get_query( self, From 0d4020e3edc487f4f441ed0aa41fad489e4501d6 Mon Sep 17 00:00:00 2001 From: Aradhya Date: Sat, 20 Aug 2022 19:27:14 +0530 Subject: [PATCH 35/43] refactor: removed excess branching --- frappe/database/query.py | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/frappe/database/query.py b/frappe/database/query.py index 6706087116..743272a5a0 100644 --- a/frappe/database/query.py +++ b/frappe/database/query.py @@ -163,11 +163,7 @@ def has_function(field): def table_from_string(table: str) -> "DocType": table_name = table.split("`", maxsplit=1)[1].split(".")[0][3:] - if "`" in table_name: - return frappe.qb.DocType(table_name=table_name.replace("`", "")) - else: - return frappe.qb.DocType(table_name=table_name) - + return frappe.qb.DocType(table_name=table_name.replace("`", "")) # default operators OPERATOR_MAP: dict[str, Callable] = { From 535b64931e3aa7fb6adb62b4f0784fcd3c5228fd Mon Sep 17 00:00:00 2001 From: Aradhya Date: Mon, 22 Aug 2022 14:49:30 +0530 Subject: [PATCH 36/43] refactor: using enumerate to iterate over fields & warn when passing query object to def sql --- frappe/database/database.py | 7 +++++++ frappe/database/query.py | 5 ++--- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/frappe/database/database.py b/frappe/database/database.py index 33f1b94cdb..39c37b3a3f 100644 --- a/frappe/database/database.py +++ b/frappe/database/database.py @@ -30,6 +30,8 @@ from frappe.query_builder.functions import Count from frappe.query_builder.utils import DocType from frappe.utils import cast as cast_fieldtype from frappe.utils import get_datetime, get_table_name, getdate, now, sbool +from pypika.dialects import MySQLQueryBuilder, PostgreSQLQueryBuilder + IFNULL_PATTERN = re.compile(r"ifnull\(", flags=re.IGNORECASE) INDEX_PATTERN = re.compile(r"\s*\([^)]+\)\s*") @@ -175,6 +177,11 @@ class Database: {"name": "a%", "owner":"test@example.com"}) """ + if isinstance(query, (MySQLQueryBuilder, PostgreSQLQueryBuilder)): + frappe.errprint( + "Use run method to execute SQL queries generated by Query Engine" + ) + debug = debug or getattr(self, "debug", False) query = str(query) if not run: diff --git a/frappe/database/query.py b/frappe/database/query.py index 743272a5a0..8cd344c83a 100644 --- a/frappe/database/query.py +++ b/frappe/database/query.py @@ -495,9 +495,8 @@ class Engine: def get_fieldnames_from_child_table(self, doctype, fields): # convert child_table.fieldname to `tabChild DocType`.`fieldname` - for field in fields: + for idx, field in enumerate(fields, start=0): if "." in field and "tab" not in field: - original_field = field alias = None if " as " in field: field, alias = field.split(" as ") @@ -510,7 +509,7 @@ class Engine: field = f"`tab{self.linked_doctype}`.`{linked_fieldname}`" if alias: field = f"{field} as {alias}" - fields[fields.index(original_field)] = field + fields[idx] = field return fields From 712416ceb0befc6b88018bbd7550492a8056d965 Mon Sep 17 00:00:00 2001 From: Aradhya Date: Mon, 22 Aug 2022 14:52:41 +0530 Subject: [PATCH 37/43] refactor: changed function name to better define usage --- frappe/database/query.py | 9 +++++---- frappe/database/utils.py | 2 +- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/frappe/database/query.py b/frappe/database/query.py index 8cd344c83a..140b200a43 100644 --- a/frappe/database/query.py +++ b/frappe/database/query.py @@ -3,13 +3,13 @@ import re from ast import literal_eval from functools import cached_property from types import BuiltinFunctionType -from typing import TYPE_CHECKING, Any, Callable +from typing import TYPE_CHECKING, Callable from pypika.dialects import MySQLQueryBuilder, PostgreSQLQueryBuilder import frappe from frappe import _ -from frappe.database.utils import is_function_object +from frappe.database.utils import 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.query_builder.functions import Function, SqlFunctions @@ -165,6 +165,7 @@ def table_from_string(table: str) -> "DocType": table_name = table.split("`", maxsplit=1)[1].split(".")[0][3:] return frappe.qb.DocType(table_name=table_name.replace("`", "")) + # default operators OPERATOR_MAP: dict[str, Callable] = { "+": operator.add, @@ -586,7 +587,7 @@ class Engine: for field in fields: # Only perform this bit if foreign doctype in fields if ( - not is_function_object(field) + not is_pypika_function_object(field) and str(field).startswith("`tab") and (f"`tab{table}`" not in str(field)) ): @@ -603,7 +604,7 @@ class Engine: if has_join: for field in fields: - if not is_function_object(field): + if not is_pypika_function_object(field): field = field if isinstance(field, str) else field.get_sql() if "tab" not in str(field): fields[fields.index(field)] = getattr(frappe.qb.DocType(table), field) diff --git a/frappe/database/utils.py b/frappe/database/utils.py index 1d44b358fe..c1f70d388e 100644 --- a/frappe/database/utils.py +++ b/frappe/database/utils.py @@ -23,7 +23,7 @@ def is_query_type(query: str, query_type: str | tuple[str]) -> bool: return query.lstrip().split(maxsplit=1)[0].lower().startswith(query_type) -def is_function_object(field: str) -> bool: +def is_pypika_function_object(field: str) -> bool: return getattr(field, "__module__", None) == "pypika.functions" or isinstance(field, Function) From 8fe5a641402b528e93fceaed0d010cf961b80213 Mon Sep 17 00:00:00 2001 From: Aradhya Date: Wed, 24 Aug 2022 14:39:43 +0530 Subject: [PATCH 38/43] fix: fixed complex toupper implementation --- frappe/database/query.py | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/frappe/database/query.py b/frappe/database/query.py index 140b200a43..074ca3d83e 100644 --- a/frappe/database/query.py +++ b/frappe/database/query.py @@ -466,12 +466,7 @@ class Engine: for function in function_objects: if isinstance(fields, str): fields = _remove_string_aliasing(function, fields) - fields = BRACKETS_PATTERN.sub("", fields.casefold().replace(function.name.casefold(), "")) - # Converting back to capitalized doctype names. - if "tab" in fields: - fields = TABLE_PATTERN.sub( - lambda p: p.group(0)[:3] + p.group(0)[3].upper() + p.group(0)[3 + 1 :], 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 = "" From f36eda1e838ad70df74c04c2d9cc5a21dbed2e75 Mon Sep 17 00:00:00 2001 From: Aradhya Date: Wed, 24 Aug 2022 14:44:18 +0530 Subject: [PATCH 39/43] refactor: using enumerate while indexing on the fly --- 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 074ca3d83e..281551a7f6 100644 --- a/frappe/database/query.py +++ b/frappe/database/query.py @@ -598,7 +598,7 @@ class Engine: has_join = True if has_join: - for field in fields: + for idx, field in enumerate(fields): if not is_pypika_function_object(field): field = field if isinstance(field, str) else field.get_sql() if "tab" not in str(field): @@ -606,7 +606,7 @@ class Engine: else: field.args = [getattr(frappe.qb.DocType(table), arg.get_sql()) for arg in field.args] field.args[0] = getattr(frappe.qb.DocType(table), field.args[0].get_sql()) - fields[fields.index(field)] = field + fields[idx] = field if len(self.tables) > 1: primary_table = self.tables[table] From edfa63a4cfcaff0bcec60913f2fba3045922610b Mon Sep 17 00:00:00 2001 From: Aradhya Date: Sun, 28 Aug 2022 00:08:13 +0530 Subject: [PATCH 40/43] fix: lint --- frappe/database/database.py | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/frappe/database/database.py b/frappe/database/database.py index 39c37b3a3f..25e82135df 100644 --- a/frappe/database/database.py +++ b/frappe/database/database.py @@ -10,6 +10,7 @@ import traceback from contextlib import contextmanager from time import time +from pypika.dialects import MySQLQueryBuilder, PostgreSQLQueryBuilder from pypika.terms import Criterion, NullValue import frappe @@ -30,8 +31,6 @@ from frappe.query_builder.functions import Count from frappe.query_builder.utils import DocType from frappe.utils import cast as cast_fieldtype from frappe.utils import get_datetime, get_table_name, getdate, now, sbool -from pypika.dialects import MySQLQueryBuilder, PostgreSQLQueryBuilder - IFNULL_PATTERN = re.compile(r"ifnull\(", flags=re.IGNORECASE) INDEX_PATTERN = re.compile(r"\s*\([^)]+\)\s*") @@ -178,9 +177,7 @@ class Database: """ if isinstance(query, (MySQLQueryBuilder, PostgreSQLQueryBuilder)): - frappe.errprint( - "Use run method to execute SQL queries generated by Query Engine" - ) + frappe.errprint("Use run method to execute SQL queries generated by Query Engine") debug = debug or getattr(self, "debug", False) query = str(query) From ec100d7a8ae623de0390feeb1551b8b15c6dd953 Mon Sep 17 00:00:00 2001 From: Aradhya Date: Tue, 30 Aug 2022 15:47:07 +0530 Subject: [PATCH 41/43] fix: misc fixes --- frappe/database/query.py | 6 +++--- frappe/model/db_query.py | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/frappe/database/query.py b/frappe/database/query.py index 281551a7f6..36dc21d9f2 100644 --- a/frappe/database/query.py +++ b/frappe/database/query.py @@ -490,6 +490,7 @@ class Engine: 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` for idx, field in enumerate(fields, start=0): if "." in field and "tab" not in field: @@ -602,15 +603,14 @@ class Engine: if not is_pypika_function_object(field): field = field if isinstance(field, str) else field.get_sql() if "tab" not in str(field): - fields[fields.index(field)] = getattr(frappe.qb.DocType(table), field) + fields[idx] = getattr(frappe.qb.DocType(table), field) else: field.args = [getattr(frappe.qb.DocType(table), arg.get_sql()) for arg in field.args] field.args[0] = getattr(frappe.qb.DocType(table), field.args[0].get_sql()) fields[idx] = field if len(self.tables) > 1: - primary_table = self.tables[table] - del self.tables[table] + primary_table = self.tables.pop(table) for table_object in self.tables.values(): criterion = getattr(criterion, join)(table_object).on( table_object.parent == primary_table.name diff --git a/frappe/model/db_query.py b/frappe/model/db_query.py index 254a7ebe12..a2f597f7bd 100644 --- a/frappe/model/db_query.py +++ b/frappe/model/db_query.py @@ -312,7 +312,7 @@ class DatabaseQuery: except ValueError: self.fields = [f.strip() for f in self.fields.split(",")] - # remove empty st1rings / nulls in fields + # remove empty strings / nulls in fields self.fields = [f for f in self.fields if f] # convert child_table.fieldname to `tabChild DocType`.`fieldname` From c55afe23f80969be2af905c79f0b3cb90d992896 Mon Sep 17 00:00:00 2001 From: Aradhya Date: Tue, 30 Aug 2022 16:40:47 +0530 Subject: [PATCH 42/43] refactor: removed flaky implicit joins --- 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 36dc21d9f2..ed45703ff8 100644 --- a/frappe/database/query.py +++ b/frappe/database/query.py @@ -552,7 +552,7 @@ class Engine: updated_fields = [] if "*" in fields: return fields - fields = self.get_fieldnames_from_child_table(doctype=table, fields=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: From e576d16855f66cd8a27f13983fec4df3567462cb Mon Sep 17 00:00:00 2001 From: Aradhya Date: Wed, 31 Aug 2022 15:44:43 +0530 Subject: [PATCH 43/43] refactor: using refactored TABLE_PATTERN for finding fields --- frappe/database/query.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/frappe/database/query.py b/frappe/database/query.py index ed45703ff8..5726b760ee 100644 --- a/frappe/database/query.py +++ b/frappe/database/query.py @@ -23,7 +23,7 @@ WORDS_PATTERN = re.compile(r"\w+") BRACKETS_PATTERN = re.compile(r"\(.*?\)|$") SQL_FUNCTIONS = [sql_function.value for sql_function in SqlFunctions] COMMA_PATTERN = re.compile(r",\s*(?![^()]*\))") -TABLE_PATTERN = re.compile(r"\btab\w+") +TABLE_PATTERN = re.compile(r"`\btab\w+") def like(key: Field, value: str) -> frappe.qb: @@ -602,7 +602,7 @@ class Engine: for idx, field in enumerate(fields): if not is_pypika_function_object(field): field = field if isinstance(field, str) else field.get_sql() - if "tab" not in str(field): + if not TABLE_PATTERN.search(str(field)): fields[idx] = getattr(frappe.qb.DocType(table), field) else: field.args = [getattr(frappe.qb.DocType(table), arg.get_sql()) for arg in field.args] @@ -619,7 +619,6 @@ class Engine: return criterion, fields - # try meta to validate fields and doctypes def get_query( self, table: str,