- Fixed unexpected-use regression where in the odd case that the

primaryjoin of a relationship involved comparison to an unhashable
type such as an HSTORE, lazy loads would fail due to a hash-oriented
check on the statement parameters, modified in 1.0 as a result of
:ticket:`3061` to use hashing and modified in :ticket:`3368`
to occur in cases more common than "load on pending".
The values are now checked for the ``__hash__`` attribute beforehand.
fixes #3416

diff --git a/doc/build/changelog/changelog_10.rst b/doc/build/changelog/changelog_10.rst
index 65a0511362616efa96962b44a1e2a2b94748c812..e51d885605fbfaee5d5801efd0723b4ea3ef0801 100644 (file)
--- a/doc/build/changelog/changelog_10.rst
+++ b/doc/build/changelog/changelog_10.rst
@@ -18,6 +18,18 @@
 .. changelog::
     :version: 1.0.4
 
+    .. change::
+        :tags: bug, orm
+        :tickets: 3416
+
+        Fixed unexpected-use regression where in the odd case that the
+        primaryjoin of a relationship involved comparison to an unhashable
+        type such as an HSTORE, lazy loads would fail due to a hash-oriented
+        check on the statement parameters, modified in 1.0 as a result of
+        :ticket:`3061` to use hashing and modified in :ticket:`3368`
+        to occur in cases more common than "load on pending".
+        The values are now checked for the ``__hash__`` attribute beforehand.
+
     .. change::
         :tags: bug, orm
         :tickets: 3412, 3347

diff --git a/lib/sqlalchemy/orm/strategies.py b/lib/sqlalchemy/orm/strategies.py
index 85d233a05e1ec7c178b9243df3215e11b6e00d11..78e9293458be95696bace6175ba460cef5fbaa90 100644 (file)
--- a/lib/sqlalchemy/orm/strategies.py
+++ b/lib/sqlalchemy/orm/strategies.py
@@ -586,9 +586,11 @@ class LazyLoader(AbstractRelationshipLoader, util.MemoizedSlots):
         lazy_clause, params = self._generate_lazy_clause(
             state, passive=passive)
 
-        if pending and orm_util._none_set.intersection(params.values()):
-            return None
-        elif orm_util._never_set.intersection(params.values()):
+        if pending:
+            if util.has_intersection(
+                    orm_util._none_set, params.values()):
+                return None
+        elif util.has_intersection(orm_util._never_set, params.values()):
             return None
 
         q = q.filter(lazy_clause).params(params)

diff --git a/lib/sqlalchemy/util/__init__.py b/lib/sqlalchemy/util/__init__.py
index d777d2e06f3fbccf973662bb9ef643fd51393626..ed968f1681cec0c127a6a980c855430d30f22797 100644 (file)
--- a/lib/sqlalchemy/util/__init__.py
+++ b/lib/sqlalchemy/util/__init__.py
@@ -19,7 +19,7 @@ from ._collections import KeyedTuple, ImmutableContainer, immutabledict, \
     OrderedSet, IdentitySet, OrderedIdentitySet, column_set, \
     column_dict, ordered_column_set, populate_column_dict, unique_list, \
     UniqueAppender, PopulateDict, EMPTY_SET, to_list, to_set, \
-    to_column_set, update_copy, flatten_iterator, \
+    to_column_set, update_copy, flatten_iterator, has_intersection, \
     LRUCache, ScopedRegistry, ThreadLocalRegistry, WeakSequence, \
     coerce_generator_arg, lightweight_named_tuple
 

diff --git a/lib/sqlalchemy/util/_collections.py b/lib/sqlalchemy/util/_collections.py
index db2c21949b1a6f9b97513d04136323d5ee34b927..5c62ebed86e7ffd03cbb764abb8c37624e4e2ca4 100644 (file)
--- a/lib/sqlalchemy/util/_collections.py
+++ b/lib/sqlalchemy/util/_collections.py
@@ -800,6 +800,19 @@ def to_list(x, default=None):
         return list(x)
 
 
+def has_intersection(set_, iterable):
+    """return True if any items of set_ are present in iterable.
+
+    Goes through special effort to ensure __hash__ is not called
+    on items in iterable that don't support it.
+
+    """
+    # TODO: optimize, write in C, etc.
+    return bool(
+        set_.intersection([i for i in iterable if i.__hash__])
+    )
+
+
 def to_set(x):
     if x is None:
         return set()

diff --git a/test/orm/test_lazy_relations.py b/test/orm/test_lazy_relations.py
index 166ee90cfc3c9d05d7da1f9bb9efafedae74a4a4..cfdfdf47c10e98a5884104c047bdf834b7781a4c 100644 (file)
--- a/test/orm/test_lazy_relations.py
+++ b/test/orm/test_lazy_relations.py
@@ -9,6 +9,7 @@ from sqlalchemy import Integer, String, ForeignKey, SmallInteger, Boolean
 from sqlalchemy.types import TypeDecorator
 from sqlalchemy.testing.schema import Table
 from sqlalchemy.testing.schema import Column
+from sqlalchemy import orm
 from sqlalchemy.orm import mapper, relationship, create_session, Session
 from sqlalchemy.testing import eq_
 from sqlalchemy.testing import fixtures
@@ -559,6 +560,58 @@ class GetterStateTest(_fixtures.FixtureTest):
 
     run_inserts = None
 
+    def _unhashable_fixture(self, metadata, load_on_pending=False):
+        class MyHashType(sa.TypeDecorator):
+            impl = sa.String(100)
+
+            def process_bind_param(self, value, dialect):
+                return ";".join(
+                    "%s=%s" % (k, v)
+                       for k, v in sorted(value.items(), lambda key: key[0]))
+
+            def process_result_value(self, value, dialect):
+                return dict(elem.split("=", 1) for elem in value.split(";"))
+
+        category = Table(
+            'category', metadata,
+            Column('id', Integer, primary_key=True),
+            Column('data', MyHashType())
+        )
+        article = Table(
+            'article', metadata,
+            Column('id', Integer, primary_key=True),
+            Column('data', MyHashType())
+        )
+
+        class Category(fixtures.ComparableEntity):
+            pass
+
+        class Article(fixtures.ComparableEntity):
+            pass
+
+        mapper(Category, category)
+        mapper(Article, article, properties={
+            "category": relationship(
+                Category,
+                primaryjoin=orm.foreign(article.c.data) == category.c.data,
+                load_on_pending=load_on_pending
+            )
+        })
+
+        metadata.create_all()
+        sess = Session(autoflush=False)
+        data = {"im": "unhashable"}
+        a1 = Article(id=1, data=data)
+        c1 = Category(id=1, data=data)
+        if load_on_pending:
+            sess.add(c1)
+        else:
+            sess.add_all([c1, a1])
+        sess.flush()
+        if load_on_pending:
+            sess.add(a1)
+        return Category, Article, sess, a1, c1
+
     def _u_ad_fixture(self, populate_user, dont_use_get=False):
         users, Address, addresses, User = (
             self.tables.users,
@@ -602,6 +655,34 @@ class GetterStateTest(_fixtures.FixtureTest):
             0
         )
 
+    @testing.provide_metadata
+    def test_no_use_get_params_not_hashable(self):
+        Category, Article, sess, a1, c1 = \
+            self._unhashable_fixture(self.metadata)
+
+        def go():
+            eq_(a1.category, c1)
+
+        self.assert_sql_count(
+            testing.db,
+            go,
+            1
+        )
+
+    @testing.provide_metadata
+    def test_no_use_get_params_not_hashable_on_pending(self):
+        Category, Article, sess, a1, c1 = \
+            self._unhashable_fixture(self.metadata, load_on_pending=True)
+
+        def go():
+            eq_(a1.category, c1)
+
+        self.assert_sql_count(
+            testing.db,
+            go,
+            1
+        )
+
     def test_get_empty_passive_return_never_set(self):
         User, Address, sess, a1 = self._u_ad_fixture(False)
         eq_(