- [bug] Fixed bug in declarative

whereby the precedence of columns
in a joined-table, composite
column (typically for id) would fail to
be correct if the columns contained
names distinct from their attribute
names.  This would cause things like
primaryjoin conditions made against the
entity attributes to be incorrect.  Related
to [ticket:1892] as this was supposed
to be part of that, this is [ticket:2491].
Also in 0.7.8.

diff --git a/CHANGES b/CHANGES
index 37eebed2ea99cae7d59ef7535e1ea4dece27763e..f37170c0af6122ac5541ad479355c2da2cac454c 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -118,6 +118,19 @@ CHANGES
     would fail to function subsequent to this
     warning in any case.  [ticket:2405]
 
+  - [bug] Fixed bug in declarative 
+    whereby the precedence of columns 
+    in a joined-table, composite
+    column (typically for id) would fail to
+    be correct if the columns contained
+    names distinct from their attribute 
+    names.  This would cause things like
+    primaryjoin conditions made against the
+    entity attributes to be incorrect.  Related
+    to [ticket:1892] as this was supposed
+    to be part of that, this is [ticket:2491].
+    Also in 0.7.8.
+
   - [feature] Query now "auto correlates" by 
     default in the same way as select() does.
     Previously, a Query used as a subquery

diff --git a/lib/sqlalchemy/ext/declarative.py b/lib/sqlalchemy/ext/declarative.py
index 25dd72b240ad7a3802eb1114f6923c5a89f37793..893fc988d0964fe1cd2db619c35779c307a0ccb4 100755 (executable)
--- a/lib/sqlalchemy/ext/declarative.py
+++ b/lib/sqlalchemy/ext/declarative.py
@@ -1388,11 +1388,9 @@ class _MapperConfig(object):
                 if k in inherited_mapper._props:
                     p = inherited_mapper._props[k]
                     if isinstance(p, ColumnProperty):
-                        # note here we place the superclass column
-                        # first.  this corresponds to the 
-                        # append() in mapper._configure_property().
-                        # change this ordering when we do [ticket:1892]
-                        properties[k] = p.columns + [col]
+                        # note here we place the subclass column
+                        # first.  See [ticket:1892] for background.
+                        properties[k] = [col] + p.columns
 
         result_mapper_args = mapper_args.copy()
         result_mapper_args['properties'] = properties
@@ -1401,7 +1399,6 @@ class _MapperConfig(object):
     def map(self):
         self.configs.pop(self.cls, None)
         mapper_args = self._prepare_mapper_arguments()
-
         self.cls.__mapper__ = self.mapper_cls(
             self.cls,
             self.local_table,

diff --git a/test/ext/test_declarative_inheritance.py b/test/ext/test_declarative_inheritance.py
index e2a8dcc9a05737431088f541f89f3c2db80e2912..2d63e2de29d19fdc70c34d393d04ddb4d5a9fd4c 100644 (file)
--- a/test/ext/test_declarative_inheritance.py
+++ b/test/ext/test_declarative_inheritance.py
@@ -805,6 +805,61 @@ class DeclarativeInheritanceTest(DeclarativeTestBase):
 
         assert A.__mapper__.inherits is a_1.__mapper__
 
+
+class OverlapColPrecedenceTest(DeclarativeTestBase):
+    """test #1892 cases when declarative does column precedence."""
+
+    def _run_test(self, Engineer, e_id, p_id):
+        p_table = Base.metadata.tables['person']
+        e_table = Base.metadata.tables['engineer']
+        assert Engineer.id.property.columns[0] is e_table.c[e_id]
+        assert Engineer.id.property.columns[1] is p_table.c[p_id]
+
+    def test_basic(self):
+        class Person(Base):
+            __tablename__ = 'person'
+            id = Column(Integer, primary_key=True)
+
+        class Engineer(Person):
+            __tablename__ = 'engineer'
+            id = Column(Integer, ForeignKey('person.id'), primary_key=True)
+
+        self._run_test(Engineer, "id", "id")
+
+    def test_alt_name_base(self):
+        class Person(Base):
+            __tablename__ = 'person'
+            id = Column("pid", Integer, primary_key=True)
+
+        class Engineer(Person):
+            __tablename__ = 'engineer'
+            id = Column(Integer, ForeignKey('person.pid'), primary_key=True)
+
+        self._run_test(Engineer, "id", "pid")
+
+    def test_alt_name_sub(self):
+        class Person(Base):
+            __tablename__ = 'person'
+            id = Column(Integer, primary_key=True)
+
+        class Engineer(Person):
+            __tablename__ = 'engineer'
+            id = Column("eid", Integer, ForeignKey('person.id'), primary_key=True)
+
+        self._run_test(Engineer, "eid", "id")
+
+    def test_alt_name_both(self):
+        class Person(Base):
+            __tablename__ = 'person'
+            id = Column("pid", Integer, primary_key=True)
+
+        class Engineer(Person):
+            __tablename__ = 'engineer'
+            id = Column("eid", Integer, ForeignKey('person.pid'), primary_key=True)
+
+        self._run_test(Engineer, "eid", "pid")
+
+
 from test.orm.test_events import _RemoveListeners
 class ConcreteInhTest(_RemoveListeners, DeclarativeTestBase):
     def _roundtrip(self, Employee, Manager, Engineer, Boss, polymorphic=True):

diff --git a/test/ext/test_declarative_mixin.py b/test/ext/test_declarative_mixin.py
index 9de56134f804be6ced7b0df4bd8d4292a27a448e..db176aa6de60ee606977888878321ba77d0fce28 100644 (file)
--- a/test/ext/test_declarative_mixin.py
+++ b/test/ext/test_declarative_mixin.py
@@ -314,8 +314,8 @@ class DeclarativeMixinTest(DeclarativeTestBase):
         assert len(General.bar.prop.columns) == 1
         assert Specific.bar.prop is not General.bar.prop
         assert len(Specific.bar.prop.columns) == 2
-        assert Specific.bar.prop.columns[0] is General.__table__.c.bar_newname
-        assert Specific.bar.prop.columns[1] is Specific.__table__.c.bar_newname
+        assert Specific.bar.prop.columns[0] is Specific.__table__.c.bar_newname
+        assert Specific.bar.prop.columns[1] is General.__table__.c.bar_newname
 
     def test_column_join_checks_superclass_type(self):
         """Test that the logic which joins subclass props to those