`update_inward_order_received_items_for_manufacture` unpacks
`zip(*item_code_wh.keys(), strict=True)`. When the manufacture entry has no
raw-material rows (all rows are finished/secondary/scrap), `item_code_wh` is
empty and the unpack raises `ValueError: not enough values to unpack`.
Return early when there are no such rows, mirroring the `if secondary_items:`
guard already present in `update_inward_order_secondary_items`.
In `validate_manufacture`, `customer_warehouse` is read only inside the
`skip_transfer` branch but was fetched unconditionally, wasting a lookup on
the non-skip-transfer path. Move it inside the branch that uses it.
`validate_material_transfer` ran the `Work Order Item` query and rebuilt
`wo_item_dict` inside the per-item loop, even though both depend only on
`self.work_order`. For an entry with N customer-provided rows that meant N
identical queries. Build the lookup once before the loop.
`validate_manufacture` already builds the analogous dict once up front, so
this also aligns the two methods.
In `update_inward_order_item`, the walrus assignment `scio_item_name :=` is
already part of the truthy `if` condition, so the nested `if scio_item_name:`
is always true. Remove it and dedent the body.
`validate_delivery_on_save` imported `pypika.terms.ValueWrapper` inside its
per-item loop, re-running the import on every iteration. Move it to the
module-level imports.
The "exceeds quantity available" throw in `validate_manufacture` passes a
third positional arg (`item.transfer_qty`), but the message only has `{0}`
and `{1}` placeholders, so `str.format` silently discards it. Remove the
dead argument; no behaviour change.
`validate_manufacture` builds its "Target Warehouse for Finished Good must
be same as Finished Good Warehouse ..." message with placeholders `{1}` and
`{2}`, but only passes two positional args (indices 0 and 1). `str.format`
raises `IndexError: Replacement index 2 out of range` instead of rendering
the message, so a user who sets the wrong FG target warehouse gets an opaque
traceback rather than the intended validation error.
Renumber the placeholders to `{0}` and `{1}` to match the args.
Add a regression test for the one-row-per-item invariant: a BOM that lists the
same raw item on two lines at different qty must still be counted once in the
report ("# Req'd Items" == 1). The test fails on the pre-fix multi-column GROUP
BY (which split the item into one row per distinct stock_qty -> 2) and passes
after the fix, on both MariaDB and Postgres.
The earlier parity fix aggregated the non-key descriptive columns for the Item
and Customer based-on paths but left Supplier grouping by all three selected
columns (supplier, supplier_name, supplier_group). supplier_name is a stored
per-transaction field, so historical purchase docs holding a divergent value for
the same supplier would split one supplier into multiple rows — diverging from
the original MariaDB output, which grouped by t1.supplier only.
Aggregate supplier_name with Max() and keep only supplier + the FD master column
supplier_group in GROUP BY, restoring one row per supplier on both engines.
Add regression tests for the Supplier (purchase) and Customer (sales) paths that
assert a single row per key even when stored descriptive fields diverge; both
fail on the pre-fix multi-column GROUP BY and pass after the fix, on MariaDB and
Postgres.
#56192 made the trends queries Postgres-strict-GROUP-BY-valid by widening based_on_group_by
to include the selected descriptive columns. For Item it added t2.item_name, for Customer
t1.territory (and customer_name) — but item_name is an editable per-line field and territory an
editable per-document field, not functionally dependent on the item_code/customer key. On MariaDB
(ONLY_FULL_GROUP_BY off) this SPLITS the single row per key into one row per distinct
(key, item_name)/(key, territory), so a customer transacting across two territories (or an item
with an edited item_name) now shows duplicate rows with fractured per-period subtotals.
Group by the KEY only and aggregate the non-key descriptive columns with Max(): one row per
based-on key (identical to the pre-#56192 MariaDB output) and still Postgres-valid. Supplier
columns are master-joined / fetch-locked (functionally dependent) so they stay unchanged.
#56196's Postgres GROUP BY fix added bom.quantity, bom_item.stock_qty and
bin.actual_qty to the GROUP BY. bom.quantity and bin.actual_qty are pinned to a
single value by the WHERE/join, but a BOM may list the same item_code on multiple
lines with different stock_qty (validate_materials does not dedupe), so grouping by
stock_qty SPLITS the row and changes req_items/instock on MariaDB for such BOMs.
Aggregate build_qty with Max() and group by item_code only: one row per item_code
(identical to the pre-#56196 single-line result; deterministic for duplicate lines),
and Postgres-valid. MariaDB output is unchanged for the common single-line case and
its row count is restored for the duplicate-line case.
- 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.
The Postgres CI site only listed erpnext in install_apps, so the payments app
(fetched and built by install.sh via 'bench get-app payments') was never
installed on the site — leaving 'tabPayment Gateway' absent. test_payment_request
(and other payment-gateway-dependent tests) then errored on Postgres with
'relation "tabPayment Gateway" does not exist', while MariaDB passed because its
site_config already lists ["payments", "erpnext"]. Match that ordering for parity.
Postgres fsyncs on every commit by default, which dominates a commit-heavy test suite.
Turn off synchronous_commit/fsync/full_page_writes on the throwaway CI database (reload-
time settings, no restart). MariaDB CI is unaffected (DB != postgres).
The MariaDB job is named 'Python Unit Tests', and 'Python Unit Tests (1..4)' are the
required status checks on develop. Naming the Postgres matrix job the same made its
checks report under those required contexts, effectively gating every (labelled) PR on
Postgres. Rename it to 'Postgres Unit Tests' so its contexts are distinct and the
workflow stays non-required until we deliberately add it to branch protection.
Bring the Server (Postgres) workflow in line with Server (MariaDB) internals while
keeping it opt-in for now: pull_request runs still require the 'postgres' label, but the
job now uses the full 4-container matrix (was 1), adds the nightly schedule /
workflow_dispatch / repository_dispatch triggers (which always run), and uploads
coverage. Builds ERPNext against frappe `develop` (PostgreSQL query-builder/ORM support
is merged there), so no fork override is needed.
The ERPNext server suite now passes on PostgreSQL and MariaDB from a single codebase;
flipping this to run on every PR / become a required check is a later, separate step.
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.
