fix: duplicate unique index when column is altered

frappe/database/mariadb/schema.py

@@ -73,13 +73,19 @@ class MariaDBTable(DBTable):
 		add_index_query = []
 		drop_index_query = []
 
-		columns_to_modify = set(self.change_type + self.add_unique + self.set_default)
-
 		for col in self.add_column:
 			add_column_query.append(f"ADD COLUMN `{col.fieldname}` {col.get_definition()}")
 
+		columns_to_modify = set(self.change_type + self.set_default)
 		for col in columns_to_modify:
-			modify_column_query.append(f"MODIFY `{col.fieldname}` {col.get_definition()}")
+			modify_column_query.append(
+				f"MODIFY `{col.fieldname}` {col.get_definition(for_modification=True)}"
+			)
+
+		for col in self.add_unique:
+			modify_column_query.append(
+				f"ADD UNIQUE INDEX IF NOT EXISTS {col.fieldname} (`{col.fieldname}`)"
+			)
 
 		for col in self.add_index:
 			# if index key does not exists

frappe/database/schema.py

@@ -187,7 +187,7 @@ class DbColumn:
 		self.unique = unique
 		self.precision = precision
 
-	def get_definition(self, with_default=1):
+	def get_definition(self, for_modification=False):
 		column_def = get_definition(self.fieldtype, precision=self.precision, length=self.length)
 
 		if not column_def:
@@ -209,7 +209,7 @@ class DbColumn:
 		):
 			column_def += f" default {frappe.db.escape(self.default)}"
 
-		if self.unique and (column_def not in ("text", "longtext")):
+		if self.unique and not for_modification and (column_def not in ("text", "longtext")):
 			column_def += " unique"
 
 		return column_def

frappe/tests/test_db_update.py

@@ -2,6 +2,8 @@ import frappe
 from frappe.core.doctype.doctype.test_doctype import new_doctype
 from frappe.core.utils import find
 from frappe.custom.doctype.property_setter.property_setter import make_property_setter
+from frappe.query_builder.utils import db_type_is
+from frappe.tests.test_query_builder import run_only_if
 from frappe.tests.utils import FrappeTestCase
 from frappe.utils import cstr
 
@@ -97,6 +99,7 @@ class TestDBUpdate(FrappeTestCase):
 			len(indexes), 1, msg=f"There should be 1 index on {doctype}.{field}, found {indexes}"
 		)
 
+	@run_only_if(db_type_is.MARIADB)
 	def test_unique_index_on_install(self):
 		"""Only one unique index should be added"""
 		for dt in frappe.get_all("DocType", {"is_virtual": 0, "issingle": 0}, pluck="name"):
@@ -106,18 +109,27 @@ class TestDBUpdate(FrappeTestCase):
 				with self.subTest(f"Checking index {doctype.name} - {field.fieldname}"):
 					self.check_unique_indexes(doctype.name, field.fieldname)
 
+	@run_only_if(db_type_is.MARIADB)
 	def test_unique_index_on_alter(self):
 		"""Only one unique index should be added"""
 
 		doctype = new_doctype(unique=1).insert()
 		field = "some_fieldname"
 
+		self.check_unique_indexes(doctype.name, field)
 		doctype.fields[0].length = 142
 		doctype.save()
 
-		doctype.delete()
+		doctype.fields[0].unique = 0
+		doctype.save()
+
+		doctype.fields[0].unique = 1
+		doctype.save()
 		self.check_unique_indexes(doctype.name, field)
 
+		doctype.delete()
+		frappe.db.commit()
+
 
 def get_fieldtype_from_def(field_def):
 	fieldtuple = frappe.db.type_map.get(field_def.fieldtype, ("", 0))