diff options
| author | Mike Bayer <mike_mp@zzzcomputing.com> | 2015-05-06 17:07:24 -0400 |
|---|---|---|
| committer | Mike Bayer <mike_mp@zzzcomputing.com> | 2015-05-06 17:07:24 -0400 |
| commit | 1b120563905e29f9769fd6d2ce39922d15cebf7f (patch) | |
| tree | f987041976e8aa32b8f3e554cf1c50ba1fec55b2 | |
| parent | 5c852409219e18fb3d3b1403bfc872b3fe99f802 (diff) | |
| download | sqlalchemy-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.rst | 12 | ||||
| -rw-r--r-- | lib/sqlalchemy/orm/strategies.py | 8 | ||||
| -rw-r--r-- | lib/sqlalchemy/util/__init__.py | 2 | ||||
| -rw-r--r-- | lib/sqlalchemy/util/_collections.py | 13 | ||||
| -rw-r--r-- | test/orm/test_lazy_relations.py | 81 |
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_( |
