diff --git a/frappe/core/doctype/doctype/doctype.py b/frappe/core/doctype/doctype/doctype.py index 6bd7f2306f..4e21f3bcb4 100644 --- a/frappe/core/doctype/doctype/doctype.py +++ b/frappe/core/doctype/doctype/doctype.py @@ -100,6 +100,7 @@ class DocType(Document): self.set_default_in_list_view() self.set_default_translatable() validate_series(self) + self.set("can_change_name_type", validate_autoincrement_autoname(self)) self.validate_document_type() validate_fields(self) @@ -124,12 +125,6 @@ class DocType(Document): if self.default_print_format and not self.custom: frappe.throw(_("Standard DocType cannot have default print format, use Customize Form")) - if check_if_can_change_name_type(self): - change_name_column_type( - self.name, - "bigint" if self.autoname == "autoincrement" else f"varchar({frappe.db.VARCHAR_LEN})", - ) - def validate_field_name_conflicts(self): """Check if field names dont conflict with controller properties and methods""" core_doctypes = [ @@ -374,6 +369,10 @@ class DocType(Document): def on_update(self): """Update database schema, make controller templates if `custom` is not set and clear cache.""" + + if self.get("can_change_name_type"): + self.setup_autoincrement_and_sequence() + try: frappe.db.updatedb(self.name, Meta(self)) except Exception as e: @@ -413,6 +412,17 @@ class DocType(Document): clear_linked_doctype_cache() + def setup_autoincrement_and_sequence(self): + """Changes name type and makes sequence on change (if required)""" + + name_type = f"varchar({frappe.db.VARCHAR_LEN})" + + if self.autoname == "autoincrement": + name_type = "bigint" + frappe.db.create_sequence(self.name, check_not_exists=True, cache=frappe.db.SEQUENCE_CACHE) + + change_name_column_type(self.name, name_type) + def sync_global_search(self): """If global search settings are changed, rebuild search properties for this table""" global_search_fields_before_update = [ @@ -903,26 +913,25 @@ def validate_series(dt, autoname=None, name=None): frappe.throw(_("Series {0} already used in {1}").format(prefix, used_in[0][0])) -def check_if_can_change_name_type(dt: DocType, raise_err: bool = True) -> bool: - def get_autoname_before_save(doctype: str, to_be_customized_dt: str) -> str: - if doctype == "Customize Form": - property_value = frappe.db.get_value( - "Property Setter", {"doc_type": to_be_customized_dt, "property": "autoname"}, "value" - ) +def validate_autoincrement_autoname(dt: DocType) -> bool: + """Checks if can doctype can change to/from autoincrement autoname""" + def get_autoname_before_save(dt: DocType) -> str: + if dt.name == "Customize Form": + property_value = frappe.db.get_value( + "Property Setter", {"doc_type": dt.doc_type, "property": "autoname"}, "value" + ) # initially no property setter is set, # hence getting autoname value from the doctype itself if not property_value: - return frappe.db.get_value("DocType", to_be_customized_dt, "autoname") or "" + return frappe.db.get_value("DocType", dt.doc_type, "autoname") or "" return property_value return getattr(dt.get_doc_before_save(), "autoname", "") - doctype_name = dt.doc_type if dt.doctype == "Customize Form" else dt.name - if not dt.is_new(): - autoname_before_save = get_autoname_before_save(dt.doctype, doctype_name) + autoname_before_save = get_autoname_before_save(dt) is_autoname_autoincrement = dt.autoname == "autoincrement" if ( @@ -930,23 +939,35 @@ def check_if_can_change_name_type(dt: DocType, raise_err: bool = True) -> bool: and autoname_before_save != "autoincrement" or (not is_autoname_autoincrement and autoname_before_save == "autoincrement") ): - if not frappe.get_all(doctype_name, limit=1): + + if frappe.get_meta(dt.name).issingle: + if dt.name == "Customize Form": + frappe.throw(_("Cannot change to/from autoincrement autoname in Customize Form")) + + return False + + if not frappe.get_all(dt.name, limit=1): # allow changing the column type if there is no data return True - if raise_err: - frappe.throw( - _("Can only change to/from Autoincrement naming rule when there is no data in the doctype") - ) + frappe.throw( + _("Can only change to/from Autoincrement naming rule when there is no data in the doctype") + ) return False def change_name_column_type(doctype_name: str, type: str) -> None: - return frappe.db.change_column_type( - doctype_name, "name", type, True if frappe.db.db_type == "mariadb" else False + """Changes name column type""" + + args = ( + (doctype_name, "name", type, False, True) + if (frappe.db.db_type == "postgres") + else (doctype_name, "name", type, True) ) + frappe.db.change_column_type(*args) + def validate_links_table_fieldnames(meta): """Validate fieldnames in Links table""" diff --git a/frappe/core/doctype/doctype/test_doctype.py b/frappe/core/doctype/doctype/test_doctype.py index 59475a95a7..11f5ef8a69 100644 --- a/frappe/core/doctype/doctype/test_doctype.py +++ b/frappe/core/doctype/doctype/test_doctype.py @@ -38,6 +38,52 @@ class TestDocType(unittest.TestCase): doc = new_doctype(name).insert() doc.delete() + def test_making_sequence_on_change(self): + frappe.delete_doc_if_exists("DocType", self._testMethodName) + dt = new_doctype(self._testMethodName).insert(ignore_permissions=True) + autoname = dt.autoname + + # change autoname + dt.autoname = "autoincrement" + dt.save() + + # check if name type has been changed + self.assertEqual( + frappe.db.sql( + f"""select data_type FROM information_schema.columns + where column_name = 'name' and table_name = 'tab{self._testMethodName}'""" + )[0][0], + "bigint", + ) + + if frappe.db.db_type == "mariadb": + table_name = "information_schema.tables" + conditions = f"table_type = 'sequence' and table_name = '{self._testMethodName}_id_seq'" + else: + table_name = "information_schema.sequences" + conditions = f"sequence_name = '{self._testMethodName}_id_seq'" + + # check if sequence table is created + self.assertTrue( + frappe.db.sql( + f"""select * from {table_name} + where {conditions}""" + ) + ) + + # change the autoname/naming rule back to original + dt.autoname = autoname + dt.save() + + # check if name type has changed + self.assertEqual( + frappe.db.sql( + f"""select data_type FROM information_schema.columns + where column_name = 'name' and table_name = 'tab{self._testMethodName}'""" + )[0][0], + "varchar" if frappe.db.db_type == "mariadb" else "character varying", + ) + def test_doctype_unique_constraint_dropped(self): if frappe.db.exists("DocType", "With_Unique"): frappe.delete_doc("DocType", "With_Unique") diff --git a/frappe/custom/doctype/customize_form/customize_form.json b/frappe/custom/doctype/customize_form/customize_form.json index a0bc994c45..4c40b80f53 100644 --- a/frappe/custom/doctype/customize_form/customize_form.json +++ b/frappe/custom/doctype/customize_form/customize_form.json @@ -287,7 +287,7 @@ "label": "Naming" }, { - "description": "Naming Options:\n
  1. field:[fieldname] - By Field
    2. \n
  2. autoincrement - Uses Databases' Auto Increment feature
  3. naming_series: - By Naming Series (field called naming_series must be present
  4. Prompt - Prompt user for a name
  5. [series] - Series by prefix (separated by a dot); for example PRE.#####
    6. \n
  6. format:EXAMPLE-{MM}morewords{fieldname1}-{fieldname2}-{#####} - Replace all braced words (fieldnames, date words (DD, MM, YY), series) with their value. Outside braces, any characters can be used.
", + "description": "Naming Options:\n
  1. field:[fieldname] - By Field
  2. naming_series: - By Naming Series (field called naming_series must be present)
  3. Prompt - Prompt user for a name
  4. [series] - Series by prefix (separated by a dot); for example PRE.#####
    5. \n
  5. format:EXAMPLE-{MM}morewords{fieldname1}-{fieldname2}-{#####} - Replace all braced words (fieldnames, date words (DD, MM, YY), series) with their value. Outside braces, any characters can be used.
", "fieldname": "autoname", "fieldtype": "Data", "label": "Auto Name" @@ -319,7 +319,7 @@ "index_web_pages_for_search": 1, "issingle": 1, "links": [], - "modified": "2022-04-21 15:36:16.772277", + "modified": "2022-05-12 15:36:16.772277", "modified_by": "Administrator", "module": "Custom", "name": "Customize Form", diff --git a/frappe/custom/doctype/customize_form/customize_form.py b/frappe/custom/doctype/customize_form/customize_form.py index 262542fd4b..903d7fc27d 100644 --- a/frappe/custom/doctype/customize_form/customize_form.py +++ b/frappe/custom/doctype/customize_form/customize_form.py @@ -11,9 +11,8 @@ import frappe import frappe.translate from frappe import _ from frappe.core.doctype.doctype.doctype import ( - change_name_column_type, check_email_append_to, - check_if_can_change_name_type, + validate_autoincrement_autoname, validate_fields_for_doctype, validate_series, ) @@ -163,7 +162,7 @@ class CustomizeForm(Document): return validate_series(self, self.autoname, self.doc_type) - can_change_name_type = check_if_can_change_name_type(self) + validate_autoincrement_autoname(self) self.flags.update_db = False self.flags.rebuild_doctype_for_global_search = False self.set_property_setters() @@ -172,12 +171,6 @@ class CustomizeForm(Document): validate_fields_for_doctype(self.doc_type) check_email_append_to(self) - if can_change_name_type: - change_name_column_type( - self.doc_type, - "bigint" if self.autoname == "autoincrement" else f"varchar({frappe.db.VARCHAR_LEN})", - ) - if self.flags.update_db: frappe.db.updatedb(self.doc_type) diff --git a/frappe/custom/doctype/customize_form/test_customize_form.py b/frappe/custom/doctype/customize_form/test_customize_form.py index 5a1f629beb..b00f45f5d2 100644 --- a/frappe/custom/doctype/customize_form/test_customize_form.py +++ b/frappe/custom/doctype/customize_form/test_customize_form.py @@ -396,3 +396,10 @@ class TestCustomizeForm(unittest.TestCase): d.label = "" d.run_method("save_customization") self.assertEqual(d.label, "") + + def test_change_to_autoincrement_autoname(self): + d = self.get_customize_form("Event") + d.autoname = "autoincrement" + + with self.assertRaises(frappe.ValidationError): + d.run_method("save_customization") diff --git a/frappe/database/mariadb/database.py b/frappe/database/mariadb/database.py index 28d78471d2..7505ef3a7f 100644 --- a/frappe/database/mariadb/database.py +++ b/frappe/database/mariadb/database.py @@ -19,6 +19,15 @@ class MariaDBDatabase(Database): DataError = pymysql.err.DataError REGEX_CHARACTER = "regexp" + # NOTE: using a very small 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. + # issue link: https://jira.mariadb.org/browse/MDEV-21786 + SEQUENCE_CACHE = 50 + def setup_type_map(self): self.db_type = "mariadb" self.type_map = { diff --git a/frappe/database/mariadb/schema.py b/frappe/database/mariadb/schema.py index 784fa23c13..f402b4ec74 100644 --- a/frappe/database/mariadb/schema.py +++ b/frappe/database/mariadb/schema.py @@ -40,14 +40,7 @@ class MariaDBTable(DBTable): not self.meta.issingle and self.meta.autoname == "autoincrement" ) or self.doctype in log_types: - # NOTE: using a very small cache - as during backup, if the sequence was used in anyform, - # it drops the cache and uses the next non cached value in setval func 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. - # issue link: https://jira.mariadb.org/browse/MDEV-21786 - frappe.db.create_sequence(self.doctype, check_not_exists=True, cache=50) + frappe.db.create_sequence(self.doctype, check_not_exists=True, cache=frappe.db.SEQUENCE_CACHE) # 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/database.py b/frappe/database/postgres/database.py index d69e0bea94..8bd4113823 100644 --- a/frappe/database/postgres/database.py +++ b/frappe/database/postgres/database.py @@ -31,6 +31,12 @@ class PostgresDatabase(Database): InterfaceError = psycopg2.InterfaceError REGEX_CHARACTER = "~" + # NOTE; The sequence cache for postgres is per connection. + # Since we're opening and closing connections for every transaction 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 setup_type_map(self): self.db_type = "postgres" self.type_map = { @@ -209,18 +215,19 @@ class PostgresDatabase(Database): ) def change_column_type( - self, doctype: str, column: str, type: str, nullable: bool = False + self, doctype: str, column: str, type: str, nullable: bool = False, use_cast: bool = False ) -> Union[List, Tuple]: table_name = get_table_name(doctype) null_constraint = "SET NOT NULL" if not nullable else "DROP NOT NULL" + using_cast = f'using "{column}"::{type}' if use_cast else "" # postgres allows ddl in transactions but since we've currently made # things same as mariadb (raising exception on ddl commands if the transaction has any writes), # hence using sql_ddl here for committing and then moving forward. return self.sql_ddl( f"""ALTER TABLE "{table_name}" - ALTER COLUMN "{column}" TYPE {type}, - ALTER COLUMN "{column}" {null_constraint}""" + ALTER COLUMN "{column}" TYPE {type} {using_cast}, + ALTER COLUMN "{column}" {null_constraint}""" ) def create_auth_table(self): diff --git a/frappe/database/postgres/schema.py b/frappe/database/postgres/schema.py index 2abd5f37c7..ef7ba33e12 100644 --- a/frappe/database/postgres/schema.py +++ b/frappe/database/postgres/schema.py @@ -34,11 +34,7 @@ class PostgresTable(DBTable): not self.meta.issingle and self.meta.autoname == "autoincrement" ) or self.doctype in log_types: - # The sequence cache is per connection. - # Since we're opening and closing connections for every transaction 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 - frappe.db.create_sequence(self.doctype, check_not_exists=True) + frappe.db.create_sequence(self.doctype, check_not_exists=True, cache=frappe.db.SEQUENCE_CACHE) name_column = "name bigint primary key" # TODO: set docstatus length diff --git a/frappe/database/sequence.py b/frappe/database/sequence.py index ede4689485..6a352d20d1 100644 --- a/frappe/database/sequence.py +++ b/frappe/database/sequence.py @@ -5,7 +5,7 @@ def create_sequence( doctype_name: str, *, slug: str = "_id_seq", - temporary=False, + temporary: bool = False, check_not_exists: bool = False, cycle: bool = False, cache: int = 0, @@ -51,7 +51,7 @@ def create_sequence( else: query += " cycle" - db.sql(query) + db.sql_ddl(query) return sequence_name diff --git a/frappe/desk/doctype/event/event.json b/frappe/desk/doctype/event/event.json index 2f67c36fc0..bce3b1e65a 100644 --- a/frappe/desk/doctype/event/event.json +++ b/frappe/desk/doctype/event/event.json @@ -277,7 +277,7 @@ "icon": "fa fa-calendar", "idx": 1, "links": [], - "modified": "2021-11-18 05:06:24.881742", + "modified": "2022-05-12 05:43:27.935510", "modified_by": "Administrator", "module": "Desk", "name": "Event", @@ -312,6 +312,7 @@ "sender_field": "sender", "sort_field": "modified", "sort_order": "DESC", + "states": [], "subject_field": "subject", "title_field": "subject", "track_changes": 1, diff --git a/frappe/model/db_query.py b/frappe/model/db_query.py index acb63b5bfa..005b7e3741 100644 --- a/frappe/model/db_query.py +++ b/frappe/model/db_query.py @@ -920,12 +920,12 @@ def cast_name(column: str) -> str: kwargs = {"string": column, "flags": re.IGNORECASE} if "cast(" not in column.lower() and "::" not in column: - if re.search(r"locate\(([^,]+),\s*([`\"]?name[`\"]?)\s*\)", **kwargs): + if re.search(r"locate\([^,]+,\s*[`\"]?name[`\"]?\s*\)", **kwargs): return re.sub( - r"locate\(([^,]+),\s*([`\"]?name[`\"]?)\)", r"locate(\1, cast(\2 as varchar))", **kwargs + r"locate\(([^,]+),\s*([`\"]?name[`\"]?)\s*\)", r"locate(\1, cast(\2 as varchar))", **kwargs ) - elif match := re.search(r"(strpos|ifnull|coalesce)\(\s*([`\"]?name[`\"]?)\s*,", **kwargs): + elif match := re.search(r"(strpos|ifnull|coalesce)\(\s*[`\"]?name[`\"]?\s*,", **kwargs): func = match.groups()[0] return re.sub(rf"{func}\(\s*([`\"]?name[`\"]?)\s*,", rf"{func}(cast(\1 as varchar),", **kwargs) diff --git a/frappe/public/js/frappe/form/controls/base_input.js b/frappe/public/js/frappe/form/controls/base_input.js index 5a5af389ee..b7fe61b385 100644 --- a/frappe/public/js/frappe/form/controls/base_input.js +++ b/frappe/public/js/frappe/form/controls/base_input.js @@ -65,7 +65,11 @@ frappe.ui.form.ControlInput = class ControlInput extends frappe.ui.form.Control }; var update_input = function() { - me.set_input(me.value); + if (me.doctype && me.docname) { + me.set_input(me.value); + } else { + me.set_input(me.value || null); + } }; if (me.disp_status != "None") { diff --git a/frappe/tests/test_db.py b/frappe/tests/test_db.py index 4d105ef28e..338bde7502 100644 --- a/frappe/tests/test_db.py +++ b/frappe/tests/test_db.py @@ -551,10 +551,16 @@ class TestDDLCommandsMaria(unittest.TestCase): ) def test_change_type(self) -> None: + def get_table_description(): + return frappe.db.sql(f"DESC `tab{self.test_table_name}`") + + # try changing from int to varchar frappe.db.change_column_type("TestNotes", "id", "varchar(255)") - test_table_description = frappe.db.sql(f"DESC tab{self.test_table_name};") - self.assertGreater(len(test_table_description), 0) - self.assertIn("varchar(255)", test_table_description[0]) + self.assertIn("varchar(255)", get_table_description()[0]) + + # try changing from varchar to bigint + frappe.db.change_column_type("TestNotes", "id", "bigint") + self.assertIn("bigint(20)", get_table_description()[0]) def test_add_index(self) -> None: index_name = "test_index" @@ -763,21 +769,34 @@ class TestDDLCommandsPost(unittest.TestCase): self.assertEqual([("id",), ("content",)], frappe.db.describe(self.test_table_name)) def test_change_type(self) -> None: + from psycopg2.errors import DatatypeMismatch + + def get_table_description(): + return frappe.db.sql( + f""" + SELECT + table_name, + column_name, + data_type + FROM + information_schema.columns + WHERE + table_name = 'tab{self.test_table_name}'""" + ) + + # try changing from int to varchar frappe.db.change_column_type(self.test_table_name, "id", "varchar(255)") - check_change = frappe.db.sql( - f""" - SELECT - table_name, - column_name, - data_type - FROM - information_schema.columns - WHERE - table_name = 'tab{self.test_table_name}' - """ - ) - self.assertGreater(len(check_change), 0) - self.assertIn("character varying", check_change[0]) + self.assertIn("character varying", get_table_description()[0]) + + # try changing from varchar to int + try: + frappe.db.change_column_type(self.test_table_name, "id", "bigint") + except DatatypeMismatch: + frappe.db.rollback() + + # try changing from varchar to int (using cast) + frappe.db.change_column_type(self.test_table_name, "id", "bigint", use_cast=True) + self.assertIn("bigint", get_table_description()[0]) def test_add_index(self) -> None: index_name = "test_index" @@ -792,7 +811,6 @@ class TestDDLCommandsPost(unittest.TestCase): ) self.assertEqual(len(indexs_in_table), 1) - @run_only_if(db_type_is.POSTGRES) def test_modify_query(self): from frappe.database.postgres.database import modify_query @@ -810,7 +828,6 @@ class TestDDLCommandsPost(unittest.TestCase): modify_query(query), ) - @run_only_if(db_type_is.POSTGRES) def test_modify_values(self): from frappe.database.postgres.database import modify_values