Commit Graph

6 Commits

Author SHA1 Message Date
Mihir Kandoi
f751f80158 ci(postgres): flag division by a possibly-zero divisor in the compat guard (#56363)
Adds the division-by-zero divergence class to the PG-compat review tooling:
on a divisor that the data can drive to 0 (e.g. Sum(a)/Sum(b)), MariaDB
returns NULL for division by zero while PostgreSQL raises `division by zero`
and aborts the query. The portable fix is to wrap the divisor in
NullIf(divisor, 0), which yields NULL on both engines (matching MariaDB).

- .greptile/config.json: add it to the "would ERROR on PostgreSQL" list.
- .github/POSTGRES_COMPATIBILITY.md: document it under §1 (hard breaks).
- .github/helper/postgres_compat.py: note it in the docstring as a
  deliberately-not-statically-checked semantic divergence (data-dependent,
  like integer-division intent), so it stays a reviewer/Greptile concern.

Tooling-only; no source query changes. The instance fix shipped in #56361.
2026-06-23 12:00:50 +00:00
Mihir Kandoi
331f383777 ci(postgres): match CAST AS CHAR with nested parens in the checker
The [^)]* span stopped at the first inner ')', so CAST(ABS(col) AS CHAR) slipped through. Use a non-greedy .+? with re.S; still zero production false positives (verified). Addresses review feedback.
2026-06-23 13:24:49 +05:30
Mihir Kandoi
fc9544435e ci(postgres): teach the PG-compat tooling the audit 6-9 divergence classes
The whole-repo MariaDB<->PostgreSQL audits surfaced classes the checker and
review guide did not yet cover. Add them:

Static checker (.github/helper/postgres_compat.py) - new mechanical breaks:
- .rlike() / raw RLIKE: frappe rewrites REGEXP->~* on Postgres but NOT RLIKE.
- Cast(x, "char") / raw CAST AS CHAR: bare CHAR is character(1) on Postgres
  and truncates multi-digit values; use "varchar".
(Both flag zero production code; the only repo hit is in patches/, which the
hook already excludes.)

Greptile config + POSTGRES_COMPATIBILITY.md - new semantic/hard classes:
- aggregate (Sum/Count) selected next to bare columns with no GROUP BY at all.
- .like()/LIKE on a non-text column (bigint ILIKE) -> Cast_ to varchar.
- get_all(fields=["CapitalCase"]) identifier-case (extends the get_value case).
- bool into a Check column via qb.update().set() (extends set_value/db_set).
- int/int division: float a literal (col/1440 -> col/1440.0).
- Concat over a nullable column leaking a bare prefix on Postgres.
- clarify REGEXP/.regexp() is translated but RLIKE/.rlike() is not.
2026-06-23 12:22:22 +05:30
Mihir Kandoi
ad237e5ec5 ci(postgres): flag get_all(distinct=True, order_by=...) in the static checker
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.
2026-06-22 23:22:52 +05:30
Mihir Kandoi
b2ee8cb1b9 ci(postgres): fix semgrep + two review findings in the checker
- semgrep: annotate the source-reading open() with # nosemgrep for the
  frappe-security-file-traversal rule (dev-only lint tool; path comes from pre-commit,
  not user input).
- bool-scan: only inspect the field *value* arg (db_set args[1]/dict args[0];
  set_value args[3]/dict args[2]) so a positional update_modified=False
  (e.g. db_set('f', 0, False)) no longer false-positives.
- # pg-ok: also honour the annotation on a multi-line call's closing paren line
  (scan one line past the node's end).
2026-06-21 17:10:34 +05:30
Mihir Kandoi
549a24f7b9 ci(postgres): add a static pre-commit check for MySQL-only SQL
The Postgres test job is label-gated, so it does not run on every PR. This adds an
always-on pre-commit hook that statically flags the *mechanical* breaks: MySQL-only
functions (timestamp(date,time), timediff, str_to_date, date_format/add/sub,
group_concat, period_diff, SQL IF()), SHOW INDEX/TABLES/COLUMNS, single-quoted
aliases, UPDATE..JOIN, interpolated/f-string SQL carrying MySQL-isms,
set_value/db_set(<Check>, bool), and MySQL SHOW INDEX result keys.

It deliberately does NOT flag the framework auto-translations (ifnull->coalesce,
backtick/locate/REGEXP, .like()->ILIKE) nor the *semantic* divergences (loose GROUP
BY, case-sensitive ==/IN, NULL ordering, tiebreakers) — those need the test suite,
which remains the backstop. AST + structure-gated regex keep false positives near
zero (docstrings and prose skipped); '# pg-ok' exempts intentional MariaDB-only
branches. Scoped to erpnext/ excluding patches/. Includes a unit test of the checker.
2026-06-21 16:53:53 +05:30