cloned selectable plus append_column causes internal error [patch]

This test:

    def test_append_column_with_cloned_select(self):
        sel = select([literal_column('1').label('a')](literal_column('1').label('a')))
        cloned = sel._clone()
        cloned._copy_internals()
        cloned.append_column(literal_column('2').label('b'))
        eq_(cloned.c.keys(), ['b']('a',))

fails with an internal error when calling .c:

  File "/Users/gthb/extsvn/sqlalchemy/./lib/sqlalchemy/util/langhelpers.py", line 485, in __get__
    obj.__dict__[self.__name__](self.__name__) = result = self.fget(obj)
  File "/Users/gthb/extsvn/sqlalchemy/./lib/sqlalchemy/sql/expression.py", line 2554, in columns
    self._populate_column_collection()
  File "/Users/gthb/extsvn/sqlalchemy/./lib/sqlalchemy/sql/expression.py", line 5251, in _populate_column_collection
    and c._label or None)
  File "/Users/gthb/extsvn/sqlalchemy/./lib/sqlalchemy/sql/expression.py", line 3964, in _make_proxy
    e = self.element._make_proxy(selectable, name=name or self.name)
  File "/Users/gthb/extsvn/sqlalchemy/./lib/sqlalchemy/sql/expression.py", line 4137, in _make_proxy
    selectable._is_clone_of.columns[c.name](c.name)
  File "/Users/gthb/extsvn/sqlalchemy/./lib/sqlalchemy/util/_collections.py", line 88, in __getitem__
    return self._data[key](key)
KeyError: u'b'

This test is distilled from usage in sqla_hierarchy, which was broken by 0536c48dafb670d34fc96d26078b41ed6accf01f (between 0.7.5 and 0.7.6).

This usage may well be hacky and make improper use of internal functions _clone and _copy_internals — but in any case the code added in that changeset assumes, for a selectable that originated by cloning another, that every column in the clone exists also in the original. Using append_column after cloning (which I think should be a valid thing to do) breaks that assumption.

Attached patch adds a failing test (possibly in the wrong place?), trivially fixes it (and thus gets sqla_hierarchy working again, at least the way I'm using it), and doesn't break any existing tests.

Comments (5)