diff options
| -rw-r--r-- | CHANGES | 15 | ||||
| -rw-r--r-- | lib/sqlalchemy/engine/base.py | 16 | ||||
| -rw-r--r-- | lib/sqlalchemy/orm/interfaces.py | 4 | ||||
| -rw-r--r-- | lib/sqlalchemy/orm/query.py | 35 | ||||
| -rw-r--r-- | lib/sqlalchemy/orm/shard.py | 10 | ||||
| -rw-r--r-- | lib/sqlalchemy/orm/strategies.py | 26 | ||||
| -rw-r--r-- | lib/sqlalchemy/sql/expression.py | 8 | ||||
| -rw-r--r-- | test/sql/query.py | 16 |
8 files changed, 60 insertions, 70 deletions
@@ -22,6 +22,16 @@ strong reference to those instances until changes are flushed. - Added full list of SQLite reserved keywords so that they get escaped properly. +- tightened up the relationship between the Query's generation + of "eager load" aliases, and Query.instances() which actually grabs the + eagerly loaded rows. If the aliases were not specifically generated for + that statement by EagerLoader, the EagerLoader will not take effect + when the rows are fetched. This prevents columns from being grabbed accidentally + as being part of an eager load when they were not meant for such, which can happen + with textual SQL as well as some inheritance situations. It's particularly important + since the "anonymous aliasing" of columns uses simple integer counts now to generate + labels. + - Removed "parameters" argument from clauseelement.compile(), replaced with "column_keys". The parameters sent to execute() only interact with the insert/update statement compilation process in terms of the column names @@ -42,11 +52,6 @@ properly. - added "schema" argument to Sequence; use this with Postgres /Oracle when the sequence is located in an alternate schema. Implements part of [ticket:584], should fix [ticket:761]. -- columns from Alias objects, when used to target result-row columns, must match exactly - to the label used in the generated statement. This is so searching for columns in a - result row which match aliases won't accidentally match non-aliased columns. - fixes errors which can arise in eager loading scenarios. - - Fixed reflection of the empty string for mysql enums. - Added 'passive_deletes="all"' flag to relation(), disables all nulling-out diff --git a/lib/sqlalchemy/engine/base.py b/lib/sqlalchemy/engine/base.py index 7e9d57d97..ad2b5df96 100644 --- a/lib/sqlalchemy/engine/base.py +++ b/lib/sqlalchemy/engine/base.py @@ -1287,19 +1287,9 @@ class ResultProxy(object): elif isinstance(key, basestring) and key.lower() in props: rec = props[key.lower()] elif isinstance(key, expression.ColumnElement): - try: - if getattr(key, '_exact_match', False): - # exact match flag means the label must be present in the - # generated column_labels - label = context.column_labels[key._label].lower() - else: - # otherwise, fall back to the straight name of the column - # if not in generated labels - label = context.column_labels.get(key._label, key.name).lower() - if label in props: - rec = props[label] - except KeyError: - pass + label = context.column_labels.get(key._label, key.name).lower() + if label in props: + rec = props[label] if not "rec" in locals(): raise exceptions.NoSuchColumnError("Could not locate column in row for column '%s'" % (str(key))) diff --git a/lib/sqlalchemy/orm/interfaces.py b/lib/sqlalchemy/orm/interfaces.py index e0f7799b7..8da7d917c 100644 --- a/lib/sqlalchemy/orm/interfaces.py +++ b/lib/sqlalchemy/orm/interfaces.py @@ -493,10 +493,10 @@ class OperationContext(object): Accept ``MapperOption`` objects which may modify its state before proceeding. """ - def __init__(self, mapper, options): + def __init__(self, mapper, options, attributes=None): self.mapper = mapper self.options = options - self.attributes = {} + self.attributes = attributes or {} self.recursion_stack = util.Set() for opt in util.flatten_iterator(options): self.accept_option(opt) diff --git a/lib/sqlalchemy/orm/query.py b/lib/sqlalchemy/orm/query.py index cb30eafcf..0c62737cb 100644 --- a/lib/sqlalchemy/orm/query.py +++ b/lib/sqlalchemy/orm/query.py @@ -612,16 +612,16 @@ class Query(object): raise exceptions.InvalidRequestError('Multiple rows returned for one()') def __iter__(self): - statement = self.compile() - statement.use_labels = True + context = self._compile_context() + context.statement.use_labels = True if self._autoflush and not self._populate_existing: self.session._autoflush() - return self._execute_and_instances(statement) + return self._execute_and_instances(context) - def _execute_and_instances(self, statement): - result = self.session.execute(statement, params=self._params, mapper=self.mapper) + def _execute_and_instances(self, querycontext): + result = self.session.execute(querycontext.statement, params=self._params, mapper=self.mapper) try: - return iter(self.instances(result)) + return iter(self.instances(result, querycontext=querycontext)) finally: result.close() @@ -784,10 +784,16 @@ class Query(object): def compile(self): """compiles and returns a SQL statement based on the criterion and conditions within this Query.""" + return self._compile_context().statement + def _compile_context(self): + + context = QueryContext(self) + if self._statement: self._statement.use_labels = True - return self._statement + context.statement = self._statement + return context whereclause = self._criterion @@ -807,7 +813,6 @@ class Query(object): # get/create query context. get the ultimate compile arguments # from there - context = QueryContext(self) order_by = context.order_by from_obj = context.from_obj lockmode = context.lockmode @@ -887,7 +892,7 @@ class Query(object): m = clauses.adapt_clause(m) statement.append_column(m) - return statement + return context def _get_entity_clauses(self, m): """for tuples added via add_entity() or add_column(), attempt to locate @@ -1209,17 +1214,27 @@ class SelectionContext(OperationContext): Indicates if mappers that have version_id columns should verify that instances existing already within the Session should have this attribute compared to the freshly loaded value. + + querycontext + the QueryContext, if any, used to generate the executed statement. + If present, the attribute dictionary from this Context will be used + as the basis for this SelectionContext's attribute dictionary. This + allows query-compile-time operations to send messages to the + result-processing-time operations. """ def __init__(self, mapper, session, extension, **kwargs): self.populate_existing = kwargs.pop('populate_existing', False) self.version_check = kwargs.pop('version_check', False) + querycontext = kwargs.pop('querycontext', None) + if querycontext: + kwargs['attributes'] = querycontext.attributes self.session = session self.extension = extension self.identity_map = {} self.stack = LoaderStack() super(SelectionContext, self).__init__(mapper, kwargs.pop('with_options', []), **kwargs) - + def accept_option(self, opt): """Accept a MapperOption which will process (modify) the state of this SelectionContext. diff --git a/lib/sqlalchemy/orm/shard.py b/lib/sqlalchemy/orm/shard.py index 48e057966..c38bcdd96 100644 --- a/lib/sqlalchemy/orm/shard.py +++ b/lib/sqlalchemy/orm/shard.py @@ -78,19 +78,19 @@ class ShardedQuery(Query): q._shard_id = shard_id return q - def _execute_and_instances(self, statement): + def _execute_and_instances(self, context): if self._shard_id is not None: - result = self.session.connection(mapper=self.mapper, shard_id=self._shard_id).execute(statement, **self._params) + result = self.session.connection(mapper=self.mapper, shard_id=self._shard_id).execute(context.statement, **self._params) try: - return iter(self.instances(result)) + return iter(self.instances(result, querycontext=context)) finally: result.close() else: partial = [] for shard_id in self.query_chooser(self): - result = self.session.connection(mapper=self.mapper, shard_id=shard_id).execute(statement, **self._params) + result = self.session.connection(mapper=self.mapper, shard_id=shard_id).execute(context.statement, **self._params) try: - partial = partial + list(self.instances(result)) + partial = partial + list(self.instances(result, querycontext=context)) finally: result.close() # if some kind of in memory 'sorting' were done, this is where it would happen diff --git a/lib/sqlalchemy/orm/strategies.py b/lib/sqlalchemy/orm/strategies.py index f3a0029f5..b93993af8 100644 --- a/lib/sqlalchemy/orm/strategies.py +++ b/lib/sqlalchemy/orm/strategies.py @@ -506,12 +506,17 @@ class EagerLoader(AbstractRelationLoader): break else: raise exceptions.InvalidRequestError("EagerLoader cannot locate a clause with which to outer join to, in query '%s' %s" % (str(statement), localparent.mapped_table)) - + + # create AliasedClauses object to build up the eager query. this is cached after 1st creation. try: clauses = self.clauses[path] except KeyError: clauses = mapperutil.PropertyAliasedClauses(self.parent_property, self.parent_property.polymorphic_primaryjoin, self.parent_property.polymorphic_secondaryjoin, parentclauses) self.clauses[path] = clauses + + # place the "row_decorator" from the AliasedClauses into the QueryContext, where it will + # be picked up in create_row_processor() when results are fetched + context.attributes[("eager_row_processor", path)] = clauses.row_decorator if self.secondaryjoin is not None: statement._outerjoin = sql.outerjoin(towrap, clauses.secondary, clauses.primaryjoin).outerjoin(clauses.alias, clauses.secondaryjoin) @@ -545,19 +550,18 @@ class EagerLoader(AbstractRelationLoader): # custom row decoration function, placed in the selectcontext by the # contains_eager() mapper option decorator = selectcontext.attributes[("eager_row_processor", self.parent_property)] + # key was present, but no decorator; therefore just use the row as is if decorator is None: decorator = lambda row: row + # check for an AliasedClauses row decorator that was set up by query._compile_context(). + # a further refactoring (described in [ticket:777]) will simplify this so that the + # contains_eager() option generates the same key as this one + elif ("eager_row_processor", path) in selectcontext.attributes: + decorator = selectcontext.attributes[("eager_row_processor", path)] else: - try: - # decorate the row according to the stored AliasedClauses for this eager load - clauses = self.clauses[path] - decorator = clauses.row_decorator - except KeyError, k: - # no stored AliasedClauses: eager loading was not set up in the query and - # AliasedClauses never got initialized - if self._should_log_debug: - self.logger.debug("Could not locate aliased clauses for key: " + str(path)) - return None + if self._should_log_debug: + self.logger.debug("Could not locate aliased clauses for key: " + str(path)) + return None try: decorated_row = decorator(row) diff --git a/lib/sqlalchemy/sql/expression.py b/lib/sqlalchemy/sql/expression.py index d649fc0ff..4bacf4be3 100644 --- a/lib/sqlalchemy/sql/expression.py +++ b/lib/sqlalchemy/sql/expression.py @@ -2425,14 +2425,6 @@ class Alias(FromClause): def _get_from_objects(self, **modifiers): return [self] - def _proxy_column(self, column): - c = column._make_proxy(self) - # send a note to ResultProxy to not "approximate" - # this column based on its name when targeting result columns - # see test/sql/query.py QueryTest.test_exact_match - c._exact_match = True - return c - bind = property(lambda s: s.selectable.bind) class _ColumnElementAdapter(ColumnElement): diff --git a/test/sql/query.py b/test/sql/query.py index e64425e91..a519dd974 100644 --- a/test/sql/query.py +++ b/test/sql/query.py @@ -429,22 +429,6 @@ class QueryTest(PersistTest): self.assertEqual([x.lower() for x in r.keys()], ['user_name', 'user_id']) self.assertEqual(r.values(), ['foo', 1]) - def test_exact_match(self): - """test that an Alias object only targets result columns that were generated - from that Alias. This is also part of eager_relations.py/test_no_false_hits. - """ - - users.insert().execute(user_id=1, user_name='ed') - users_alias = users.alias() - result = users.select().execute() - row = result.fetchone() - assert users_alias.c.user_id not in row - - result = users_alias.select().execute() - row = result.fetchone() - assert users_alias.c.user_id in row - - @testing.unsupported('oracle', 'firebird') def test_column_accessor_shadow(self): meta = MetaData(testbase.db) |