From d4512b76aa25844584b0cd5f3311a1bd53b9f828 Mon Sep 17 00:00:00 2001 From: Mike Bayer Date: Fri, 28 Aug 2009 20:29:39 +0000 Subject: [PATCH] - Fixed an obscure issue whereby a joined-table subclass with a self-referential eager load on the base class would populate the related object's "subclass" table with data from the "subclass" table of the parent. [ticket:1485] --- CHANGES | 6 ++++ lib/sqlalchemy/orm/strategies.py | 4 +-- lib/sqlalchemy/orm/util.py | 4 +-- lib/sqlalchemy/sql/util.py | 13 ++++++- test/orm/inheritance/test_basic.py | 55 ++++++++++++++++++++++++++++++ 5 files changed, 77 insertions(+), 5 deletions(-) diff --git a/CHANGES b/CHANGES index 2c33b78618..44b74f36c0 100644 --- a/CHANGES +++ b/CHANGES @@ -10,6 +10,12 @@ CHANGES composite primary key would fail on updates. Continuation of [ticket:1300]. + - Fixed an obscure issue whereby a joined-table subclass + with a self-referential eager load on the base class + would populate the related object's "subclass" table with + data from the "subclass" table of the parent. + [ticket:1485] + - relations() now have greater ability to be "overridden", meaning a subclass that explicitly specifies a relation() overriding that of the parent class will be honored diff --git a/lib/sqlalchemy/orm/strategies.py b/lib/sqlalchemy/orm/strategies.py index f739fb1dd0..fd767c8655 100644 --- a/lib/sqlalchemy/orm/strategies.py +++ b/lib/sqlalchemy/orm/strategies.py @@ -115,7 +115,7 @@ class ColumnLoader(LoaderStrategy): key, col = self.key, self.columns[0] if adapter: col = adapter.columns[col] - if col in row: + if col is not None and col in row: def new_execute(state, dict_, row, **flags): dict_[key] = row[col] @@ -678,7 +678,7 @@ class EagerLoader(AbstractRelationLoader): # create AliasedClauses object to build up the eager query. clauses = mapperutil.ORMAdapter(mapperutil.AliasedClass(self.mapper), - equivalents=self.mapper._equivalent_columns) + equivalents=self.mapper._equivalent_columns, adapt_required=True) join_to_left = False if adapter: diff --git a/lib/sqlalchemy/orm/util.py b/lib/sqlalchemy/orm/util.py index c858ca1026..bc23d8c6d8 100644 --- a/lib/sqlalchemy/orm/util.py +++ b/lib/sqlalchemy/orm/util.py @@ -257,13 +257,13 @@ class ORMAdapter(sql_util.ColumnAdapter): and the AliasedClass if any is referenced. """ - def __init__(self, entity, equivalents=None, chain_to=None): + def __init__(self, entity, equivalents=None, chain_to=None, adapt_required=False): self.mapper, selectable, is_aliased_class = _entity_info(entity) if is_aliased_class: self.aliased_class = entity else: self.aliased_class = None - sql_util.ColumnAdapter.__init__(self, selectable, equivalents, chain_to) + sql_util.ColumnAdapter.__init__(self, selectable, equivalents, chain_to, adapt_required=adapt_required) def replace(self, elem): entity = elem._annotations.get('parentmapper', None) diff --git a/lib/sqlalchemy/sql/util.py b/lib/sqlalchemy/sql/util.py index 27ae3e6247..9be405e219 100644 --- a/lib/sqlalchemy/sql/util.py +++ b/lib/sqlalchemy/sql/util.py @@ -498,11 +498,12 @@ class ColumnAdapter(ClauseAdapter): adapted_row() factory. """ - def __init__(self, selectable, equivalents=None, chain_to=None, include=None, exclude=None): + def __init__(self, selectable, equivalents=None, chain_to=None, include=None, exclude=None, adapt_required=False): ClauseAdapter.__init__(self, selectable, equivalents, include, exclude) if chain_to: self.chain(chain_to) self.columns = util.populate_column_dict(self._locate_col) + self.adapt_required = adapt_required def wrap(self, adapter): ac = self.__class__.__new__(self.__class__) @@ -530,6 +531,16 @@ class ColumnAdapter(ClauseAdapter): # anonymize labels in case they have a hardcoded name if isinstance(c, expression._Label): c = c.label(None) + + # adapt_required indicates that if we got the same column + # back which we put in (i.e. it passed through), + # it's not correct. this is used by eagerloading which + # knows that all columns and expressions need to be adapted + # to a result row, and a "passthrough" is definitely targeting + # the wrong column. + if self.adapt_required and c is col: + return None + return c def adapted_row(self, row): diff --git a/test/orm/inheritance/test_basic.py b/test/orm/inheritance/test_basic.py index bad6920de7..f93dfd5476 100644 --- a/test/orm/inheritance/test_basic.py +++ b/test/orm/inheritance/test_basic.py @@ -343,7 +343,62 @@ class EagerLazyTest(_base.MappedTest): self.assert_(len(q.first().lazy) == 1) self.assert_(len(q.first().eager) == 1) +class EagerTargetingTest(_base.MappedTest): + """test a scenario where joined table inheritance might be confused as an eagerly loaded joined table.""" + + @classmethod + def define_tables(cls, metadata): + Table('a_table', metadata, + Column('id', Integer, primary_key=True), + Column('name', String(50)), + Column('type', String(30), nullable=False), + Column('parent_id', Integer, ForeignKey('a_table.id')) + ) + + Table('b_table', metadata, + Column('id', Integer, ForeignKey('a_table.id'), primary_key=True), + Column('b_data', String(50)), + ) + + @testing.resolve_artifact_names + def test_adapt_stringency(self): + class A(_base.ComparableEntity): + pass + class B(A): + pass + + mapper(A, a_table, polymorphic_on=a_table.c.type, polymorphic_identity='A', + properties={ + 'children': relation(A, order_by=a_table.c.name) + }) + + mapper(B, b_table, inherits=A, polymorphic_identity='B', properties={ + 'b_derived':column_property(b_table.c.b_data + "DATA") + }) + + sess=create_session() + + b1=B(id=1, name='b1',b_data='i') + sess.add(b1) + sess.flush() + + b2=B(id=2, name='b2', b_data='l', parent_id=1) + sess.add(b2) + sess.flush() + bid=b1.id + + sess.expunge_all() + node = sess.query(B).filter(B.id==bid).all()[0] + eq_(node, B(id=1, name='b1',b_data='i')) + eq_(node.children[0], B(id=2, name='b2',b_data='l')) + + sess.expunge_all() + node = sess.query(B).options(eagerload(B.children)).filter(B.id==bid).all()[0] + eq_(node, B(id=1, name='b1',b_data='i')) + eq_(node.children[0], B(id=2, name='b2',b_data='l')) + + class FlushTest(_base.MappedTest): """test dependency sorting among inheriting mappers""" @classmethod -- 2.47.3