- merge rfc0b9df5d9e0 from 0.6 branch

diff --git a/CHANGES b/CHANGES
index 5e79a27aec7345e8f732d31fc655e125c45da77a..2a6bb1e797f7e7463eeb162b3a5f893a1ffe9ebc 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -46,6 +46,11 @@ CHANGES
     objects, despite passive_deletes remaining at 
     its default of False.  [ticket:2002]
     
+  - A warning is emitted when version_id_col is specified
+    on an inheriting mapper when the inherited mapper
+    already has one, if those column expressions are not
+    the same.  [ticket:1987]
+    
   - "innerjoin" flag doesn't take effect along the chain
     of joinedload() joins if a previous join in that chain
     is an outer join, thus allowing primary rows without

diff --git a/lib/sqlalchemy/orm/mapper.py b/lib/sqlalchemy/orm/mapper.py
index e9271008edae1fda5bd98b109678387d3c9bd7d5..346d7d4bf6d39cfc4cab6d028fe19774044289ec 100644 (file)
--- a/lib/sqlalchemy/orm/mapper.py
+++ b/lib/sqlalchemy/orm/mapper.py
@@ -279,6 +279,17 @@ class Mapper(object):
             if self.version_id_col is None:
                 self.version_id_col = self.inherits.version_id_col
                 self.version_id_generator = self.inherits.version_id_generator
+            elif self.inherits.version_id_col is not None and \
+                self.version_id_col is not self.inherits.version_id_col:
+                util.warn(
+                    "Inheriting version_id_col '%s' does not match inherited "
+                    "version_id_col '%s' and will not automatically populate "
+                    "the inherited versioning column. "
+                    "version_id_col should only be specified on "
+                    "the base-most mapper that includes versioning." %
+                    (self.version_id_col.description, 
+                    self.inherits.version_id_col.description)
+                )
 
             if self.order_by is False and \
                         not self.concrete and \

diff --git a/test/orm/test_versioning.py b/test/orm/test_versioning.py
index 75f7fbb6e85ae666eb35a234a79cec1c71a96a75..94c0efc10fb3874d90a0a5ca87201f784ac3255e 100644 (file)
--- a/test/orm/test_versioning.py
+++ b/test/orm/test_versioning.py
@@ -1,8 +1,10 @@
 import sqlalchemy as sa
 from test.lib import engines, testing
-from sqlalchemy import Integer, String, ForeignKey, literal_column, orm, exc
+from sqlalchemy import Integer, String, ForeignKey, literal_column, \
+    orm, exc, select
 from test.lib.schema import Table, Column
-from sqlalchemy.orm import mapper, relationship, create_session, column_property, sessionmaker
+from sqlalchemy.orm import mapper, relationship, Session, \
+    create_session, column_property, sessionmaker
 from test.lib.testing import eq_, ne_, assert_raises, assert_raises_message
 from test.orm import _base, _fixtures
 from test.engine import _base as engine_base
@@ -396,7 +398,93 @@ class AlternateGeneratorTest(_base.MappedTest):
             sess2.commit
         )
         
+
+class InheritanceTwoVersionIdsTest(_base.MappedTest):
+    """Test versioning where both parent/child table have a
+    versioning column.
+    
+    """
+    @classmethod
+    def define_tables(cls, metadata):
+        Table('base', metadata,
+            Column('id', Integer, primary_key=True),
+            Column('version_id', Integer, nullable=True),
+            Column('data', String(50))
+        )
+        Table('sub', metadata,
+            Column('id', Integer, ForeignKey('base.id'), primary_key=True),
+            Column('version_id', Integer, nullable=False),
+            Column('sub_data', String(50))
+        )
+
+    @classmethod
+    def setup_classes(cls):
+        class Base(_base.ComparableEntity):
+            pass
+        class Sub(Base):
+            pass
+
+    @testing.resolve_artifact_names
+    def test_base_both(self):
+        mapper(Base, base, 
+                version_id_col=base.c.version_id)
+        mapper(Sub, sub, inherits=Base)
+
+        session = Session()
+        b1 = Base(data='b1')
+        session.add(b1)
+        session.commit()
+        eq_(b1.version_id, 1)
+        # base is populated
+        eq_(select([base.c.version_id]).scalar(), 1)
+
+    @testing.resolve_artifact_names
+    def test_sub_both(self):
+        mapper(Base, base, 
+                version_id_col=base.c.version_id)
+        mapper(Sub, sub, inherits=Base)
+
+        session = Session()
+        s1 = Sub(data='s1', sub_data='s1')
+        session.add(s1)
+        session.commit()
+
+        # table is populated
+        eq_(select([sub.c.version_id]).scalar(), 1)
+
+        # base is populated
+        eq_(select([base.c.version_id]).scalar(), 1)
+    
+    @testing.resolve_artifact_names
+    def test_sub_only(self):
+        mapper(Base, base)
+        mapper(Sub, sub, inherits=Base, 
+                version_id_col=sub.c.version_id)
+
+        session = Session()
+        s1 = Sub(data='s1', sub_data='s1')
+        session.add(s1)
+        session.commit()
+
+        # table is populated
+        eq_(select([sub.c.version_id]).scalar(), 1)
+
+        # base is not
+        eq_(select([base.c.version_id]).scalar(), None)
         
+    @testing.resolve_artifact_names
+    def test_mismatch_version_col_warning(self):
+        mapper(Base, base, 
+                version_id_col=base.c.version_id)
         
-        
+        assert_raises_message(
+            exc.SAWarning,
+            "Inheriting version_id_col 'version_id' does not "
+            "match inherited version_id_col 'version_id' and will not "
+            "automatically populate the inherited versioning column. "
+            "version_id_col should only be specified on "
+            "the base-most mapper that includes versioning.",
+            mapper,
+            Sub, sub, inherits=Base, 
+                version_id_col=sub.c.version_id)
         
\ No newline at end of file