Merge pull request #56220 from mihir-kandoi/pg-item

fix(stock): convert item.py raw SQL → qb/ORM (Postgres)

erpnext/stock/doctype/item/item.py

@@ -534,14 +534,16 @@ class Item(Document):
 			for item_barcode in self.barcodes:
 				options = frappe.get_meta("Item Barcode").get_options("barcode_type").split("\n")
 				if item_barcode.barcode:
-					duplicate = frappe.db.sql(
-						"""select parent from `tabItem Barcode` where barcode = %s and parent != %s""",
-						(item_barcode.barcode, self.name),
+					duplicate = frappe.get_all(
+						"Item Barcode",
+						filters={"barcode": item_barcode.barcode, "parent": ["!=", self.name]},
+						pluck="parent",
+						limit=1,  # only the first conflicting item is reported
 					)
 					if duplicate:
 						frappe.throw(
 							_("Barcode {0} already used in Item {1}").format(
-								item_barcode.barcode, duplicate[0][0]
+								item_barcode.barcode, duplicate[0]
 							)
 						)
 
@@ -602,12 +604,8 @@ class Item(Document):
 
 	def stock_ledger_created(self):
 		if not hasattr(self, "_stock_ledger_created"):
-			self._stock_ledger_created = len(
-				frappe.db.sql(
-					"""select name from `tabStock Ledger Entry`
-				where item_code = %s and is_cancelled = 0 limit 1""",
-					self.name,
-				)
+			self._stock_ledger_created = bool(
+				frappe.db.exists("Stock Ledger Entry", {"item_code": self.name, "is_cancelled": 0})
 			)
 		return self._stock_ledger_created
 
@@ -618,26 +616,18 @@ class Item(Document):
 		if self.is_new():
 			return
 
-		frappe.db.sql(
-			"""
-				UPDATE `tabItem Price`
-				SET
-					item_name=%(item_name)s,
-					item_description=%(item_description)s,
-					brand=%(brand)s
-				WHERE item_code=%(item_code)s
-			""",
-			dict(
-				item_name=self.item_name,
-				item_description=self.description,
-				brand=self.brand,
-				item_code=self.name,
-			),
-		)
+		item_price = frappe.qb.DocType("Item Price")
+		(
+			frappe.qb.update(item_price)
+			.set(item_price.item_name, self.item_name)
+			.set(item_price.item_description, self.description)
+			.set(item_price.brand, self.brand)
+			.where(item_price.item_code == self.name)
+		).run()
 
 	def on_trash(self):
-		frappe.db.sql("""delete from tabBin where item_code=%s""", self.name)
-		frappe.db.sql("delete from `tabItem Price` where item_code=%s", self.name)
+		frappe.db.delete("Bin", {"item_code": self.name})
+		frappe.db.delete("Item Price", {"item_code": self.name})
 		for variant_of in frappe.get_all("Item", filters={"variant_of": self.name}):
 			frappe.delete_doc("Item", variant_of.name)
 
@@ -669,15 +659,15 @@ class Item(Document):
 		frappe.db.delete("Bin", {"item_code": old_name})
 
 	def validate_duplicate_item_in_stock_reconciliation(self, old_name, new_name):
-		records = frappe.db.sql(
-			""" SELECT parent, COUNT(*) as records
-			FROM `tabStock Reconciliation Item`
-			WHERE item_code = %s and docstatus = 1
-			GROUP By item_code, warehouse, parent
-			HAVING records > 1
-		""",
-			new_name,
-			as_dict=1,
+		sri = frappe.qb.DocType("Stock Reconciliation Item")
+		records = (
+			frappe.qb.from_(sri)
+			.select(sri.parent, Count("*").as_("records"))
+			.where((sri.item_code == new_name) & (sri.docstatus == 1))
+			.groupby(sri.item_code, sri.warehouse, sri.parent)
+			# HAVING references the aggregate itself; postgres rejects the SELECT alias here
+			.having(Count("*") > 1)
+			.run(as_dict=1)
 		)
 
 		if not records:
@@ -757,32 +747,26 @@ class Item(Document):
 			return
 
 		if self.db_get("description") != self.description:
-			frappe.db.sql(
-				"""
-				update `tabBOM`
-				set description = %s
-				where item = %s and docstatus < 2
-			""",
-				(self.description, self.name),
-			)
+			bom = frappe.qb.DocType("BOM")
+			(
+				frappe.qb.update(bom)
+				.set(bom.description, self.description)
+				.where((bom.item == self.name) & (bom.docstatus < 2))
+			).run()
 
-			frappe.db.sql(
-				"""
-				update `tabBOM Item`
-				set description = %s
-				where item_code = %s and docstatus < 2
-			""",
-				(self.description, self.name),
-			)
+			bom_item = frappe.qb.DocType("BOM Item")
+			(
+				frappe.qb.update(bom_item)
+				.set(bom_item.description, self.description)
+				.where((bom_item.item_code == self.name) & (bom_item.docstatus < 2))
+			).run()
 
-			frappe.db.sql(
-				"""
-				update `tabBOM Explosion Item`
-				set description = %s
-				where item_code = %s and docstatus < 2
-			""",
-				(self.description, self.name),
-			)
+			bom_explosion_item = frappe.qb.DocType("BOM Explosion Item")
+			(
+				frappe.qb.update(bom_explosion_item)
+				.set(bom_explosion_item.description, self.description)
+				.where((bom_explosion_item.item_code == self.name) & (bom_explosion_item.docstatus < 2))
+			).run()
 
 	def validate_item_defaults(self):
 		companies = {row.company for row in self.item_defaults}
@@ -1380,14 +1364,17 @@ def check_stock_uom_with_bin(item, stock_uom):
 				).format(item)
 			)
 
-	bin_list = frappe.db.sql(
-		"""
-			select * from `tabBin` where item_code = %s
-				and (reserved_qty > 0 or ordered_qty > 0 or indented_qty > 0 or planned_qty > 0)
-				and stock_uom != %s
-			""",
-		(item, stock_uom),
-		as_dict=1,
+	bin_list = frappe.get_all(
+		"Bin",
+		filters={"item_code": item, "stock_uom": ["!=", stock_uom]},
+		or_filters=[
+			["reserved_qty", ">", 0],
+			["ordered_qty", ">", 0],
+			["indented_qty", ">", 0],
+			["planned_qty", ">", 0],
+		],
+		pluck="name",  # only used as an existence check below
+		limit=1,
 	)
 
 	if bin_list:
@@ -1398,7 +1385,8 @@ def check_stock_uom_with_bin(item, stock_uom):
 		)
 
 	# No SLE or documents against item. Bin UOM can be changed safely.
-	frappe.db.sql("""update `tabBin` set stock_uom=%s where item_code=%s""", (stock_uom, item))
+	bin_dt = frappe.qb.DocType("Bin")
+	frappe.qb.update(bin_dt).set(bin_dt.stock_uom, stock_uom).where(bin_dt.item_code == item).run()
 
 
 def get_item_defaults(item_code, company):
@@ -1468,19 +1456,16 @@ def get_uom_conv_factor(uom: str | None, stock_uom: str | None):
 	# 			 g -> mg = 1000
 	# 			 g -> kg = 0.001
 	# therefore	 kg -> mg = 1000  / 0.001 = 1,000,000
-	intermediate_match = frappe.db.sql(
-		"""
-			select (first.value / second.value) as value
-			from `tabUOM Conversion Factor` first
-			join `tabUOM Conversion Factor` second
-				on first.from_uom = second.from_uom
-			where
-				first.to_uom = %(to_uom)s
-				and second.to_uom = %(from_uom)s
-			limit 1
-			""",
-		{"to_uom": to_uom, "from_uom": from_uom},
-		as_dict=1,
+	first = frappe.qb.DocType("UOM Conversion Factor").as_("first")
+	second = frappe.qb.DocType("UOM Conversion Factor").as_("second")
+	intermediate_match = (
+		frappe.qb.from_(first)
+		.join(second)
+		.on(first.from_uom == second.from_uom)
+		.select((first.value / second.value).as_("value"))
+		.where((first.to_uom == to_uom) & (second.to_uom == from_uom))
+		.limit(1)
+		.run(as_dict=1)
 	)
 
 	if intermediate_match:

erpnext/stock/doctype/item/test_item.py

@@ -76,6 +76,18 @@ class TestItem(ERPNextTestSuite):
 		super().setUp()
 		frappe.flags.attribute_values = None
 
+	def test_merge_rename_runs_duplicate_stock_reco_check(self):
+		# after_rename(merge=True) calls validate_duplicate_item_in_stock_reconciliation, whose query
+		# uses HAVING Count(*) > 1. The raw version referenced the SELECT alias in HAVING, which
+		# Postgres rejects; this exercises the converted query on both engines.
+		make_item("_Test Item Merge Source", {"is_stock_item": 1, "is_purchase_item": 1})
+		make_item("_Test Item Merge Target", {"is_stock_item": 1, "is_purchase_item": 1})
+
+		frappe.rename_doc("Item", "_Test Item Merge Source", "_Test Item Merge Target", merge=True)
+
+		self.assertTrue(frappe.db.exists("Item", "_Test Item Merge Target"))
+		self.assertFalse(frappe.db.exists("Item", "_Test Item Merge Source"))
+
 	def get_item(self, idx):
 		item_code = self.globalTestRecords["Item"][idx].get("item_code")
 		if not frappe.db.exists("Item", item_code):