The contact lookup orders only by is_primary_contact desc then takes contacts[0]; contacts commonly
tie (the no-primary case), so MariaDB and Postgres could pick a different contact. Add a parent
(contact name) tiebreaker.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The Pick List Item get_value orders only by qty desc; a pick list can hold multiple rows for the same
SO item split across warehouses/batches/serials that tie on qty, so MariaDB and Postgres could stamp a
different warehouse/batch/serial onto the packed item. Add a name tiebreaker.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
get_open_payment_requests_for_references orders by Coalesce(transaction_date, creation); when
transaction_date is set the coalesce never falls back to creation, so PRs sharing a transaction_date
have no tiebreaker and MariaDB/Postgres can allocate a different PR first. Append creation, name keys.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
get_item_price ORDER BYs valid_from/batch_no/uom/party then LIMIT 1 with no unique key. Two Item
Price rows tied on all of those but differing price_list_rate would be picked arbitrarily -- MariaDB
and Postgres can return a different rate. Append a name tiebreaker; for exact ties MariaDB's pick was
already undefined, so its output is preserved.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The QI-by-purpose check required an inspection on every outgoing
(s_warehouse) row for any purpose that was not incoming. Material
Consumption for Manufacture rows are source-only and the Work Order
mapper copies inspection_required from the BOM, so this silently
blocked submission. An inspection_required BOM inspects the finished
good, not each consumed raw material.
Replace the "anything not incoming" fallthrough with an explicit
QI_OUTGOING_PURPOSES allow-list (mirrored in transaction.js) so a new
purpose cannot silently start requiring a QI. Consumption and Return
Raw Material to Customer now need no QI; Issue, Transfer, Transfer for
Manufacture, Send to Subcontractor, Subcontracting Delivery and
Disassemble keep their outgoing checks. Scope item_query to the same
set and add a regression test.
The closing paragraph named only the §2/§3 semantic divergences as static-checker-invisible;
§6 (refactor/conversion row-set changes) is equally invisible and belongs there too. (greptile nit.)
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Mirror the two new POSTGRES_COMPATIBILITY.md rules into the greptile instructions so the bot flags
them on changed queries: (1) adding an ORDER BY column to a SELECT DISTINCT grows the distinct key
and the MariaDB row count unless it is functionally dependent; (2) a refactor / raw-frappe.db.sql->qb
conversion can silently change the WHERE/row set on both engines -- review the predicate, not just
the query shape.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Two review lessons from the post-merge net-diff/whole-repo re-audit of the SQL-dialect classes:
- Section 3 (row-count trap) now covers SELECT DISTINCT too: adding the ORDER BY column to the
select to satisfy Postgres grows the DISTINCT key and changes the MariaDB row count when the
column is not single-valued per distinct row -- sort in Python instead.
- New section 6: a 'refactor' / raw-SQL->qb conversion is not automatically 1:1. Diff the
WHERE/predicate and the resulting row set, not just the SELECT shape -- a conversion that widens
a filter (e.g. posting_datetime > X gaining an OR (== X AND creation > ...) branch under a
sql->qb refactor) changes the rows touched on both engines and hides under a refactor label.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(stock): value batch/serial return from ledger when original receipt has no bundle
* test(stock): add test to validate the valuation of serial/batch for return when original receipt has no bundle
create_address is a helper called by create_prospect/create_customer AFTER they insert the Prospect/Customer. Its full frappe.db.rollback() on an address-save failure rolled back the caller's just-inserted parent doc, then swallowed the exception, so the caller returned a Prospect/Customer name that no longer existed. Scope the rollback to savepoint('crm_create_address') so only the address work is undone; the parent doc survives and the failed address is just logged.
link_existing_conversations is the Contact after_insert hook; a full frappe.db.rollback() on a failed call_log.save() would discard the triggering Contact insert itself (and, in test mode, the whole unit of work). Savepoint the hook's DB work and roll back only to it.
from_detailed_data inserts tax templates/accounts before update_regional_tax_settings in the same transaction; a full frappe.db.rollback() on regional-setup failure discarded those templates while the wizard continued. Take a savepoint before the regional call and roll back only to it.
After rollback(save_point=reorder_mr) discards the just-inserted Material Request, mr.log_error() left a dangling Error Log reference. Use frappe.log_error(title=...).
The savepoint rollback erases the just-inserted Bank Transaction row, so bank_transaction.log_error() created an Error Log pointing at a row that no longer exists. Use frappe.log_error(title=...) with no doc reference.
send_mail (called per campaign schedule in a loop) inserts a Communication via make(); on failure the except calls frappe.log_error with no rollback, raising InFailedSqlTransaction on Postgres and poisoning subsequent sends. Savepoint before make() + rollback(save_point=) before log_error. No-op on MariaDB.
make_depreciation_entry posts a Journal Entry per schedule row in a loop; the except only stored the error, so the next row's je.save()/submit() ran on the Postgres-poisoned txn (InFailedSqlTransaction). Savepoint per iteration + rollback(save_point=) before storing the error; the final raise of the collected error is unchanged. No-op on MariaDB.
trigger_invoice_update_for_subscriptions loops invoices calling refresh_subscription_status (db_set/save); on failure the except calls frappe.log_error with no rollback, raising InFailedSqlTransaction on Postgres, and the next invoice runs in the poisoned txn. Savepoint per iteration + rollback(save_point=) before log_error. No-op on MariaDB.
start_merge merges accounts in a loop; on failure it only rolled back when not in_test, so in tests a failed merge_account left the Postgres txn poisoned and the except log_error + the finally db_set(status) raised InFailedSqlTransaction. Wrap each row in savepoint('ledger_merge_row') and rollback to it unconditionally before log_error - this recovers the txn in both paths without the full rollback discarding the rest of the test transaction. Production still commits per successful merge, so the per-iteration savepoint rollback is equivalent to the prior full rollback. No-op on MariaDB.
create_prospect/create_address/create_customer insert docs and on failure call frappe.log_error with no rollback; on Postgres (untrusted external CRM webhook input) a failed insert poisons the txn so log_error raises InFailedSqlTransaction. Full frappe.db.rollback() before each log_error. No-op on MariaDB.
book_deferred_entries' make_gl_entries failure path: the else branch already rolls back before log_error, but the frappe.in_test branch ran doc.log_error then re-raised with no rollback -> on Postgres log_error hits InFailedSqlTransaction and masks the original error. Rollback before log_error in the in_test branch too. No-op on MariaDB.
create_production_plan_bom (background job) save+submits BOMs in a loop; on failure the except runs self.db_set(status=Failed, error_log) with no rollback, raising InFailedSqlTransaction on Postgres so status is never set. Full frappe.db.rollback() at the top of the except. No-op on MariaDB.
prepare_closing_stock_balance (background job) saves Stock Closing Balance rows + db_set status; on failure the except runs db_set('Failed')+log_error with no rollback, raising InFailedSqlTransaction on Postgres so the doc is never marked Failed and the job dies. Full frappe.db.rollback() before the handler's db_set. No-op on MariaDB.
Regional tax-template setup writes docs; on failure the except calls frappe.log_error with no rollback -> InFailedSqlTransaction on Postgres. Full rollback before log_error. No-op on MariaDB.
Regional fixture setup writes docs; on failure the except calls frappe.log_error before frappe.throw with no rollback -> InFailedSqlTransaction on Postgres. Full rollback before log_error. No-op on MariaDB.
The hook saves Call Logs in a loop; on failure the except calls frappe.log_error (INSERT) with no rollback, raising InFailedSqlTransaction on Postgres (it runs on every Contact create/update). Full frappe.db.rollback() before log_error. No-op on MariaDB.
