From 27f8a1ea56fd56a3af43ccaa6a26828aef5d0472 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Thu, 23 Dec 2021 18:34:36 +0530 Subject: [PATCH 1/5] refactor: Fractional ratings patch * Change column types of rating fields * Workaround added for truncated/NULL values Co-authored-by: Ankush Menat Co-authored-by: Suraj Shetty --- .../patches/v14_0/save_ratings_in_fraction.py | 46 ++++++++++++++++--- 1 file changed, 39 insertions(+), 7 deletions(-) diff --git a/frappe/patches/v14_0/save_ratings_in_fraction.py b/frappe/patches/v14_0/save_ratings_in_fraction.py index bdc5dfee3d..15a8c9ddf1 100644 --- a/frappe/patches/v14_0/save_ratings_in_fraction.py +++ b/frappe/patches/v14_0/save_ratings_in_fraction.py @@ -1,12 +1,44 @@ import frappe +from frappe.query_builder import DocType + def execute(): - rating_fields = frappe.get_all("DocField", fields=["parent", "fieldname"], filters={"fieldtype": "Rating"}) + rating_fields = frappe.get_all( + "DocField", fields=["parent", "fieldname"], filters={"fieldtype": "Rating"} + ) + + custom_rating_fields = frappe.get_all( + "Custom Field", fields=["dt", "fieldname"], filters={"fieldtype": "Rating"} + ) + + for _field in rating_fields + custom_rating_fields: + doctype_name = _field.get("parent") or _field.get("dt") + doctype = DocType(doctype_name) + field = _field.fieldname + + # update NULL values to 0 to avoid data truncated error (temp) + # commit for upcoming DLL + frappe.qb.update(doctype).set( + doctype[field], 0 + ).where( + doctype[field].isnull() + ).run() + frappe.db.commit() + + # alter column types for rating fieldtype + frappe.db.change_column_type(doctype_name, column=field, type="decimal(3,2)") + + # update data: int => decimal + frappe.qb.update(doctype).set( + doctype[field], doctype[field] / 5 + ).run() - custom_rating_fields = frappe.get_all("Custom Field", fields=["dt", "fieldname"], filters={"fieldtype": "Rating"}) + # revert 0 to NULL conversion + frappe.qb.update(doctype).set( + doctype[field], None + ).where( + doctype[field] == 0 + ).run() - for field in rating_fields + custom_rating_fields: - doctype_name = field.get("parent") or field.get("dt") - doctype = frappe.qb.DocType(doctype_name) - field = field.fieldname - (frappe.qb.update(doctype_name).set(doctype[field], doctype[field]/5)).run() + # commit to flush updated rows + frappe.db.commit() From fd11b0e8e76b18ab77d884c9dd46ffa9c4774e22 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Thu, 23 Dec 2021 18:40:53 +0530 Subject: [PATCH 2/5] fix: Re-run patch to ensure column conversion Co-authored-by: Ankush Menat Co-authored-by: Suraj Shetty --- frappe/patches.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/patches.txt b/frappe/patches.txt index 1760a04f35..27ba1a145d 100644 --- a/frappe/patches.txt +++ b/frappe/patches.txt @@ -188,5 +188,5 @@ frappe.patches.v14_0.copy_mail_data #08.03.21 frappe.patches.v14_0.update_workspace2 # 20.09.2021 frappe.patches.v14_0.update_github_endpoints #08-11-2021 frappe.patches.v14_0.remove_db_aggregation -frappe.patches.v14_0.save_ratings_in_fraction +frappe.patches.v14_0.save_ratings_in_fraction #23-12-2021 frappe.patches.v14_0.update_color_names_in_kanban_board_column From 141f4922e7f132e9c6845a77fb47c71bb38d8d13 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Thu, 23 Dec 2021 19:09:22 +0530 Subject: [PATCH 3/5] fix: Skip patch if column already converted Co-authored-by: Ankush Menat Co-authored-by: Suraj Shetty --- frappe/patches/v14_0/save_ratings_in_fraction.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/frappe/patches/v14_0/save_ratings_in_fraction.py b/frappe/patches/v14_0/save_ratings_in_fraction.py index 15a8c9ddf1..c37d3f134b 100644 --- a/frappe/patches/v14_0/save_ratings_in_fraction.py +++ b/frappe/patches/v14_0/save_ratings_in_fraction.py @@ -3,6 +3,7 @@ from frappe.query_builder import DocType def execute(): + RATING_FIELD_TYPE = "decimal(3,2)" rating_fields = frappe.get_all( "DocField", fields=["parent", "fieldname"], filters={"fieldtype": "Rating"} ) @@ -16,6 +17,13 @@ def execute(): doctype = DocType(doctype_name) field = _field.fieldname + # TODO: Add postgres support (for the check) + if ( + frappe.conf.db_type == "mariadb" + and frappe.db.get_column_type(doctype_name, field) == RATING_FIELD_TYPE + ): + continue + # update NULL values to 0 to avoid data truncated error (temp) # commit for upcoming DLL frappe.qb.update(doctype).set( @@ -26,7 +34,7 @@ def execute(): frappe.db.commit() # alter column types for rating fieldtype - frappe.db.change_column_type(doctype_name, column=field, type="decimal(3,2)") + frappe.db.change_column_type(doctype_name, column=field, type=RATING_FIELD_TYPE) # update data: int => decimal frappe.qb.update(doctype).set( From 600156b1a17b12b86bc6fa94706f49738048d857 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Fri, 24 Dec 2021 15:29:19 +0530 Subject: [PATCH 4/5] refactor!: allow modified column to be nullable * updated change_column_type to allow making columns nullable. * breaking change: in postgres the method was previously nullable by default, changed it to be consistent with mariadb. --- frappe/database/mariadb/database.py | 5 +++-- frappe/database/postgres/database.py | 7 +++++-- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/frappe/database/mariadb/database.py b/frappe/database/mariadb/database.py index afd912bc6b..6b827a4e89 100644 --- a/frappe/database/mariadb/database.py +++ b/frappe/database/mariadb/database.py @@ -135,9 +135,10 @@ class MariaDBDatabase(Database): table_name = get_table_name(doctype) return self.sql(f"DESC `{table_name}`") - def change_column_type(self, doctype: str, column: str, type: str) -> Union[List, Tuple]: + def change_column_type(self, doctype: str, column: str, type: str, nullable: bool = False) -> Union[List, Tuple]: table_name = get_table_name(doctype) - return self.sql(f"ALTER TABLE `{table_name}` MODIFY `{column}` {type} NOT NULL") + null_constraint = "NOT NULL" if not nullable else "" + return self.sql(f"ALTER TABLE `{table_name}` MODIFY `{column}` {type} {null_constraint}") # exception types @staticmethod diff --git a/frappe/database/postgres/database.py b/frappe/database/postgres/database.py index 3cea1440cf..008635b1b3 100644 --- a/frappe/database/postgres/database.py +++ b/frappe/database/postgres/database.py @@ -183,9 +183,12 @@ class PostgresDatabase(Database): table_name = get_table_name(doctype) return self.sql(f"SELECT COLUMN_NAME FROM information_schema.COLUMNS WHERE TABLE_NAME = '{table_name}'") - def change_column_type(self, doctype: str, column: str, type: str) -> Union[List, Tuple]: + def change_column_type(self, doctype: str, column: str, type: str, nullable: bool = False) -> Union[List, Tuple]: table_name = get_table_name(doctype) - return self.sql(f'ALTER TABLE "{table_name}" ALTER COLUMN "{column}" TYPE {type}') + null_constraint = "SET NOT NULL" if not nullable else "DROP NOT NULL" + return self.sql(f"""ALTER TABLE "{table_name}" + ALTER COLUMN "{column}" TYPE {type}, + ALTER COLUMN "{column}" {null_constraint}""") def create_auth_table(self): self.sql_ddl("""create table if not exists "__Auth" ( From 0b7f2804d79dda620982063579600f86f4803714 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Fri, 24 Dec 2021 15:30:25 +0530 Subject: [PATCH 5/5] refactor: make ratings field nullable while altering --- .../patches/v14_0/save_ratings_in_fraction.py | 17 ++--------------- 1 file changed, 2 insertions(+), 15 deletions(-) diff --git a/frappe/patches/v14_0/save_ratings_in_fraction.py b/frappe/patches/v14_0/save_ratings_in_fraction.py index c37d3f134b..c933179b44 100644 --- a/frappe/patches/v14_0/save_ratings_in_fraction.py +++ b/frappe/patches/v14_0/save_ratings_in_fraction.py @@ -24,29 +24,16 @@ def execute(): ): continue - # update NULL values to 0 to avoid data truncated error (temp) - # commit for upcoming DLL - frappe.qb.update(doctype).set( - doctype[field], 0 - ).where( - doctype[field].isnull() - ).run() + # commit any changes so far for upcoming DDL frappe.db.commit() # alter column types for rating fieldtype - frappe.db.change_column_type(doctype_name, column=field, type=RATING_FIELD_TYPE) + frappe.db.change_column_type(doctype_name, column=field, type=RATING_FIELD_TYPE, nullable=True) # update data: int => decimal frappe.qb.update(doctype).set( doctype[field], doctype[field] / 5 ).run() - # revert 0 to NULL conversion - frappe.qb.update(doctype).set( - doctype[field], None - ).where( - doctype[field] == 0 - ).run() - # commit to flush updated rows frappe.db.commit()