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

Issue #3788 resolved
Michael Bayer
repo owner created an issue
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)

  1. Michael Bayer reporter

    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

    → <<cset ce577d484495>>

  2. Log in to comment