diff --git a/frappe/core/doctype/doctype/doctype.py b/frappe/core/doctype/doctype/doctype.py index d428373c05..d4646b3c03 100644 --- a/frappe/core/doctype/doctype/doctype.py +++ b/frappe/core/doctype/doctype/doctype.py @@ -539,7 +539,7 @@ class DocType(Document): if self.autoname == "autoincrement": name_type = "bigint" - frappe.db.create_sequence(self.name, check_not_exists=True, cache=frappe.db.SEQUENCE_CACHE) + frappe.db.create_sequence(self.name, check_not_exists=True) change_name_column_type(self.name, name_type) diff --git a/frappe/database/database.py b/frappe/database/database.py index 3c42b65e9b..ba5e6c891e 100644 --- a/frappe/database/database.py +++ b/frappe/database/database.py @@ -59,23 +59,6 @@ class Database: CHILD_TABLE_COLUMNS = ("parent", "parenttype", "parentfield") MAX_WRITES_PER_TRANSACTION = 200_000 - # NOTE: - # FOR MARIADB - using no cache - as during backup, if the sequence was used in anyform, - # it drops the cache and uses the next non cached value in setval query and - # puts that in the backup file, which will start the counter - # from that value when inserting any new record in the doctype. - # By default the cache is 1000 which will mess up the sequence when - # using the system after a restore. - # - # Another case could be if the cached values expire then also there is a chance of - # the cache being skipped. - # - # FOR POSTGRES - The sequence cache for postgres is per connection. - # Since we're opening and closing connections for every request this results in skipping the cache - # to the next non-cached value hence not using cache in postgres. - # ref: https://stackoverflow.com/questions/21356375/postgres-9-0-4-sequence-skipping-numbers - SEQUENCE_CACHE = 0 - class InvalidColumnName(frappe.ValidationError): pass diff --git a/frappe/database/mariadb/schema.py b/frappe/database/mariadb/schema.py index 63cdca2736..209535b9fd 100644 --- a/frappe/database/mariadb/schema.py +++ b/frappe/database/mariadb/schema.py @@ -3,7 +3,6 @@ from pymysql.constants.ER import DUP_ENTRY import frappe from frappe import _ from frappe.database.schema import DBTable -from frappe.model import log_types class MariaDBTable(DBTable): @@ -36,11 +35,8 @@ class MariaDBTable(DBTable): additional_definitions.append("index modified(modified)") # creating sequence(s) - if ( - not self.meta.issingle and self.meta.autoname == "autoincrement" - ) or self.doctype in log_types: - - frappe.db.create_sequence(self.doctype, check_not_exists=True, cache=frappe.db.SEQUENCE_CACHE) + if not self.meta.issingle and self.meta.autoname == "autoincrement": + frappe.db.create_sequence(self.doctype, check_not_exists=True) # NOTE: not used nextval func as default as the ability to restore # database with sequences has bugs in mariadb and gives a scary error. diff --git a/frappe/database/postgres/schema.py b/frappe/database/postgres/schema.py index b0a878fa8a..073fafd8c7 100644 --- a/frappe/database/postgres/schema.py +++ b/frappe/database/postgres/schema.py @@ -1,7 +1,6 @@ import frappe from frappe import _ from frappe.database.schema import DBTable, get_definition -from frappe.model import log_types from frappe.utils import cint, flt @@ -30,11 +29,8 @@ class PostgresTable(DBTable): ) # creating sequence(s) - if ( - not self.meta.issingle and self.meta.autoname == "autoincrement" - ) or self.doctype in log_types: - - frappe.db.create_sequence(self.doctype, check_not_exists=True, cache=frappe.db.SEQUENCE_CACHE) + if not self.meta.issingle and self.meta.autoname == "autoincrement": + frappe.db.create_sequence(self.doctype, check_not_exists=True) name_column = "name bigint primary key" # TODO: set docstatus length diff --git a/frappe/database/sequence.py b/frappe/database/sequence.py index 54362a5895..1b22dfdd6a 100644 --- a/frappe/database/sequence.py +++ b/frappe/database/sequence.py @@ -1,5 +1,22 @@ from frappe import db, scrub +# NOTE: +# FOR MARIADB - using no cache - as during backup, if the sequence was used in anyform, +# it drops the cache and uses the next non cached value in setval query and +# puts that in the backup file, which will start the counter +# from that value when inserting any new record in the doctype. +# By default the cache is 1000 which will mess up the sequence when +# using the system after a restore. +# +# Another case could be if the cached values expire then also there is a chance of +# the cache being skipped. +# +# FOR POSTGRES - The sequence cache for postgres is per connection. +# Since we're opening and closing connections for every request this results in skipping the cache +# to the next non-cached value hence not using cache in postgres. +# ref: https://stackoverflow.com/questions/21356375/postgres-9-0-4-sequence-skipping-numbers +SEQUENCE_CACHE = 0 + def create_sequence( doctype_name: str, @@ -8,7 +25,7 @@ def create_sequence( temporary: bool = False, check_not_exists: bool = False, cycle: bool = False, - cache: int = 0, + cache: int = SEQUENCE_CACHE, start_value: int = 0, increment_by: int = 0, min_value: int = 0, diff --git a/frappe/model/__init__.py b/frappe/model/__init__.py index 32e46c83c2..9c0282a24d 100644 --- a/frappe/model/__init__.py +++ b/frappe/model/__init__.py @@ -115,8 +115,6 @@ core_doctypes_list = ( "Client Script", ) -# NOTE: this is being used for dynamic autoincrement in new sites, -# removing any of these will require patches. log_types = ( "Version", "Error Log", diff --git a/frappe/model/naming.py b/frappe/model/naming.py index c90b7f517b..7a06e4d248 100644 --- a/frappe/model/naming.py +++ b/frappe/model/naming.py @@ -3,7 +3,6 @@ import datetime import re -from collections import defaultdict from collections.abc import Callable from typing import TYPE_CHECKING, Optional @@ -12,7 +11,6 @@ from frappe import _ from frappe.model import log_types from frappe.query_builder import DocType from frappe.utils import cint, cstr, now_datetime -from frappe.utils.caching import redis_cache if TYPE_CHECKING: from frappe.model.document import Document @@ -177,28 +175,15 @@ def set_new_name(doc): def is_autoincremented(doctype: str, meta: Optional["Meta"] = None) -> bool: """Checks if the doctype has autoincrement autoname set""" - if doctype in log_types: - return _implicitly_auto_incremented(doctype) - else: - if not meta: - meta = frappe.get_meta(doctype) + if not meta: + meta = frappe.get_meta(doctype) - if not getattr(meta, "issingle", False) and meta.autoname == "autoincrement": - return True + if not getattr(meta, "issingle", False) and meta.autoname == "autoincrement": + return True return False -@redis_cache -def _implicitly_auto_incremented(doctype) -> bool: - query = f"""select data_type FROM information_schema.columns where column_name = 'name' and table_name = 'tab{doctype}'""" - values = () - if frappe.db.db_type == "mariadb": - query += " and table_schema = %s" - values = (frappe.db.db_name,) - return frappe.db.sql(query, values)[0][0] == "bigint" - - def set_name_from_naming_options(autoname, doc): """ Get a name based on the autoname field option diff --git a/frappe/patches.txt b/frappe/patches.txt index 120a2b07d8..d0dae01e67 100644 --- a/frappe/patches.txt +++ b/frappe/patches.txt @@ -1,4 +1,5 @@ [pre_model_sync] +frappe.patches.v15_0.remove_implicit_primary_key frappe.patches.v12_0.remove_deprecated_fields_from_doctype #3 execute:frappe.utils.global_search.setup_global_search_table() execute:frappe.reload_doc('core', 'doctype', 'doctype_action', force=True) #2019-09-23 diff --git a/frappe/patches/v15_0/remove_implicit_primary_key.py b/frappe/patches/v15_0/remove_implicit_primary_key.py new file mode 100644 index 0000000000..1503c752bb --- /dev/null +++ b/frappe/patches/v15_0/remove_implicit_primary_key.py @@ -0,0 +1,46 @@ +import frappe +from frappe.model.naming import is_autoincremented + +possible_log_types = ( + "Version", + "Error Log", + "Scheduled Job Log", + "Event Sync Log", + "Event Update Log", + "Access Log", + "View Log", + "Activity Log", + "Energy Point Log", + "Notification Log", + "Email Queue", + "DocShare", + "Document Follow", + "Console Log", +) + + +def execute(): + """Few doctypes had int PKs even though schema didn't mention them, this requires detecting it + at runtime which is prone to bugs and adds unnecessary overhead. + + This patch converts them back to varchar. + """ + for doctype in possible_log_types: + if _is_implicit_int_pk(doctype) and not is_autoincremented(doctype): + frappe.db.change_column_type( + doctype, + "name", + type=f"varchar({frappe.db.VARCHAR_LEN})", + nullable=True, + ) + + +def _is_implicit_int_pk(doctype: str) -> bool: + query = f"""select data_type FROM information_schema.columns where column_name = 'name' and table_name = 'tab{doctype}'""" + values = () + if frappe.db.db_type == "mariadb": + query += " and table_schema = %s" + values = (frappe.db.db_name,) + + col_type = frappe.db.sql(query, values) + return bool(col_type and col_type[0][0] == "bigint")