- 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).
Remove erpnext/tests/test_postgres_compat.py (and its pre-commit exclude); a unit
test for the dev-tooling lint helper isn't needed in the app test suite.
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.
get_timeline_data uses UnixTimestamp(posting_date); on Postgres that is the date's
midnight epoch in the DB session timezone, which can sit up to a day ahead of the
Python time.time() instant when the app timezone is ahead of UTC. The strict
'<= now' upper bound is therefore flaky on Postgres. Allow a day of slack on the
upper bound; MariaDB's UNIX_TIMESTAMP stays <= now so its pass/fail is unchanged.
create_bank_account() inserts a bank Account and swallows DuplicateEntryError
('bank account same as a CoA entry'). On Postgres the failed insert aborts the
transaction, so the rest of company setup ran against a poisoned transaction.
Take a savepoint and roll back to it in the handler. No-op on MariaDB.
add_bank_accounts() inserts a Bank Account per Plaid account in a loop. On a
duplicate the bare insert raises UniqueValidationError, which on Postgres aborts
the whole transaction; the handler only msgprint'd and continued, so the next
iteration's insert died with InFailedSqlTransaction. Wrap each iteration in a
savepoint and roll back to it in the handlers (the pattern frappe#40075 prescribes
after dropping the blanket per-insert savepoint). No-op on MariaDB.
SHOW INDEX is MySQL-only and errored on Postgres. Add a db-aware helper that reads
the leading index column from pg_index on Postgres and keeps SHOW INDEX on
MariaDB; both assert the field is the first column of some index.
The deliberate UniqueValidationError when re-adding a barcode aborts the
transaction on Postgres, so the next frappe.get_doc() failed with
InFailedSqlTransaction. Wrap the expected-failure save in a savepoint and roll
back to it. No-op on MariaDB.
The deliberate UniqueValidationError from the second Bin insert aborts the
transaction on Postgres, so the following _create_bin() (which takes its own
savepoint) failed with InFailedSqlTransaction. Wrap the expected-failure insert in
a savepoint and roll back to it, mirroring _create_bin's 'preserve transaction in
postgres' pattern. No-op on MariaDB.
frappe.db.set_value(..., "reconciled", True) renders SET reconciled=true; the
column is smallint, which Postgres rejects (DatatypeMismatch). MariaDB coerces the
boolean to 1. Pass 1 so both engines store the same value.
_bom_contains_item() lowercased the item name and then reused that lowercased
value as a doc name in frappe.db.get_value("Item", item, "variant_of"). Doc
names are case-sensitive on Postgres, so the lowercased name matched no row,
variant_of came back NULL, and a Work Order for a variant item built from the
template's BOM was wrongly rejected with 'BOM ... does not belong to Item ...'.
Keep the original case for the Item lookup; the comparisons stay case-insensitive.
MariaDB is unchanged (its name lookup was case-insensitive either way).
It never references `self`. The deterministic-serial-value test added in #56249
called it as `get_incoming_value_for_serial_nos(None, sle, serial_nos)` — passing
None for self, which is fragile: a future `self.*` access would fail with an opaque
AttributeError. Declaring it @staticmethod makes the call honest
(`get_incoming_value_for_serial_nos(sle, serial_nos)`) and is backward compatible —
the method has no in-repo callers besides that test, and any `self.`-style call still
binds correctly to a staticmethod.
Addresses Greptile review feedback on #56249.
The repo-wide query audit fixed runtime/source queries, but test files carry their
own raw SQL helpers that were never swept and only fail when the suite runs on
Postgres. Port the staging branch's already-green fixes for them:
- timestamp(posting_date, posting_time) (raw + qb Timestamp) -> posting_datetime /
CombineDatetime (test_stock_ledger_entry, test_stock_balance, test_utils)
- HAVING <select-alias> -> qb .having(<expr>) (test_asset_capitalization, test_purchase_order)
- capital-cased identifiers ("Status", "Name") -> lowercase (test_delivery_note,
test_purchase_order, test_employee)
- raw GL/SLE select helpers -> frappe.get_all / qb, with order-independent
comparisons where account ordering is collation-dependent across engines
(test_purchase_invoice, test_sales_invoice, test_payment_entry, test_asset,
test_purchase_receipt, test_payment_request, test_repost_accounting_ledger,
test_journal_entry)
All changes are test-only and behaviour-identical on MariaDB (lowercase column names
resolve the same; posting_datetime == timestamp(posting_date, posting_time); HAVING on
the expression is the same computation). Verified: the heavy modules pass on both
MariaDB and Postgres, and MariaDB output is unchanged.
The report's batch_no filter used an exact `==`, which is case-sensitive on Postgres -- a
differently-cased batch_no missed Job Cards that MariaDB (case-insensitive collation)
matches. Add a dedicated batch_no branch wrapping both sides in Lower() (keeping the exact
match, not a substring like serial_no): MariaDB result is unchanged, Postgres now matches.
The serial-no filter used serial_batch_entry.serial_no.isin(serial_nos), which is
case-sensitive on Postgres -- a differently-cased serial no missed Serial and Batch
Entry rows that MariaDB (case-insensitive collation) matches (the OR'd regexp branch
only covers the legacy Stock Ledger Entry.serial_no text, empty for bundle-tracked
serials). Lower() both sides: MariaDB result unchanged, Postgres now matches too.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
get_item_codes_by_attributes compared Item Variant Attribute `attribute`/`attribute_value`
with raw equality/IN, which is case-sensitive on Postgres -- a differently-cased website
filter value missed variants that MariaDB (case-insensitive collation) matches. Lower()
both sides: MariaDB result is unchanged (already case-insensitive), Postgres now matches too.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
test_stock_entry.get_sle ordered by `timestamp(posting_date, posting_time)`, a
MySQL-only two-arg function that errors on Postgres ("function timestamp(date, time)
does not exist"), so every test using get_sle (test_fifo, test_stock_entry_qty, ...)
failed to run on Postgres. Order by the precomputed `posting_datetime` column instead
(identical value on MariaDB, valid on both engines).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
batch.get_batches(item_code, warehouse, ...) was added by #55647 and has no callers
anywhere in erpnext, frappe, or payments (not whitelisted, not referenced from JS/hooks).
It is also obsolete: it joins Stock Ledger Entry on `batch_no`, which the Serial and
Batch Bundle system no longer populates, so it returns nothing even on MariaDB. Its
query was additionally Postgres-invalid (GROUP BY batch_id with ORDER BY expiry_date/
creation -> GroupingError, since batch_id is not the primary key).
Remove the dead function (and its now-unused CurDate/Sum import) rather than fix a query
that nothing can reach. Live batch-quantity lookups go through get_batch_qty() /
get_auto_batch_nos(), which use the bundle model.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
get_timesheets_list selected `timesheet.sales_invoice | detail.sales_invoice`,
intending COALESCE (pick the parent timesheet's invoice, else the detail's) -- the
original raw SQL was COALESCE(ts.sales_invoice, tsd.sales_invoice). pypika's `|` is a
bitwise OR, not a coalesce:
- Postgres: `varchar | varchar` -> "operator does not exist" (hard error).
- MariaDB: bitwise OR coerces the operands to integers; with a NULL detail invoice the
result is NULL, so the portal showed no invoice even when the timesheet was billed.
Replace with Coalesce(table.sales_invoice, child_table.sales_invoice).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
test_index_creation used `frappe.db.sql("show index from tabItem")` and the MySQL-only
result key "Column_name". "SHOW INDEX" errors on Postgres, so the test could not run
there. Use the db-agnostic frappe.db.get_column_index("tabItem", column) (checking both
unique and non-unique single-column indexes) instead.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
test_index_exists used `frappe.db.sql("show index from tabBin ...")`. "SHOW INDEX"
is MySQL-only syntax and errors on Postgres (syntax error at "from"), so the test
could not run there. Use the db-agnostic frappe.db.has_index("tabBin",
"unique_item_warehouse") instead.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
rfq_transaction_list had two defects introduced when it was converted to the query
builder:
1. `party.supplier == party[0]` compared supplier to a column literally named "0"
(a stray index on the DocType, not the intended `parties[0]` value). This renders
as `supplier = \`0\`` / `supplier = "0"` and errors on BOTH engines
(MariaDB: Unknown column '0'; Postgres: column "0" does not exist), so the
supplier portal RFQ list was completely broken.
2. SELECT DISTINCT ordered by `creation`, which is not in the select list. Postgres
rejects this ("for SELECT DISTINCT, ORDER BY expressions must appear in select list").
Compare against `parties[0]` and add `creation` to the select list.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The "Lost Quotations %" column computed Count(distinct) / total_quotations * 100,
where both operands are integers. Postgres does integer division on int/int, so any
group that is a strict minority of the total truncated to 0 (e.g. 1 of 4 -> 0%);
MariaDB always divides as decimal. Multiply by 100.0 before dividing so the division
is done in floating point on both engines.
The "Lost Value %" column already divided Sum(Currency)/Sum(Currency) (numeric), so it
was unaffected; left unchanged.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
When Accounts Settings -> general_ledger_remarks_length is set, the GL report
adds `substr(remarks, 1, n) as 'remarks'` to its raw SQL. Postgres treats a
single-quoted column alias as a string literal and raises a syntax error, so
the General Ledger report is broken on Postgres whenever that setting is on.
Use a bare alias (`as remarks`). substr() itself is portable.
Adds a test that sets general_ledger_remarks_length and runs the report,
asserting it executes (and returns rows) on both engines.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
get_vendor_invoice_query filtered unclaimed invoices with
.having(unclaimed_amount > 0), but the query has no GROUP BY/aggregate and
unclaimed_amount is a SELECT alias. Postgres rejects HAVING on a SELECT alias
(and HAVING without GROUP BY on a non-aggregated column); MariaDB allowed it.
Move the threshold into WHERE on the underlying expression.
Behaviour is identical on MariaDB (same rows); fixes a hard error on Postgres.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
update_variant_attribute_values (propagates renamed Item Attribute Values to
variant items) used a qb UPDATE ... JOIN. Postgres has no UPDATE..JOIN syntax,
so renaming an Item Attribute Value errored on Postgres.
Rewrite as a correlated UPDATE that restricts to variant items via a subquery
on the parent (item_variant_table.parent.isin(variant Items)) instead of
joining the Item table. MariaDB behaviour is unchanged.
Covered by the existing test_item.test_rename_attribute_value_updates_variants
and test_swapped_attribute_value_renames_update_variants, which errored on
Postgres before and now pass on both engines.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
get_timeline_data grouped Timesheet Detail by Date(from_time) but selected
UnixTimestamp(from_time) (the full timestamp, ungrouped). MariaDB
arbitrary-picks a row's timestamp; Postgres rejects it ("must appear in the
GROUP BY clause"), so the Project timeline (calendar heatmap) is broken on PG.
Select UnixTimestamp(Date(from_time)) — the day's epoch — which is the
timeline key and matches the GROUP BY. CurDate() - Interval(years=1) is
portable and kept as-is.
Adds a test (no coverage existed) that records a timesheet against a project
and asserts get_timeline_data returns day-bucketed counts, on both engines.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
get_purchase_details grouped Purchase Order Item by (item_code, warehouse)
while selecting `qty` ungrouped/unaggregated. MariaDB arbitrary-picks one
row's qty; Postgres rejects the query ("must appear in the GROUP BY clause"),
so the report is broken on Postgres.
Sum the qty per item+warehouse ({"SUM": "qty"}). The column is the "Arrival
Qty" (quantity on order arriving) display figure; summing the open PO lines is
the meaningful planning number, and is deterministic vs MariaDB's arbitrary
single-line pick (which only differed when an item+warehouse had multiple open
PO lines).
Adds a test (no test file existed) that creates a Work Order plus two PO lines
for a BOM raw material and asserts the report runs and reports arrival_qty = 7
(3 + 4), on both engines.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
