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.
12 KiB
PostgreSQL compatibility — review guide
ERPNext targets both MariaDB and PostgreSQL from a single codebase. The full server
test suite passes on both, but the PostgreSQL CI job is label-gated (it does not run on
every PR), so until it is required this guide is the always-on guard. Greptile loads it as
review context (.greptile/config.json).
When reviewing a PR, flag any new or changed query (raw frappe.db.sql, frappe.qb,
frappe.get_all/get_list/get_value, report SQL) that would error on PostgreSQL or
return different results on the two engines.
The one rule that governs everything
MariaDB behaviour must not change; PostgreSQL is brought into line with MariaDB — never the reverse. A "fix" that changes the value, row count, or ordering MariaDB produced is a regression, even if the new behaviour looks more correct. The only accepted MariaDB-output change is replacing a genuinely undefined/arbitrary result with a deterministic one (row count preserved) — and that should be called out explicitly.
There are two failure modes to watch for:
- Hard breaks — PostgreSQL raises an exception; MariaDB is green. Easy to catch in CI, but the gated job may not run.
- Silent divergences — both engines succeed but return different results. CI on one engine stays green; the bug only shows on a PostgreSQL site. These are the dangerous ones.
1. Hard breaks — would error on PostgreSQL
Flag a changed query that uses any of these:
- Loose
GROUP BY— selecting/ordering a column that is neither inGROUP BYnor wrapped in an aggregate. MariaDB tolerates it; PostgreSQL errors (must appear in the GROUP BY clause or be used in an aggregate function). This also covers an aggregate (Sum/Count/…) selected alongside bare columns with NO.groupby()at all — MariaDB silently collapses every row into one arbitrary-valued row (often a wrong-output bug there too), PostgreSQL errors. Fix: add the bare column toGROUP BYif it is functionally dependent on the group key, otherwise wrap it inMax()/Min(). See §3 — the row-count trap — before suggesting "add it to GROUP BY". - MySQL-only functions —
TIMESTAMP(date,time),TIMEDIFF,STR_TO_DATE,DATE_FORMAT,DATE_ADD/SUB,GROUP_CONCAT,PERIOD_DIFF, SQLIF(cond,a,b). Use the portablefrappe.query_builder.functionsequivalents (CombineDatetime,DateDiff,Case,GroupConcat, …) or a precomputed column (e.g.posting_datetime). UPDATE … JOIN— not valid on PostgreSQL. Rewrite asUPDATE … WHERE name IN (subquery).HAVINGreferencing aSELECTalias — PostgreSQL rejects output-column aliases inHAVING(regardless of whether the query has aGROUP BY; MariaDB allows them). Repeat the underlying expression inHAVING, or move a non-aggregate predicate intoWHERE.SELECT DISTINCT … ORDER BY <expr not in the select list>— add the expr to the select.- Single-quoted column alias
AS 'x'— PostgreSQL reads'x'as a string literal. Use an unquoted (or double-quoted) alias. varchar | varchar(bitwise OR misused as a coalesce) — errors on PostgreSQL. UseCoalesce(...).- Capital-cased identifiers used as column/field names in
get_value(dt, dn, "Status"),get_all(dt, fields=["Account"]), and similar — PostgreSQL quotes the identifier and matches it case-sensitively; a stored column namedstatus/accountwon't match"Status"/"Account"(column "Account" does not exist). Use the exact stored (lower-case) fieldname. - Boolean passed where an integer column is expected —
frappe.db.set_value(dt, dn, check_field, True),doc.db_set(field, False), orfrappe.qb.update(dt).set(check_field, True)emitSET col = true, which PostgreSQL rejects on asmallint/Checkcolumn (column is of type smallint but expression is of type boolean). Pass1/0. .like()/.ilike()(or rawLIKE) on a NON-text column —idx,docstatus, a date, etc. frappe maps.like()→ILIKE, and PostgreSQL has nobigint ILIKE textoperator (operator does not exist: bigint ~~* unknown). Cast the column to text first —Cast_(col, "varchar"), notCast(col, "char")(see below). MariaDB coerces the int implicitly, so the cast is a no-op there.CAST(… AS CHAR)/Cast(x, "char")— on PostgreSQL bareCHARischaracter(1), soCAST(12 AS CHAR)→'1'(silently truncates multi-digit values); MariaDB gives the full string. UseVARCHAR/Cast_(x, "varchar")..rlike()/ rawRLIKE— frappe rewritesREGEXP→~*on PostgreSQL but does not translateRLIKE(no such PostgreSQL operator). Use.regexp()(or.like()for a simple prefix).IfNull/Coalesceof a typed column with a different-typed literal —IfNull(asset.disposal_date, 0)rendersCOALESCE("disposal_date", 0), coalescing a DATE with an integer. PostgreSQL requiresCOALESCEargs to share a type (DatatypeMismatch: COALESCE types date and integer cannot be matched); MariaDB'sIFNULLis permissive. The common shape isIfNull(date_col, 0) != 0 / == 0as a presence test — replace withdate_col.isnotnull()/date_col.isnull()(identical, and valid on both). Otherwise coalesce to a same-type default (Coalesce(date_col, '1900-01-01'),Coalesce(text_col, '')).- Division by a possibly-zero divisor —
Sum(a) / Sum(b),x / col, etc. where the divisor can be0/empty. MariaDB returnsNULLfor division by zero; PostgreSQL raisesdivision by zeroand aborts the query. Wrap the divisor inNullIf(divisor, 0)— that yieldsNULLon both engines, matching MariaDB's value. (Only the literal/ 0is a parse constant; the trap is a divisor that is an aggregate or column the data can drive to zero.)
2. Silent divergences — succeeds on both, returns different results
These don't error, so a one-engine CI stays green. Flag them:
- Case sensitivity on text equality —
==,.isin(),Strpos/Locateon free-text columns are case-sensitive on PostgreSQL but case-insensitive under MariaDB's default collation.Lower()both sides. (Not.like()/["like", …]— those already render asILIKEon PostgreSQL; see §4.) - Case sensitivity in a doc-
namelookup — lower-casing a value then using it as a document name inget_value/get_doc/existsmisses on PostgreSQL (names are case-sensitive). Keep original case for the identifier; lower-case only comparison operands. - Empty string vs NULL — PostgreSQL stores a blank link/data field as
NULLon some paths while MariaDB keeps'';Concat/Concat_wsthen diverge. Prefer the stored full value, orCoalesce(col, '')per argument. - NULL ordering — MariaDB sorts
NULLfirst, PostgreSQL sorts it last. ForORDER BY … LIMIT 1/[0]on a nullable column, guard withCoalesce/isnotnull(). ORDER BY … LIMIT 1with no unique tiebreaker — when rows tie on the ordered column the two engines may pick different rows. Add acreation/nametiebreaker only if it does not change MariaDB's current pick (see §4).- Integer division —
int / inttruncates on PostgreSQL but is decimal on MariaDB, e.g.COUNT(...) / COUNT(...) * 100→0, ormanufacturing_time_in_mins / 1440flooring a lead-time to whole days. Force float: multiply by100.0, or make a literal a float (/ 1440→/ 1440.0), or cast an operand. (Only SQL-level/on integer columns/literals — Python/is already float.) DISTINCTlist ordering —frappe.get_all(distinct=True, order_by=…)/SELECT DISTINCT … ORDER BY: frappe'sdb_querysilently dropsORDER BYfor distinct queries on PostgreSQL, so the result is unordered there. Sort in Python instead — and usekey=str.casefold, because baresorted()is case-sensitive (ASCII) while MariaDB's collation is case-insensitive, so a plain sort reorders MariaDB's output.- Engine-specific function rewrites — e.g. a PostgreSQL
regexp_replacebranch reimplementing MariaDB'sCAST(SUBSTRING_INDEX(name,' ',-1) AS UNSIGNED)(leading digits of the last whitespace token). Verify the rewrite matches MariaDB on edge cases ("X - 3a"→3,"X - 1.5"→1) by diffing both engines on literal rows. UnixTimestamp(date)/ date→epoch is timezone-dependent (midnight in the DB session TZ), so a strictepoch <= nowbound is flaky on PostgreSQL.
3. The GROUP BY row-count trap (the single most important rule)
When making a loose GROUP BY PostgreSQL-valid, do not add a non-functionally-dependent
column to the GROUP BY just to satisfy PostgreSQL — that turns one group row into N and
changes the MariaDB row count (a regression). The classic traps are adding the child/row
primary key or an editable per-row field. Instead Max()/Min()-wrap the offending
column: the row count is preserved and the value goes from arbitrary (MariaDB's old loose pick)
to deterministic.
Judge functional dependence by the source table, not the column name:
- A column from a master joined on the group key (
t3.xwheret1.key = t3.name) is FD → safe to keep inGROUP BY. - A descriptive field on the transaction table (
t1.supplier_name,t1.territory,t1.item_name— fetched/editable, can differ across historical rows for the same key) is not FD even though it looks master-derived →Max()-wrap it.
Conversely, do not suggest changing a Max()/Min()-wrapped column to Sum() (or vice
versa) to make a number "more correct" — that changes the MariaDB value. The wrap reproduces
MariaDB's prior one-value-per-group output; a different aggregate is a product change, out of
scope for a portability fix.
4. False positives — do NOT flag these
These are auto-handled by the framework and are not breaks:
.like()/["like", …]already renders asILIKEon PostgreSQL — not a case-sensitivity bug. (Exception:.like()on a non-text column —idx,docstatus— is a hard break,bigint ILIKE; see §1.)- Raw
ifnull(...)insidefrappe.db.sql()is rewritten tocoalesce(...)on all engines. - Backticks,
LOCATE,REGEXP/.regexp()in raw SQL are auto-translated on PostgreSQL (REGEXP→~*). ButRLIKE/.rlike()is NOT translated — that one is a hard break (see §1). - An
ORDER BY … LIMIT 1tie where the two engines already agree, or where adding a tiebreaker would change MariaDB's current pick — leave it; "fixing" it would either change MariaDB or has no observable effect.
5. Transaction / runtime (not query-shape, still PostgreSQL-only)
- Catch-and-continue inserts — on PostgreSQL a failed
insert()aborts the whole transaction, so code that swallows a duplicate and keeps going dies on the next statement withInFailedSqlTransaction(frappe dropped its blanket per-statement savepoint in frappe#40075). Such a handler must wrap the fallible insert infrappe.db.savepoint(name)+rollback(save_point=name)— unless it re-throws with no DB call before the throw, or the insert usesignore_if_duplicate=True/autoname="hash"(→ON CONFLICT DO NOTHING).
How to review
For every changed query: does it (a) use a construct from §1 (would error on PostgreSQL), or (b) match a divergence in §2/§3 (different result across engines)? If so, comment with the portable fix and confirm it leaves MariaDB output unchanged. Skip the §4 false positives. Prefer a comment that names the rule (e.g. "loose GROUP BY — Max()-wrap, don't add to GROUP BY: splits the row count") so the fix is unambiguous.
The static pre-commit checker (.github/helper/postgres_compat.py) catches the mechanical
§1 breaks; the semantic §2/§3 divergences are exactly what a reviewer (and this guide) must
cover, because no static check can see them.