From 01b37f1561c3a58568329cc9b11920a07f7b8464 Mon Sep 17 00:00:00 2001 From: Mike Bayer Date: Sat, 26 May 2012 13:13:04 -0400 Subject: [PATCH] - [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]. --- CHANGES | 12 +++++++ lib/sqlalchemy/ext/declarative.py | 8 ++--- test/ext/test_declarative.py | 59 +++++++++++++++++++++++++++++-- 3 files changed, 72 insertions(+), 7 deletions(-) diff --git a/CHANGES b/CHANGES index 0b22a43f65..f8aba424e4 100644 --- a/CHANGES +++ b/CHANGES @@ -12,6 +12,18 @@ CHANGES distinct class encountered in the polymorphic result. [ticket:2480] + - [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]. + - oracle - [bug] Added ROWID to oracle.*, [ticket:2483] diff --git a/lib/sqlalchemy/ext/declarative.py b/lib/sqlalchemy/ext/declarative.py index faf575da1c..ecdae68d6b 100755 --- a/lib/sqlalchemy/ext/declarative.py +++ b/lib/sqlalchemy/ext/declarative.py @@ -1322,11 +1322,9 @@ def _as_declarative(cls, classname, dict_): 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] - our_stuff[k] = p.columns + [col] + # note here we place the subclass column + # first. See [ticket:1892] for background. + our_stuff[k] = [col] + p.columns cls.__mapper__ = mapper_cls(cls, diff --git a/test/ext/test_declarative.py b/test/ext/test_declarative.py index 69042b5c84..4500b74cf2 100644 --- a/test/ext/test_declarative.py +++ b/test/ext/test_declarative.py @@ -2085,6 +2085,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): @@ -2785,8 +2840,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 -- 2.47.2