sqlalchemy / sqlalchemy

The Database Toolkit for Python
https://www.sqlalchemy.org
MIT License
9.59k stars 1.43k forks source link

same named relationship from two subclasses cant be targeted #2614

Closed sqlalchemy-bot closed 11 years ago

sqlalchemy-bot commented 11 years ago

Migrated issue, originally created by Michael Bayer (@zzzeek)

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

Attachments: 2614.patch

sqlalchemy-bot commented 11 years ago

Michael Bayer (@zzzeek) wrote:

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
sqlalchemy-bot commented 11 years ago

Michael Bayer (@zzzeek) wrote:

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
sqlalchemy-bot commented 11 years ago

Michael Bayer (@zzzeek) wrote:

partial patch to start allowing more accurate path in query.setup

sqlalchemy-bot commented 11 years ago

Michael Bayer (@zzzeek) wrote:

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.

sqlalchemy-bot commented 11 years ago

Michael Bayer (@zzzeek) wrote:

it's an exception raise, and it's in e2697d547ec8c24c9a37a72fc60abe73b7dee81b

sqlalchemy-bot commented 11 years ago

Michael Bayer (@zzzeek) wrote:

progress continues at https://bitbucket.org/zzzeek/sqla_2614

sqlalchemy-bot commented 11 years ago

Michael Bayer (@zzzeek) wrote:

going to try hitting this for 0.8final.

sqlalchemy-bot commented 11 years ago

Michael Bayer (@zzzeek) wrote:

b66dad46f31961ad9f2271e6dae377e38fc67979

sqlalchemy-bot commented 11 years ago

Changes by Michael Bayer (@zzzeek):

sqlalchemy-bot commented 11 years ago

Changes by Michael Bayer (@zzzeek):

sqlalchemy-bot commented 11 years ago

Changes by Michael Bayer (@zzzeek):