perf: drop ifnull from IS SET filter (#21822)

- Kinda confuses query planner (idk why it's not smart enough to
  understand but there are probably edge cases where it can't be done)
- `null != null` and `'' != null` both yield `null` which is falsy and
  won't be shown in results.

Alternate fix to https://github.com/frappe/frappe/pull/21817
This commit is contained in:
Ankush Menat 2023-07-27 10:58:20 +05:30 committed by GitHub
parent b4629d8c02
commit 8930d4b5e1
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 18 additions and 3 deletions

View file

@ -815,14 +815,19 @@ class DatabaseQuery:
elif f.operator.lower() == "is":
if f.value == "set":
f.operator = "!="
# Value can technically be null, but comparing with null will always be falsy
# Not using coalesce here is faster because indexes can be used.
# null != '' -> null ~ falsy
# '' != '' -> false
can_be_null = False
elif f.value == "not set":
f.operator = "="
fallback = "''"
can_be_null = True
value = ""
fallback = "''"
can_be_null = True
if "ifnull" not in column_name.lower():
if can_be_null and "ifnull" not in column_name.lower():
column_name = f"ifnull({column_name}, {fallback})"
elif df and df.fieldtype == "Date":

View file

@ -704,6 +704,11 @@ class TestDBQuery(FrappeTestCase):
self.assertTrue({"name": "Prepared Report"} in res)
self.assertFalse({"name": "Property Setter"} in res)
frappe.db.set_value("DocType", "Property Setter", "autoname", None, update_modified=False)
res = DatabaseQuery("DocType").execute(filters={"autoname": ["is", "set"]})
self.assertFalse({"name": "Property Setter"} in res)
def test_set_field_tables(self):
# Tests _in_standard_sql_methods method in test_set_field_tables
# The following query will break if the above method is broken

View file

@ -158,3 +158,8 @@ class TestPerformance(FrappeTestCase):
frappe.get_list("User")
with self.assertQueryCount(1):
frappe.get_list("User")
def test_no_ifnull_checks(self):
query = frappe.get_all("DocType", {"autoname": ("is", "set")}, run=0).lower()
self.assertNotIn("coalesce", query)
self.assertNotIn("ifnull", query)