.. changelog::
:version: 0.9.4
+ .. change::
+ :tags: bug, orm
+ :tickets: 2948
+
+ Fixed a very old behavior where the lazy load emitted for a one-to-many
+ could inappropriately pull in the parent table, and also return results
+ inconsistent based on what's in the parent table, when the primaryjoin
+ includes some kind of discriminator against the parent table, such
+ as ``and_(parent.id == child.parent_id, parent.deleted == False)``.
+ While this primaryjoin doesn't make that much sense for a one-to-many,
+ it is slightly more common when applied to the many-to-one side, and
+ the one-to-many comes as a result of a backref.
+ Loading rows from ``child`` in this case would keep ``parent.deleted == False``
+ as is within the query, thereby yanking it into the FROM clause
+ and doing a cartesian product. The new behavior will now substitute
+ the value of the local "parent.deleted" for that parameter as is
+ appropriate. Though typically, a real-world app probably wants to use a
+ different primaryjoin for the o2m side in any case.
+
.. change::
:tags: bug, orm
:tickets: 2965
binds = util.column_dict()
lookup = util.column_dict()
equated_columns = util.column_dict()
+ being_replaced = set()
if reverse_direction and self.secondaryjoin is None:
for l, r in self.local_remote_pairs:
_list.append((r, l))
equated_columns[l] = r
else:
+ # replace all "local side" columns, which is
+ # anything that isn't marked "remote"
+ being_replaced.update(self.local_columns)
for l, r in self.local_remote_pairs:
_list = lookup.setdefault(l, [])
_list.append((l, r))
equated_columns[r] = l
def col_to_bind(col):
- if col in lookup:
- for tobind, equated in lookup[col]:
- if equated in binds:
- return None
+ if col in being_replaced or col in lookup:
+ if col in lookup:
+ for tobind, equated in lookup[col]:
+ if equated in binds:
+ return None
+ else:
+ assert not reverse_direction
if col not in binds:
binds[col] = sql.bindparam(
None, None, type_=col.type, unique=True)
from sqlalchemy import exc as sa_exc
from sqlalchemy.orm import attributes, exc as orm_exc
import sqlalchemy as sa
-from sqlalchemy import testing
-from sqlalchemy import Integer, String, ForeignKey, SmallInteger
+from sqlalchemy import testing, and_
+from sqlalchemy import Integer, String, ForeignKey, SmallInteger, Boolean
from sqlalchemy.types import TypeDecorator
from sqlalchemy.testing.schema import Table
from sqlalchemy.testing.schema import Column
-from sqlalchemy.orm import mapper, relationship, create_session
+from sqlalchemy.orm import mapper, relationship, create_session, Session
from sqlalchemy.testing import eq_
from sqlalchemy.testing import fixtures
from test.orm import _fixtures
+from sqlalchemy.testing.assertsql import AllOf, CompiledSQL
class LazyTest(_fixtures.FixtureTest):
User(name='user3', stuff=[Stuff(id=5, date=datetime.date(2007, 6, 15))])
])
+class O2MWOSideFixedTest(fixtures.MappedTest):
+ # test #2948 - o2m backref with a "m2o does/does not count"
+ # criteria doesn't scan the "o" table
+ @classmethod
+ def define_tables(self, meta):
+ Table('city', meta,
+ Column('id', Integer, primary_key=True),
+ Column('deleted', Boolean),
+ )
+ Table('person', meta,
+ Column('id', Integer, primary_key=True),
+ Column('city_id', ForeignKey('city.id'))
+ )
+
+
+ @classmethod
+ def setup_classes(cls):
+ class Person(cls.Basic):
+ pass
+
+ class City(cls.Basic):
+ pass
+
+ @classmethod
+ def setup_mappers(cls):
+ Person, City = cls.classes.Person, cls.classes.City
+ city, person = cls.tables.city, cls.tables.person
+
+ mapper(Person, person, properties={
+ 'city':relationship(City,
+ primaryjoin=and_(
+ person.c.city_id==city.c.id,
+ city.c.deleted == False),
+ backref='people'
+ )
+ })
+ mapper(City, city)
+
+ def _fixture(self, include_other):
+ city, person = self.tables.city, self.tables.person
+
+ if include_other:
+ city.insert().execute(
+ {"id": 1, "deleted": False},
+ )
+
+ person.insert().execute(
+ {"id": 1, "city_id": 1},
+ {"id": 2, "city_id": 1},
+ )
+
+ city.insert().execute(
+ {"id": 2, "deleted": True},
+ )
+
+ person.insert().execute(
+ {"id": 3, "city_id": 2},
+ {"id": 4, "city_id": 2},
+ )
+
+
+ def test_lazyload_assert_expected_sql(self):
+ self._fixture(True)
+ Person, City = self.classes.Person, self.classes.City
+ sess = Session(testing.db)
+ c1, c2 = sess.query(City).order_by(City.id).all()
+
+ def go():
+ eq_(
+ [p.id for p in c2.people],
+ []
+ )
+
+ self.assert_sql_execution(
+ testing.db,
+ go,
+ CompiledSQL(
+ "SELECT person.id AS person_id, person.city_id AS "
+ "person_city_id FROM person "
+ "WHERE person.city_id = :param_1 AND :param_2 = 0",
+ {"param_1": 2, "param_2": 1}
+ )
+ )
+
+ def test_lazyload_people_other_exists(self):
+ self._fixture(True)
+ Person, City = self.classes.Person, self.classes.City
+ sess = Session(testing.db)
+ c1, c2 = sess.query(City).order_by(City.id).all()
+ eq_(
+ [p.id for p in c1.people],
+ [1, 2]
+ )
+
+ eq_(
+ [p.id for p in c2.people],
+ []
+ )
+
+ def test_lazyload_people_no_other_exists(self):
+ # note that if we revert #2948, *this still passes!*
+ # e.g. due to the scan of the "o" table, whether or not *another*
+ # row exists determines if this works.
+
+ self._fixture(False)
+ Person, City = self.classes.Person, self.classes.City
+ sess = Session(testing.db)
+ c2, = sess.query(City).order_by(City.id).all()
+
+ eq_(
+ [p.id for p in c2.people],
+ []
+ )
**kw
)
+
+ cls.left = Table('lft', m,
+ Column('id', Integer, primary_key=True),
+ Column('x', Integer),
+ Column('y', Integer),
+ )
+ cls.right = Table('rgt', m,
+ Column('id', Integer, primary_key=True),
+ Column('lid', Integer, ForeignKey('lft.id')),
+ Column('x', Integer),
+ Column('y', Integer),
+ )
+
+ def _join_fixture_o2m_o_side_none(self, **kw):
+ return relationships.JoinCondition(
+ self.left,
+ self.right,
+ self.left,
+ self.right,
+ primaryjoin=and_(self.left.c.id == self.right.c.lid,
+ self.left.c.x == 5),
+ **kw
+ )
+
def _assert_non_simple_warning(self, fn):
assert_raises_message(
exc.SAWarning,
"lft.id = :param_1"
)
+ def test_lazy_clause_o2m_o_side_none(self):
+ # test for #2948. When the join is "o.id == m.oid AND o.something == something",
+ # we don't want 'o' brought into the lazy load for 'm'
+ joincond = self._join_fixture_o2m_o_side_none()
+ lazywhere, bind_to_col, equated_columns = joincond.create_lazy_clause()
+ self.assert_compile(
+ lazywhere,
+ ":param_1 = rgt.lid AND :param_2 = :x_1",
+ checkparams={'param_1': None, 'param_2': None, 'x_1': 5}
+ )
+
+ def test_lazy_clause_o2m_o_side_none_reverse(self):
+ # continued test for #2948.
+ joincond = self._join_fixture_o2m_o_side_none()
+ lazywhere, bind_to_col, equated_columns = joincond.create_lazy_clause(reverse_direction=True)
+ self.assert_compile(
+ lazywhere,
+ "lft.id = :param_1 AND lft.x = :x_1",
+ checkparams= {'param_1': None, 'x_1': 5}
+ )
projects / thirdparty / sqlalchemy / sqlalchemy.git / commitdiff
