relationship aliased on subclass causes infinite recursion

[Former user]

Having additional names for a relationship in a subclass works with sqlalchemy <= 0.8.4 With 0.9.0 setting the attribute as seen below gives RuntimeError: maximum recursion depth exceeded

from sqlalchemy.ext.declarative import declarative_base
from sqlalchemy import Column, Integer, ForeignKey
from sqlalchemy.orm import relationship, backref

Base = declarative_base()

class Child(Base):
    __tablename__ = 'child'
    id = Column(Integer, primary_key=True)
    foo_id = Column(Integer, ForeignKey('foo.id'))
    foo = relationship('Foo', backref=backref('children'))

class FooChild(Child):
    parent = Child.foo

class Foo(Base):
    __tablename__ = 'foo'
    id = Column(Integer, primary_key=True)

FooChild(parent=Foo())

[Mike Bayer]

mmm OK, well use a synonym for that now, the thing you're doing there, if it works in 0.8, works entirely by accident:

class FooChild(Child):
    parent = synonym("foo")

the way that is now, the relationship() is probably being set up incorrectly and when I look into it, i might prefer that pattern just raise an exception, or make an implicit synonym, not really sure.

[Mike Bayer]

OK figuring out what's happening here, and also considering what exactly is synonym() for really, if we can just do this? What is basically the pattern here is that the InstrumentedAttribute is just assigned a second time somewhere else. declarative isn't really getting involved here other than just setting the attribute. same pattern in classical:

from sqlalchemy import Column, Integer, ForeignKey, Table, MetaData
from sqlalchemy.orm import relationship, backref, mapper

m = MetaData()
child = Table('child', m,
        Column('id', Integer, primary_key=True),
        Column('foo_id', ForeignKey('foo.id'))
    )

foo = Table('foo', m,
        Column('id', Integer, primary_key=True)
    )
class Child(object):
    pass

class Foo(object):
    pass

class FooChild(Child):
    pass

mapper(Child, child, properties={
        'foo': relationship(Foo, backref=backref('children'))
    })

mapper(Foo, foo)
mapper(FooChild, None, inherits=Child)

Child.parent = Child.foo
Child.brap = Child.foo_id

fc = FooChild()
fc.parent = Foo()

[Mike Bayer]

so, if we can just have an InstrumentedAttribute referred to by many keys, what does synonym() really get us if you aren't using descriptor overrides? obviously we have to fix the recursion catch in backrefs here. but should we try to build in "mirror the instrumented attribute" behavior at the declared level also, then this means we're revising #2828. #2828 inconsistently doesn't seem to apply to relationships either? seems like we might have screwed up on this one.

[Mike Bayer]

looking into the backref issue specifically, it is related to inheritance and the fact that Child.foo doesn't belong on FooChild.foo:

# breaks
FooChild.parent = Child.foo

# works
#FooChild.parent = FooChild.foo

FooChild(parent=Foo())

[Mike Bayer]

i think just having decalrative turn them into synonyms is the most straightforward route, otherwise we really can't be too smart about what's going on, e.g. in the original case where FooChild has "parent = Child.foo" inline:

diff --git a/lib/sqlalchemy/ext/declarative/base.py b/lib/sqlalchemy/ext/declarative/base.py
index f7668a5..a571d07 100644
--- a/lib/sqlalchemy/ext/declarative/base.py
+++ b/lib/sqlalchemy/ext/declarative/base.py
@@ -6,9 +6,10 @@
 """Internal implementation for declarative."""

 from ...schema import Table, Column
-from ...orm import mapper, class_mapper
+from ...orm import mapper, class_mapper, synonym
 from ...orm.interfaces import MapperProperty
 from ...orm.properties import ColumnProperty, CompositeProperty
+from ...orm.attributes import QueryableAttribute
 from ...orm.base import _is_mapped_class
 from ... import util, exc
 from ...sql import expression
@@ -148,6 +149,15 @@ def _as_declarative(cls, classname, dict_):
         if isinstance(value, declarative_props):
             value = getattr(cls, k)

+        elif isinstance(value, QueryableAttribute) and \
+                value.class_ is not cls and \
+                value.key != k:
+            # detect a QueryableAttribute that's already mapped being
+            # assigned elsewhere in userland, turn into a synonym()
+            value = synonym(value.key)
+            setattr(cls, k, value)
+
+
         if (isinstance(value, tuple) and len(value) == 1 and
             isinstance(value[0](0), (Column, MapperProperty))):
             util.warn("Ignoring declarative-like tuple value of attribute "
@@ -397,6 +407,7 @@ def _add_attribute(cls, key, value):
     adds it to the Mapper, adds a column to the mapped Table, etc.

     """
+
     if '__mapper__' in cls.__dict__:
         if isinstance(value, Column):
             _undefer_column_name(key, value)
@@ -413,6 +424,11 @@ def _add_attribute(cls, key, value):
                 key,
                 clsregistry._deferred_relationship(cls, value)
             )
+        elif isinstance(value, QueryableAttribute) and value.key != key:
+            # detect a QueryableAttribute that's already mapped being
+            # assigned elsewhere in userland, turn into a synonym()
+            value = synonym(value.key)
+            type.__setattr__(cls, key, value)
         else:
             type.__setattr__(cls, key, value)
     else:

[Mike Bayer]

I'd like to shore up the "how do we tell what kind of attribute this is" test a bit better, just checking for different key is not strong enough.

[Mike Bayer]

changed my mind on the key being strong enough based on some existing tests. 8a7fdd4e5cf5e4d9ba71c66a