create_lazy_clause() is only turning a target col into a bind on the first occurrence

Issue #3300 resolved
Mike Bayer repo owner created an issue

the logic commented out below has the effect that if a "local" column appears more than once in the clause, it only gets turned to a bound param once. need to determine the rationale for this logic and get it to be more specific to the case where it is needed.

diff --git a/lib/sqlalchemy/orm/relationships.py b/lib/sqlalchemy/orm/relationships.py
index df2250a..9a5e1ba 100644
--- a/lib/sqlalchemy/orm/relationships.py
+++ b/lib/sqlalchemy/orm/relationships.py
@@ -2709,10 +2709,10 @@ class JoinCondition(object):
         def col_to_bind(col):
             if (reverse_direction and col in lookup) or \
                     (not reverse_direction and "local" in col._annotations):
-                if col in lookup:
-                    for tobind, equated in lookup[col]:
-                        if equated in binds:
-                            return None
+                #if col in lookup:
+                #    for tobind, equated in lookup[col]:
+                #        if equated in binds:
+                #            return None
                 if col not in binds:
                     binds[col] = sql.bindparam(
                         None, None, type_=col.type, unique=True)

this script illustrates three different join conditions, the third one is pretty simple, which all illustrate the same thing happening:

from sqlalchemy import *
from sqlalchemy.orm import *
from sqlalchemy.ext.declarative import declarative_base

Base = declarative_base()


# NOT (:param_1 != a.attr OR a.attr IS NULL OR a.attr IS NULL)
# OR a.attr IS NULL AND a.attr IS NULL
notdistinct_1 = lambda a, b: ~(
    (a != b) | (a == None) |
    (b == None)) | ((a == None) & (b == None))

# :param_1 != a.attr OR a.attr IS NULL OR a.attr IS NULL
notdistinct_2 = lambda a, b: (
    (a != b) | (a == None) | (b == None))

# :param_1 = a.attr OR a.attr = a.attr
notdistinct_3 = lambda a, b: ((a == b) | (b == a))


class A(Base):
    __tablename__ = 'a'
    id = Column(Integer, primary_key=True)
    attr = Column(Integer)
    collection = relationship(
        "A",
        viewonly=True,
        primaryjoin=lambda: and_(
            notdistinct_3(A.attr, foreign(remote(A.attr)))
        )
    )

configure_mappers()

print A.collection.property.strategy._lazywhere

Comments (3)

  1. Mike Bayer reporter

    the logic is needed for the reverse case. Patch so far. it is scary, I'll have to review this very closely because I really want this in 0.9.

  2. Mike Bayer reporter
    • Fixed bug in lazy loading SQL construction whereby a complex primaryjoin that referred to the same "local" column multiple times in the "column that points to itself" style of self-referential join would not be substituted in all cases. The logic to determine substitutions here has been reworked to be more open-ended. fixes #3300

    → <<cset 9ea19b374630>>

  3. Mike Bayer reporter
    • Fixed bug in lazy loading SQL construction whereby a complex primaryjoin that referred to the same "local" column multiple times in the "column that points to itself" style of self-referential join would not be substituted in all cases. The logic to determine substitutions here has been reworked to be more open-ended. fixes #3300

    Conflicts: test/orm/test_relationships.py

    → <<cset 19776f3585f8>>

  4. Log in to comment