From 2484ef34c27f3342e62bd6285bb3668e2c913090 Mon Sep 17 00:00:00 2001 From: Mike Bayer Date: Mon, 15 Oct 2012 20:07:13 -0400 Subject: [PATCH] - [feature] The Query can now load entity/scalar-mixed "tuple" rows that contain types which aren't hashable, by setting the flag "hashable=False" on the corresponding TypeEngine object in use. Custom types that return unhashable types (typically lists) can set this flag to False. [ticket:2592] - [bug] Applying a column expression to a select statement using a label with or without other modifying constructs will no longer "target" that expression to the underlying Column; this affects ORM operations that rely upon Column targeting in order to retrieve results. That is, a query like query(User.id, User.id.label('foo')) will now track the value of each "User.id" expression separately instead of munging them together. It is not expected that any users will be impacted by this; however, a usage that uses select() in conjunction with query.from_statement() and attempts to load fully composed ORM entities may not function as expected if the select() named Column objects with arbitrary .label() names, as these will no longer target to the Column objects mapped by that entity. [ticket:2591] --- CHANGES | 26 ++++++++++++++++++++++++++ lib/sqlalchemy/engine/result.py | 8 ++++++-- lib/sqlalchemy/orm/query.py | 14 +++++++++++--- lib/sqlalchemy/sql/compiler.py | 4 ++-- lib/sqlalchemy/types.py | 6 ++++++ test/orm/test_query.py | 28 +++++++++++++++++++++++++--- test/sql/test_compiler.py | 18 +++++++++++++++++- test/sql/test_query.py | 15 ++++++++++++++- test/sql/test_type_expressions.py | 2 +- 9 files changed, 108 insertions(+), 13 deletions(-) diff --git a/CHANGES b/CHANGES index da57c086a9..97f5061c0f 100644 --- a/CHANGES +++ b/CHANGES @@ -178,6 +178,14 @@ underneath "0.7.xx". when a session first becomes active and when it deactivates. + - [feature] The Query can now load entity/scalar-mixed + "tuple" rows that contain + types which aren't hashable, by setting the flag + "hashable=False" on the corresponding TypeEngine object + in use. Custom types that return unhashable types + (typically lists) can set this flag to False. + [ticket:2592] + - [bug] Improvements to joined/subquery eager loading dealing with chains of subclass entities sharing a common base, with no specific "join depth" @@ -620,6 +628,24 @@ underneath "0.7.xx". by setting "case_insensitive=False" on create_engine(). [ticket:2423] + - [bug] Applying a column expression to a select + statement using a label with or without other + modifying constructs will no longer "target" that + expression to the underlying Column; this affects + ORM operations that rely upon Column targeting + in order to retrieve results. That is, a query + like query(User.id, User.id.label('foo')) will now + track the value of each "User.id" expression separately + instead of munging them together. It is not expected + that any users will be impacted by this; however, + a usage that uses select() in conjunction with + query.from_statement() and attempts to load fully + composed ORM entities may not function as expected + if the select() named Column objects with arbitrary + .label() names, as these will no longer target to + the Column objects mapped by that entity. + [ticket:2591] + - [feature] The "unconsumed column names" warning emitted when keys are present in insert.values() or update.values() that aren't in the target table is now an exception. diff --git a/lib/sqlalchemy/engine/result.py b/lib/sqlalchemy/engine/result.py index 9fb735f468..6962a4d1e2 100644 --- a/lib/sqlalchemy/engine/result.py +++ b/lib/sqlalchemy/engine/result.py @@ -228,16 +228,20 @@ class ResultMetaData(object): # is interpreted later as an AmbiguousColumnError, # but only when actually accessed. Columns # colliding by name is not a problem if those names - # aren't used; integer and ColumnElement access is always + # aren't used; integer access is always # unambiguous. primary_keymap[name if self.case_sensitive - else name.lower()] = rec = (processor, obj, None) + else name.lower()] = rec = (None, obj, None) self.keys.append(colname) if obj: for o in obj: keymap[o] = rec + # technically we should be doing this but we + # are saving on callcounts by not doing so. + # if keymap.setdefault(o, rec) is not rec: + # keymap[o] = (None, obj, None) if translate_colname and \ untranslated: diff --git a/lib/sqlalchemy/orm/query.py b/lib/sqlalchemy/orm/query.py index 35d32651fd..a6d20a9730 100644 --- a/lib/sqlalchemy/orm/query.py +++ b/lib/sqlalchemy/orm/query.py @@ -29,7 +29,8 @@ from .util import ( _is_aliased_class, _is_mapped_class, _orm_columns, join as orm_join,with_parent, aliased ) -from .. import sql, util, log, exc as sa_exc, inspect, inspection +from .. import sql, util, log, exc as sa_exc, inspect, inspection, \ + types as sqltypes from ..sql.expression import _interpret_as_from from ..sql import ( util as sql_util, @@ -2930,12 +2931,20 @@ class _ColumnEntity(_QueryEntity): if c is not column: return + if not isinstance(column, sql.ColumnElement): raise sa_exc.InvalidRequestError( "SQL expression, column, or mapped entity " "expected - got '%r'" % (column, ) ) + type_ = column.type + if type_.hashable: + self.filter_fn = lambda item: item + else: + counter = util.counter() + self.filter_fn = lambda item: counter() + # If the Column is unnamed, give it a # label() so that mutable column expressions # can be located in the result even @@ -2972,6 +2981,7 @@ class _ColumnEntity(_QueryEntity): else: self.entity_zero = None + @property def entity_zero_or_selectable(self): if self.entity_zero is not None: @@ -2985,8 +2995,6 @@ class _ColumnEntity(_QueryEntity): def type(self): return self.column.type - def filter_fn(self, item): - return item def adapt_to_selectable(self, query, sel): c = _ColumnEntity(query, sel.corresponding_column(self.column)) diff --git a/lib/sqlalchemy/sql/compiler.py b/lib/sqlalchemy/sql/compiler.py index f705a216e8..5fe30a8ff0 100644 --- a/lib/sqlalchemy/sql/compiler.py +++ b/lib/sqlalchemy/sql/compiler.py @@ -158,7 +158,7 @@ class _CompileLabel(visitors.Visitable): def __init__(self, col, name, alt_names=()): self.element = col self.name = name - self._alt_names = alt_names + self._alt_names = (col,) + alt_names @property def proxy_set(self): @@ -391,7 +391,7 @@ class SQLCompiler(engine.Compiled): add_to_result_map( labelname, label.name, - (label, label.element, labelname, ) + label._alt_names, + (label, labelname, ) + label._alt_names, label.type ) diff --git a/lib/sqlalchemy/types.py b/lib/sqlalchemy/types.py index 71bd39ba6e..ebcffca6e2 100644 --- a/lib/sqlalchemy/types.py +++ b/lib/sqlalchemy/types.py @@ -57,6 +57,12 @@ class TypeEngine(AbstractType): def __reduce__(self): return _reconstitute_comparator, (self.expr, ) + hashable = True + """Flag, if False, means values from this type aren't hashable. + + Used by the ORM when uniquing result lists. + + """ comparator_factory = Comparator """A :class:`.TypeEngine.Comparator` class which will apply diff --git a/test/orm/test_query.py b/test/orm/test_query.py index 11d86a5f0f..137fdcd95d 100644 --- a/test/orm/test_query.py +++ b/test/orm/test_query.py @@ -51,12 +51,12 @@ class RowTupleTest(QueryTest): User, users = self.classes.User, self.tables.users mapper(User, users, properties={ - 'uname':users.c.name + 'uname': users.c.name }) row = create_session().\ query(User.id, User.uname).\ - filter(User.id==7).first() + filter(User.id == 7).first() assert row.id == 7 assert row.uname == 'jack' @@ -106,7 +106,7 @@ class RowTupleTest(QueryTest): sess.query(name_label, fn), [ {'name':'uname', 'type':users.c.name.type, - 'aliased':False,'expr':name_label}, + 'aliased':False, 'expr':name_label}, {'name':None, 'type':fn.type, 'aliased':False, 'expr':fn }, @@ -118,6 +118,28 @@ class RowTupleTest(QueryTest): asserted ) + def test_unhashable_type(self): + from sqlalchemy.types import TypeDecorator, Integer + from sqlalchemy.sql import type_coerce + + class MyType(TypeDecorator): + impl = Integer + hashable = False + def process_result_value(self, value, dialect): + return [value] + + User, users = self.classes.User, self.tables.users + + mapper(User, users) + + s = Session() + row = s.\ + query(User, type_coerce(users.c.id, MyType).label('foo')).\ + filter(User.id == 7).first() + eq_( + row, (User(id=7), [7]) + ) + class RawSelectTest(QueryTest, AssertsCompiledSQL): __dialect__ = 'default' diff --git a/test/sql/test_compiler.py b/test/sql/test_compiler.py index 53e6260106..5b7a5d1d72 100644 --- a/test/sql/test_compiler.py +++ b/test/sql/test_compiler.py @@ -3245,6 +3245,22 @@ class ResultMapTest(fixtures.TestBase): {'a': ('a', (t.c.a, 'a', 'a'), t.c.a.type)}, ) + def test_label_plus_element(self): + t = Table('t', MetaData(), Column('a', Integer)) + l1 = t.c.a.label('bar') + tc = type_coerce(t.c.a, String) + stmt = select([t.c.a, l1, tc]) + comp = stmt.compile() + tc_anon_label = comp.result_map['a_1'][1][0] + eq_( + comp.result_map, + { + 'a': ('a', (t.c.a, 'a', 'a'), t.c.a.type), + 'bar': ('bar', (l1, 'bar'), l1.type), + 'a_1': ('%%(%d a)s' % id(tc), (tc_anon_label, 'a_1'), tc.type), + }, + ) + def test_label_conflict_union(self): t1 = Table('t1', MetaData(), Column('a', Integer), Column('b', Integer)) t2 = Table('t2', MetaData(), Column('t1_a', Integer)) @@ -3259,5 +3275,5 @@ class ResultMapTest(fixtures.TestBase): set(['t1_1_b', 't1_1_a', 't1_a', 't1_b']) ) is_( - comp.result_map['t1_a'][1][1], t1.c.a + comp.result_map['t1_a'][1][2], t1.c.a ) \ No newline at end of file diff --git a/test/sql/test_query.py b/test/sql/test_query.py index e2f2544c86..f8f5953c5a 100644 --- a/test/sql/test_query.py +++ b/test/sql/test_query.py @@ -993,6 +993,19 @@ class QueryTest(fixtures.TestBase): lambda: row[u2.c.user_id] ) + def test_ambiguous_column_by_col_plus_label(self): + users.insert().execute(user_id=1, user_name='john') + result = select([users.c.user_id, + type_coerce(users.c.user_id, Integer).label('foo')] + ).execute() + row = result.first() + eq_( + row[users.c.user_id], 1 + ) + eq_( + row[1], 1 + ) + @testing.requires.subqueries def test_column_label_targeting(self): users.insert().execute(user_id=7, user_name='ed') @@ -1641,7 +1654,7 @@ class KeyTargetingTest(fixtures.TablesTest): def test_column_label_overlap_fallback(self): content, bar = self.tables.content, self.tables.bar row = testing.db.execute(select([content.c.type.label("content_type")])).first() - assert content.c.type in row + assert content.c.type not in row assert bar.c.content_type not in row assert sql.column('content_type') in row diff --git a/test/sql/test_type_expressions.py b/test/sql/test_type_expressions.py index 4bb8a928d6..b65e4f24f3 100644 --- a/test/sql/test_type_expressions.py +++ b/test/sql/test_type_expressions.py @@ -56,7 +56,7 @@ class SelectTest(_ExprFixture, fixtures.TestBase, AssertsCompiledSQL): # the lower() function goes into the result_map, we don't really # need this but it's fine self.assert_compile( - compiled.result_map['test_table_y'][1][1], + compiled.result_map['test_table_y'][1][2], "lower(test_table.y)" ) # then the original column gets put in there as well. -- 2.47.3