Repair foreign_keys population for Join._refresh_for_new_column

Fixed bug where setting up a single-table inh subclass of a joined-table
subclass which included an extra column would corrupt the foreign keys
collection of the mapped table, thereby interfering with the
initialization of relationships.

Change-Id: I04a0cf98fd456d12d5a5b9e77a46a01246969a63
Fixes: #3797
(cherry picked from commit 8967099c3ba8b04fa20536bc0a026f6adc8e096f)

diff --git a/doc/build/changelog/changelog_10.rst b/doc/build/changelog/changelog_10.rst
index e6875d5fddcffbf61d2a70ed32cd51406107491d..d726435cf885018e4eb6c65a39612e70a390ef63 100644 (file)
--- a/doc/build/changelog/changelog_10.rst
+++ b/doc/build/changelog/changelog_10.rst
@@ -18,6 +18,16 @@
 .. changelog::
     :version: 1.0.16
 
+    .. change::
+        :tags: bug, orm.declarative
+        :tickets: 3797
+        :versions: 1.1.0
+
+        Fixed bug where setting up a single-table inh subclass of a joined-table
+        subclass which included an extra column would corrupt the foreign keys
+        collection of the mapped table, thereby interfering with the
+        initialization of relationships.
+
 .. changelog::
     :version: 1.0.15
     :released: September 1, 2016

diff --git a/lib/sqlalchemy/sql/selectable.py b/lib/sqlalchemy/sql/selectable.py
index 0ad2d983b5fcb9d2380ec25b2e96a9b7306affa1..87eddc49cb69e72097b672c5f9c0a3cd0326c748 100644 (file)
--- a/lib/sqlalchemy/sql/selectable.py
+++ b/lib/sqlalchemy/sql/selectable.py
@@ -793,7 +793,7 @@ class Join(FromClause):
         if col is not None:
             if self._cols_populated:
                 self._columns[col._label] = col
-                self.foreign_keys.add(col)
+                self.foreign_keys.update(col.foreign_keys)
                 if col.primary_key:
                     self.primary_key.add(col)
                 return col

diff --git a/lib/sqlalchemy/testing/__init__.py b/lib/sqlalchemy/testing/__init__.py
index 4e02227c31da08ee1e20c56e0f529430c6e51ee1..c002fcb1e95af0fa4f992016966c49e1b9524cc2 100644 (file)
--- a/lib/sqlalchemy/testing/__init__.py
+++ b/lib/sqlalchemy/testing/__init__.py
@@ -22,7 +22,7 @@ from .assertions import emits_warning, emits_warning_on, uses_deprecated, \
     eq_, ne_, le_, is_, is_not_, startswith_, assert_raises, \
     assert_raises_message, AssertsCompiledSQL, ComparesTables, \
     AssertsExecutionResults, expect_deprecated, expect_warnings, \
-    in_, not_in_
+    in_, not_in_, is_true
 
 from .util import run_as_contextmanager, rowset, fail, \
     provide_metadata, adict, force_drop_names, \

diff --git a/lib/sqlalchemy/testing/assertions.py b/lib/sqlalchemy/testing/assertions.py
index 492adcd62ae485b4dd9a3f32ff059b95071622f7..dc90bc42debf35bb1880c7e8511340170cfe9257 100644 (file)
--- a/lib/sqlalchemy/testing/assertions.py
+++ b/lib/sqlalchemy/testing/assertions.py
@@ -218,6 +218,8 @@ def le_(a, b, msg=None):
     """Assert a <= b, with repr messaging on failure."""
     assert a <= b, msg or "%r != %r" % (a, b)
 
+def is_true(a, msg=None):
+    is_(a, True, msg=msg)
 
 def is_(a, b, msg=None):
     """Assert a is b, with repr messaging on failure."""

diff --git a/test/ext/declarative/test_inheritance.py b/test/ext/declarative/test_inheritance.py
index 274a6aa285ec643625945b53724ef669824f8ada..a5a86b7c6f6165d31ecec3cc46d1225f4fcfe895 100644 (file)
--- a/test/ext/declarative/test_inheritance.py
+++ b/test/ext/declarative/test_inheritance.py
@@ -1,6 +1,6 @@
 
 from sqlalchemy.testing import eq_, assert_raises, \
-    assert_raises_message, is_
+    assert_raises_message, is_, is_true
 from sqlalchemy.ext import declarative as decl
 import sqlalchemy as sa
 from sqlalchemy import testing
@@ -485,6 +485,56 @@ class DeclarativeInheritanceTest(DeclarativeTestBase):
                                            ).one(),
             Engineer(name='vlad', primary_language='cobol'))
 
+    def test_single_cols_on_sub_to_joined(self):
+        """test [ticket:3797]"""
+
+        class BaseUser(Base):
+            __tablename__ = 'root'
+
+            id = Column(Integer, primary_key=True)
+            row_type = Column(String)
+
+            __mapper_args__ = {
+                'polymorphic_on': row_type,
+                'polymorphic_identity': 'baseuser'
+            }
+
+        class User(BaseUser):
+            __tablename__ = 'user'
+
+            __mapper_args__ = {
+                'polymorphic_identity': 'user'
+            }
+
+            baseuser_id = Column(
+                Integer, ForeignKey('root.id'), primary_key=True)
+
+        class Bat(Base):
+            __tablename__ = 'bat'
+            id = Column(Integer, primary_key=True)
+
+        class Thing(Base):
+            __tablename__ = 'thing'
+
+            id = Column(Integer, primary_key=True)
+
+            owner_id = Column(Integer, ForeignKey('user.baseuser_id'))
+            owner = relationship('User')
+
+        class SubUser(User):
+            __mapper_args__ = {
+                'polymorphic_identity': 'subuser'
+            }
+
+            sub_user_custom_thing = Column(Integer, ForeignKey('bat.id'))
+
+        eq_(
+            User.__table__.foreign_keys,
+            User.baseuser_id.foreign_keys.union(
+                SubUser.sub_user_custom_thing.foreign_keys))
+        is_true(Thing.owner.property.primaryjoin.compare(
+            Thing.owner_id == User.baseuser_id))
+
     def test_single_constraint_on_sub(self):
         """test the somewhat unusual case of [ticket:3341]"""
 

diff --git a/test/sql/test_selectable.py b/test/sql/test_selectable.py
index 9d714164b915d7d4068035f369d1d0c45a0fca76..8f2b06123c1e7148f513d7d0d3ae2e5d0ea22d8a 100644 (file)
--- a/test/sql/test_selectable.py
+++ b/test/sql/test_selectable.py
@@ -892,6 +892,44 @@ class RefreshForNewColTest(fixtures.TestBase):
         j._refresh_for_new_column(q)
         assert j.c.b_q is q
 
+    def test_fk_table(self):
+        m = MetaData()
+        fk = ForeignKey('x.id')
+        Table('x', m, Column('id', Integer))
+        a = Table('a', m, Column('x', Integer, fk))
+        a.c
+
+        q = Column('q', Integer)
+        a.append_column(q)
+        a._refresh_for_new_column(q)
+        eq_(a.foreign_keys, set([fk]))
+
+        fk2 = ForeignKey('g.id')
+        p = Column('p', Integer, fk2)
+        a.append_column(p)
+        a._refresh_for_new_column(p)
+        eq_(a.foreign_keys, set([fk, fk2]))
+
+    def test_fk_join(self):
+        m = MetaData()
+        fk = ForeignKey('x.id')
+        Table('x', m, Column('id', Integer))
+        a = Table('a', m, Column('x', Integer, fk))
+        b = Table('b', m, Column('y', Integer))
+        j = a.join(b, a.c.x == b.c.y)
+        j.c
+
+        q = Column('q', Integer)
+        b.append_column(q)
+        j._refresh_for_new_column(q)
+        eq_(j.foreign_keys, set([fk]))
+
+        fk2 = ForeignKey('g.id')
+        p = Column('p', Integer, fk2)
+        b.append_column(p)
+        j._refresh_for_new_column(p)
+        eq_(j.foreign_keys, set([fk, fk2]))
+
 
 class AnonLabelTest(fixtures.TestBase):