Fixed bug where using server_onupdate=<FetchedValue|DefaultClause>

without passing the "for_update=True" flag would apply the default
object to the server_default, blowing away whatever was there.
The explicit for_update=True argument shouldn't be needed with this usage
(especially since the documentation shows an example without it being
used) so it is now arranged internally using a copy of the given default
object, if the flag isn't set to what corresponds to that argument.
[ticket:2631]

diff --git a/doc/build/changelog/changelog_07.rst b/doc/build/changelog/changelog_07.rst
index 5132bd961321bb2cf4571c61a081da7334288677..a4e6b442a300ac3742183f19aeafdae7460b8342 100644 (file)
--- a/doc/build/changelog/changelog_07.rst
+++ b/doc/build/changelog/changelog_07.rst
@@ -27,6 +27,18 @@
       to the MSSQL dialect's "schema rendering"
       logic's failure to take .key into account.
 
+    .. change::
+        :tags: sql, bug
+        :tickets: 2631
+
+      Fixed bug where using server_onupdate=<FetchedValue|DefaultClause>
+      without passing the "for_update=True" flag would apply the default
+      object to the server_default, blowing away whatever was there.
+      The explicit for_update=True argument shouldn't be needed with this usage
+      (especially since the documentation shows an example without it being
+      used) so it is now arranged internally using a copy of the given default
+      object, if the flag isn't set to what corresponds to that argument.
+
     .. change::
         :tags: oracle, bug
         :tickets: 2620

diff --git a/lib/sqlalchemy/schema.py b/lib/sqlalchemy/schema.py
index 8545a0f9b18bff018b850e1260d9956744f8333a..00537f1f9866cd694afa0a95c938da43374ede43 100644 (file)
--- a/lib/sqlalchemy/schema.py
+++ b/lib/sqlalchemy/schema.py
@@ -923,7 +923,7 @@ class Column(SchemaItem, expression.ColumnClause):
 
         if self.server_default is not None:
             if isinstance(self.server_default, FetchedValue):
-                args.append(self.server_default)
+                args.append(self.server_default._as_for_update(False))
             else:
                 args.append(DefaultClause(self.server_default))
 
@@ -935,7 +935,7 @@ class Column(SchemaItem, expression.ColumnClause):
 
         if self.server_onupdate is not None:
             if isinstance(self.server_onupdate, FetchedValue):
-                args.append(self.server_onupdate)
+                args.append(self.server_onupdate._as_for_update(True))
             else:
                 args.append(DefaultClause(self.server_onupdate,
                                             for_update=True))
@@ -1763,6 +1763,19 @@ class FetchedValue(_NotAColumnExpr, events.SchemaEventTarget):
     def __init__(self, for_update=False):
         self.for_update = for_update
 
+    def _as_for_update(self, for_update):
+        if for_update == self.for_update:
+            return self
+        else:
+            return self._clone(for_update)
+
+    def _clone(self, for_update):
+        n = self.__class__.__new__(self.__class__)
+        n.__dict__.update(self.__dict__)
+        n.__dict__.pop('column', None)
+        n.for_update = for_update
+        return n
+
     def _set_parent(self, column):
         self.column = column
         if self.for_update:

diff --git a/test/sql/test_metadata.py b/test/sql/test_metadata.py
index 7e77adf5c38a2a916c5f254ff102373c9daddc68..781d4a3da8f700e029b4ee923bf137c16284ba3c 100644 (file)
--- a/test/sql/test_metadata.py
+++ b/test/sql/test_metadata.py
@@ -1229,6 +1229,37 @@ class ColumnDefaultsTest(fixtures.TestBase):
         assert c.server_default is target
         assert target.column is c
 
+    def test_onupdate_default_not_server_default_one(self):
+        target1 = schema.DefaultClause('y')
+        target2 = schema.DefaultClause('z')
+
+        c = self._fixture(server_default=target1, server_onupdate=target2)
+        eq_(c.server_default.arg, 'y')
+        eq_(c.server_onupdate.arg, 'z')
+
+    def test_onupdate_default_not_server_default_two(self):
+        target1 = schema.DefaultClause('y', for_update=True)
+        target2 = schema.DefaultClause('z', for_update=True)
+
+        c = self._fixture(server_default=target1, server_onupdate=target2)
+        eq_(c.server_default.arg, 'y')
+        eq_(c.server_onupdate.arg, 'z')
+
+    def test_onupdate_default_not_server_default_three(self):
+        target1 = schema.DefaultClause('y', for_update=False)
+        target2 = schema.DefaultClause('z', for_update=True)
+
+        c = self._fixture(target1, target2)
+        eq_(c.server_default.arg, 'y')
+        eq_(c.server_onupdate.arg, 'z')
+
+    def test_onupdate_default_not_server_default_four(self):
+        target1 = schema.DefaultClause('y', for_update=False)
+
+        c = self._fixture(server_onupdate=target1)
+        is_(c.server_default, None)
+        eq_(c.server_onupdate.arg, 'y')
+
     def test_server_default_keyword_as_schemaitem(self):
         target = schema.DefaultClause('y')
         c = self._fixture(server_default=target)