simple many-to-one comparison is sensitive to order of clauses

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

Comments (1)