From: Mike Bayer Date: Tue, 24 Apr 2012 15:24:23 +0000 (-0400) Subject: - [bug] The names of the columns on the X-Git-Tag: rel_0_8_0b1~467 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=9b73e997b2325018555268e1f1069e88e80fdb85;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git - [bug] The names of the columns on the .c. attribute of a select().apply_labels() is now based on _ instead of _, for those columns that have a distinctly named .key. [ticket:2397] --- diff --git a/CHANGES b/CHANGES index b2eaaa004d..3320bbce78 100644 --- a/CHANGES +++ b/CHANGES @@ -103,6 +103,13 @@ CHANGES also in 0.7.7. - sql + - [bug] The names of the columns on the + .c. attribute of a select().apply_labels() + is now based on _ instead + of _, for those columns + that have a distinctly named .key. + [ticket:2397] + - [feature] The Inspector object can now be acquired using the new inspect() service, part of [ticket:2208] diff --git a/lib/sqlalchemy/schema.py b/lib/sqlalchemy/schema.py index 9746af2287..0a22d8855d 100644 --- a/lib/sqlalchemy/schema.py +++ b/lib/sqlalchemy/schema.py @@ -1084,7 +1084,7 @@ class Column(SchemaItem, expression.ColumnClause): c.dispatch._update(self.dispatch) return c - def _make_proxy(self, selectable, name=None): + def _make_proxy(self, selectable, name=None, key=None): """Create a *proxy* for this column. This is a copy of this ``Column`` referenced by a different parent @@ -1102,7 +1102,7 @@ class Column(SchemaItem, expression.ColumnClause): c = self._constructor( expression._as_truncated(name or self.name), self.type, - key = name or self.key, + key = key if key else name if name else self.key, primary_key = self.primary_key, nullable = self.nullable, quote=self.quote, _proxies=[self], *fk) diff --git a/lib/sqlalchemy/sql/expression.py b/lib/sqlalchemy/sql/expression.py index d8ad7c3fa0..6147b1640a 100644 --- a/lib/sqlalchemy/sql/expression.py +++ b/lib/sqlalchemy/sql/expression.py @@ -2163,7 +2163,7 @@ class ColumnElement(ClauseElement, _CompareMixin): return hasattr(other, 'name') and hasattr(self, 'name') and \ other.name == self.name - def _make_proxy(self, selectable, name=None): + def _make_proxy(self, selectable, name=None, **kw): """Create a new :class:`.ColumnElement` representing this :class:`.ColumnElement` as it appears in the select list of a descending selectable. @@ -2171,14 +2171,10 @@ class ColumnElement(ClauseElement, _CompareMixin): """ if name is None: name = self.anon_label - # TODO: may want to change this to anon_label, - # or some value that is more useful than the - # compiled form of the expression key = str(self) else: key = name - - co = ColumnClause(_as_truncated(name), + co = ColumnClause(_as_truncated(name), selectable, type_=getattr(self, 'type', None)) @@ -3981,8 +3977,9 @@ class _Label(ColumnElement): def _from_objects(self): return self.element._from_objects - def _make_proxy(self, selectable, name = None): - e = self.element._make_proxy(selectable, name=name or self.name) + def _make_proxy(self, selectable, name=None, **kw): + e = self.element._make_proxy(selectable, + name=name if name else self.name) e.proxies.append(self) return e @@ -4142,12 +4139,12 @@ class ColumnClause(_Immutable, ColumnElement): _compared_to_type=self.type, unique=True) - def _make_proxy(self, selectable, name=None, attach=True): + def _make_proxy(self, selectable, name=None, attach=True, **kw): # propagate the "is_literal" flag only if we are keeping our name, # otherwise its considered to be a label is_literal = self.is_literal and (name is None or name == self.name) c = self._constructor( - _as_truncated(name or self.name), + _as_truncated(name if name else self.name), selectable=selectable, type_=self.type, is_literal=is_literal @@ -4158,7 +4155,7 @@ class ColumnClause(_Immutable, ColumnElement): selectable._is_clone_of.columns[c.name] if attach: - selectable._columns[c.name] = c + selectable._columns[c.key] = c return c class TableClause(_Immutable, FromClause): @@ -4584,8 +4581,9 @@ class _ScalarSelect(_Grouping): def self_group(self, **kwargs): return self - def _make_proxy(self, selectable, name): - return list(self.inner_columns)[0]._make_proxy(selectable, name) + def _make_proxy(self, selectable, name=None, **kw): + return list(self.inner_columns)[0]._make_proxy( + selectable, name=name) class CompoundSelect(_SelectBase): """Forms the basis of ``UNION``, ``UNION ALL``, and other @@ -4649,8 +4647,9 @@ class CompoundSelect(_SelectBase): # ForeignKeys in. this would allow the union() to have all # those fks too. - proxy = cols[0]._make_proxy(self, name=self.use_labels - and cols[0]._label or None) + proxy = cols[0]._make_proxy(self, + name=cols[0]._label if self.use_labels else None, + key=cols[0]._key_label if self.use_labels else None) # hand-construct the "proxies" collection to include all # derived columns place a 'weight' annotation corresponding @@ -5268,8 +5267,8 @@ class Select(_SelectBase): for c in self.inner_columns: if hasattr(c, '_make_proxy'): c._make_proxy(self, - name=self.use_labels - and c._label or None) + name=c._label if self.use_labels else None, + key=c._key_label if self.use_labels else None) def self_group(self, against=None): """return a 'grouping' construct as per the ClauseElement diff --git a/test/sql/test_compiler.py b/test/sql/test_compiler.py index feb7405db5..d5d5eaa2d0 100644 --- a/test/sql/test_compiler.py +++ b/test/sql/test_compiler.py @@ -248,14 +248,12 @@ class SelectTest(fixtures.TestBase, AssertsCompiledSQL): "SELECT sum(lala(mytable.myid)) AS bar FROM mytable" ) - # changes with #2397 self.assert_compile( select([keyed]), "SELECT keyed.x, keyed.y" ", keyed.z FROM keyed" ) - # changes with #2397 self.assert_compile( select([keyed]).apply_labels(), "SELECT keyed.x AS keyed_x, keyed.y AS " @@ -316,7 +314,6 @@ class SelectTest(fixtures.TestBase, AssertsCompiledSQL): ) # using alternate keys. - # this will change with #2397 a, b, c = Column('a', Integer, key='b'), \ Column('b', Integer), \ Column('c', Integer, key='a') @@ -370,7 +367,6 @@ class SelectTest(fixtures.TestBase, AssertsCompiledSQL): 'AS anon_1') def test_nested_label_targeting_keyed(self): - # this behavior chagnes with #2397 s1 = keyed.select() s2 = s1.alias() s3 = select([s2], use_labels=True) diff --git a/test/sql/test_query.py b/test/sql/test_query.py index 9f0c2dab09..f315d6621c 100644 --- a/test/sql/test_query.py +++ b/test/sql/test_query.py @@ -1280,7 +1280,7 @@ class KeyTargetingTest(fixtures.TablesTest): keyed2 = self.tables.keyed2 row = testing.db.execute(select([keyed1, keyed2])).first() - # without #2397, row.b is unambiguous + # row.b is unambiguous eq_(row.b, "b2") # row.a is ambiguous assert_raises_message( @@ -1289,21 +1289,6 @@ class KeyTargetingTest(fixtures.TablesTest): getattr, row, "a" ) - @testing.fails_if(lambda: True, "Possible future behavior") - def test_keyed_accessor_composite_conflict_2397(self): - keyed1 = self.tables.keyed1 - keyed2 = self.tables.keyed2 - - row = testing.db.execute(select([keyed1, keyed2])).first() - # with #2397, row.a is unambiguous - eq_(row.a, "a2") - # row.b is ambiguous - assert_raises_message( - exc.InvalidRequestError, - "Ambiguous column name 'b'", - getattr, row, 'b' - ) - def test_keyed_accessor_composite_names_precedent(self): keyed1 = self.tables.keyed1 keyed4 = self.tables.keyed4 @@ -1328,17 +1313,6 @@ class KeyTargetingTest(fixtures.TablesTest): ) eq_(row.d, "d3") - @testing.fails_if(lambda: True, "Possible future behavior") - def test_keyed_accessor_composite_2397(self): - keyed1 = self.tables.keyed1 - keyed3 = self.tables.keyed3 - - row = testing.db.execute(select([keyed1, keyed3])).first() - eq_(row.b, "a1") - eq_(row.q, "c1") - eq_(row.a, "a3") - eq_(row.d, "d3") - def test_keyed_accessor_composite_labeled(self): keyed1 = self.tables.keyed1 keyed2 = self.tables.keyed2 @@ -1366,22 +1340,12 @@ class KeyTargetingTest(fixtures.TablesTest): assert sql.column('content_type') in row def test_column_label_overlap_fallback_2(self): - # this fails with #2397 content, bar = self.tables.content, self.tables.bar row = testing.db.execute(content.select(use_labels=True)).first() assert content.c.type in row assert bar.c.content_type not in row assert sql.column('content_type') not in row - @testing.fails_if(lambda: True, "Possible future behavior") - def test_column_label_overlap_fallback_3(self): - # this passes with #2397 - content, bar = self.tables.content, self.tables.bar - row = testing.db.execute(content.select(use_labels=True)).first() - assert content.c.type in row - assert bar.c.content_type not in row - assert sql.column('content_type') in row - class LimitTest(fixtures.TestBase): diff --git a/test/sql/test_selectable.py b/test/sql/test_selectable.py index dde832e7da..e3508f77dc 100644 --- a/test/sql/test_selectable.py +++ b/test/sql/test_selectable.py @@ -100,8 +100,6 @@ class SelectableTest(fixtures.TestBase, AssertsExecutionResults, AssertsCompiled s = select([keyed]) eq_(s.c.colx.key, 'colx') - # this would change to 'colx' - # with #2397 eq_(s.c.colx.name, 'x') assert s.corresponding_column(keyed.c.colx) is s.c.colx @@ -113,6 +111,17 @@ class SelectableTest(fixtures.TestBase, AssertsExecutionResults, AssertsCompiled assert sel2.corresponding_column(keyed.c.coly) is sel2.c.coly assert sel2.corresponding_column(keyed.c.z) is sel2.c.z + def test_keyed_label_gen(self): + s = select([keyed]).apply_labels() + + assert s.corresponding_column(keyed.c.colx) is s.c.keyed_colx + assert s.corresponding_column(keyed.c.coly) is s.c.keyed_coly + assert s.corresponding_column(keyed.c.z) is s.c.keyed_z + + sel2 = s.alias() + assert sel2.corresponding_column(keyed.c.colx) is sel2.c.keyed_colx + assert sel2.corresponding_column(keyed.c.coly) is sel2.c.keyed_coly + assert sel2.corresponding_column(keyed.c.z) is sel2.c.keyed_z def test_distance_on_aliases(self): a1 = table1.alias('a1')