Support django 4.2

jayvynl · web-flow · commit bca7a05c4be9 · 2023-06-24T15:02:38.000+08:00
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -4,6 +4,7 @@
 - Fix str(queryset.query) when default database is not clickhouse.
 - Fix [bug when save django model instance](https://github.com/jayvynl/django-clickhouse-backend/issues/9).
 - Support [clickhouse-driver 0.2.6](https://github.com/mymarilyn/clickhouse-driver), drop support for python3.6.
+- Support [Django 4.2](https://docs.djangoproject.com).
 
 ### 1.0.2 (2023-02-28)
 - Fix test db name when NAME not provided in DATABASES setting.
diff --git a/clickhouse_backend/backend/base.py b/clickhouse_backend/backend/base.py
@@ -203,6 +203,15 @@ def init_connection_state(self):
     def create_cursor(self, name=None):
         return self.connection.cursor()
 
+    def _savepoint(self, sid):
+        pass
+
+    def _savepoint_rollback(self, sid):
+        pass
+
+    def _savepoint_commit(self, sid):
+        pass
+
     def _set_autocommit(self, autocommit):
         pass
 
diff --git a/clickhouse_backend/backend/operations.py b/clickhouse_backend/backend/operations.py
@@ -344,21 +344,6 @@ def last_insert_id(self, cursor, table_name, pk_name):
         cursor.execute(query % params)
         return cursor.fetchone()[0]
 
-    def savepoint_create_sql(self, sid):
-        if self.fake_transaction:
-            return "SELECT 1"
-        return super().savepoint_create_sql(sid)
-
-    def savepoint_commit_sql(self, sid):
-        if self.fake_transaction:
-            return "SELECT 1"
-        return super().savepoint_commit_sql(sid)
-
-    def savepoint_rollback_sql(self, sid):
-        if self.fake_transaction:
-            return "SELECT 1"
-        return super().savepoint_rollback_sql(sid)
-
     def last_executed_query(self, cursor, sql, params):
         if params:
             if insert_pattern.match(sql):
diff --git a/clickhouse_backend/backend/schema.py b/clickhouse_backend/backend/schema.py
@@ -10,6 +10,7 @@
 from django.db.models.expressions import ExpressionList
 from django.db.models.indexes import IndexExpression
 
+from clickhouse_backend import compat
 from clickhouse_backend.driver.escape import escape_param
 
 
@@ -364,6 +365,15 @@ def _field_should_be_altered(self, old_field, new_field):
             (old_path, old_args, old_kwargs) != (new_path, new_args, new_kwargs)
         )
 
+    def _alter_column_type_sql(
+        self, model, old_field, new_field, new_type, old_collation=None, new_collation=None
+    ):
+        """Django4.2 add old_collation, new_collation"""
+        if compat.dj_ge42:
+            return super()._alter_column_type_sql(model, old_field, new_field, new_type, old_collation, new_collation)
+        else:
+            return super()._alter_column_type_sql(model, old_field, new_field, new_type)
+
     def _alter_field(self, model, old_field, new_field, old_type, new_type,
                      old_db_params, new_db_params, strict=False):
         # Change check constraints?
diff --git a/clickhouse_backend/compat.py b/clickhouse_backend/compat.py
@@ -4,3 +4,4 @@
 dj4 = (4, ) <= django.VERSION < (5, )
 dj_ge4 = django.VERSION >= (4, )
 dj_ge41 = django.VERSION >= (4, 1)
+dj_ge42 = django.VERSION >= (4, 2)
diff --git a/clickhouse_backend/models/sql/compiler.py b/clickhouse_backend/models/sql/compiler.py
@@ -1,12 +1,15 @@
 from django.db.models.fields import BigAutoField
 from django.db.models.sql import compiler
 
+from clickhouse_backend import compat
 from clickhouse_backend.idworker import id_worker
 
+if compat.dj_ge42:
+    from django.core.exceptions import FullResultSet
+
 
 class ClickhouseMixin:
-    def as_sql(self, *args, **kwargs):
-        sql, params = super().as_sql(*args, **kwargs)
+    def _add_explain_sql(self, sql, params):
         # Backward compatible for django 3.2
         explain_info = getattr(self.query, "explain_info", None)
         if explain_info:
@@ -18,6 +21,9 @@ def as_sql(self, *args, **kwargs):
             sql = "%s %s" % (prefix, sql.lstrip())
             if suffix:
                 sql = "%s %s" % (sql, suffix)
+        return sql, params
+
+    def _add_settings_sql(self, sql, params):
         if getattr(self.query, "setting_info", None):
             setting_sql, setting_params = self.connection.ops.settings_sql(
                 **self.query.setting_info
@@ -26,12 +32,30 @@ def as_sql(self, *args, **kwargs):
             params = (*params, *setting_params)
         return sql, params
 
+    def _compile_where(self, table):
+        if compat.dj_ge42:
+            try:
+                where, params = self.compile(self.query.where)
+            except FullResultSet:
+                where, params = "", ()
+        else:
+            where, params = self.compile(self.query.where)
+        if where:
+            where = where.replace(table + ".", "")
+        else:
+            where = "1"
+        return where, params
+
 
 class SQLCompiler(ClickhouseMixin, compiler.SQLCompiler):
-    pass
+    def as_sql(self, *args, **kwargs):
+        sql, params = super().as_sql(*args, **kwargs)
+        sql, params = self._add_settings_sql(sql, params)
+        sql, params = self._add_explain_sql(sql, params)
+        return sql, params
 
 
-class SQLInsertCompiler(ClickhouseMixin, compiler.SQLInsertCompiler):
+class SQLInsertCompiler(compiler.SQLInsertCompiler):
     def as_sql(self):
         # We don't need quote_name_unless_alias() here, since these are all
         # going to be column names (so we can avoid the extra overhead).
@@ -62,11 +86,16 @@ def as_sql(self):
         ]
 
         placeholder_rows, param_rows = self.assemble_as_sql(fields, value_rows)
+        # https://clickhouse.com/docs/en/sql-reference/statements/insert-into
+        # If you want to specify SETTINGS for INSERT query then you have to do it before FORMAT clause
+        # since everything after FORMAT format_name is treated as data.
+        if getattr(self.query, "setting_info", None):
+            setting_sql, setting_params = self.connection.ops.settings_sql(
+                **self.query.setting_info
+            )
+            result.append(setting_sql % setting_params)
+
         result.append(self.connection.ops.bulk_insert_sql(fields, placeholder_rows))
-        if hasattr(self.query, "get_settings"):
-            settings_string = self.query.get_settings()
-            if settings_string:
-                result.append(settings_string)
         return [(" ".join(result), param_rows)]
 
     def execute_sql(self, returning_fields=None):
@@ -84,16 +113,14 @@ def _as_sql(self, query):
         "table"."column" in WHERE clause.
         """
         table = self.quote_name_unless_alias(query.base_table)
-        result = [
-            "ALTER TABLE %s DELETE" % table
-        ]
-        where, params = self.compile(query.where)
-        where = where.replace(table + ".", "")
-        if where:
-            result.append("WHERE %s" % where)
-        else:
-            result.append("WHERE 1")
-        return " ".join(result), tuple(params)
+        delete = "ALTER TABLE %s DELETE" % table
+        where, params = self._compile_where(table)
+        return f"{delete} WHERE {where}", tuple(params)
+
+    def as_sql(self):
+        sql, params = super().as_sql()
+        sql, params = self._add_settings_sql(sql, params)
+        return sql, params
 
 
 class SQLUpdateCompiler(ClickhouseMixin, compiler.SQLUpdateCompiler):
@@ -159,23 +186,15 @@ def as_sql(self):
             "ALTER TABLE %s UPDATE" % table,
             ", ".join(values).replace(table + ".", ""),
         ]
-        where, params = self.compile(self.query.where)
-        where = where.replace(table + ".", "")
-
-        if where:
-            result.append("WHERE %s" % where)
-        else:
-            result.append("WHERE 1")
-
+        where, params = self._compile_where(table)
+        result.append(f"WHERE {where}")
         params = (*update_params, *params)
-        if getattr(self.query, "setting_info", None):
-            setting_sql, setting_params = self.connection.ops.settings_sql(
-                **self.query.setting_info
-            )
-            result.append(setting_sql)
-            params = (*params, *setting_params)
-        return " ".join(result), params
+        return self._add_settings_sql(" ".join(result), params)
 
 
 class SQLAggregateCompiler(ClickhouseMixin, compiler.SQLAggregateCompiler):
-    pass
+    def as_sql(self):
+        sql, params = super().as_sql()
+        sql, params = self._add_settings_sql(sql, params)
+        sql, params = self._add_explain_sql(sql, params)
+        return sql, params
diff --git a/setup.py b/setup.py
@@ -33,6 +33,7 @@ def read(name):
         "Framework :: Django :: 4",
         "Framework :: Django :: 4.0",
         "Framework :: Django :: 4.1",
+        "Framework :: Django :: 4.2",
         "Intended Audience :: Developers",
         "License :: OSI Approved :: MIT License",
         "Programming Language :: Python",
diff --git a/tests/aggregation/tests.py b/tests/aggregation/tests.py
@@ -1106,14 +1106,6 @@ def test_annotate_over_annotate(self):
 
         self.assertEqual(author.sum_age, other_author.sum_age)
 
-    def test_aggregate_over_aggregate(self):
-        msg = "Cannot compute Avg('age'): 'age' is an aggregate"
-        with self.assertRaisesMessage(FieldError, msg):
-            Author.objects.annotate(age_alias=F("age"),).aggregate(
-                age=Sum(F("age")),
-                avg_age=Avg(F("age")),
-            )
-
     def test_annotated_aggregate_over_annotated_aggregate(self):
         with self.assertRaisesMessage(
             FieldError, "Cannot compute Sum('id__max'): 'id__max' is an aggregate"
diff --git a/tests/basic/tests.py b/tests/basic/tests.py
@@ -12,6 +12,7 @@
 )
 from django.utils.translation import gettext_lazy
 
+from clickhouse_backend import compat
 from .models import (
     Article,
     ArticleSelectOnSave,
@@ -148,7 +149,15 @@ def test_save_primary_with_default(self):
     def test_save_parent_primary_with_default(self):
         # An UPDATE attempt is skipped when an inherited primary key has
         # default.
-        with self.assertNumQueries(2):
+        #
+        if compat.dj_ge42:
+            # Django 4.2 add BEGIN and COMMIT debug queries.
+            # https://github.com/django/django/blob/a18e0f44d5692b656bd8ea178e830ebdc80a000d/django/db/backends/base/base.py#L496
+            # https://github.com/django/django/blob/a18e0f44d5692b656bd8ea178e830ebdc80a000d/django/db/backends/base/base.py#L312
+            num_queries = 4
+        else:
+            num_queries = 2
+        with self.assertNumQueries(num_queries):
             ChildPrimaryKeyWithDefault().save()
 
 
diff --git a/tests/migrations/test_autodetector.py b/tests/migrations/test_autodetector.py
@@ -2107,25 +2107,6 @@ def test_same_app_circular_fk_dependency(self):
         self.assertOperationAttributes(changes, "testapp", 0, 2, name="publisher")
         self.assertMigrationDependencies(changes, "testapp", 0, [])
 
-    def test_same_app_circular_fk_dependency_with_unique_together_and_indexes(self):
-        """
-        #22275 - A migration with circular FK dependency does not try
-        to create unique together constraint and indexes before creating all
-        required fields first.
-        """
-        changes = self.get_changes([], [self.knight, self.rabbit])
-        # Right number/type of migrations?
-        self.assertNumberMigrations(changes, "eggs", 1)
-        self.assertOperationTypes(
-            changes,
-            "eggs",
-            0,
-            ["CreateModel", "CreateModel", "AddIndex", "AlterUniqueTogether"],
-        )
-        self.assertNotIn("unique_together", changes["eggs"][0].operations[0].options)
-        self.assertNotIn("unique_together", changes["eggs"][0].operations[1].options)
-        self.assertMigrationDependencies(changes, "eggs", 0, [])
-
     def test_alter_db_table_add(self):
         """Tests detection for adding db_table in model's options."""
         changes = self.get_changes(
@@ -2366,37 +2347,6 @@ def test(from_state, to_state, msg):
         for t in tests:
             test(*t)
 
-    def test_create_model_with_indexes(self):
-        """Test creation of new model with indexes already defined."""
-        author = ModelState(
-            "otherapp",
-            "Author",
-            [
-                ("id", models.BigAutoField(primary_key=True)),
-                ("name", models.CharField(max_length=200)),
-            ],
-            {
-                "indexes": [
-                    models.Index(fields=["name"], name="create_model_with_indexes_idx")
-                ]
-            },
-        )
-        changes = self.get_changes([], [author])
-        added_index = models.Index(
-            fields=["name"], name="create_model_with_indexes_idx"
-        )
-        # Right number of migrations?
-        self.assertEqual(len(changes["otherapp"]), 1)
-        # Right number of actions?
-        migration = changes["otherapp"][0]
-        self.assertEqual(len(migration.operations), 2)
-        # Right actions order?
-        self.assertOperationTypes(changes, "otherapp", 0, ["CreateModel", "AddIndex"])
-        self.assertOperationAttributes(changes, "otherapp", 0, 0, name="Author")
-        self.assertOperationAttributes(
-            changes, "otherapp", 0, 1, model_name="author", index=added_index
-        )
-
     def test_add_indexes(self):
         """Test change detection of new indexes."""
         changes = self.get_changes(
@@ -3617,63 +3567,6 @@ def test_add_model_order_with_respect_to_unique_together(self):
             },
         )
 
-    def test_add_model_order_with_respect_to_index_constraint(self):
-        tests = [
-            (
-                "AddIndex",
-                {
-                    "indexes": [
-                        models.Index(fields=["_order"], name="book_order_idx"),
-                    ]
-                },
-            ),
-            (
-                "AddConstraint",
-                {
-                    "constraints": [
-                        models.CheckConstraint(
-                            check=models.Q(_order__gt=1),
-                            name="book_order_gt_1",
-                        ),
-                    ]
-                },
-            ),
-        ]
-        for operation, extra_option in tests:
-            with self.subTest(operation=operation):
-                after = ModelState(
-                    "testapp",
-                    "Author",
-                    [
-                        ("id", models.BigAutoField(primary_key=True)),
-                        ("name", models.CharField(max_length=200)),
-                        ("book", models.ForeignKey("otherapp.Book", models.CASCADE)),
-                    ],
-                    options={
-                        "order_with_respect_to": "book",
-                        **extra_option,
-                    },
-                )
-                changes = self.get_changes([], [self.book, after])
-                self.assertNumberMigrations(changes, "testapp", 1)
-                self.assertOperationTypes(
-                    changes,
-                    "testapp",
-                    0,
-                    [
-                        "CreateModel",
-                        operation,
-                    ],
-                )
-                self.assertOperationAttributes(
-                    changes,
-                    "testapp",
-                    0,
-                    0,
-                    name="Author",
-                    options={"order_with_respect_to": "book"},
-                )
-
     def test_set_alter_order_with_respect_to_index_constraint_unique_together(self):
         tests = [
             (
diff --git a/tox.ini b/tox.ini
