From 5ed696cd3d9eae548dfc2b595e653f92ac1ea3ed Mon Sep 17 00:00:00 2001 From: Aradhya Tripathi <67282231+Aradhya-Tripathi@users.noreply.github.com> Date: Tue, 1 Nov 2022 11:25:02 +0530 Subject: [PATCH] fix(qb): fixed implicit join for child tables (#18692) * test: Added test for multiple joins --- frappe/database/query.py | 32 +++++++++++++++++--------------- frappe/tests/test_query.py | 14 ++++++++++++++ 2 files changed, 31 insertions(+), 15 deletions(-) diff --git a/frappe/database/query.py b/frappe/database/query.py index 2e1635101a..0fc041f7bc 100644 --- a/frappe/database/query.py +++ b/frappe/database/query.py @@ -603,6 +603,8 @@ class Engine: def join_(self, criterion, fields, table, join): """Handles all join operations on criterion objects""" has_join = False + joined_tables = {} + if not isinstance(fields, Criterion): for field in fields: # Only perform this bit if foreign doctype in fields @@ -611,24 +613,24 @@ class Engine: and str(field).startswith("`tab") 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") - ) has_join = True + table_to_join_on = table_from_string(str(field)) + if joined_tables.get(join) != table_to_join_on: + criterion = getattr(criterion, join)(table_to_join_on).on( + getattr(table_to_join_on, "parent") == getattr(frappe.qb.DocType(table), "name") + ) + joined_tables[join] = table_to_join_on if has_join: - def _update_pypika_fields(field): if not is_pypika_function_object(field): - field = field if isinstance(field, str) else field.get_sql() + field = field if isinstance(field,(str, PseudoColumn)) else field.get_sql() if not TABLE_PATTERN.search(str(field)): + if isinstance(field, PseudoColumn): + field = field.get_sql() return getattr(frappe.qb.DocType(table), field) + else: + return field else: field.args = [getattr(frappe.qb.DocType(table), arg.get_sql()) for arg in field.args] return field @@ -638,10 +640,10 @@ class Engine: if len(self.tables) > 1: 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 - ) - has_join = True + if joined_tables.get("left_join") != table_object: + criterion = getattr(criterion, join)(table_object).on( + table_object.parent == primary_table.name + ) return criterion, fields diff --git a/frappe/tests/test_query.py b/frappe/tests/test_query.py index 1804f1672c..500b46ffae 100644 --- a/frappe/tests/test_query.py +++ b/frappe/tests/test_query.py @@ -202,6 +202,20 @@ class TestQuery(FrappeTestCase): ), ) + self.assertEqual( + frappe.qb.engine.get_query( + "Note", + filters={"name": "Test Note Title"}, + fields=["name", "`tabNote Seen By`.`user` as seen_by", "`tabNote Seen By`.`idx` as idx"], + ).run(as_dict=1), + frappe.get_list( + "Note", + filters={"name": "Test Note Title"}, + fields=["name", "`tabNote Seen By`.`user` as seen_by", "`tabNote Seen By`.`idx` as idx"], + ), + + ) + @run_only_if(db_type_is.MARIADB) def test_comment_stripping(self): self.assertNotIn(