From 308c6ffb4fb09169ff7541e14057fb179b4c3aee Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Thu, 21 Dec 2023 21:39:20 +0530 Subject: [PATCH] perf: Drop unused/duplicate/sub-optimal indexes (backport #38884) (#38913) perf: Drop unused/duplicate/sub-optimal indexes (#38884) * ci: enable more checks * perf: Drop unused/duplicate indexes (cherry picked from commit 787333896c3710d16b1fa72432db0fe59c74dfa8) Co-authored-by: Ankush Menat --- .pre-commit-config.yaml | 6 +++- .../accounts/doctype/gl_entry/gl_entry.json | 8 ++---- .../purchase_invoice/purchase_invoice.py | 4 --- .../doctype/sales_invoice/sales_invoice.py | 4 --- .../purchase_order_item.json | 3 +- erpnext/patches.txt | 4 +-- erpnext/stock/doctype/bin/bin.json | 3 +- .../drop_unused_return_against_index.py | 28 +++++++++++++------ erpnext/tests/test_perf.py | 24 ++++++++++++++++ 9 files changed, 56 insertions(+), 28 deletions(-) create mode 100644 erpnext/tests/test_perf.py diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 30be903ae8..6ea121f298 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -5,7 +5,7 @@ fail_fast: false 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 @@ repos: 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 5063ec6076..de2a9db2c8 100644 --- a/erpnext/accounts/doctype/gl_entry/gl_entry.json +++ b/erpnext/accounts/doctype/gl_entry/gl_entry.json @@ -142,8 +142,7 @@ "label": "Against Voucher Type", "oldfieldname": "against_voucher_type", "oldfieldtype": "Data", - "options": "DocType", - "search_index": 1 + "options": "DocType" }, { "fieldname": "against_voucher", @@ -162,8 +161,7 @@ "label": "Voucher Type", "oldfieldname": "voucher_type", "oldfieldtype": "Select", - "options": "DocType", - "search_index": 1 + "options": "DocType" }, { "fieldname": "voucher_no", @@ -321,4 +319,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 7156fa2682..db33271bcc 100644 --- a/erpnext/accounts/doctype/purchase_invoice/purchase_invoice.py +++ b/erpnext/accounts/doctype/purchase_invoice/purchase_invoice.py @@ -1816,10 +1816,6 @@ def make_inter_company_sales_invoice(source_name, target_doc=None): 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 aa6daa7d42..6aba1faa84 100644 --- a/erpnext/accounts/doctype/sales_invoice/sales_invoice.py +++ b/erpnext/accounts/doctype/sales_invoice/sales_invoice.py @@ -2549,10 +2549,6 @@ def get_loyalty_programs(customer): 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 98c1b388c1..5a24cc2e92 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 f76220537b..4825e7648f 100644 --- a/erpnext/patches.txt +++ b/erpnext/patches.txt @@ -353,5 +353,5 @@ execute:frappe.db.set_single_value("Buying Settings", "project_update_frequency" 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 312470d50e..10d9511357 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 8fe4ffb58f..cc29e67fa7 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 0000000000..fc17b1dcbd --- /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""" + ) + )