A deeper re-audit (with an adversarial skeptic) of the queries left raw
in the prior commit found more that have exact ORM equivalents:
- scalar SUM/MAX -> frappe.qb + Sum/Max .run()[0][0]
- SUM ... GROUP BY -> frappe.qb .groupby().select(Sum().as_()) run(as_dict)
- name IN (values) -> get_all(filters={'f': ['in', ...]})
- sql_list(select col) -> get_all(pluck='col')
- bulk UPDATE ... = NULL/value -> frappe.db.set_value(filters, field, val)
- positional as_list reads -> get_all(..., as_list=True) (+ sorted())
Note: get_value(dt, filters, 'sum(x)') and get_all(fields=['sum(x)'])
are rejected by frappe ('SQL functions are not allowed as strings'), so
aggregates go through frappe.qb. get_all(as_list=True) returns a tuple
(not a list), so consumers that mutate use sorted().
All affected test modules pass on MariaDB.
Convert test-only raw frappe.db.sql calls that have an exact ORM
equivalent: full-table/filtered deletes -> frappe.db.delete, count ->
frappe.db.count, row-existence assertions -> frappe.db.exists,
single-row scalar fetches -> frappe.db.get_value, and simple
equality/range-filter selects -> frappe.get_all. No behaviour change.
Raw SQL that genuinely needs it is left as-is (dynamic identifiers,
aggregates/group-by, positional as_list consumers, DB-catalog
introspection).
Conservative cleanup of frappe.throw/msgprint messages per the message style
guide; meaning, severity, and .format() arguments are unchanged:
- index bare {} placeholders as {0}/{1}/... so translators can reorder
- move f-strings / .format() / concatenation out of _() (they break gettext
extraction and never translate)
- wrap translatable dynamic values (DocType/Select labels) in _()
- fix grammar and colloquialisms
- drop no-op _() wrapping runtime-built strings
Part of #53976.
Conservative cleanup of frappe.throw/msgprint messages per the message style
guide; meaning, severity, and .format() arguments are unchanged:
- index bare {} placeholders as {0}/{1}/... so translators can reorder
- move f-strings / .format() / concatenation out of _() (they break gettext
extraction and never translate)
- wrap translatable dynamic values (DocType/Select labels) in _()
- fix grammar and colloquialisms
- drop no-op _() wrapping runtime-built strings
Part of #53976.
_get_avg_valuation_rate_from_bins divides Sum(stock_value) by Sum(actual_qty).
The `Count(name) > 0` guard only proves a Bin row exists; Sum(actual_qty) can
still be 0 (stock depleted, or per-warehouse quantities cancelling out), and
the outer IfNull catches only NULL, not a 0 divisor.
MariaDB returns NULL for x/0 (then IfNull -> 0.0); PostgreSQL raises
`division by zero` and aborts BOM costing. Wrapping the divisor in
NullIf(Sum(actual_qty), 0) keeps the identical 0.0 result on MariaDB and
avoids the error on PostgreSQL.
The item_code field in the Job Card Item child table was optional,
allowing job cards to be saved without a raw material item linked.
Set reqd=1 in the JSON and update the Python type annotation accordingly.
get_item_lead_time in Master Production Schedule computes
manufacturing_time_in_mins / 1440 + purchase_time + buffer_time. As in the
MRP report, manufacturing_time_in_mins is an Int column and 1440 an int
literal, so the division truncates on PostgreSQL (720/1440 -> 0) while
MariaDB yields 0.5. The value is summed over the BOM tree, ceil'd, and
drives the planned order-release date, so it diverged by engine.
Use a float numerator (1440.0). MariaDB output is unchanged; PostgreSQL
now matches it.
get_item_lead_time computed the manufacturing lead time as
1440 / manufacturing_time_in_mins + buffer_time. Both columns are Int, so
integer/integer division truncates on PostgreSQL (1440/7 -> 205) while
MariaDB yields a decimal (205.71). The value feeds math.ceil() and then
release_date = add_days(delivery_date, -lead_time), so a user could see a
release date that differs by a day between engines.
Make the numerator a float literal (1440.0) so both engines do decimal
division. MariaDB already returned a decimal, so its output is unchanged;
PostgreSQL now matches it.
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.
The over-order check sums the same item across multiple order rows. Add a
test where one item is split into two Sales Order rows against the same
blanket order and together exceed its quantity.
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.
#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.
_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).
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.
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>
get_item_list() computes build_qty as IfNull(bin.actual_qty * bom.quantity
/ bom_item.stock_qty, 0) while grouping only by bom_item.item_code. The
three operand columns are neither grouped nor aggregated, so MariaDB
arbitrary-picks them but Postgres rejects the query with "column ... must
appear in the GROUP BY clause".
Add bom.quantity, bom_item.stock_qty and bin.actual_qty to the GROUP BY.
The WHERE pins bom/item and the join pins warehouse to single rows (Bin is
unique per item+warehouse), so the result stays one row per item and
MariaDB behaviour is unchanged.
Adds a test (no test file existed) that runs the report against a Work
Order and asserts it is listed, exercising the query on both engines.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
_query_bom_items / _build_base_bom_items_query / _add_*_item_columns selected
non-grouped columns (idx, item_name, image, project, item-default fields, BOM
Item attributes) alongside `group by item_code` -> arbitrary pick on MariaDB,
GroupingError on Postgres. Wrap them in Max() (Min() for idx, preserving the
original ordering). Every wrapped column is functionally dependent on the
grouped item_code (item attributes / the single BOM's project / one Item
Default per item+company), so Max()/Min() returns exactly the value MySQL
picked arbitrarily -> MariaDB output unchanged.
This was previously shipped in #56008 and reverted with that batch; re-applied
in isolation here. Verified: test_work_order 85/85 on BOTH MariaDB (no change)
and Postgres (was 85/85 failing on this query).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Same class as the sub_assembly_queries / bom_stock_analysis fixes in this PR: two
more merged queries wrapped a non-functionally-dependent column in Max(), so the
engines can disagree and the value is wrong for the case the column drives.
bom_explosion._subitems_query — Max(is_phantom_item):
Rows are grouped by item_code and get_subitems() drops any grouped row whose
is_phantom_item is truthy. When one item_code is listed in a BOM both as a
phantom sub-assembly and as a plain raw material, Max() returns 1 and the real
raw material is silently dropped from the plan. Use Min(): an item is phantom
only when EVERY line for it is phantom, so a real material is never lost.
required_items._material_transfer_qty_by_item — Max(original_item):
original_item is the output dict key. The same item B can be transferred both
for itself (original_item NULL) and as a substitute for required item A
(original_item=A). Grouping by item_code alone with Max() merged the two and
credited B's whole transfer to A, leaving B at 0. Group by
(item_code, original_item) and accumulate into the keyed dict so each transfer
is credited to the right required item (two rows can resolve to one key, e.g.
A's own transfer and B-for-A, hence += not plain assignment).
Both were previously undefined SQL (loose GROUP BY); the fix makes MariaDB and
Postgres agree on the correct, deterministic value. Other Max()-wrapped columns
in these queries are functionally dependent on the grouped item and unchanged.
Tests (fail on the old code, pass on both engines):
- test_subitems_query_keeps_real_rm_listed_alongside_phantom
- test_transferred_qty_not_misattributed_between_item_and_its_substitute
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Address review on #56090: get_bom_data groups components by item_code, so it
picks one representative BOM Item line for the (bom_no, is_phantom_item) pair.
Taking the first line by idx dropped the phantom flag when a non-phantom line
was listed before the phantom one, so explode_phantom_boms skipped the sub-BOM.
Keep one row per item_code (preserving the qty_per_unit total per component
rather than widening the GROUP BY), but make the representative phantom-
preferring: the first line, upgraded to the first phantom line if any exists.
A phantom sub-BOM is therefore never dropped due to line order, on either engine.
Adds test_phantom_explosion_when_phantom_line_is_not_first (phantom line at
idx 2) alongside the existing idx-1 case; both pass on MariaDB and Postgres and
the new one fails on the naive first-line representative.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
When a component is listed more than once in a BOM pointing at different
sub-BOMs (e.g. one phantom, one not), two queries grouped the duplicate
lines into a single row and aggregated bom_no and is_phantom_item with
*independent* Max(). That could pair the phantom flag of one line with the
bom_no of another, so the consumer recursed into the wrong sub-BOM:
- sub_assembly_queries._sub_assembly_rm_query keys on (item_code, bom_no)
and recurses on is_phantom_item. An incoherent pair sent raw-material
resolution down the wrong sub-assembly BOM.
- bom_stock_analysis.get_bom_data: explode_phantom_boms recurses into
bom_no only when is_phantom_item is set; an incoherent pair exploded a
non-phantom sub-BOM as if it were phantom (or vice-versa).
Fix:
- sub_assembly_queries: group also by (bom_no, is_phantom_item) so each
distinct sub-BOM is its own coherent row.
- bom_stock_analysis: drop the two independent Max()es and attach a single
representative line (lowest idx) per item_code before exploding.
This was previously undefined SQL (loose GROUP BY); the fix makes MariaDB
and Postgres agree on a deterministic, coherent pairing. Other Max()-wrapped
columns are functionally dependent on the grouped item and keep their value
on both engines.
Tests (fail on the old code, pass on both engines):
- test_phantom_explosion_picks_coherent_sub_bom: duplicate-component BOM
explodes the phantom sub-BOM, not the lexically-larger non-phantom one.
- test_sub_assembly_rm_query_keeps_bom_no_phantom_pair_coherent: the query
returns one coherent row per distinct sub-BOM with the right phantom flag.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Wrap the non-aggregated, functionally-dependent column(s) in Max()/Min() (or add
them to GROUP BY) so the report's grouped query is valid under PostgreSQL's strict
GROUP BY. No behaviour change on MariaDB.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Wrap the non-aggregated, functionally-dependent column(s) in Max()/Min() (or add
them to GROUP BY) so the report's grouped query is valid under PostgreSQL's strict
GROUP BY. No behaviour change on MariaDB.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The "item is not discontinued" checks treat an item as alive when its
`end_of_life` is unset, in the future, or the MariaDB zero-date `'0000-00-00'`.
`'0000-00-00'` is an invalid date literal on PostgreSQL (it errors), and a
"not set" end_of_life is `NULL` there anyway — already covered by the existing
`end_of_life IS NULL` term. So the zero-date comparison is applied on MariaDB
only; PostgreSQL keeps the `IS NULL` / future-date terms. No behaviour change on
MariaDB.
Sites: work order item-master selection (`mapper.py`), reorder-level item
selection (`reorder_item.py`), and the Stock Projected Qty report.
Part of the staged MariaDB<->PostgreSQL parity rollout (one problem class).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
refactor(manufacturing, projects): make raw SQL portable to PostgreSQL
Convert the MariaDB-only raw `frappe.db.sql` in the Manufacturing and Projects
modules to the cross-database query builder / ORM, and fix the non-portable
constructs that remain. Every change is a no-op on MariaDB (identical rendered
SQL / identical results) and only brings PostgreSQL — standards-strict where
MySQL is lax — in line.
Areas: BOM (cost/where-used/explosion), Work Order (operations, required items,
mapper, stock report), Workstation, Production Plan sub-assembly/explosion
queries, BOM Stock Analysis / Process Loss / Work Order Stock reports; Projects
(project, task, timesheet, activity cost, project update), Daily Timesheet
Summary and Project-wise Stock Tracking reports.
Part of the staged MariaDB<->PostgreSQL parity rollout (module 2 of 9).
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
