Skip to content

Commit

Permalink
feat: Add identified index from UI
Browse files Browse the repository at this point in the history
  • Loading branch information
ankush committed May 14, 2024
1 parent 4e251e9 commit b169f87
Show file tree
Hide file tree
Showing 6 changed files with 80 additions and 121 deletions.
98 changes: 0 additions & 98 deletions frappe/commands/site.py
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down Expand Up @@ -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,
Expand Down
20 changes: 20 additions & 0 deletions frappe/core/doctype/recorder/recorder.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
40 changes: 37 additions & 3 deletions frappe/core/doctype/recorder/recorder.py
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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):
Expand Down
21 changes: 20 additions & 1 deletion frappe/core/doctype/recorder/test_recorder.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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")
Original file line number Diff line number Diff line change
@@ -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": [
{
Expand All @@ -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",
Expand Down
9 changes: 0 additions & 9 deletions frappe/tests/test_commands.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit b169f87

Please sign in to comment.