From b169f8780ab543a21ee091c1d105c10d2c22f076 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Tue, 14 May 2024 17:58:31 +0530 Subject: [PATCH] feat: Add identified index from UI --- frappe/commands/site.py | 98 ------------------- frappe/core/doctype/recorder/recorder.js | 20 ++++ frappe/core/doctype/recorder/recorder.py | 40 +++++++- frappe/core/doctype/recorder/test_recorder.py | 21 +++- .../recorder_suggested_index.json | 13 +-- frappe/tests/test_commands.py | 9 -- 6 files changed, 80 insertions(+), 121 deletions(-) diff --git a/frappe/commands/site.py b/frappe/commands/site.py index abd5d9464e5..641f70555e5 100644 --- a/frappe/commands/site.py +++ b/frappe/commands/site.py @@ -587,103 +587,6 @@ def add_db_index(context, doctype, column): raise SiteNotSpecifiedError -@click.command("describe-database-table") -@click.option("--doctype", help="DocType to describe") -@click.option( - "--column", - multiple=True, - help="Explicitly fetch accurate cardinality from table data. This can be quite slow on large tables.", -) -@pass_context -def describe_database_table(context, doctype, column): - """Describes various statistics about the table. - - This is useful to build integration like - This includes: - 1. Schema - 2. Indexes - 3. stats - total count of records - 4. if column is specified then extra stats are generated for column: - Distinct values count in column - """ - import json - - for site in context.sites: - frappe.init(site=site) - frappe.connect() - try: - data = _extract_table_stats(doctype, column) - # NOTE: Do not print anything else in this to avoid clobbering the output. - print(json.dumps(data, indent=2)) - finally: - frappe.destroy() - - if not context.sites: - raise SiteNotSpecifiedError - - -def _extract_table_stats(doctype: str, columns: list[str]) -> dict: - from frappe.utils import cint, cstr, get_table_name - - def sql_bool(val): - return cstr(val).lower() in ("yes", "1", "true") - - table = get_table_name(doctype, wrap_in_backticks=True) - - schema = [] - for field in frappe.db.sql(f"describe {table}", as_dict=True): - schema.append( - { - "column": field["Field"], - "type": field["Type"], - "is_nullable": sql_bool(field["Null"]), - "default": field["Default"], - } - ) - - def update_cardinality(column, value): - for col in schema: - if col["column"] == column: - col["cardinality"] = value - break - - indexes = [] - for idx in frappe.db.sql(f"show index from {table}", as_dict=True): - indexes.append( - { - "unique": not sql_bool(idx["Non_unique"]), - "cardinality": idx["Cardinality"], - "name": idx["Key_name"], - "sequence": idx["Seq_in_index"], - "nullable": sql_bool(idx["Null"]), - "column": idx["Column_name"], - "type": idx["Index_type"], - } - ) - if idx["Seq_in_index"] == 1: - update_cardinality(idx["Column_name"], idx["Cardinality"]) - - total_rows = cint( - frappe.db.sql( - f"""select table_rows - from information_schema.tables - where table_name = 'tab{doctype}'""" - )[0][0] - ) - - # fetch accurate cardinality for columns by query. WARN: This can take a lot of time. - for column in columns: - cardinality = frappe.db.sql(f"select count(distinct {column}) from {table}")[0][0] - update_cardinality(column, cardinality) - - return { - "table_name": table.strip("`"), - "total_rows": total_rows, - "schema": schema, - "indexes": indexes, - } - - @click.command("add-system-manager") @click.argument("email") @click.option("--first-name") @@ -1602,7 +1505,6 @@ def add_new_user( add_system_manager, add_user_for_sites, add_db_index, - describe_database_table, backup, drop_site, install_app, diff --git a/frappe/core/doctype/recorder/recorder.js b/frappe/core/doctype/recorder/recorder.js index e58f55c442b..735511f5b16 100644 --- a/frappe/core/doctype/recorder/recorder.js +++ b/frappe/core/doctype/recorder/recorder.js @@ -22,6 +22,26 @@ frappe.ui.form.on("Recorder", { frm.reload_doc(); setTimeout(() => frm.scroll_to_field("suggested_indexes"), 1500); }); + + let index_grid = frm.fields_dict.suggested_indexes.grid; + index_grid.wrapper.find(".grid-footer").toggle(true); + index_grid.toggle_checkboxes(true); + index_grid.df.cannot_delete_rows = true; + index_grid.add_custom_button(__("Add Indexes"), function () { + let indexes_to_add = index_grid.get_selected_children().map((row) => { + return { + column: row.column, + table: row.table, + }; + }); + if (!indexes_to_add.length) { + frappe.toast(__("You need to select indexes you want to add first.")); + return; + } + frappe.xcall("frappe.core.doctype.recorder.recorder.add_indexes", { + indexes: indexes_to_add, + }); + }); }, setup_sort: function (frm) { diff --git a/frappe/core/doctype/recorder/recorder.py b/frappe/core/doctype/recorder/recorder.py index cc894365787..fa8b1d14a7b 100644 --- a/frappe/core/doctype/recorder/recorder.py +++ b/frappe/core/doctype/recorder/recorder.py @@ -1,11 +1,13 @@ # Copyright (c) 2023, Frappe Technologies and contributors # For license information, please see license.txt +import json from collections import Counter, defaultdict import frappe from frappe import _ from frappe.core.doctype.recorder.db_optimizer import DBOptimizer, DBTable +from frappe.custom.doctype.property_setter.property_setter import make_property_setter from frappe.model.document import Document from frappe.recorder import RECORDER_REQUEST_HASH from frappe.recorder import get as get_recorder_data @@ -113,6 +115,34 @@ def serialize_request(request): return request +@frappe.whitelist() +def add_indexes(indexes): + frappe.only_for("Administrator") + indexes = json.loads(indexes) + + for index in indexes: + frappe.enqueue(_add_index, table=index["table"], column=index["column"]) + frappe.msgprint(_("Enqueued creation of indexes"), alert=True) + + +def _add_index(table, column): + doctype = get_doctype_name(table) + frappe.db.add_index(doctype, [column]) + make_property_setter( + doctype, + column, + property="search_index", + value="1", + property_type="Check", + for_doctype=False, # Applied on docfield + ) + frappe.msgprint( + _("Index created successfully on column {0} of doctype {1}").format(column, doctype), + alert=True, + realtime=True, + ) + + @frappe.whitelist() def optimize(recorder_id: str): frappe.only_for("Administrator") @@ -152,14 +182,18 @@ def _optimize(recorder_id): ] if not suggested_indexes: - frappe.msgprint(_("No optimization suggestions."), realtime=True) + frappe.msgprint( + _("No automatic optimization suggestions available."), + title=_("No Suggestions"), + realtime=True, + ) return - frappe.msgprint(_("Query analysis complete. Check suggested indexes."), realtime=True, alert=True) data = frappe.cache.hget(RECORDER_REQUEST_HASH, record.name) data["suggested_indexes"] = [{"table": idx[0][0], "column": idx[0][1]} for idx in suggested_indexes] frappe.cache.hset(RECORDER_REQUEST_HASH, record.name, data) - frappe.publish_realtime("recorder-analysis-complete") + frappe.publish_realtime("recorder-analysis-complete", user=frappe.session.user) + frappe.msgprint(_("Query analysis complete. Check suggested indexes."), realtime=True, alert=True) def _optimize_query(query): diff --git a/frappe/core/doctype/recorder/test_recorder.py b/frappe/core/doctype/recorder/test_recorder.py index aad47cadf5b..3a35925c753 100644 --- a/frappe/core/doctype/recorder/test_recorder.py +++ b/frappe/core/doctype/recorder/test_recorder.py @@ -5,8 +5,10 @@ import frappe import frappe.recorder -from frappe.core.doctype.recorder.recorder import serialize_request +from frappe.core.doctype.recorder.recorder import _optimize_query, serialize_request +from frappe.query_builder.utils import db_type_is from frappe.recorder import get as get_recorder_data +from frappe.tests.test_query_builder import run_only_if from frappe.tests.utils import FrappeTestCase from frappe.utils import set_request @@ -75,3 +77,20 @@ def test_recorder_serialization(self): requests = frappe.get_all("Recorder") request_doc = get_recorder_data(requests[0].name) self.assertIsInstance(serialize_request(request_doc), dict) + + +class TestQueryOptimization(FrappeTestCase): + @run_only_if(db_type_is.MARIADB) + def test_query_optimizer(self): + suggested_index = _optimize_query( + """select name from + `tabUser` u + join `tabHas Role` r + on r.parent = u.name + where email='xyz' + and creation > '2023' + and bio like '%xyz%' + """ + ) + self.assertEqual(suggested_index.table, "tabUser") + self.assertEqual(suggested_index.column, "email") diff --git a/frappe/core/doctype/recorder_suggested_index/recorder_suggested_index.json b/frappe/core/doctype/recorder_suggested_index/recorder_suggested_index.json index 1a265f347be..eb6f4b1e4da 100644 --- a/frappe/core/doctype/recorder_suggested_index/recorder_suggested_index.json +++ b/frappe/core/doctype/recorder_suggested_index/recorder_suggested_index.json @@ -1,14 +1,13 @@ { "actions": [], "allow_rename": 1, - "creation": "2024-05-14 15:03:46.138438", + "creation": "2024-05-14 16:23:33.466465", "doctype": "DocType", "editable_grid": 1, "engine": "InnoDB", "field_order": [ "table", - "column", - "add_index" + "column" ], "fields": [ { @@ -22,19 +21,13 @@ "fieldtype": "Data", "in_list_view": 1, "label": "Column" - }, - { - "columns": 2, - "fieldname": "add_index", - "fieldtype": "Button", - "label": "Add Index" } ], "index_web_pages_for_search": 1, "is_virtual": 1, "istable": 1, "links": [], - "modified": "2024-05-14 15:18:51.371808", + "modified": "2024-05-14 17:43:57.231051", "modified_by": "Administrator", "module": "Core", "name": "Recorder Suggested Index", diff --git a/frappe/tests/test_commands.py b/frappe/tests/test_commands.py index 68e5ed30fc9..6b07ec41e8d 100644 --- a/frappe/tests/test_commands.py +++ b/frappe/tests/test_commands.py @@ -925,15 +925,6 @@ def test_db_add_index(self): meta = frappe.get_meta("User", cached=False) self.assertTrue(meta.get_field(field).search_index) - @run_only_if(db_type_is.MARIADB) - def test_describe_table(self): - self.execute("bench --site {site} describe-database-table --doctype User", {}) - self.assertIn("user_type", self.stdout) - - # Ensure that output is machine parseable - stats = json.loads(self.stdout) - self.assertIn("total_rows", stats) - class TestSchedulerUtils(BaseTestCommands): # Retry just in case there are stuck queued jobs