same named relationship from two subclasses cant be targeted

Issue #2614 resolved
Mike Bayer repo owner created an issue

due to the path reduction. a simple switch to use non-reduce paths fails to even initiate the eagerload at all.

from sqlalchemy.ext.declarative import declarative_base
from sqlalchemy import *
from sqlalchemy.orm import relationship, subqueryload, Session

Base = declarative_base()

class A(Base):
    __tablename__ = 'a'
    id = Column(Integer, primary_key=True)
    type = Column(String)

    __mapper_args__ = {'polymorphic_on': type}

class B(A):
    __tablename__ = 'b'
    id = Column(Integer, ForeignKey('a.id'), primary_key=True)

    btod = Table('btod', Base.metadata,
        Column('bid', Integer, ForeignKey('b.id'), nullable=False),
        Column('did', Integer, ForeignKey('d.id'), nullable=False)
    )
    related = relationship("D", secondary=btod)
    __mapper_args__ = {'polymorphic_identity': 'b'}

class C(A):
    __tablename__ = 'c'
    id = Column(Integer, ForeignKey('a.id'), primary_key=True)
    ctod = Table('ctod', Base.metadata,
        Column('cid', Integer, ForeignKey('c.id'), nullable=False),
        Column('did', Integer, ForeignKey('d.id'), nullable=False)
    )
    related = relationship("D", secondary=ctod)
    __mapper_args__ = {'polymorphic_identity': 'c'}

class D(Base):
    __tablename__ = 'd'
    id = Column(Integer, primary_key=True)

engine = create_engine('sqlite://', echo=True)
Base.metadata.create_all(engine)
session = Session(engine)

d = D()
session.add_all([   B(related=[d](
)),
    C(related=[d](d))
])
session.commit()

for a in session.query(A).with_polymorphic('*').\
        options(subqueryload(B.related), subqueryload(C.related)):
    print a.related

Comments (9)

  1. Mike Bayer reporter

    this is the workaround people can use who really need this pattern:

    class A(..):
        myChildrenA = relationship(...)
    
       @property
       def myChildren(self):
            return myChildrenA
    
    class B(..):
        myChildrenB = relationship(...)
    
       @property
       def myChildren(self):
            return myChildrenB
    
  2. Mike Bayer reporter

    here's the beginning of a patch but this messes up a lot of tests. so this is a big rethink kind of issue, as we've not tried to figure out how to target those paths more accurately as of yet.

    diff -r 0f0ce7c9b7c4b3a9f1329dfa9c42d0d69386d48f lib/sqlalchemy/orm/interfaces.py
    --- a/lib/sqlalchemy/orm/interfaces.py  Thu Nov 22 23:45:24 2012 -0500
    +++ b/lib/sqlalchemy/orm/interfaces.py  Fri Nov 23 17:15:28 2012 -0500
    @@ -414,7 +414,7 @@
    
         def _get_context_strategy(self, context, path):
             # this is essentially performance inlining.
    -        key = ('loaderstrategy', path.reduced_path + (self.key,))
    +        key = ('loaderstrategy', path.path + (self.key,))
             cls = None
             if key in context.attributes:
                 cls = context.attributes[key](key)
    @@ -648,7 +648,8 @@
                                                 raiseerr)
                         if not entity:
                             return no_result
    -                    path_element = entity.entity_zero
    +                    path_element = token._parententity
    +                    #path_element = entity.entity_zero
                         mapper = entity.mapper
                 else:
                     raise sa_exc.ArgumentError(
    diff -r 0f0ce7c9b7c4b3a9f1329dfa9c42d0d69386d48f lib/sqlalchemy/orm/query.py
    --- a/lib/sqlalchemy/orm/query.py   Thu Nov 22 23:45:24 2012 -0500
    +++ b/lib/sqlalchemy/orm/query.py   Fri Nov 23 17:15:28 2012 -0500
    @@ -2882,7 +2882,7 @@
                 value.setup(
                     context,
                     self,
    -                self.path,
    +                value.parent._sa_path_registry,  # self.path,
                     adapter,
                     only_load_props=query._only_load_props,
                     column_collection=context.primary_columns
    diff -r 0f0ce7c9b7c4b3a9f1329dfa9c42d0d69386d48f lib/sqlalchemy/orm/strategies.py
    --- a/lib/sqlalchemy/orm/strategies.py  Thu Nov 22 23:45:24 2012 -0500
    +++ b/lib/sqlalchemy/orm/strategies.py  Fri Nov 23 17:15:28 2012 -0500
    @@ -679,7 +679,6 @@
                             path, adapter,
                             column_collection=None,
                             parentmapper=None, **kwargs):
    -
             if not context.query._enable_eagerloads:
                 return
    
    diff -r 0f0ce7c9b7c4b3a9f1329dfa9c42d0d69386d48f lib/sqlalchemy/orm/util.py
    --- a/lib/sqlalchemy/orm/util.py    Thu Nov 22 23:45:24 2012 -0500
    +++ b/lib/sqlalchemy/orm/util.py    Fri Nov 23 17:15:28 2012 -0500
    @@ -277,13 +277,13 @@
                 self.path == other.path
    
         def set(self, reg, key, value):
    -        reg._attributes[self.reduced_path)]((key,) = value
    +        reg._attributes[self.path)]((key,) = value
    
         def setdefault(self, reg, key, value):
    -        reg._attributes.setdefault((key, self.reduced_path), value)
    +        reg._attributes.setdefault((key, self.path), value)
    
         def get(self, reg, key, value=None):
    -        key = (key, self.reduced_path)
    +        key = (key, self.path)
             if key in reg._attributes:
                 return reg._attributes[key](key)
             else:
    @@ -300,7 +300,7 @@
             return mapper in self.path
    
         def contains(self, reg, key):
    -        return (key, self.reduced_path) in reg._attributes
    +        return (key, self.path) in reg._attributes
    
         def serialize(self):
             path = self.path
    
  3. Mike Bayer reporter

    the attached patch attempts to get _MapperEntity.setup_context() to pass accurate pathing information to value.setup(). It requires changes to AliasedClass in that the AC needs to carry along the with_polymorphic of the parent mapper, as well as that an AC like Person.Engineer needs to "reduce" down to Person. Unfortunately, even with these adjustments, some very complex subqueryload tests are still failing, and this is only a small portion of the total change. Will attempt to emit a warning for this condition so that the release of 0.8 can continue.

  4. Log in to comment