From: Mike Bayer Date: Mon, 24 Sep 2007 19:27:52 +0000 (+0000) Subject: - columns from Alias objects, when used to target result-row columns, must match... X-Git-Tag: rel_0_4beta6~13 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=a0838e0c476f7deab5b67f431c8ce107974b9313;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git - 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. --- diff --git a/CHANGES b/CHANGES index 63959fdd62..05c00b92da 100644 --- a/CHANGES +++ b/CHANGES @@ -36,7 +36,12 @@ CHANGES - 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 cc2ea6d5de..7e9d57d971 100644 --- a/lib/sqlalchemy/engine/base.py +++ b/lib/sqlalchemy/engine/base.py @@ -1287,10 +1287,19 @@ class ResultProxy(object): elif isinstance(key, basestring) and key.lower() in props: rec = props[key.lower()] elif isinstance(key, expression.ColumnElement): - label = context.column_labels.get(key._label, key.name).lower() - if label in props: - rec = props[label] - + 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 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/strategies.py b/lib/sqlalchemy/orm/strategies.py index 8016673ff5..f3a0029f57 100644 --- a/lib/sqlalchemy/orm/strategies.py +++ b/lib/sqlalchemy/orm/strategies.py @@ -615,6 +615,10 @@ class EagerLoader(AbstractRelationLoader): selectcontext.stack.pop() selectcontext.stack.pop() + + if self._should_log_debug: + self.logger.debug("Returning eager instance loader for %s" % str(self)) + return (execute, execute, None) else: if self._should_log_debug: diff --git a/lib/sqlalchemy/sql/expression.py b/lib/sqlalchemy/sql/expression.py index f7514fec9d..dac5d7a743 100644 --- a/lib/sqlalchemy/sql/expression.py +++ b/lib/sqlalchemy/sql/expression.py @@ -2423,6 +2423,14 @@ 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/orm/eager_relations.py b/test/orm/eager_relations.py index 749c286572..c1aa0e9ef4 100644 --- a/test/orm/eager_relations.py +++ b/test/orm/eager_relations.py @@ -225,6 +225,25 @@ class EagerTest(QueryTest): ] == q.all() self.assert_sql_count(testbase.db, go, 1) + + def test_no_false_hits(self): + """test that eager loaders don't interpret main table columns as part of their eager load.""" + + mapper(User, users, properties={ + 'addresses':relation(Address, lazy=False), + 'orders':relation(Order, lazy=False) + }) + mapper(Address, addresses) + mapper(Order, orders) + + allusers = create_session().query(User).all() + + # using a textual select, the columns will be 'id' and 'name'. + # the eager loaders have aliases which should not hit on those columns, they should + # be required to locate only their aliased/fully table qualified column name. + noeagers = create_session().query(User).from_statement("select * from users").all() + assert 'orders' not in noeagers[0].__dict__ + assert 'addresses' not in noeagers[0].__dict__ def test_limit(self): """test limit operations combined with lazy-load relationships.""" diff --git a/test/sql/query.py b/test/sql/query.py index a519dd974b..e64425e914 100644 --- a/test/sql/query.py +++ b/test/sql/query.py @@ -429,6 +429,22 @@ 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)