Skip to content

Commit

Permalink
perf: Avoid coalesce for between filters (#26531) (#26532)
Browse files Browse the repository at this point in the history
- Avoid on `between` + date
- Avoid on timestamp fields
- Avoid on `>` and `>=` comparisons

(cherry picked from commit 005e74b)

Co-authored-by: Ankush Menat <ankush@frappe.io>
  • Loading branch information
mergify[bot] and ankush committed May 22, 2024
1 parent e7e55db commit 8263898
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 2 deletions.
23 changes: 21 additions & 2 deletions frappe/model/db_query.py
Original file line number Diff line number Diff line change
Expand Up @@ -723,7 +723,7 @@ def prepare_filter_condition(self, f):
meta = frappe.get_meta(f.doctype)

# primary key is never nullable, modified is usually indexed by default and always present
can_be_null = f.fieldname not in ("name", "modified")
can_be_null = f.fieldname not in ("name", "modified", "creation")

# prepare in condition
if f.operator.lower() in NestedSetHierarchy:
Expand Down Expand Up @@ -802,19 +802,38 @@ def prepare_filter_condition(self, f):

if f.operator.lower() in ("previous", "next", "timespan"):
date_range = get_date_range(f.operator.lower(), f.value)
f.operator = "Between"
f.operator = "between"
f.value = date_range
fallback = f"'{FallBackDateTimeStr}'"

if f.operator.lower() in (">", ">=") and (
f.fieldname in ("creation", "modified")
or (df and (df.fieldtype == "Date" or df.fieldtype == "Datetime"))
):
# Null values can never be greater than any non-null value
can_be_null = False

if f.operator in (">", "<", ">=", "<=") and (f.fieldname in ("creation", "modified")):
value = cstr(f.value)
can_be_null = False
fallback = f"'{FallBackDateTimeStr}'"

elif f.operator.lower() in ("between") and (
f.fieldname in ("creation", "modified")
or (df and (df.fieldtype == "Date" or df.fieldtype == "Datetime"))
):
escape = False
# Between operator never needs to check for null
# Explanation: Consider SQL -> `COLUMN between X and Y`
# Actual computation:
# for row in rows:
# if Y > row.COLUMN > X:
# yield row

# Since Y and X can't be null, null value in column will never match filter, so
# coalesce is extra cost that prevents index usage
can_be_null = False

value = get_between_date_filter(f.value, df)
fallback = f"'{FallBackDateTimeStr}'"

Expand Down
9 changes: 9 additions & 0 deletions frappe/tests/test_db_query.py
Original file line number Diff line number Diff line change
Expand Up @@ -1112,6 +1112,15 @@ def test_coalesce_with_in_ops(self):
self.assertNotIn("ifnull", frappe.get_all("User", {"name": ("in", (""))}, run=0))
self.assertNotIn("ifnull", frappe.get_all("User", {"name": ("in", ())}, run=0))

def test_coalesce_with_datetime_ops(self):
self.assertNotIn("ifnull", frappe.get_all("User", {"last_active": (">", "2022-01-01")}, run=0))
self.assertNotIn("ifnull", frappe.get_all("User", {"creation": ("<", "2022-01-01")}, run=0))
self.assertNotIn(
"ifnull",
frappe.get_all("User", {"last_active": ("between", ("2022-01-01", "2023-01-01"))}, run=0),
)
self.assertIn("ifnull", frappe.get_all("User", {"last_active": ("<", "2022-01-01")}, run=0))

def test_ambiguous_linked_tables(self):
from frappe.desk.reportview import get

Expand Down

0 comments on commit 8263898

Please sign in to comment.