diff --git a/django/contrib/admin/utils.py b/django/contrib/admin/utils.py index 8263b6f9e2a1..e21a6102b590 100644 --- a/django/contrib/admin/utils.py +++ b/django/contrib/admin/utils.py @@ -552,21 +552,21 @@ def construct_change_message(form, formsets, add): Translations are deactivated so that strings are stored untranslated. Translation happens later on LogEntry access. """ + change_message = [] + if add: + change_message.append({"added": {}}) # Evaluating `form.changed_data` prior to disabling translations is # required to avoid fields affected by localization from being included # incorrectly, e.g. where date formats differ such as MM/DD/YYYY vs # DD/MM/YYYY. - changed_data = form.changed_data - with translation_override(None): - # Deactivate translations while fetching verbose_name for form - # field labels and using `field_name`, if verbose_name is not provided. - # Translations will happen later on LogEntry access. - changed_field_labels = _get_changed_field_labels_from_form(form, changed_data) - - change_message = [] - if add: - change_message.append({"added": {}}) - elif form.changed_data: + elif changed_data := form.changed_data: + with translation_override(None): + # Deactivate translations while fetching verbose_name for form + # field labels and using `field_name`, if verbose_name is not + # provided. Translations will happen later on LogEntry access. + changed_field_labels = _get_changed_field_labels_from_form( + form, changed_data + ) change_message.append({"changed": {"fields": changed_field_labels}}) if formsets: with translation_override(None): diff --git a/django/db/backends/base/operations.py b/django/db/backends/base/operations.py index e34570143898..fac62158996c 100644 --- a/django/db/backends/base/operations.py +++ b/django/db/backends/base/operations.py @@ -2,12 +2,14 @@ import decimal import json from importlib import import_module +from itertools import chain import sqlparse from django.conf import settings from django.db import NotSupportedError, transaction from django.db.models.expressions import Col +from django.db.models.fields.composite import CompositePrimaryKey from django.utils import timezone from django.utils.duration import duration_microseconds from django.utils.encoding import force_str @@ -78,7 +80,17 @@ def bulk_batch_size(self, fields, objs): are the fields going to be inserted in the batch, the objs contains all the objects to be inserted. """ - return len(objs) + if self.connection.features.max_query_params is None or not fields: + return len(objs) + + return self.connection.features.max_query_params // len( + list( + chain.from_iterable( + field.fields if isinstance(field, CompositePrimaryKey) else [field] + for field in fields + ) + ) + ) def format_for_duration_arithmetic(self, sql): raise NotImplementedError( diff --git a/django/db/backends/oracle/operations.py b/django/db/backends/oracle/operations.py index 75f80941b3bb..802f27a3c6a2 100644 --- a/django/db/backends/oracle/operations.py +++ b/django/db/backends/oracle/operations.py @@ -1,7 +1,6 @@ import datetime import uuid from functools import lru_cache -from itertools import chain from django.conf import settings from django.db import NotSupportedError @@ -9,7 +8,6 @@ from django.db.backends.utils import split_tzname_delta, strip_quotes, truncate_name from django.db.models import ( AutoField, - CompositePrimaryKey, Exists, ExpressionWrapper, Lookup, @@ -707,18 +705,6 @@ def subtract_temporals(self, internal_type, lhs, rhs): ) return super().subtract_temporals(internal_type, lhs, rhs) - def bulk_batch_size(self, fields, objs): - """Oracle restricts the number of parameters in a query.""" - fields = list( - chain.from_iterable( - field.fields if isinstance(field, CompositePrimaryKey) else [field] - for field in fields - ) - ) - if fields: - return self.connection.features.max_query_params // len(fields) - return len(objs) - def conditional_expression_supported_in_where_clause(self, expression): """ Oracle supports only EXISTS(...) or filters in the WHERE clause, others diff --git a/django/db/backends/postgresql/features.py b/django/db/backends/postgresql/features.py index 7d313467d41f..b663adc90cf2 100644 --- a/django/db/backends/postgresql/features.py +++ b/django/db/backends/postgresql/features.py @@ -151,6 +151,12 @@ def uses_server_side_binding(self): options = self.connection.settings_dict["OPTIONS"] return is_psycopg3 and options.get("server_side_binding") is True + @cached_property + def max_query_params(self): + if self.uses_server_side_binding: + return 2**16 - 1 + return None + @cached_property def prohibits_null_characters_in_text_exception(self): if is_psycopg3: diff --git a/django/db/backends/sqlite3/operations.py b/django/db/backends/sqlite3/operations.py index ac98324b2e30..23c17054d260 100644 --- a/django/db/backends/sqlite3/operations.py +++ b/django/db/backends/sqlite3/operations.py @@ -29,26 +29,6 @@ class DatabaseOperations(BaseDatabaseOperations): # SQLite. Use JSON_TYPE() instead. jsonfield_datatype_values = frozenset(["null", "false", "true"]) - def bulk_batch_size(self, fields, objs): - """ - SQLite has a variable limit defined by SQLITE_LIMIT_VARIABLE_NUMBER - (reflected in max_query_params). - """ - fields = list( - chain.from_iterable( - ( - field.fields - if isinstance(field, models.CompositePrimaryKey) - else [field] - ) - for field in fields - ) - ) - if fields: - return self.connection.features.max_query_params // len(fields) - else: - return len(objs) - def check_expression_support(self, expression): bad_fields = (models.DateField, models.DateTimeField, models.TimeField) bad_aggregates = (models.Sum, models.Avg, models.Variance, models.StdDev) diff --git a/django/db/models/sql/query.py b/django/db/models/sql/query.py index e3e6100f24f0..4be450167d78 100644 --- a/django/db/models/sql/query.py +++ b/django/db/models/sql/query.py @@ -2585,10 +2585,17 @@ def set_values(self, fields): annotation_names.append(f) selected[f] = f elif f in self.annotations: - raise FieldError( - f"Cannot select the '{f}' alias. Use annotate() to " - "promote it." - ) + if self.annotation_select: + raise FieldError( + f"Cannot select the '{f}' alias. It was excluded " + f"by a previous values() or values_list() call. " + f"Include '{f}' in that call to select it." + ) + else: + raise FieldError( + f"Cannot select the '{f}' alias. Use annotate() " + f"to promote it." + ) else: # Call `names_to_path` to ensure a FieldError including # annotations about to be masked as valid choices if diff --git a/tests/annotations/tests.py b/tests/annotations/tests.py index 6336cabafae9..69a2b4a7c7bc 100644 --- a/tests/annotations/tests.py +++ b/tests/annotations/tests.py @@ -482,6 +482,17 @@ def test_values_wrong_annotation(self): with self.assertRaisesMessage(FieldError, expected_message % article_fields): Book.objects.annotate(annotation=Value(1)).values_list("annotation_typo") + def test_chained_values_masked_annotation_error_message(self): + msg = ( + "Cannot select the 'author_id' alias. It was excluded by a " + "previous values() or values_list() call. Include 'author_id' in " + "that call to select it." + ) + with self.assertRaisesMessage(FieldError, msg): + Book.objects.annotate( + author_name=F("authors__name"), author_id=F("authors__id") + ).values("author_name").values("author_id") + def test_decimal_annotation(self): salary = Decimal(10) ** -Employee._meta.get_field("salary").decimal_places Employee.objects.create( diff --git a/tests/backends/base/test_operations.py b/tests/backends/base/test_operations.py index 96fadb4c7afa..8be1d3e841b1 100644 --- a/tests/backends/base/test_operations.py +++ b/tests/backends/base/test_operations.py @@ -1,7 +1,7 @@ import decimal from django.core.management.color import no_style -from django.db import NotSupportedError, connection, transaction +from django.db import NotSupportedError, connection, models, transaction from django.db.backends.base.operations import BaseDatabaseOperations from django.db.models import DurationField from django.db.models.expressions import Col @@ -11,10 +11,11 @@ TransactionTestCase, override_settings, skipIfDBFeature, + skipUnlessDBFeature, ) from django.utils import timezone -from ..models import Author, Book +from ..models import Author, Book, Person class SimpleDatabaseOperationTests(SimpleTestCase): @@ -201,6 +202,55 @@ def test_subtract_temporals(self): with self.assertRaisesMessage(NotSupportedError, msg): self.ops.subtract_temporals(duration_field_internal_type, None, None) + @skipUnlessDBFeature("max_query_params") + def test_bulk_batch_size_limited(self): + max_query_params = connection.features.max_query_params + objects = range(max_query_params + 1) + first_name_field = Person._meta.get_field("first_name") + last_name_field = Person._meta.get_field("last_name") + composite_pk = models.CompositePrimaryKey("first_name", "last_name") + composite_pk.fields = [first_name_field, last_name_field] + + self.assertEqual(connection.ops.bulk_batch_size([], objects), len(objects)) + self.assertEqual( + connection.ops.bulk_batch_size([first_name_field], objects), + max_query_params, + ) + self.assertEqual( + connection.ops.bulk_batch_size( + [first_name_field, last_name_field], objects + ), + max_query_params // 2, + ) + self.assertEqual( + connection.ops.bulk_batch_size([composite_pk, first_name_field], objects), + max_query_params // 3, + ) + + @skipIfDBFeature("max_query_params") + def test_bulk_batch_size_unlimited(self): + objects = range(2**16 + 1) + first_name_field = Person._meta.get_field("first_name") + last_name_field = Person._meta.get_field("last_name") + composite_pk = models.CompositePrimaryKey("first_name", "last_name") + composite_pk.fields = [first_name_field, last_name_field] + + self.assertEqual(connection.ops.bulk_batch_size([], objects), len(objects)) + self.assertEqual( + connection.ops.bulk_batch_size([first_name_field], objects), + len(objects), + ) + self.assertEqual( + connection.ops.bulk_batch_size( + [first_name_field, last_name_field], objects + ), + len(objects), + ) + self.assertEqual( + connection.ops.bulk_batch_size([composite_pk, first_name_field], objects), + len(objects), + ) + class SqlFlushTests(TransactionTestCase): available_apps = ["backends"] diff --git a/tests/backends/oracle/test_operations.py b/tests/backends/oracle/test_operations.py index 1f9447bde709..197ac5a422ba 100644 --- a/tests/backends/oracle/test_operations.py +++ b/tests/backends/oracle/test_operations.py @@ -1,7 +1,7 @@ import unittest from django.core.management.color import no_style -from django.db import connection, models +from django.db import connection from django.test import TransactionTestCase from ..models import Person, Tag @@ -17,31 +17,6 @@ def test_sequence_name_truncation(self): ) self.assertEqual(seq_name, "SCHEMA_AUTHORWITHEVENLOB0B8_SQ") - def test_bulk_batch_size(self): - # Oracle restricts the number of parameters in a query. - objects = range(2**16) - self.assertEqual(connection.ops.bulk_batch_size([], objects), len(objects)) - # Each field is a parameter for each object. - first_name_field = Person._meta.get_field("first_name") - last_name_field = Person._meta.get_field("last_name") - self.assertEqual( - connection.ops.bulk_batch_size([first_name_field], objects), - connection.features.max_query_params, - ) - self.assertEqual( - connection.ops.bulk_batch_size( - [first_name_field, last_name_field], - objects, - ), - connection.features.max_query_params // 2, - ) - composite_pk = models.CompositePrimaryKey("first_name", "last_name") - composite_pk.fields = [first_name_field, last_name_field] - self.assertEqual( - connection.ops.bulk_batch_size([composite_pk, first_name_field], objects), - connection.features.max_query_params // 3, - ) - def test_sql_flush(self): statements = connection.ops.sql_flush( no_style(), diff --git a/tests/backends/postgresql/test_features.py b/tests/backends/postgresql/test_features.py new file mode 100644 index 000000000000..c63f26a7bb13 --- /dev/null +++ b/tests/backends/postgresql/test_features.py @@ -0,0 +1,14 @@ +import unittest + +from django.db import connection +from django.test import TestCase + + +@unittest.skipUnless(connection.vendor == "postgresql", "PostgreSQL tests") +class FeaturesTests(TestCase): + def test_max_query_params_respects_server_side_params(self): + if connection.features.uses_server_side_binding: + limit = 2**16 - 1 + else: + limit = None + self.assertEqual(connection.features.max_query_params, limit) diff --git a/tests/backends/sqlite/test_operations.py b/tests/backends/sqlite/test_operations.py index 0c2772301b17..dee55b639067 100644 --- a/tests/backends/sqlite/test_operations.py +++ b/tests/backends/sqlite/test_operations.py @@ -2,7 +2,7 @@ import unittest from django.core.management.color import no_style -from django.db import connection, models +from django.db import connection from django.test import TestCase from ..models import Person, Tag @@ -88,29 +88,6 @@ def test_sql_flush_sequences_allow_cascade(self): statements[-1], ) - def test_bulk_batch_size(self): - self.assertEqual(connection.ops.bulk_batch_size([], [Person()]), 1) - first_name_field = Person._meta.get_field("first_name") - last_name_field = Person._meta.get_field("last_name") - self.assertEqual( - connection.ops.bulk_batch_size([first_name_field], [Person()]), - connection.features.max_query_params, - ) - self.assertEqual( - connection.ops.bulk_batch_size( - [first_name_field, last_name_field], [Person()] - ), - connection.features.max_query_params // 2, - ) - composite_pk = models.CompositePrimaryKey("first_name", "last_name") - composite_pk.fields = [first_name_field, last_name_field] - self.assertEqual( - connection.ops.bulk_batch_size( - [composite_pk, first_name_field], [Person()] - ), - connection.features.max_query_params // 3, - ) - def test_bulk_batch_size_respects_variable_limit(self): first_name_field = Person._meta.get_field("first_name") last_name_field = Person._meta.get_field("last_name") diff --git a/tests/composite_pk/tests.py b/tests/composite_pk/tests.py index 3001847455cb..3653beceedfe 100644 --- a/tests/composite_pk/tests.py +++ b/tests/composite_pk/tests.py @@ -149,21 +149,18 @@ def test_in_bulk(self): def test_in_bulk_batching(self): Comment.objects.all().delete() - batching_required = connection.features.max_query_params is not None - expected_queries = 2 if batching_required else 1 + num_objects = 10 + connection.features.__dict__.pop("max_query_params", None) with unittest.mock.patch.object( - type(connection.features), "max_query_params", 10 + type(connection.features), "max_query_params", num_objects ): - num_requiring_batching = ( - connection.ops.bulk_batch_size([Comment._meta.pk], []) + 1 - ) comments = [ Comment(id=i, tenant=self.tenant, user=self.user) - for i in range(1, num_requiring_batching + 1) + for i in range(1, num_objects + 1) ] Comment.objects.bulk_create(comments) id_list = list(Comment.objects.values_list("pk", flat=True)) - with self.assertNumQueries(expected_queries): + with self.assertNumQueries(2): comment_dict = Comment.objects.in_bulk(id_list=id_list) self.assertQuerySetEqual(comment_dict, id_list) diff --git a/tests/gis_tests/gdal_tests/test_driver.py b/tests/gis_tests/gdal_tests/test_driver.py index 4bcea5f16c86..71ff5a5aab35 100644 --- a/tests/gis_tests/gdal_tests/test_driver.py +++ b/tests/gis_tests/gdal_tests/test_driver.py @@ -9,7 +9,7 @@ "MapInfo File", "S57", "DGN", - "Memory", + "Memory" if GDAL_VERSION <= (3, 10) else "MEM", "CSV", "GML", "KML",