nsinnovations/erpnext
2
0
Fork 0
mirror of https://github.com/frappe/erpnext.git synced 2026-08-13 22:51:49 +00:00
Code Issues Packages Projects Releases Wiki Activity

ci(postgres): flag get_all(distinct=True, order_by=...) in the static checker

Browse Source 
frappe's db_query SILENTLY drops ORDER BY for distinct queries on Postgres (the ORDER BY
column must appear in the SELECT-DISTINCT list), so `get_all/get_list(distinct=True,
order_by="<col>")` is a no-op there and the result comes back unordered — the root cause of
the Sales Register, Purchase Register and Sales Analytics ordering fixes. Add an AST rule to
.github/helper/postgres_compat.py that flags this (literal order_by only; an empty order_by=""
suppression and a dynamic/variable order_by are not flagged). `# pg-ok` escape hatch as usual.

Grandfather the three pre-existing low-impact sites the rule surfaces (paging/iteration order
only, not data): job_card operation autocomplete, inventory_dimension config list, and a
work_order test loop.
This commit is contained in:
Mihir Kandoi
2026-06-22 23:22:52 +05:30
parent f026d1dac8
commit ad237e5ec5
4 changed files with 27 additions and 3 deletions
Download Patch File Download Diff File Expand all files Collapse all files

24
.github/helper/postgres_compat.py vendored
View File

@@ -71,6 +71,11 @@ MYSQL_RESULT_KEYS = {"Column_name", "Key_name", "Seq_in_index", "Non_unique", "I
SET_BOOL_FUNCS = {"set_value", "db_set"}
# frappe.get_all / get_list: frappe's db_query SILENTLY drops ORDER BY for `distinct` queries on
# Postgres (the ORDER BY column must appear in the SELECT-DISTINCT list), so `distinct=True` together
# with a literal `order_by` is a no-op on PG and the result comes back unordered.
DISTINCT_ORDER_FUNCS = {"get_all", "get_list"}
def _docstring_ids(tree: ast.AST) -> set[int]:
"""ids of Constant nodes that are docstrings (so prose describing the rules isn't flagged)."""
@@ -154,6 +159,25 @@ class Visitor(ast.NodeVisitor):
if isinstance(a, ast.Constant) and isinstance(a.value, bool):
self._flag(node, f"{name}(..., {a.value}) sets an int/Check column with a bool -> pass 1/0 (Postgres rejects bool->smallint)")
# frappe.get_all/get_list(..., distinct=True, order_by="<col>") -> ORDER BY is silently dropped
# for distinct queries on Postgres, so the result is unordered there. Sort in python instead
# (e.g. sorted(frappe.get_all(..., distinct=True), key=str.casefold)). An empty order_by="" (the
# explicit "suppress the injected default" idiom) and a dynamic/variable order_by are not flagged.
if name in DISTINCT_ORDER_FUNCS:
has_distinct = any(
kw.arg == "distinct" and isinstance(kw.value, ast.Constant) and kw.value.value
for kw in node.keywords
)
order_kw = next((kw for kw in node.keywords if kw.arg == "order_by"), None)
has_literal_order = (
order_kw is not None
and isinstance(order_kw.value, ast.Constant)
and isinstance(order_kw.value.value, str)
and order_kw.value.value.strip()
)
if has_distinct and has_literal_order:
self._flag(node, f"{name}(distinct=True, order_by=...) -> frappe drops ORDER BY for distinct queries on Postgres; sort in python instead, e.g. sorted(..., key=str.casefold)")
self.generic_visit(node)
def visit_Subscript(self, node: ast.Subscript) -> None: