- A :func:`.bindparam` construct with a "null" type (e.g. no type

specified) is now copied when used in a typed expression, and the
new copy is assigned the actual type of the compared column.  Previously,
this logic would occur on the given :func:`.bindparam` in place.
Additionally, a similar process now occurs for :func:`.bindparam` constructs
passed to :meth:`.ValuesBase.values` for a :class:`.Insert` or
:class:`.Update` construct. [ticket:2850]

5 files changed, 110 insertions, 14 deletions

diff --git a/doc/build/changelog/changelog_09.rst b/doc/build/changelog/changelog_09.rst
index c6901c14a..16297e23f 100644
--- a/doc/build/changelog/changelog_09.rst
+++ b/doc/build/changelog/changelog_09.rst
@@ -14,6 +14,25 @@
 
     .. change::
         :tags: feature, sql
+        :tickets: 2850
+
+        A :func:`.bindparam` construct with a "null" type (e.g. no type
+        specified) is now copied when used in a typed expression, and the
+        new copy is assigned the actual type of the compared column.  Previously,
+        this logic would occur on the given :func:`.bindparam` in place.
+        Additionally, a similar process now occurs for :func:`.bindparam` constructs
+        passed to :meth:`.ValuesBase.values` for a :class:`.Insert` or
+        :class:`.Update` construct.
+
+        These are both subtle behavioral changes which may impact some
+        usages.
+
+        .. seealso::
+
+            :ref:`migration_2850`
+
+    .. change::
+        :tags: feature, sql
         :tickets: 2838
 
         The typing system now handles the task of rendering "literal bind" values,
diff --git a/doc/build/changelog/migration_09.rst b/doc/build/changelog/migration_09.rst
index 969e2b5da..d80bc3040 100644
--- a/doc/build/changelog/migration_09.rst
+++ b/doc/build/changelog/migration_09.rst
@@ -321,6 +321,56 @@ against ``b_value`` directly.
 
 :ticket:`2751`
 
+.. _migration_2850:
+
+A bindparam() construct with no type gets upgraded via copy when a type is available
+------------------------------------------------------------------------------------
+
+The logic which "upgrades" a :func:`.bindparam` construct to take on the
+type of the enclosing expression has been improved in two ways.  First, the
+:func:`.bindparam` object is **copied** before the new type is assigned, so that
+the given :func:`.bindparam` is not mutated in place.  Secondly, this same
+operation occurs within the :meth:`.Values.values` method of :class:`.Insert`
+and :class:`.Update`.
+
+If given an untyped :func:`.bindparam`::
+
+    bp = bindparam("some_col")
+
+If we use this parameter as follows::
+
+    expr = mytable.c.col == bp
+
+The type for ``bp`` remains as ``NullType``, however if ``mytable.c.col``
+is of type ``String``, then ``expr.right``, that is the right side of the
+binary expression, will take on the ``String`` type.   Previously, ``bp`` itself
+would have been changed in place to have ``String`` as its type.
+
+Similarly, this operation occurs in an :class:`.Insert` or :class:`.Update`::
+
+    stmt = mytable.update().values(col=bp)
+
+Above, ``bp`` remains unchanged, but the ``String`` type will be used when
+the statement is executed, which we can see by examining the ``binds`` dictionary::
+
+    >>> compiled = stmt.compile()
+    >>> compiled.binds['some_col'].type
+    String
+
+The feature allows custom types to take their expected effect within INSERT/UPDATE
+statements without needing to explicitly specify those types within every
+:func:`.bindparam` expression.
+
+The potentially backwards-compatible changes involve two unlikely
+scenarios.  Since the the bound parameter is
+**cloned**, users should not be relying upon making in-place changes to a
+:func:`.bindparam` construct once created.   Additionally, code which uses
+:func:`.bindparam` within an :class:`.Insert` or :class:`.Update` statement
+which is relying on the fact that the :func:`.bindparam` is not typed according
+to the column being assigned towards will no longer function in that way.
+
+:ticket:`2850`
+
 .. _change_2812:
 
 Schema identifiers now carry along their own quoting information
diff --git a/lib/sqlalchemy/sql/compiler.py b/lib/sqlalchemy/sql/compiler.py
index 5c7a29f99..f526203ac 100644
--- a/lib/sqlalchemy/sql/compiler.py
+++ b/lib/sqlalchemy/sql/compiler.py
@@ -1934,16 +1934,22 @@ class SQLCompiler(Compiled):
                                         if not stmt._has_multi_parameters
                                         else "%s_0" % c.key
                                     )
-                elif c.primary_key and implicit_returning:
-                    self.returning.append(c)
-                    value = self.process(value.self_group(), **kw)
-                elif implicit_return_defaults and \
-                    c in implicit_return_defaults:
-                    self.returning.append(c)
-                    value = self.process(value.self_group(), **kw)
                 else:
-                    self.postfetch.append(c)
-                    value = self.process(value.self_group(), **kw)
+                    if isinstance(value, elements.BindParameter) and \
+                        value.type._isnull:
+                        value = value._clone()
+                        value.type = c.type
+
+                    if c.primary_key and implicit_returning:
+                        self.returning.append(c)
+                        value = self.process(value.self_group(), **kw)
+                    elif implicit_return_defaults and \
+                        c in implicit_return_defaults:
+                        self.returning.append(c)
+                        value = self.process(value.self_group(), **kw)
+                    else:
+                        self.postfetch.append(c)
+                        value = self.process(value.self_group(), **kw)
                 values.append((c, value))
 
             elif self.isinsert:
diff --git a/lib/sqlalchemy/sql/default_comparator.py b/lib/sqlalchemy/sql/default_comparator.py
index c9125108a..7c803ac4c 100644
--- a/lib/sqlalchemy/sql/default_comparator.py
+++ b/lib/sqlalchemy/sql/default_comparator.py
@@ -261,10 +261,7 @@ class _DefaultColumnComparator(operators.ColumnOperators):
         if isinstance(other, (ColumnElement, TextClause)):
             if isinstance(other, BindParameter) and \
                     other.type._isnull:
-                # TODO: perhaps we should not mutate the incoming
-                # bindparam() here and instead make a copy of it.
-                # this might be the only place that we're mutating
-                # an incoming construct.
+                other = other._clone()
                 other.type = expr.type
             return other
         elif hasattr(other, '__clause_element__'):
diff --git a/test/sql/test_types.py b/test/sql/test_types.py
index f739707f3..13d4e378e 100644
--- a/test/sql/test_types.py
+++ b/test/sql/test_types.py
@@ -1057,6 +1057,8 @@ class ExpressionTest(fixtures.TestBase, AssertsExecutionResults, AssertsCompiled
                 def process(value):
                     return value / 10
                 return process
+
+        class MyOldCustomType(MyCustomType):
             def adapt_operator(self, op):
                 return {operators.add: operators.sub,
                     operators.sub: operators.add}.get(op, op)
@@ -1133,6 +1135,26 @@ class ExpressionTest(fixtures.TestBase, AssertsExecutionResults, AssertsCompiled
                 datetime.date(2007, 10, 15), 25, 'BIND_INfooBIND_OUT')]
         )
 
+    def test_bind_adapt_update(self):
+        bp = bindparam("somevalue")
+        stmt = test_table.update().values(avalue=bp)
+        compiled = stmt.compile()
+        eq_(bp.type._type_affinity, types.NullType)
+        eq_(compiled.binds['somevalue'].type._type_affinity, MyCustomType)
+
+    def test_bind_adapt_insert(self):
+        bp = bindparam("somevalue")
+        stmt = test_table.insert().values(avalue=bp)
+        compiled = stmt.compile()
+        eq_(bp.type._type_affinity, types.NullType)
+        eq_(compiled.binds['somevalue'].type._type_affinity, MyCustomType)
+
+    def test_bind_adapt_expression(self):
+        bp = bindparam("somevalue")
+        stmt = test_table.c.avalue == bp
+        eq_(bp.type._type_affinity, types.NullType)
+        eq_(stmt.right.type._type_affinity, MyCustomType)
+
     def test_literal_adapt(self):
         # literals get typed based on the types dictionary, unless
         # compatible with the left side type
@@ -1264,7 +1286,9 @@ class ExpressionTest(fixtures.TestBase, AssertsExecutionResults, AssertsCompiled
         assert expr.right.type._type_affinity is MyFoobarType
 
         # untyped bind - it gets assigned MyFoobarType
-        expr = column("foo", MyFoobarType) + bindparam("foo")
+        bp = bindparam("foo")
+        expr = column("foo", MyFoobarType) + bp
+        assert bp.type._type_affinity is types.NullType
         assert expr.right.type._type_affinity is MyFoobarType
 
         expr = column("foo", MyFoobarType) + bindparam("foo", type_=Integer)