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.
Also in 0.7.10. [ticket:2631]

diff --git a/doc/build/changelog/changelog_07.rst b/doc/build/changelog/changelog_07.rst
index 092e8af40fbc77e5348beac23c891dc1d64db769..d8e4e06cec7a7f7cdea2212dadcc437597bb53af 100644 (file)
--- a/doc/build/changelog/changelog_07.rst
+++ b/doc/build/changelog/changelog_07.rst
@@ -8,6 +8,18 @@
     :version: 0.7.10
     :released:
 
+    .. 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/doc/build/changelog/changelog_08.rst b/doc/build/changelog/changelog_08.rst
index 012a052c8300ce0ddada0adc372bdbd5b2af9e41..c472b444a77cc27a87bc0764ef16ceb1e721e062 100644 (file)
--- a/doc/build/changelog/changelog_08.rst
+++ b/doc/build/changelog/changelog_08.rst
@@ -6,6 +6,19 @@
 .. changelog::
     :version: 0.8.0b2
 
+    .. 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.
+      Also in 0.7.10.
+
     .. change::
         :tags: sql, bug
         :tickets: 2610

diff --git a/lib/sqlalchemy/schema.py b/lib/sqlalchemy/schema.py
index 4f3655a1dac4447c2a6bc9a442dabfcdaaade786..553936299f273a7fed94765a77628ec20b42201d 100644 (file)
--- a/lib/sqlalchemy/schema.py
+++ b/lib/sqlalchemy/schema.py
@@ -945,7 +945,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))
 
@@ -957,7 +957,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))
@@ -1854,6 +1854,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 ba5c9e9874d32a410a15af7da26dd018856d0c0d..4627a966278ec732a01581c4d62c8338a12c2cae 100644 (file)
--- a/test/sql/test_metadata.py
+++ b/test/sql/test_metadata.py
@@ -1300,6 +1300,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)