Fixed #36727 -- Deprecated Field.get_placeholder in favor of get_placeholder_sql.

The lack of ability of the get_placeholder call chain to return SQL and
parameters separated so they can be mogrified by the backend at execution time
forced implementations to dangerously interpolate potentially user controlled
values.

The get_placeholder_sql name was chosen due to its proximity to the previous
method, but other options such as Field.as_sql were considered but ultimately
rejected due to its different input signature compared to Expression.as_sql
that might have lead to confusion.

There is a lot of overlap between what Field.get_db_prep_value and
get_placeholder_sql do but folding the latter in the former would require
changing its return signature to return expression which is a way more invasive
change than what is proposed here.

Given we always call get_db_prep_value it might still be an avenue worth
exploring in the future to offer a publicly documented interface to allow field
to take an active part in the compilation chain.

Thanks Jacob for the review.

7 files changed, 45 insertions, 41 deletions

diff --git a/django/contrib/gis/db/backends/base/operations.py b/django/contrib/gis/db/backends/base/operations.py
index e468c6f610..3ff13dfc86 100644
--- a/django/contrib/gis/db/backends/base/operations.py
+++ b/django/contrib/gis/db/backends/base/operations.py
@@ -115,7 +115,7 @@ class BaseSpatialOperations:
             "Distance operations not available on this spatial backend."
         )
 
-    def get_geom_placeholder(self, f, value, compiler):
+    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
@@ -123,28 +123,26 @@ class BaseSpatialOperations:
         backend.
         """
 
-        def transform_value(value, field):
+        def must_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 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 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 1c09281809..918ab7ce64 100644
--- a/django/contrib/gis/db/backends/oracle/operations.py
+++ b/django/contrib/gis/db/backends/oracle/operations.py
@@ -204,10 +204,10 @@ class OracleOperations(BaseSpatialOperations, DatabaseOperations):
 
         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 f4e70c9204..3c5ea7687f 100644
--- a/django/contrib/gis/db/backends/postgis/operations.py
+++ b/django/contrib/gis/db/backends/postgis/operations.py
@@ -304,7 +304,7 @@ class PostGISOperations(BaseSpatialOperations, DatabaseOperations):
 
         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 @@ class PostGISOperations(BaseSpatialOperations, DatabaseOperations):
         """
         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 @@ class PostGISOperations(BaseSpatialOperations, DatabaseOperations):
         # 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 d1c1a5937e..14076d305b 100644
--- a/django/contrib/gis/db/models/fields.py
+++ b/django/contrib/gis/db/models/fields.py
@@ -135,12 +135,12 @@ class BaseSpatialField(Field):
         """
         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 c0cb80ea70..3c2f4c3be4 100644
--- a/django/contrib/gis/db/models/lookups.py
+++ b/django/contrib/gis/db/models/lookups.py
@@ -62,12 +62,12 @@ class GISLookup(Lookup):
             # 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 f867571215..58341e02ab 100644
--- a/django/contrib/postgres/fields/array.py
+++ b/django/contrib/postgres/fields/array.py
@@ -124,8 +124,12 @@ class ArrayField(CheckPostgresInstalledMixin, CheckFieldDefaultMixin, Field):
         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 33c6611f86..22a56a3633 100644
--- a/django/contrib/postgres/fields/ranges.py
+++ b/django/contrib/postgres/fields/ranges.py
@@ -84,8 +84,12 @@ class RangeField(CheckPostgresInstalledMixin, models.Field):
     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: