diff options
| -rw-r--r-- | CHANGES | 6 | ||||
| -rw-r--r-- | lib/sqlalchemy/orm/interfaces.py | 36 | ||||
| -rw-r--r-- | test/orm/test_eager_relations.py | 4 | ||||
| -rw-r--r-- | test/orm/test_mapper.py | 4 | ||||
| -rw-r--r-- | test/orm/test_query.py | 71 |
5 files changed, 107 insertions, 14 deletions
@@ -47,6 +47,12 @@ CHANGES to state actually within the current flush. [ticket:2082] + - Improvements to the error messages emitted when + querying against column-only entities in conjunction + with (typically incorrectly) using loader options, + where the parent entity is not fully present. + [ticket:2069] + - sql - Added a fully descriptive error message for the case where Column is subclassed and _make_proxy() diff --git a/lib/sqlalchemy/orm/interfaces.py b/lib/sqlalchemy/orm/interfaces.py index 907dd4bf5..8055aa504 100644 --- a/lib/sqlalchemy/orm/interfaces.py +++ b/lib/sqlalchemy/orm/interfaces.py @@ -422,7 +422,7 @@ class PropertyOption(MapperOption): state['key'] = tuple(ret) self.__dict__ = state - def _find_entity( self, query, mapper, raiseerr): + def _find_entity_prop_comparator(self, query, token, mapper, raiseerr): if mapperutil._is_aliased_class(mapper): searchfor = mapper isa = False @@ -435,9 +435,27 @@ class PropertyOption(MapperOption): return ent else: if raiseerr: - raise sa_exc.ArgumentError("Can't find entity %s in " - "Query. Current list: %r" % (searchfor, - [str(m.path_entity) for m in query._entities])) + raise sa_exc.ArgumentError( + "Can't find property '%s' on any entity " + "specified in this Query." % (token,) + ) + else: + return None + + def _find_entity_basestring(self, query, token, raiseerr): + for ent in query._mapper_entities: + # return only the first _MapperEntity when searching + # based on string prop name. Ideally object + # attributes are used to specify more exactly. + return ent + else: + if raiseerr: + raise sa_exc.ArgumentError( + "Can't find property named '%s' on the first mapped " + "entity in this Query. " + "Consider using an attribute object instead of a " + "string name to target a specific entity." % (token, ) + ) else: return None @@ -459,13 +477,13 @@ class PropertyOption(MapperOption): sub_tokens = token.split(".", 1) token = sub_tokens[0] tokens.extendleft(sub_tokens[1:]) - if not entity: if current_path: if current_path[1] == token: current_path = current_path[2:] continue - entity = query._entity_zero() + entity = self._find_entity_basestring(query, + token, raiseerr) path_element = entity.path_entity mapper = entity.mapper mappers.append(mapper) @@ -473,7 +491,6 @@ class PropertyOption(MapperOption): prop = mapper.get_property(token) else: prop = None - key = token elif isinstance(token, PropComparator): prop = token.property if not entity: @@ -482,14 +499,13 @@ class PropertyOption(MapperOption): prop.key]: current_path = current_path[2:] continue - entity = self._find_entity(query, - token.parententity, raiseerr) + entity = self._find_entity_prop_comparator(query, + prop.key, token.parententity, raiseerr) if not entity: return [], [] path_element = entity.path_entity mapper = entity.mapper mappers.append(prop.parent) - key = prop.key else: raise sa_exc.ArgumentError('mapper option expects ' 'string key or list of attributes') diff --git a/test/orm/test_eager_relations.py b/test/orm/test_eager_relations.py index 095254b27..8fdb844b4 100644 --- a/test/orm/test_eager_relations.py +++ b/test/orm/test_eager_relations.py @@ -4,7 +4,7 @@ from test.lib.testing import eq_, is_, is_not_ import sqlalchemy as sa from test.lib import testing from sqlalchemy.orm import joinedload, deferred, undefer, \ - joinedload_all, backref + joinedload_all, backref, eagerload from sqlalchemy import Integer, String, Date, ForeignKey, and_, select, \ func from test.lib.schema import Table, Column @@ -332,7 +332,6 @@ class EagerTest(_fixtures.FixtureTest, testing.AssertsCompiledSQL): mapper(Item, items, properties = dict( keywords = relationship(Keyword, secondary=item_keywords, lazy='select', order_by=keywords.c.id))) - q = create_session().query(Item) def go(): @@ -342,6 +341,7 @@ class EagerTest(_fixtures.FixtureTest, testing.AssertsCompiledSQL): self.assert_sql_count(testing.db, go, 1) + @testing.resolve_artifact_names def test_cyclical(self): """A circular eager relationship breaks the cycle with a lazy loader""" diff --git a/test/orm/test_mapper.py b/test/orm/test_mapper.py index e26097116..552b631c2 100644 --- a/test/orm/test_mapper.py +++ b/test/orm/test_mapper.py @@ -1493,8 +1493,8 @@ class DeepOptionsTest(_fixtures.FixtureTest): assert_raises_message( sa.exc.ArgumentError, - r"Can't find entity Mapper\|Order\|orders in Query. " - r"Current list: \['Mapper\|User\|users'\]", + "Can't find property 'items' on any entity " + "specified in this Query.", sess.query(User).options, sa.orm.joinedload(Order.items)) # joinedload "keywords" on items. it will lazy load "orders", then diff --git a/test/orm/test_query.py b/test/orm/test_query.py index 8e94f56c9..6a7e7d624 100644 --- a/test/orm/test_query.py +++ b/test/orm/test_query.py @@ -2137,3 +2137,74 @@ class OptionsTest(QueryTest): opt = self._option_fixture(Address.user, User.addresses) self._assert_path_result(opt, q, [], []) +class OptionsNoPropTest(_base.MappedTest): + """test the error messages emitted when using property + options in conjunection with column-only entities. + + """ + + @testing.resolve_artifact_names + def test_option_with_mapper_using_basestring(self): + self._assert_option([Item], 'keywords') + + @testing.resolve_artifact_names + def test_option_with_mapper_using_PropCompatator(self): + self._assert_option([Item], Item.keywords) + + @testing.resolve_artifact_names + def test_option_with_mapper_then_column_using_basestring(self): + self._assert_option([Item, Item.id], 'keywords') + + @testing.resolve_artifact_names + def test_option_with_mapper_then_column_using_PropComparator(self): + self._assert_option([Item, Item.id], Item.keywords) + + @testing.resolve_artifact_names + def test_option_with_column_then_mapper_using_basestring(self): + self._assert_option([Item.id, Item], 'keywords') + + @testing.resolve_artifact_names + def test_option_with_column_then_mapper_using_PropComparator(self): + self._assert_option([Item.id, Item], Item.keywords) + + @testing.resolve_artifact_names + def test_option_with_column_using_basestring(self): + message = \ + "Can't find property named 'keywords' on the first mapped "\ + "entity in this Query. Consider using an attribute object "\ + "instead of a string name to target a specific entity." + self._assert_eager_with_just_column_exception(Item.id, + 'keywords', message) + + @testing.resolve_artifact_names + def test_option_with_column_using_PropComparator(self): + message = \ + "Can't find property 'keywords' on any entity specified "\ + "in this Query\." + self._assert_eager_with_just_column_exception(Item.id, + Item.keywords, message) + + @classmethod + def define_tables(cls, metadata): + pass + + @classmethod + @testing.resolve_artifact_names + def setup_mappers(cls): + mapper(Keyword, keywords) + mapper(Item, items, + properties=dict(keywords=relationship(Keyword, + secondary=item_keywords))) + + @testing.resolve_artifact_names + def _assert_option(self, entity_list, option): + q = create_session().query(*entity_list).\ + options(eagerload(option)) + key = ('loaderstrategy', (class_mapper(Item), 'keywords')) + assert key in q._attributes + + def _assert_eager_with_just_column_exception(self, column, + eager_option, message): + assert_raises_message(sa.exc.ArgumentError, message, + create_session().query(column).options, + eagerload(eager_option)) |