From 4012d32be6730a46dc448e56b8b037e61f12fb09 Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Tue, 13 Sep 2022 12:09:53 +0530 Subject: [PATCH] fix: coalesce `not in` queries (#18099) (#18101) * fix: get workspaces with empty module fields * Revert "fix: get workspaces with empty module fields" This reverts commit 1f194be2c3642e31ebe2165e461b2f24be8cda4c. * fix: always coalesce `not in` queries Co-authored-by: Ankush Menat (cherry picked from commit 235171796d07a246be727ec1c244fbe54779e90c) Co-authored-by: Shariq Ansari <30859809+shariquerik@users.noreply.github.com> --- frappe/model/db_query.py | 5 ++++- frappe/tests/test_db_query.py | 4 ++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/frappe/model/db_query.py b/frappe/model/db_query.py index a43d0ad2dc..87e3d83e9c 100644 --- a/frappe/model/db_query.py +++ b/frappe/model/db_query.py @@ -613,7 +613,10 @@ class DatabaseQuery: elif f.operator.lower() in ("in", "not in"): # if values contain '' or falsy values then only coalesce column - can_be_null = not f.value or any(v is None or v == "" for v in f.value) + # for `in` query this is only required if values contain '' or values are empty. + # for `not in` queries we can't be sure as column values might contain null. + if f.operator.lower() == "in": + can_be_null = not f.value or any(v is None or v == "" for v in f.value) values = f.value or "" if isinstance(values, str): diff --git a/frappe/tests/test_db_query.py b/frappe/tests/test_db_query.py index 881b90f399..9e61c5d3ec 100644 --- a/frappe/tests/test_db_query.py +++ b/frappe/tests/test_db_query.py @@ -749,6 +749,10 @@ class TestReportview(FrappeTestCase): self.assertNotIn("ifnull", frappe.get_all("User", {"name": ("in", ["a", "b"])}, run=0)) self.assertIn("ifnull", frappe.get_all("User", {"name": ("in", ["a", None])}, run=0)) self.assertIn("ifnull", frappe.get_all("User", {"name": ("in", ["a", ""])}, run=0)) + self.assertIn("ifnull", frappe.get_all("User", {"name": ("in", [])}, run=0)) + self.assertIn("ifnull", frappe.get_all("User", {"name": ("not in", ["a"])}, run=0)) + self.assertIn("ifnull", frappe.get_all("User", {"name": ("not in", [])}, run=0)) + self.assertIn("ifnull", frappe.get_all("User", {"name": ("not in", [""])}, run=0)) def add_child_table_to_blog_post():