fix: add unique constraint after add field (#53)

fix #48

TiDB does not support multiple operations with a single DDL statement.
For more information, refer to the [MySQL Compatibility - DDL
operations](https://docs.pingcap.com/tidb/dev/mysql-compatibility#ddl-operations)
documentation. In django-tidb, we now separate the SQL into multiple DDL
statements: first, add the column, and then add the unique constraint
and foreign key.

.github/workflows/ci.yml

@@ -4,6 +4,10 @@ on:
     branches:
       - stable/3.2.x
 
+concurrency:
+  group: ${{ github.workflow }}-${{ github.ref }}
+  cancel-in-progress: true
+
 jobs:
   lint:
     name: lint
@@ -31,7 +35,7 @@ jobs:
           - '3.9'
           - '3.10'
         django-version:
-          - '3.2.20'
+          - '3.2.21'
         tidb-version:
           - 'v7.1.1'
           - 'v6.5.3'

django_tidb/features.py

@@ -117,6 +117,9 @@ def django_test_skips(self):
                 # wrong test result
                 ".test_no_duplicates_for_non_unique_related_object_in_search_fields",
                 "transaction_hooks.tests.TestConnectionOnCommit.test_inner_savepoint_does_not_affect_outer",
+                # TiDB does not support `JSON` format for `EXPLAIN ANALYZE`
+                "queries.test_explain.ExplainTests.test_mysql_analyze",
+                "queries.test_explain.ExplainTests.test_mysql_text_to_traditional",
                 # TiDB cannot guarantee to always rollback the main thread txn when deadlock occurs
                 "transactions.tests.AtomicMySQLTests.test_implicit_savepoint_rollback",
                 "filtered_relation.tests.FilteredRelationTests.test_select_for_update",
@@ -168,10 +171,8 @@ def django_test_skips(self):
                 # Unsupported add foreign key reference to themselves
                 "schema.tests.SchemaTests.test_add_inline_fk_update_data",
                 "schema.tests.SchemaTests.test_db_table",
-                "schema.tests.SchemaTests.test_indexes",
                 "schema.tests.SchemaTests.test_inline_fk",
                 "schema.tests.SchemaTests.test_remove_constraints_capital_letters",
-                "schema.tests.SchemaTests.test_remove_db_index_doesnt_remove_custom_indexes",
                 "schema.tests.SchemaTests.test_rename_column_renames_deferred_sql_references",
                 "schema.tests.SchemaTests.test_rename_referenced_field",
                 "schema.tests.SchemaTests.test_rename_table_renames_deferred_sql_references",
@@ -195,6 +196,13 @@ def django_test_skips(self):
                 "contenttypes_tests.test_models.ContentTypesMultidbTests.test_multidb",
                 # ContentType matching query does not exist.
                 "contenttypes_tests.test_models.ContentTypesTests.test_app_labeled_name",
+                "schema.tests.SchemaTests.test_add_auto_field",
+                "schema.tests.SchemaTests.test_alter_autofield_pk_to_smallautofield_pk",
+                "schema.tests.SchemaTests.test_alter_primary_key_db_collation",
+                "schema.tests.SchemaTests.test_alter_primary_key_the_same_name",
+                "schema.tests.SchemaTests.test_autofield_to_o2o",
+                "update.tests.AdvancedTests.test_update_ordered_by_inline_m2m_annotation",
+                "update.tests.AdvancedTests.test_update_ordered_by_m2m_annotation",
                 # IntegrityError not raised
                 "constraints.tests.CheckConstraintTests.test_database_constraint",
                 "constraints.tests.CheckConstraintTests.test_database_constraint_unicode",
@@ -388,7 +396,6 @@ def django_test_skips(self):
                         "migrations.test_operations.OperationTests.test_add_func_unique_constraint",
                         "migrations.test_operations.OperationTests.test_add_func_index",
                         "schema.tests.SchemaTests.test_add_auto_field",
-                        "schema.tests.SchemaTests.test_add_field_o2o_nullable",
                         "schema.tests.SchemaTests.test_alter_autofield_pk_to_smallautofield_pk",
                         "schema.tests.SchemaTests.test_alter_primary_key_db_collation",
                         "schema.tests.SchemaTests.test_alter_primary_key_the_same_name",

django_tidb/schema.py

@@ -49,3 +49,14 @@ def _field_should_be_indexed(self, model, field):
         if field.get_internal_type() == "ForeignKey" and field.db_constraint:
             return False
         return not self._is_limited_data_type(field)
+
+    def add_field(self, model, field):
+        if field.unique:
+            # TiDB does not support multiple operations with a single DDL statement,
+            # so we need to execute the unique constraint creation separately.
+            field._unique = False
+            super().add_field(model, field)
+            field._unique = True
+            self.execute(self._create_unique_sql(model, [field]))
+        else:
+            super().add_field(model, field)

run_testing_worker.py

@@ -35,9 +35,11 @@
 if not all_apps:
     exit()
 
-exitcode = os.WEXITSTATUS(os.system(
-    """DJANGO_TEST_APPS="{apps}" bash ./django_test_suite.sh""".format(
-        apps=" ".join(all_apps)
+exitcode = os.WEXITSTATUS(
+    os.system(
+        """DJANGO_TEST_APPS="{apps}" bash ./django_test_suite.sh""".format(
+            apps=" ".join(all_apps)
+        )
     )
-))
+)
 exit(exitcode)

tests/test_tidb_ddl.py

@@ -19,6 +19,16 @@ def get_indexes(self, table):
                 if c["index"] and len(c["columns"]) == 1
             ]
 
+    def get_uniques(self, table):
+        with connection.cursor() as cursor:
+            return [
+                c["columns"][0]
+                for c in connection.introspection.get_constraints(
+                    cursor, table
+                ).values()
+                if c["unique"] and len(c["columns"]) == 1
+            ]
+
     @isolate_apps("tidb")
     def test_should_create_db_index(self):
         # When define a model with db_index=True, TiDB should create a db index
@@ -32,6 +42,12 @@ class Meta:
             editor.create_model(Tag)
         self.assertIn("title", self.get_indexes("tidb_tag"))
 
+        new_field = models.CharField(max_length=255, db_index=True)
+        new_field.set_attributes_from_name("new_field")
+        with connection.schema_editor() as editor:
+            editor.add_field(Tag, new_field)
+        self.assertIn("new_field", self.get_indexes("tidb_tag"))
+
     @isolate_apps("tidb")
     def test_should_create_db_index_for_foreign_key_with_no_db_constraint(self):
         # When define a model with ForeignKey, TiDB should not create a db index
@@ -54,3 +70,28 @@ class Meta:
             editor.create_model(Node2)
 
         self.assertIn("node1_id", self.get_indexes("tidb_node2"))
+
+    @isolate_apps("tidb")
+    def test_add_unique_field(self):
+        # issue: https://github.com/pingcap/django-tidb/issues/48
+        class Node3(models.Model):
+            title = models.CharField(max_length=255, unique=True)
+
+            class Meta:
+                app_label = "tidb"
+
+        with connection.schema_editor() as editor:
+            editor.create_model(Node3)
+        self.assertIn("title", self.get_uniques("tidb_node3"))
+
+        new_field = models.CharField(max_length=255, unique=True)
+        new_field.set_attributes_from_name("new_field")
+        with connection.schema_editor() as editor:
+            editor.add_field(Node3, new_field)
+        self.assertIn("new_field", self.get_uniques("tidb_node3"))
+
+        parent = models.OneToOneField(Node3, models.CASCADE)
+        parent.set_attributes_from_name("parent")
+        with connection.schema_editor() as editor:
+            editor.add_field(Node3, parent)
+        self.assertIn("parent_id", self.get_uniques("tidb_node3"))

tidb_settings.py

@@ -13,40 +13,40 @@
 
 import os
 
-hosts = os.getenv('TIDB_HOST', '127.0.0.1')
-port = os.getenv('TIDB_PORT', 4000)
+hosts = os.getenv("TIDB_HOST", "127.0.0.1")
+port = os.getenv("TIDB_PORT", 4000)
 
 DATABASES = {
-    'default': {
-        'ENGINE': 'django_tidb',
-        'USER': 'root',
-        'PASSWORD': '',
-        'HOST': hosts,
-        'PORT': port,
-        'TEST': {
-            'NAME': 'django_tests',
-            'CHARSET': 'utf8mb4',
-            'COLLATION': 'utf8mb4_general_ci',
+    "default": {
+        "ENGINE": "django_tidb",
+        "USER": "root",
+        "PASSWORD": "",
+        "HOST": hosts,
+        "PORT": port,
+        "TEST": {
+            "NAME": "django_tests",
+            "CHARSET": "utf8mb4",
+            "COLLATION": "utf8mb4_general_ci",
         },
     },
-    'other': {
-        'ENGINE': 'django_tidb',
-        'USER': 'root',
-        'PASSWORD': '',
-        'HOST': hosts,
-        'PORT': port,
-        'TEST': {
-            'NAME': 'django_tests2',
-            'CHARSET': 'utf8mb4',
-            'COLLATION': 'utf8mb4_general_ci',
+    "other": {
+        "ENGINE": "django_tidb",
+        "USER": "root",
+        "PASSWORD": "",
+        "HOST": hosts,
+        "PORT": port,
+        "TEST": {
+            "NAME": "django_tests2",
+            "CHARSET": "utf8mb4",
+            "COLLATION": "utf8mb4_general_ci",
         },
     },
 }
-DEFAULT_AUTO_FIELD = 'django.db.models.AutoField'
+DEFAULT_AUTO_FIELD = "django.db.models.AutoField"
 USE_TZ = False
-SECRET_KEY = 'django_tests_secret_key'
+SECRET_KEY = "django_tests_secret_key"
 
 # Use a fast hasher to speed up tests.
 PASSWORD_HASHERS = [
     "django.contrib.auth.hashers.MD5PasswordHasher",
-]
+]

tox.ini

@@ -31,9 +31,10 @@ setenv =
 
 [testenv:lint]
 skip_install = True
+allowlist_externals = bash
 deps =
   flake8==6.0.0
-  black==23.1.0
+  black==23.7.0
 commands =
-  flake8 --max-line-length 130 django_tidb tests
-  black --diff --check django_tidb tests
+  bash -c "flake8 --max-line-length 130 django_tidb tests *py"
+  bash -c "black --diff --check django_tidb tests *py"