From cf1b01fb5ed9fefd180740b0f9b2e74c5ef97464 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Mon, 12 Oct 2020 15:24:42 +0530 Subject: [PATCH 1/4] fix: Remove partial support for distinct keyword * Added supporting sql functions and distinct kw in TODO --- frappe/model/db_query.py | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/frappe/model/db_query.py b/frappe/model/db_query.py index c73fa5ea09..596f69d2dd 100644 --- a/frappe/model/db_query.py +++ b/frappe/model/db_query.py @@ -171,11 +171,19 @@ class DatabaseQuery(object): fields = [] + # Wrapping fields with grave quotes to allow support for sql keywords + # TODO: Add support for wrapping fields with sql functions and distinct keyword for field in self.fields: - if field.strip().startswith(("`", "*", '"', "'")) or "(" in field: + stripped_field = field.strip().lower() + skip_wrapping = any([ + stripped_field.startswith(("`", "*", '"', "'")), + "(" in stripped_field, + "distinct" in stripped_field, + ]) + if skip_wrapping: fields.append(field) elif "as" in field.lower().split(" "): - col, _, new = field.split()[-3:] + col, _, new = field.split() fields.append("`{0}` as {1}".format(col, new)) else: fields.append("`{0}`".format(field)) From 5097d9cd6ec86eddd91d265884c2d62869673454 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Mon, 12 Oct 2020 15:47:28 +0530 Subject: [PATCH 2/4] test: Updated tests with changed DatabaseQuery.prepare_args behaviour --- frappe/tests/test_db.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/frappe/tests/test_db.py b/frappe/tests/test_db.py index 2ac0cb2631..81d7d07ab3 100644 --- a/frappe/tests/test_db.py +++ b/frappe/tests/test_db.py @@ -133,8 +133,10 @@ class TestDB(unittest.TestCase): self.assertEqual(list(frappe.get_all("ToDo", fields=[random_field], limit=1)[0])[0], random_field) self.assertEqual(list(frappe.get_all("ToDo", fields=["{0} as total".format(random_field)], limit=1)[0])[0], "total") - # Testing read for distinct keyword - Check if result contains total field - self.assertEqual(list(frappe.get_all("ToDo", fields=["distinct {0} as total".format(random_field)], limit=1)[0])[0], "total") + # Testing read for distinct and sql functions + self.assertEqual(list(frappe.get_all("ToDo", fields=["distinct `{0}` as total".format(random_field)], limit=1)[0])[0], "total") + self.assertEqual(list(frappe.get_all("ToDo", fields=["distinct `{0}`".format(random_field)], limit=1)[0])[0], random_field) + self.assertEqual(list(frappe.get_all("ToDo", fields=["count(`{0}`)".format(random_field)], limit=1)[0])[0], "count(`{0}`)".format(random_field)) # Testing update frappe.db.set_value(test_doctype, random_doc, random_field, random_value) From 992f66e044458408a19eab2504ba2f0c262c6122 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Mon, 12 Oct 2020 18:05:04 +0530 Subject: [PATCH 3/4] test: Use standard distinct --- frappe/tests/test_db.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frappe/tests/test_db.py b/frappe/tests/test_db.py index 81d7d07ab3..c4ec721b59 100644 --- a/frappe/tests/test_db.py +++ b/frappe/tests/test_db.py @@ -134,8 +134,8 @@ class TestDB(unittest.TestCase): self.assertEqual(list(frappe.get_all("ToDo", fields=["{0} as total".format(random_field)], limit=1)[0])[0], "total") # Testing read for distinct and sql functions - self.assertEqual(list(frappe.get_all("ToDo", fields=["distinct `{0}` as total".format(random_field)], limit=1)[0])[0], "total") - self.assertEqual(list(frappe.get_all("ToDo", fields=["distinct `{0}`".format(random_field)], limit=1)[0])[0], random_field) + self.assertEqual(list(frappe.get_all("ToDo", fields=["`{0}` as total".format(random_field)], limit=1, distinct=True)[0])[0], "total") + self.assertEqual(list(frappe.get_all("ToDo", fields=["`{0}`".format(random_field)], limit=1, distinct=True)[0])[0], random_field) self.assertEqual(list(frappe.get_all("ToDo", fields=["count(`{0}`)".format(random_field)], limit=1)[0])[0], "count(`{0}`)".format(random_field)) # Testing update From e812fb7da5d1503c4d3c135be3c603064531db54 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Mon, 12 Oct 2020 18:36:26 +0530 Subject: [PATCH 4/4] test: Update test_db_keywords_as_fields for postgres * Updated styling to fit word limit --- frappe/tests/test_db.py | 23 ++++++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) diff --git a/frappe/tests/test_db.py b/frappe/tests/test_db.py index c4ec721b59..6fbf247404 100644 --- a/frappe/tests/test_db.py +++ b/frappe/tests/test_db.py @@ -134,9 +134,26 @@ class TestDB(unittest.TestCase): self.assertEqual(list(frappe.get_all("ToDo", fields=["{0} as total".format(random_field)], limit=1)[0])[0], "total") # Testing read for distinct and sql functions - self.assertEqual(list(frappe.get_all("ToDo", fields=["`{0}` as total".format(random_field)], limit=1, distinct=True)[0])[0], "total") - self.assertEqual(list(frappe.get_all("ToDo", fields=["`{0}`".format(random_field)], limit=1, distinct=True)[0])[0], random_field) - self.assertEqual(list(frappe.get_all("ToDo", fields=["count(`{0}`)".format(random_field)], limit=1)[0])[0], "count(`{0}`)".format(random_field)) + self.assertEqual(list( + frappe.get_all("ToDo", + fields=["`{0}` as total".format(random_field)], + distinct=True, + limit=1, + )[0] + )[0], "total") + self.assertEqual(list( + frappe.get_all("ToDo", + fields=["`{0}`".format(random_field)], + distinct=True, + limit=1, + )[0] + )[0], random_field) + self.assertEqual(list( + frappe.get_all("ToDo", + fields=["count(`{0}`)".format(random_field)], + limit=1 + )[0] + )[0], "count" if frappe.conf.db_type == "postgres" else "count(`{0}`)".format(random_field)) # Testing update frappe.db.set_value(test_doctype, random_doc, random_field, random_value)