diff --git a/django/contrib/gis/db/backends/base/operations.py b/django/contrib/gis/db/backends/base/operations.py index e468c6f610c8..c1307440239b 100644 --- a/django/contrib/gis/db/backends/base/operations.py +++ b/django/contrib/gis/db/backends/base/operations.py @@ -115,36 +115,34 @@ def get_distance(self, f, value, lookup_type): "Distance operations not available on this spatial backend." ) - def get_geom_placeholder(self, f, value, compiler): + @staticmethod + def _must_transform_value(value, field): + return value is not None and value.srid != field.srid + + def get_geom_placeholder_sql(self, f, value, compiler): """ Return the placeholder for the given geometry field with the given value. Depending on the spatial backend, the placeholder may contain a stored procedure call to the transformation function of the spatial backend. """ - - def transform_value(value, field): - return value is not None and value.srid != field.srid - if hasattr(value, "as_sql"): - return ( - "%s(%%s, %s)" % (self.spatial_function_name("Transform"), f.srid) - if transform_value(value.output_field, f) - else "%s" - ) - if transform_value(value, f): - # Add Transform() to the SQL placeholder. - return "%s(%s(%%s,%s), %s)" % ( - self.spatial_function_name("Transform"), - self.from_text, - value.srid, - f.srid, - ) + sql, params = compiler.compile(value) + if self._must_transform_value(value.output_field, f): + transform_func = self.spatial_function_name("Transform") + sql = f"{transform_func}({sql}, %s)" + params = (*params, f.srid) + return sql, params + elif self._must_transform_value(value, f): + transform_func = self.spatial_function_name("Transform") + sql = f"{transform_func}({self.from_text}(%s, %s), %s)" + params = (value, value.srid, f.srid) + return sql, params elif self.connection.features.has_spatialrefsys_table: - return "%s(%%s,%s)" % (self.from_text, f.srid) + return f"{self.from_text}(%s, %s)", (value, f.srid) else: # For backwards compatibility on MySQL (#27464). - return "%s(%%s)" % self.from_text + return f"{self.from_text}(%s)", (value,) def check_expression_support(self, expression): if isinstance(expression, self.disallowed_aggregates): diff --git a/django/contrib/gis/db/backends/oracle/operations.py b/django/contrib/gis/db/backends/oracle/operations.py index 1c0928180974..918ab7ce6412 100644 --- a/django/contrib/gis/db/backends/oracle/operations.py +++ b/django/contrib/gis/db/backends/oracle/operations.py @@ -204,10 +204,10 @@ def get_distance(self, f, value, lookup_type): return [dist_param] - def get_geom_placeholder(self, f, value, compiler): + def get_geom_placeholder_sql(self, f, value, compiler): if value is None: - return "NULL" - return super().get_geom_placeholder(f, value, compiler) + return "NULL", () + return super().get_geom_placeholder_sql(f, value, compiler) def spatial_aggregate_name(self, agg_name): """ diff --git a/django/contrib/gis/db/backends/postgis/operations.py b/django/contrib/gis/db/backends/postgis/operations.py index f4e70c9204f0..3c5ea7687ff9 100644 --- a/django/contrib/gis/db/backends/postgis/operations.py +++ b/django/contrib/gis/db/backends/postgis/operations.py @@ -304,7 +304,7 @@ def get_distance(self, f, dist_val, lookup_type): return [dist_param] - def get_geom_placeholder(self, f, value, compiler): + def get_geom_placeholder_sql(self, f, value, compiler): """ Provide a proper substitution value for Geometries or rasters that are not in the SRID of the field. Specifically, this routine will @@ -312,11 +312,11 @@ def get_geom_placeholder(self, f, value, compiler): """ transform_func = self.spatial_function_name("Transform") if hasattr(value, "as_sql"): - if value.field.srid == f.srid: - placeholder = "%s" - else: - placeholder = "%s(%%s, %s)" % (transform_func, f.srid) - return placeholder + sql, params = compiler.compile(value) + if value.field.srid != f.srid: + sql = f"{transform_func}({sql}, %s)" + params = (*params, f.srid) + return sql, params # Get the srid for this object if value is None: @@ -327,11 +327,9 @@ def get_geom_placeholder(self, f, value, compiler): # Adding Transform() to the SQL placeholder if the value srid # is not equal to the field srid. if value_srid is None or value_srid == f.srid: - placeholder = "%s" + return "%s", (value,) else: - placeholder = "%s(%%s, %s)" % (transform_func, f.srid) - - return placeholder + return f"{transform_func}(%s, %s)", (value, f.srid) def _get_postgis_func(self, func): """ diff --git a/django/contrib/gis/db/models/fields.py b/django/contrib/gis/db/models/fields.py index d1c1a5937e33..14076d305b72 100644 --- a/django/contrib/gis/db/models/fields.py +++ b/django/contrib/gis/db/models/fields.py @@ -135,12 +135,12 @@ def geodetic(self, connection): """ return get_srid_info(self.srid, connection).geodetic - def get_placeholder(self, value, compiler, connection): + def get_placeholder_sql(self, value, compiler, connection): """ Return the placeholder for the spatial column for the given value. """ - return connection.ops.get_geom_placeholder(self, value, compiler) + return connection.ops.get_geom_placeholder_sql(self, value, compiler) def get_srid(self, obj): """ diff --git a/django/contrib/gis/db/models/lookups.py b/django/contrib/gis/db/models/lookups.py index c0cb80ea7005..3c2f4c3be461 100644 --- a/django/contrib/gis/db/models/lookups.py +++ b/django/contrib/gis/db/models/lookups.py @@ -62,12 +62,12 @@ def process_rhs(self, compiler, connection): # If rhs is some Query, don't touch it. return super().process_rhs(compiler, connection) if isinstance(self.rhs, Expression): - self.rhs = self.rhs.resolve_expression(compiler.query) - rhs, rhs_params = super().process_rhs(compiler, connection) - placeholder = connection.ops.get_geom_placeholder( - self.lhs.output_field, self.rhs, compiler + rhs = self.rhs.resolve_expression(compiler.query) + else: + rhs = connection.ops.Adapter(self.rhs) + return connection.ops.get_geom_placeholder_sql( + self.lhs.output_field, rhs, compiler ) - return placeholder % rhs, rhs_params def get_rhs_op(self, connection, rhs): # Unlike BuiltinLookup, the GIS get_rhs_op() implementation should diff --git a/django/contrib/postgres/fields/array.py b/django/contrib/postgres/fields/array.py index f867571215f2..58341e02abb7 100644 --- a/django/contrib/postgres/fields/array.py +++ b/django/contrib/postgres/fields/array.py @@ -124,8 +124,12 @@ def db_parameters(self, connection): db_params["collation"] = self.db_collation return db_params - def get_placeholder(self, value, compiler, connection): - return "%s::{}".format(self.db_type(connection)) + def get_placeholder_sql(self, value, compiler, connection): + db_type = self.db_type(connection) + if hasattr(value, "as_sql"): + sql, params = compiler.compile(value) + return f"{sql}::{db_type}", params + return f"%s::{db_type}", (value,) def get_db_prep_value(self, value, connection, prepared=False): if isinstance(value, (list, tuple)): diff --git a/django/contrib/postgres/fields/ranges.py b/django/contrib/postgres/fields/ranges.py index 33c6611f8672..22a56a36336b 100644 --- a/django/contrib/postgres/fields/ranges.py +++ b/django/contrib/postgres/fields/ranges.py @@ -84,8 +84,12 @@ def model(self, model): def _choices_is_value(cls, value): return isinstance(value, (list, tuple)) or super()._choices_is_value(value) - def get_placeholder(self, value, compiler, connection): - return "%s::{}".format(self.db_type(connection)) + def get_placeholder_sql(self, value, compiler, connection): + db_type = self.db_type(connection) + if hasattr(value, "as_sql"): + sql, params = compiler.compile(value) + return f"{sql}::{db_type}", params + return f"%s::{db_type}", (value,) def get_prep_value(self, value): if value is None: diff --git a/django/db/backends/base/operations.py b/django/db/backends/base/operations.py index bc1752df6fa4..29ef7d93d166 100644 --- a/django/db/backends/base/operations.py +++ b/django/db/backends/base/operations.py @@ -734,12 +734,14 @@ def combine_expression(self, connector, sub_expressions): def combine_duration_expression(self, connector, sub_expressions): return self.combine_expression(connector, sub_expressions) - def binary_placeholder_sql(self, value): + def binary_placeholder_sql(self, value, compiler): """ Some backends require special syntax to insert binary content (MySQL for example uses '_binary %s'). """ - return "%s" + if hasattr(value, "as_sql"): + return compiler.compile(value) + return "%s", (value,) def modify_insert_params(self, placeholder, params): """ diff --git a/django/db/backends/mysql/operations.py b/django/db/backends/mysql/operations.py index 74ba72f31614..61fc9da3f49e 100644 --- a/django/db/backends/mysql/operations.py +++ b/django/db/backends/mysql/operations.py @@ -306,10 +306,12 @@ def convert_uuidfield_value(self, value, expression, connection): value = uuid.UUID(value) return value - def binary_placeholder_sql(self, value): - return ( - "_binary %s" if value is not None and not hasattr(value, "as_sql") else "%s" - ) + def binary_placeholder_sql(self, value, compiler): + if value is None: + return "%s", (None,) + elif hasattr(value, "as_sql"): + return compiler.compile(value) + return "_binary %s", (value,) def subtract_temporals(self, internal_type, lhs, rhs): lhs_sql, lhs_params = lhs diff --git a/django/db/backends/postgresql/compiler.py b/django/db/backends/postgresql/compiler.py index 9d383b52e8cc..ad8359bfaffe 100644 --- a/django/db/backends/postgresql/compiler.py +++ b/django/db/backends/postgresql/compiler.py @@ -36,11 +36,11 @@ def assemble_as_sql(self, fields, value_rows): # Lack of fields denote the usage of the DEFAULT keyword # for the insertion of empty rows. or any(field is None for field in fields) - # Field.get_placeholder takes value as an argument, so the + # Field.get_placeholder_sql takes value as an argument, so the # resulting placeholder might be dependent on the value. # in UNNEST requires a single placeholder to "fit all values" in # the array. - or any(hasattr(field, "get_placeholder") for field in fields) + or any(hasattr(field, "get_placeholder_sql") for field in fields) # Fields that don't use standard internal types might not be # unnest'able (e.g. array and geometry types are known to be # problematic). diff --git a/django/db/models/expressions.py b/django/db/models/expressions.py index 9d3c1241e767..c6ba5c89a92e 100644 --- a/django/db/models/expressions.py +++ b/django/db/models/expressions.py @@ -705,10 +705,14 @@ def register_combinable_fields(lhs, connector, rhs, result): _connector_combinators[connector].append((lhs, rhs, result)) -for d in _connector_combinations: - for connector, field_types in d.items(): - for lhs, rhs, result in field_types: - register_combinable_fields(lhs, connector, rhs, result) +def _register_combinable_fields(): + for d in _connector_combinations: + for connector, field_types in d.items(): + for lhs, rhs, result in field_types: + register_combinable_fields(lhs, connector, rhs, result) + + +_register_combinable_fields() @functools.lru_cache(maxsize=128) @@ -1173,8 +1177,12 @@ def as_sql(self, compiler, connection): val = output_field.get_db_prep_save(val, connection=connection) else: val = output_field.get_db_prep_value(val, connection=connection) - if hasattr(output_field, "get_placeholder"): - return output_field.get_placeholder(val, compiler, connection), [val] + try: + get_placeholder_sql = output_field.get_placeholder_sql + except AttributeError: + pass + else: + return get_placeholder_sql(val, compiler, connection) if val is None: # oracledb does not always convert None to the appropriate # NULL type (like in case expressions using numbers), so we diff --git a/django/db/models/fields/__init__.py b/django/db/models/fields/__init__.py index feb0441bab80..d98319ef0076 100644 --- a/django/db/models/fields/__init__.py +++ b/django/db/models/fields/__init__.py @@ -30,6 +30,7 @@ parse_duration, parse_time, ) +from django.utils.deprecation import RemovedInDjango70Warning, django_file_prefixes from django.utils.duration import duration_string from django.utils.functional import Promise, cached_property from django.utils.ipv6 import MAX_IPV6_ADDRESS_LENGTH, clean_ipv6_address @@ -181,6 +182,32 @@ def _description(self): description = property(_description) + def __init_subclass__(cls, **kwargs): + # RemovedInDjango70Warning: When the deprecation ends, remove + # completely. + # Allow for both `get_placeholder` and `get_placeholder_sql` to + # be declared to ease the deprecation process for third-party apps. + if ( + get_placeholder := cls.__dict__.get("get_placeholder") + ) is not None and "get_placeholder_sql" not in cls.__dict__: + warnings.warn( + "Field.get_placeholder is deprecated in favor of get_placeholder_sql. " + f"Define {cls.__module__}.{cls.__qualname__}.get_placeholder_sql " + "to return both SQL and parameters instead.", + category=RemovedInDjango70Warning, + skip_file_prefixes=django_file_prefixes(), + ) + + def get_placeholder_sql(self, value, compiler, connection): + placeholder = get_placeholder(self, value, compiler, connection) + if hasattr(value, "as_sql"): + sql, params = compiler.compile(value) + return placeholder % sql, params + return placeholder, (value,) + + setattr(cls, "get_placeholder_sql", get_placeholder_sql) + return super().__init_subclass__(**kwargs) + def __init__( self, verbose_name=None, @@ -2735,8 +2762,8 @@ def deconstruct(self): def get_internal_type(self): return "BinaryField" - def get_placeholder(self, value, compiler, connection): - return connection.ops.binary_placeholder_sql(value) + def get_placeholder_sql(self, value, compiler, connection): + return connection.ops.binary_placeholder_sql(value, compiler) def get_default(self): if self.has_default() and not callable(self.default): diff --git a/django/db/models/sql/compiler.py b/django/db/models/sql/compiler.py index 8742de00d69f..6c758fb5261a 100644 --- a/django/db/models/sql/compiler.py +++ b/django/db/models/sql/compiler.py @@ -1690,11 +1690,12 @@ class SQLInsertCompiler(SQLCompiler): returning_fields = None returning_params = () - def field_as_sql(self, field, get_placeholder, val): + def field_as_sql(self, field, get_placeholder_sql, val): """ Take a field and a value intended to be saved on that field, and return placeholder SQL and accompanying params. Check for raw values, - expressions, and fields with get_placeholder() defined in that order. + fields with get_placeholder_sql(), and compilable defined in that + order. When field is None, consider the value raw and use it as the placeholder, with no corresponding parameters returned. @@ -1702,13 +1703,13 @@ def field_as_sql(self, field, get_placeholder, val): if field is None: # A field value of None means the value is raw. sql, params = val, [] + elif get_placeholder_sql is not None: + # Some fields (e.g. geo fields) need special munging before + # they can be inserted. + sql, params = get_placeholder_sql(val, self, self.connection) elif hasattr(val, "as_sql"): # This is an expression, let's compile it. sql, params = self.compile(val) - elif get_placeholder is not None: - # Some fields (e.g. geo fields) need special munging before - # they can be inserted. - sql, params = get_placeholder(val, self, self.connection), [val] else: # Return the common case for the placeholder sql, params = "%s", [val] @@ -1777,11 +1778,15 @@ def assemble_as_sql(self, fields, value_rows): # list of (sql, [params]) tuples for each object to be saved # Shape: [n_objs][n_fields][2] - get_placeholders = [getattr(field, "get_placeholder", None) for field in fields] + get_placeholder_sqls = [ + getattr(field, "get_placeholder_sql", None) for field in fields + ] rows_of_fields_as_sql = ( ( - self.field_as_sql(field, get_placeholder, value) - for field, get_placeholder, value in zip(fields, get_placeholders, row) + self.field_as_sql(field, get_placeholder_sql, value) + for field, get_placeholder_sql, value in zip( + fields, get_placeholder_sqls, row + ) ) for row in value_rows ) @@ -2078,21 +2083,20 @@ def as_sql(self): ) val = field.get_db_prep_save(val, connection=self.connection) - # Getting the placeholder for the field. - if hasattr(field, "get_placeholder"): - placeholder = field.get_placeholder(val, self, self.connection) - else: - placeholder = "%s" - name = field.column - if hasattr(val, "as_sql"): + quoted_name = qn(field.column) + if ( + get_placeholder_sql := getattr(field, "get_placeholder_sql", None) + ) is not None: + sql, params = get_placeholder_sql(val, self, self.connection) + values.append(f"{quoted_name} = {sql}") + update_params.extend(params) + elif hasattr(val, "as_sql"): sql, params = self.compile(val) - values.append("%s = %s" % (qn(name), placeholder % sql)) + values.append(f"{quoted_name} = {sql}") update_params.extend(params) - elif val is not None: - values.append("%s = %s" % (qn(name), placeholder)) - update_params.append(val) else: - values.append("%s = NULL" % qn(name)) + values.append(f"{quoted_name} = %s") + update_params.append(val) table = self.query.base_table result = [ "UPDATE %s SET" % qn(table), diff --git a/django/template/smartif.py b/django/template/smartif.py index c0682f3b1e60..ad4d01f89895 100644 --- a/django/template/smartif.py +++ b/django/template/smartif.py @@ -111,10 +111,14 @@ def eval(self, context): "<=": infix(10, lambda context, x, y: x.eval(context) <= y.eval(context)), } + # Assign 'id' to each: -for key, op in OPERATORS.items(): - op.id = key -del key, op +def _init_operators(): + for key, op in OPERATORS.items(): + op.id = key + + +_init_operators() class Literal(TokenBase): diff --git a/docs/internals/deprecation.txt b/docs/internals/deprecation.txt index 280187b36872..8962cbde62d7 100644 --- a/docs/internals/deprecation.txt +++ b/docs/internals/deprecation.txt @@ -64,6 +64,9 @@ details on these changes. :class:`~django.db.models.JSONNull` to query for a JSON ``null`` value instead. +* The ``Field.get_placeholder_sql`` shim over the deprecated + ``get_placeholder`` method will be removed. + .. _deprecation-removed-in-6.1: 6.1 diff --git a/docs/releases/6.1.txt b/docs/releases/6.1.txt index 2539e599c786..67891c5bf738 100644 --- a/docs/releases/6.1.txt +++ b/docs/releases/6.1.txt @@ -396,6 +396,15 @@ backends. * Set the new ``DatabaseFeatures.supports_inspectdb`` attribute to ``False`` if the management command isn't supported. +* The ``DatabaseOperations.binary_placeholder_sql()`` method now expects a + query compiler as an extra positional argument and should return a + two-elements tuple composed of an SQL format string and a tuple of associated + parameters. + +* The ``BaseSpatialOperations.get_geom_placeholder()`` method is renamed to + ``get_geom_placeholder_sql`` and is expected to return a two-elements tuple + composed of an SQL format string and a tuple of associated parameters. + :mod:`django.contrib.admin` --------------------------- @@ -481,6 +490,14 @@ Miscellaneous used as the top-level value. :lookup:`Key and index lookups ` are unaffected by this deprecation. +* The undocumented ``get_placeholder`` method of + :class:`~django.db.models.Field` is deprecated in favor of the newly + introduced ``get_placeholder_sql`` method, which has the same input signature + but is expected to return a two-elements tuple composed of an SQL format + string and a tuple of associated parameters. This method should now expect + to be provided expressions meant to be compiled via the provided ``compiler`` + argument. + Features removed in 6.1 ======================= diff --git a/tests/model_fields/tests.py b/tests/model_fields/tests.py index 3d856d36c56d..b27c07a92f07 100644 --- a/tests/model_fields/tests.py +++ b/tests/model_fields/tests.py @@ -1,10 +1,12 @@ import pickle +import warnings from django import forms from django.core.exceptions import ValidationError -from django.db import models +from django.db import connection, models from django.test import SimpleTestCase, TestCase from django.utils.choices import CallableChoiceIterator +from django.utils.deprecation import RemovedInDjango70Warning from django.utils.functional import lazy from .models import ( @@ -147,6 +149,41 @@ class MyModel(models.Model): self.assertEqual(field_hash, hash(field)) + def test_get_placeholder_deprecation(self): + # RemovedInDjango70Warning: When the deprecation ends, remove + # completely. + msg = ( + "Field.get_placeholder is deprecated in favor of get_placeholder_sql. " + "Define model_fields.tests.BasicFieldTests." + "test_get_placeholder_deprecation..SomeField.get_placeholder_sql " + "to return both SQL and parameters instead." + ) + with self.assertWarnsMessage(RemovedInDjango70Warning, msg): + + class SomeField(models.Field): + def get_placeholder(self, value, compiler, connection): + return "%s" + + def test_get_placeholder_sql_shim(self): + # RemovedInDjango70Warning: When the deprecation ends, remove + # completely. + with warnings.catch_warnings(record=True): + warnings.simplefilter("always") + + class SomeField(models.Field): + def get_placeholder(self, value, compiler, connection): + return "(1 + %s)" + + compiler = Bar.objects.all().query.get_compiler(connection=connection) + self.assertEqual( + SomeField().get_placeholder_sql(2, compiler, connection), + ("(1 + %s)", (2,)), + ) + self.assertEqual( + SomeField().get_placeholder_sql(models.Value(2), compiler, connection), + ("(1 + %s)", (2,)), + ) + class ChoicesTests(SimpleTestCase): @classmethod diff --git a/tests/postgres_tests/fields.py b/tests/postgres_tests/fields.py index d099effdd56c..77370611ea43 100644 --- a/tests/postgres_tests/fields.py +++ b/tests/postgres_tests/fields.py @@ -60,5 +60,9 @@ def get_prep_value(self, value): class OffByOneField(models.IntegerField): - def get_placeholder(self, value, compiler, connection): - return "(%s + 1)" + def get_placeholder_sql(self, value, compiler, connection): + if hasattr(value, "as_sql"): + sql, params = compiler.compile(value) + else: + sql, params = "%s", (value,) + return f"({sql} + %s)", (*params, 1)