summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorMike Bayer <mike_mp@zzzcomputing.com>2015-05-06 17:07:24 -0400
committerMike Bayer <mike_mp@zzzcomputing.com>2015-05-06 17:07:24 -0400
commit1b120563905e29f9769fd6d2ce39922d15cebf7f (patch)
treef987041976e8aa32b8f3e554cf1c50ba1fec55b2
parent5c852409219e18fb3d3b1403bfc872b3fe99f802 (diff)
downloadsqlalchemy-1b120563905e29f9769fd6d2ce39922d15cebf7f.tar.gz
- 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
-rw-r--r--doc/build/changelog/changelog_10.rst12
-rw-r--r--lib/sqlalchemy/orm/strategies.py8
-rw-r--r--lib/sqlalchemy/util/__init__.py2
-rw-r--r--lib/sqlalchemy/util/_collections.py13
-rw-r--r--test/orm/test_lazy_relations.py81
5 files changed, 112 insertions, 4 deletions
diff --git a/doc/build/changelog/changelog_10.rst b/doc/build/changelog/changelog_10.rst
index 65a051136..e51d88560 100644
--- a/doc/build/changelog/changelog_10.rst
+++ b/doc/build/changelog/changelog_10.rst
@@ -20,6 +20,18 @@
.. 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
Liberalized an assertion that was added as part of :ticket:`3347`
diff --git a/lib/sqlalchemy/orm/strategies.py b/lib/sqlalchemy/orm/strategies.py
index 85d233a05..78e929345 100644
--- 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 d777d2e06..ed968f168 100644
--- 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 db2c21949..5c62ebed8 100644
--- 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 166ee90cf..cfdfdf47c 100644
--- 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_(