perf: Drop unused/duplicate/sub-optimal indexes (#38884)
* ci: enable more checks
* perf: Drop unused/duplicate indexes
diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
index 30be903..6ea121f 100644
--- a/.pre-commit-config.yaml
+++ b/.pre-commit-config.yaml
@@ -5,7 +5,7 @@
repos:
- repo: https://github.com/pre-commit/pre-commit-hooks
- rev: v4.0.1
+ rev: v4.3.0
hooks:
- id: trailing-whitespace
files: "erpnext.*"
@@ -15,6 +15,10 @@
args: ['--branch', 'develop']
- id: check-merge-conflict
- id: check-ast
+ - id: check-json
+ - id: check-toml
+ - id: check-yaml
+ - id: debug-statements
- repo: https://github.com/pre-commit/mirrors-eslint
rev: v8.44.0
diff --git a/erpnext/accounts/doctype/gl_entry/gl_entry.json b/erpnext/accounts/doctype/gl_entry/gl_entry.json
index 16df40f..c4492be 100644
--- a/erpnext/accounts/doctype/gl_entry/gl_entry.json
+++ b/erpnext/accounts/doctype/gl_entry/gl_entry.json
@@ -158,8 +158,7 @@
"label": "Against Voucher Type",
"oldfieldname": "against_voucher_type",
"oldfieldtype": "Data",
- "options": "DocType",
- "search_index": 1
+ "options": "DocType"
},
{
"fieldname": "against_voucher",
@@ -178,8 +177,7 @@
"label": "Voucher Type",
"oldfieldname": "voucher_type",
"oldfieldtype": "Select",
- "options": "DocType",
- "search_index": 1
+ "options": "DocType"
},
{
"fieldname": "voucher_no",
@@ -337,4 +335,4 @@
"sort_field": "modified",
"sort_order": "DESC",
"states": []
-}
\ No newline at end of file
+}
diff --git a/erpnext/accounts/doctype/purchase_invoice/purchase_invoice.py b/erpnext/accounts/doctype/purchase_invoice/purchase_invoice.py
index 931b48d..f40824d 100644
--- a/erpnext/accounts/doctype/purchase_invoice/purchase_invoice.py
+++ b/erpnext/accounts/doctype/purchase_invoice/purchase_invoice.py
@@ -1858,10 +1858,6 @@
return make_inter_company_transaction("Purchase Invoice", source_name, target_doc)
-def on_doctype_update():
- frappe.db.add_index("Purchase Invoice", ["supplier", "is_return", "return_against"])
-
-
@frappe.whitelist()
def make_purchase_receipt(source_name, target_doc=None):
def update_item(obj, target, source_parent):
diff --git a/erpnext/accounts/doctype/sales_invoice/sales_invoice.py b/erpnext/accounts/doctype/sales_invoice/sales_invoice.py
index c0228e6..2cddb86 100644
--- a/erpnext/accounts/doctype/sales_invoice/sales_invoice.py
+++ b/erpnext/accounts/doctype/sales_invoice/sales_invoice.py
@@ -2575,10 +2575,6 @@
return lp_details
-def on_doctype_update():
- frappe.db.add_index("Sales Invoice", ["customer", "is_return", "return_against"])
-
-
@frappe.whitelist()
def create_invoice_discounting(source_name, target_doc=None):
invoice = frappe.get_doc("Sales Invoice", source_name)
diff --git a/erpnext/buying/doctype/purchase_order_item/purchase_order_item.json b/erpnext/buying/doctype/purchase_order_item/purchase_order_item.json
index 98c1b38..5a24cc2 100644
--- a/erpnext/buying/doctype/purchase_order_item/purchase_order_item.json
+++ b/erpnext/buying/doctype/purchase_order_item/purchase_order_item.json
@@ -123,8 +123,7 @@
"oldfieldname": "item_code",
"oldfieldtype": "Link",
"options": "Item",
- "reqd": 1,
- "search_index": 1
+ "reqd": 1
},
{
"fieldname": "supplier_part_no",
diff --git a/erpnext/patches.txt b/erpnext/patches.txt
index 952875c..7ade21d 100644
--- a/erpnext/patches.txt
+++ b/erpnext/patches.txt
@@ -355,5 +355,5 @@
erpnext.patches.v14_0.update_total_asset_cost_field
# below migration patch should always run last
erpnext.patches.v14_0.migrate_gl_to_payment_ledger
-erpnext.stock.doctype.delivery_note.patches.drop_unused_return_against_index
-erpnext.patches.v14_0.set_maintain_stock_for_bom_item
\ No newline at end of file
+erpnext.stock.doctype.delivery_note.patches.drop_unused_return_against_index # 2023-12-20
+erpnext.patches.v14_0.set_maintain_stock_for_bom_item
diff --git a/erpnext/stock/doctype/bin/bin.json b/erpnext/stock/doctype/bin/bin.json
index 312470d..10d9511 100644
--- a/erpnext/stock/doctype/bin/bin.json
+++ b/erpnext/stock/doctype/bin/bin.json
@@ -52,8 +52,7 @@
"oldfieldtype": "Link",
"options": "Item",
"read_only": 1,
- "reqd": 1,
- "search_index": 1
+ "reqd": 1
},
{
"default": "0.00",
diff --git a/erpnext/stock/doctype/delivery_note/patches/drop_unused_return_against_index.py b/erpnext/stock/doctype/delivery_note/patches/drop_unused_return_against_index.py
index 8fe4ffb..cc29e67 100644
--- a/erpnext/stock/doctype/delivery_note/patches/drop_unused_return_against_index.py
+++ b/erpnext/stock/doctype/delivery_note/patches/drop_unused_return_against_index.py
@@ -1,15 +1,27 @@
+import click
import frappe
+UNUSED_INDEXES = [
+ ("Delivery Note", ["customer", "is_return", "return_against"]),
+ ("Sales Invoice", ["customer", "is_return", "return_against"]),
+ ("Purchase Invoice", ["supplier", "is_return", "return_against"]),
+ ("Purchase Receipt", ["supplier", "is_return", "return_against"]),
+]
+
def execute():
- """Drop unused return_against index"""
+ for doctype, index_fields in UNUSED_INDEXES:
+ table = f"tab{doctype}"
+ index_name = frappe.db.get_index_name(index_fields)
+ drop_index_if_exists(table, index_name)
+
+
+def drop_index_if_exists(table: str, index: str):
+ if not frappe.db.has_index(table, index):
+ return
try:
- frappe.db.sql_ddl(
- "ALTER TABLE `tabDelivery Note` DROP INDEX `customer_is_return_return_against_index`"
- )
- frappe.db.sql_ddl(
- "ALTER TABLE `tabPurchase Receipt` DROP INDEX `supplier_is_return_return_against_index`"
- )
+ frappe.db.sql_ddl(f"ALTER TABLE `{table}` DROP INDEX `{index}`")
+ click.echo(f"✓ dropped {index} index from {table}")
except Exception:
- frappe.log_error("Failed to drop unused index")
+ frappe.log_error("Failed to drop index")
diff --git a/erpnext/tests/test_perf.py b/erpnext/tests/test_perf.py
new file mode 100644
index 0000000..fc17b1d
--- /dev/null
+++ b/erpnext/tests/test_perf.py
@@ -0,0 +1,24 @@
+import frappe
+from frappe.tests.utils import FrappeTestCase
+
+INDEXED_FIELDS = {
+ "Bin": ["item_code"],
+ "GL Entry": ["voucher_type", "against_voucher_type"],
+ "Purchase Order Item": ["item_code"],
+ "Stock Ledger Entry": ["warehouse"],
+}
+
+
+class TestPerformance(FrappeTestCase):
+ def test_ensure_indexes(self):
+ # These fields are not explicitly indexed BUT they are prefix in some
+ # other composite index. If those are removed this test should be
+ # updated accordingly.
+ for doctype, fields in INDEXED_FIELDS.items():
+ for field in fields:
+ self.assertTrue(
+ frappe.db.sql(
+ f"""SHOW INDEX FROM `tab{doctype}`
+ WHERE Column_name = "{field}" AND Seq_in_index = 1"""
+ )
+ )