Address Greptile review: - customContext.files scope was **/*.py only, so Query Report SQL in .js/.sql/report .json files didn't get the guide attached as context (the global instructions still applied). Widen to .py/.js/.sql/report **/*.json. - The guide's HAVING-alias rule said "with no GROUP BY"; PostgreSQL rejects a SELECT-alias in HAVING regardless of GROUP BY. Reworded to match (repeat the expression, or move a non-aggregate predicate to WHERE).
9.0 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). Fix: add it 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")and similar — PostgreSQL folds unquoted identifiers to lower case; a stored column namedstatuswon't match"Status". Use the exact stored case. - Boolean passed where an integer column is expected —
frappe.db.set_value(dt, dn, check_field, True)emitsSET col = true, which PostgreSQL rejects on asmallint(DatatypeMismatch). Pass1/0.
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 —
COUNT(...) / COUNT(...) * 100truncates to0on PostgreSQL (integer/integer) but is decimal on MariaDB. Multiply by100.0first. 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.- Raw
ifnull(...)insidefrappe.db.sql()is rewritten tocoalesce(...)on all engines. - Backticks,
LOCATE,REGEXPin raw SQL are auto-translated on PostgreSQL. - 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.