Closed
Description
Migrated issue, originally created by Michael Bayer (@zzzeek)
from sqlalchemy import *
from sqlalchemy.orm import *
from sqlalchemy.ext.declarative import declarative_base
Base = declarative_base()
class A(Base):
__tablename__ = 'a'
id = Column(Integer, primary_key=True)
assoc = relationship("AB")
class B(Base):
__tablename__ = 'b'
id = Column(Integer, primary_key=True)
class AB(Base):
__tablename__ = 'ab'
a_id = Column(ForeignKey('a.id'), primary_key=True)
b_id = Column(ForeignKey('b.id'), primary_key=True)
a = relationship("A")
b = relationship("B")
bug = True
class C(Base):
__tablename__ = 'c'
id = Column(Integer, primary_key=True)
if bug:
k2 = Column(Integer)
k1 = Column(Integer)
else:
k1 = Column(Integer)
k2 = Column(Integer)
assoc = relationship("AB")
__table_args__ = (
ForeignKeyConstraint(['k1', 'k2'], ['ab.a_id', 'ab.b_id']), {})
e = create_engine("sqlite://", echo=True)
Base.metadata.create_all(e)
s = Session(e)
a, b = A(id=100), B(id=1000)
ab = AB(a=a, b=b)
c1 = C(id=1, assoc=ab)
c2 = C(id=2, assoc=ab)
s.add_all([a, b, ab, c1, c2])
s.commit()
s.close() # clears everything
c1 = s.query(C).get(1)
print "#1 EMITS LAZYLOAD:"
assoc = c1.assoc # note we keep a strong reference here
c2 = s.query(C).get(2)
print "SHOULD NOT EMIT LAZYLOAD"
assert c2.assoc is assoc
what happens here is mapper.get_clause() is determined as an and() using the order of columns in primary key only. The relationship join is based on Join._join_condition which iterates through columns in the source selectable, so the clauses are reversed.
the fix would be in the comparison:
diff --git a/lib/sqlalchemy/sql/elements.py b/lib/sqlalchemy/sql/elements.py
index 75d5368..cff5737 100644
--- a/lib/sqlalchemy/sql/elements.py
+++ b/lib/sqlalchemy/sql/elements.py
@@ -1828,12 +1828,23 @@ class ClauseList(ClauseElement):
if not isinstance(other, ClauseList) and len(self.clauses) == 1:
return self.clauses[0].compare(other, **kw)
elif isinstance(other, ClauseList) and \
- len(self.clauses) == len(other.clauses):
- for i in range(0, len(self.clauses)):
- if not self.clauses[i].compare(other.clauses[i], **kw):
- return False
+ len(self.clauses) == len(other.clauses) and \
+ self.operator is other.operator:
+
+ if self.operator in (operators.and_, operators.or_):
+ completed = set()
+ for clause in self.clauses:
+ for other_clause in set(other.clauses).difference(completed):
+ if clause.compare(other_clause, **kw):
+ completed.add(other_clause)
+ break
+ return len(completed) == len(other.clauses)
else:
- return self.operator == other.operator
+ for i in range(0, len(self.clauses)):
+ if not self.clauses[i].compare(other.clauses[i], **kw):
+ return False
+ else:
+ return True
else:
return False
Metadata
Metadata
Assignees
Labels
Type
Projects
Relationships
Development
No branches or pull requests
Activity
sqlalchemy-bot commentedon Sep 2, 2016
Michael Bayer (@zzzeek) wrote:
Repair clauselist comparison to account for clause ordering
Fixed bug where the "simple many-to-one" condition that allows lazy
loading to use get() from identity map would fail to be invoked if the
primaryjoin of the relationship had multiple clauses separated by AND
which were not in the same order as that of the primary key columns
being compared in each clause. This ordering
difference occurs for a composite foreign key where the table-bound
columns on the referencing side were not in the same order in the .c
collection as the primary key columns on the referenced side....which
in turn occurs a lot if one is using declarative mixins and/or
declared_attr to set up columns.
Change-Id: I66cce74f614c04ed693dc0d58ac8c952b2f8ae54
Fixes: #3788
→ ce577d4
sqlalchemy-bot commentedon Sep 2, 2016
Changes by Michael Bayer (@zzzeek):