mirror of
https://github.com/frappe/erpnext.git
synced 2026-08-22 02:40:02 +00:00
fix(stock): close postgres lock-then-read races in pick list and stock reservation
Postgres has no gap locks, so the lock-then-read pattern (plain SELECT ... FOR UPDATE before a grouped read) only serializes on rows that already exist. Two sites had reachable races where the lock set is empty or disjoint: - Pick list: two pick lists against the same SO item submitted concurrently lock only docstatus=1 rows, so with no previously-submitted picks their lock sets are disjoint and both pass validate_picked_qty (over-pick; picked_qty last-writer-wins). Gate on the referenced Sales Order Item / Packed Item rows, which always exist. - Stock reservation: the first concurrent reservations for an (item, warehouse) find no SRE rows to lock, so both pass and reserved qty can exceed actual. Gate on the Bin row, which exists once there is stock. MariaDB is unchanged (its gap locks already serialize both; the gates are postgres-only). Also: ORDER BY on the small-set postgres lock selects for deterministic lock order, and the repost pre-lock in get_future_stock_vouchers selects a constant instead of shipping every matching SLE name to the client.
This commit is contained in:
@@ -13,6 +13,7 @@ from frappe.desk.reportview import build_match_conditions
|
||||
from frappe.model.meta import get_field_precision
|
||||
from frappe.model.naming import determine_consecutive_week_number
|
||||
from frappe.query_builder import AliasedQuery, Case, Criterion, Field, Table
|
||||
from frappe.query_builder.custom import ConstantColumn
|
||||
from frappe.query_builder.functions import Count, IfNull, Max, Min, Round, Sum
|
||||
from frappe.query_builder.utils import DocType
|
||||
from frappe.utils import (
|
||||
@@ -1791,9 +1792,10 @@ def get_future_stock_vouchers(posting_date, posting_time, for_warehouses=None, f
|
||||
# transaction can't modify them mid-flight (the original DISTINCT ... FOR UPDATE did this).
|
||||
# MariaDB carries the lock on the grouped query below; postgres rejects FOR UPDATE alongside
|
||||
# GROUP BY, so lock the matching rows in a separate pass first -- the row locks are held until
|
||||
# the surrounding transaction ends, giving the same protection.
|
||||
# the surrounding transaction ends, giving the same protection. Select a constant, not the
|
||||
# name: a deep backdated repost can match millions of rows and only the locks are needed.
|
||||
if frappe.db.db_type == "postgres":
|
||||
frappe.qb.from_(SLE).select(SLE.name).where(conditions).for_update().run()
|
||||
frappe.qb.from_(SLE).select(ConstantColumn(1)).where(conditions).for_update().run()
|
||||
|
||||
# distinct vouchers in chronological order; expressed as GROUP BY + Min() so it's valid on
|
||||
# postgres (SELECT DISTINCT can't ORDER BY non-selected cols, and FOR UPDATE is invalid with both).
|
||||
|
||||
@@ -952,10 +952,22 @@ def get_picked_items_qty(items, contains_packed_items=False) -> list[dict]:
|
||||
)
|
||||
|
||||
# Lock the picked-qty rows so a concurrent pick can't change them mid-transaction. MariaDB carries
|
||||
# the lock on the grouped query; postgres rejects FOR UPDATE with GROUP BY, so lock the same rows
|
||||
# in a separate plain SELECT first (held for the transaction).
|
||||
# the lock on the grouped query (its gap locks also block rows other in-flight picks are about to
|
||||
# submit); postgres has no gap locks, so first serialize on the referenced SO/packed item rows
|
||||
# (they always exist), then lock the matching picked rows in a separate plain SELECT.
|
||||
if frappe.db.db_type == "postgres":
|
||||
frappe.qb.from_(pi_item).select(pi_item.name).where(conditions).for_update().run()
|
||||
parent = frappe.qb.DocType("Packed Item" if contains_packed_items else "Sales Order Item")
|
||||
(
|
||||
frappe.qb.from_(parent)
|
||||
.select(parent.name)
|
||||
.where(parent.name.isin(items))
|
||||
.orderby(parent.name)
|
||||
.for_update()
|
||||
.run()
|
||||
)
|
||||
frappe.qb.from_(pi_item).select(pi_item.name).where(conditions).orderby(
|
||||
pi_item.name
|
||||
).for_update().run()
|
||||
else:
|
||||
query = query.for_update()
|
||||
|
||||
|
||||
@@ -716,10 +716,19 @@ def get_available_qty_to_reserve(
|
||||
conditions &= sre.name != ignore_sre
|
||||
|
||||
# Lock the rows being aggregated so a concurrent reservation can't change them mid-transaction.
|
||||
# MariaDB carries the lock on the aggregate query itself; postgres rejects FOR UPDATE with an
|
||||
# aggregate, so on postgres lock the same rows in a separate plain SELECT first (held for the txn).
|
||||
# MariaDB carries the lock on the aggregate query itself (its gap locks also serialize two
|
||||
# FIRST reservations, when no SRE rows exist yet); postgres has no gap locks, so gate on the
|
||||
# Bin row (exists once there is stock), then lock the matching SREs in a plain SELECT.
|
||||
if frappe.db.db_type == "postgres":
|
||||
frappe.qb.from_(sre).select(sre.name).where(conditions).for_update().run()
|
||||
bin_table = frappe.qb.DocType("Bin")
|
||||
(
|
||||
frappe.qb.from_(bin_table)
|
||||
.select(bin_table.name)
|
||||
.where((bin_table.item_code == item_code) & (bin_table.warehouse == warehouse))
|
||||
.for_update()
|
||||
.run()
|
||||
)
|
||||
frappe.qb.from_(sre).select(sre.name).where(conditions).orderby(sre.name).for_update().run()
|
||||
|
||||
query = (
|
||||
frappe.qb.from_(sre)
|
||||
|
||||
Reference in New Issue
Block a user