From 8163de4cc9e01460d3476b9fb3ed14a5b3e70bae Mon Sep 17 00:00:00 2001 From: Mike Bayer Date: Tue, 26 Jan 2016 16:41:26 -0500 Subject: [PATCH] - rework ColumnCollection to no longer persist "all_col_set"; we don't need this collection except in the extend/update uses where we create it ad-hoc. simplifies pickling. Compatibility with 1.0 should be OK as ColumnColleciton uses __getstate__ in any case and the __setstate__ contract hasn't changed. - Fixed bug in :class:`.Table` metadata construct which appeared around the 0.9 series where adding columns to a :class:`.Table` that was unpickled would fail to correctly establish the :class:`.Column` within the 'c' collection, leading to issues in areas such as ORM configuration. This could impact use cases such as ``extend_existing`` and others. fixes #3632 --- doc/build/changelog/changelog_10.rst | 11 +++++++++++ lib/sqlalchemy/sql/base.py | 29 +++++++++------------------- test/sql/test_metadata.py | 19 ++++++++++++++++++ 3 files changed, 39 insertions(+), 20 deletions(-) diff --git a/doc/build/changelog/changelog_10.rst b/doc/build/changelog/changelog_10.rst index a1e1fbe179..a0b1ad9572 100644 --- a/doc/build/changelog/changelog_10.rst +++ b/doc/build/changelog/changelog_10.rst @@ -19,6 +19,17 @@ :version: 1.0.12 :released: + .. change:: + :tags: bug, sql + :tickets: 3632 + + Fixed bug in :class:`.Table` metadata construct which appeared + around the 0.9 series where adding columns to a :class:`.Table` + that was unpickled would fail to correctly establish the + :class:`.Column` within the 'c' collection, leading to issues in + areas such as ORM configuration. This could impact use cases such + as ``extend_existing`` and others. + .. change:: :tags: bug, py3k :tickets: 3625 diff --git a/lib/sqlalchemy/sql/base.py b/lib/sqlalchemy/sql/base.py index eed0792387..48b9a8a2b0 100644 --- a/lib/sqlalchemy/sql/base.py +++ b/lib/sqlalchemy/sql/base.py @@ -449,11 +449,10 @@ class ColumnCollection(util.OrderedProperties): """ - __slots__ = '_all_col_set', '_all_columns' + __slots__ = '_all_columns' def __init__(self, *columns): super(ColumnCollection, self).__init__() - object.__setattr__(self, '_all_col_set', util.column_set()) object.__setattr__(self, '_all_columns', []) for c in columns: self.add(c) @@ -482,14 +481,11 @@ class ColumnCollection(util.OrderedProperties): other = self[column.name] if other.name == other.key: remove_col = other - self._all_col_set.remove(other) del self._data[other.key] if column.key in self._data: remove_col = self._data[column.key] - self._all_col_set.remove(remove_col) - self._all_col_set.add(column) self._data[column.key] = column if remove_col is not None: self._all_columns[:] = [column if c is remove_col @@ -534,7 +530,6 @@ class ColumnCollection(util.OrderedProperties): # in a _make_proxy operation util.memoized_property.reset(value, "proxy_set") - self._all_col_set.add(value) self._all_columns.append(value) self._data[key] = value @@ -543,22 +538,20 @@ class ColumnCollection(util.OrderedProperties): def remove(self, column): del self._data[column.key] - self._all_col_set.remove(column) self._all_columns[:] = [ c for c in self._all_columns if c is not column] def update(self, iter): cols = list(iter) + all_col_set = set(self._all_columns) self._all_columns.extend( - c for label, c in cols if c not in self._all_col_set) - self._all_col_set.update(c for label, c in cols) + c for label, c in cols if c not in all_col_set) self._data.update((label, c) for label, c in cols) def extend(self, iter): cols = list(iter) - self._all_columns.extend(c for c in cols if c not in - self._all_col_set) - self._all_col_set.update(cols) + all_col_set = set(self._all_columns) + self._all_columns.extend(c for c in cols if c not in all_col_set) self._data.update((c.key, c) for c in cols) __hash__ = None @@ -584,22 +577,18 @@ class ColumnCollection(util.OrderedProperties): def __setstate__(self, state): object.__setattr__(self, '_data', state['_data']) object.__setattr__(self, '_all_columns', state['_all_columns']) - object.__setattr__( - self, '_all_col_set', util.column_set(state['_all_columns'])) def contains_column(self, col): - # this has to be done via set() membership - return col in self._all_col_set + existing = self._data.get(col.key) + return existing is not None and hash(existing) == hash(col) def as_immutable(self): - return ImmutableColumnCollection( - self._data, self._all_col_set, self._all_columns) + return ImmutableColumnCollection(self._data, self._all_columns) class ImmutableColumnCollection(util.ImmutableProperties, ColumnCollection): - def __init__(self, data, colset, all_columns): + def __init__(self, data, all_columns): util.ImmutableProperties.__init__(self, data) - object.__setattr__(self, '_all_col_set', colset) object.__setattr__(self, '_all_columns', all_columns) extend = remove = util.ImmutableProperties._immutable diff --git a/test/sql/test_metadata.py b/test/sql/test_metadata.py index 47ecf5a9b0..050929d3db 100644 --- a/test/sql/test_metadata.py +++ b/test/sql/test_metadata.py @@ -1258,6 +1258,25 @@ class TableTest(fixtures.TestBase, AssertsCompiledSQL): assign2 ) + def test_c_mutate_after_unpickle(self): + m = MetaData() + + y = Column('y', Integer) + t1 = Table('t', m, Column('x', Integer), y) + + t2 = pickle.loads(pickle.dumps(t1)) + z = Column('z', Integer) + g = Column('g', Integer) + t2.append_column(z) + + is_(t1.c.contains_column(y), True) + is_(t2.c.contains_column(y), False) + y2 = t2.c.y + is_(t2.c.contains_column(y2), True) + + is_(t2.c.contains_column(z), True) + is_(t2.c.contains_column(g), False) + def test_autoincrement_replace(self): m = MetaData() -- 2.47.2