Support bindparam() with callable for primaryjoin

Fixes the comparison of bindparam() objects based on
the "callable" parameter being present which helps to correctly
detect use_get, and also checks for "callable" when detecting
parameters for value substitution and will not impact the
object if present.

Change-Id: I4c93ee5d404d2648dd9835beeae0c5fb67e37d19
Fixes: #3767

5 files changed, 75 insertions, 5 deletions

diff --git a/doc/build/changelog/changelog_11.rst b/doc/build/changelog/changelog_11.rst
index a09703489..3919c6ec6 100644
--- a/doc/build/changelog/changelog_11.rst
+++ b/doc/build/changelog/changelog_11.rst
@@ -23,6 +23,17 @@
 
     .. change::
         :tags: bug, orm
+        :tickets: 3767
+
+        The primaryjoin of a :func:`.relationship` construct can now include
+        a :func:`.bindparam` object that includes a callable function to
+        generate values.  Previously, the lazy loader strategy would
+        be incompatible with this use, and additionally would fail to correctly
+        detect if the "use_get" criteria should be used if the primary key
+        were involved with the bound parameter.
+
+    .. change::
+        :tags: bug, orm
         :tickets: 3788
 
         Fixed bug where the "simple many-to-one" condition that allows  lazy
diff --git a/lib/sqlalchemy/orm/strategies.py b/lib/sqlalchemy/orm/strategies.py
index 0b22b8486..202b652b7 100644
--- a/lib/sqlalchemy/orm/strategies.py
+++ b/lib/sqlalchemy/orm/strategies.py
@@ -478,7 +478,7 @@ class LazyLoader(AbstractRelationshipLoader, util.MemoizedSlots):
                 params.append((
                     bindparam.key, bind_to_col[bindparam._identifying_key],
                     None))
-            else:
+            elif bindparam.callable is None:
                 params.append((bindparam.key, None, bindparam.value))
 
         criterion = visitors.cloned_traverse(
diff --git a/lib/sqlalchemy/sql/elements.py b/lib/sqlalchemy/sql/elements.py
index cff57372c..768574c1a 100644
--- a/lib/sqlalchemy/sql/elements.py
+++ b/lib/sqlalchemy/sql/elements.py
@@ -1145,7 +1145,8 @@ class BindParameter(ColumnElement):
 
         return isinstance(other, BindParameter) \
             and self.type._compare_type_affinity(other.type) \
-            and self.value == other.value
+            and self.value == other.value \
+            and self.callable == other.callable
 
     def __getstate__(self):
         """execute a deferred value for serialization purposes."""
diff --git a/test/orm/test_lazy_relations.py b/test/orm/test_lazy_relations.py
index 56d1b8323..6ae7d9a55 100644
--- a/test/orm/test_lazy_relations.py
+++ b/test/orm/test_lazy_relations.py
@@ -4,7 +4,7 @@ from sqlalchemy.testing import assert_raises
 import datetime
 from sqlalchemy.orm import attributes, exc as orm_exc, configure_mappers
 import sqlalchemy as sa
-from sqlalchemy import testing, and_
+from sqlalchemy import testing, and_, bindparam
 from sqlalchemy import Integer, String, ForeignKey, SmallInteger, Boolean
 from sqlalchemy import ForeignKeyConstraint
 from sqlalchemy.types import TypeDecorator
@@ -259,6 +259,35 @@ class LazyTest(_fixtures.FixtureTest):
         u1 = s.query(User).filter(User.id == 7).one()
         assert_raises(sa.exc.SAWarning, getattr, u1, 'order')
 
+    def test_callable_bind(self):
+        Address, addresses, users, User = (
+            self.classes.Address,
+            self.tables.addresses,
+            self.tables.users,
+            self.classes.User)
+
+        mapper(User, users, properties=dict(
+            addresses=relationship(
+                mapper(Address, addresses),
+                lazy='select',
+                primaryjoin=and_(
+                    users.c.id == addresses.c.user_id,
+                    users.c.name == bindparam("name", callable_=lambda: "ed")
+                )
+            )
+        ))
+
+        s = Session()
+        ed = s.query(User).filter_by(name='ed').one()
+        eq_(ed.addresses, [
+            Address(id=2, user_id=8),
+            Address(id=3, user_id=8),
+            Address(id=4, user_id=8)
+        ])
+
+        fred = s.query(User).filter_by(name='fred').one()
+        eq_(fred.addresses, [])  # fred is missing
+
     def test_one_to_many_scalar(self):
         Address, addresses, users, User = (
             self.classes.Address,
diff --git a/test/sql/test_utils.py b/test/sql/test_utils.py
index 09d7e98af..5e54cf734 100644
--- a/test/sql/test_utils.py
+++ b/test/sql/test_utils.py
@@ -1,6 +1,6 @@
 from sqlalchemy.testing import fixtures, is_true, is_false
-from sqlalchemy import MetaData, Table, Column, Integer
-from sqlalchemy import and_, or_
+from sqlalchemy import MetaData, Table, Column, Integer, String
+from sqlalchemy import and_, or_, bindparam
 from sqlalchemy.sql.elements import ClauseList
 from sqlalchemy.sql import operators
 
@@ -76,3 +76,32 @@ class CompareClausesTest(fixtures.TestBase):
 
         is_false(l1.compare(l2))
 
+    def test_compare_binds(self):
+        b1 = bindparam("foo", type_=Integer())
+        b2 = bindparam("foo", type_=Integer())
+        b3 = bindparam("bar", type_=Integer())
+        b4 = bindparam("foo", type_=String())
+
+        c1 = lambda: 5  # noqa
+        c2 = lambda: 6  # noqa
+
+        b5 = bindparam("foo", type_=Integer(), callable_=c1)
+        b6 = bindparam("foo", type_=Integer(), callable_=c2)
+        b7 = bindparam("foo", type_=Integer(), callable_=c1)
+
+        b8 = bindparam("foo", type_=Integer, value=5)
+        b9 = bindparam("foo", type_=Integer, value=6)
+
+        is_false(b1.compare(b5))
+        is_true(b5.compare(b7))
+        is_false(b5.compare(b6))
+        is_true(b1.compare(b2))
+
+        # currently not comparing "key", as we often have to compare
+        # anonymous names.  however we should really check for that
+        is_true(b1.compare(b3))
+
+        is_false(b1.compare(b4))
+        is_false(b1.compare(b8))
+        is_false(b8.compare(b9))
+        is_true(b8.compare(b8))