- 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

diff --git a/doc/build/changelog/changelog_10.rst b/doc/build/changelog/changelog_10.rst
index a1e1fbe1796ab20bdabd2b7f07db2200d122de50..a0b1ad9572c7aad5b4adcdaae90d6140e163e1b0 100644 (file)
--- 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 eed0792387fa8d1cec1638214f1f8714d5802ce3..48b9a8a2b0b345a95a4d2912afe59f10e0ac3832 100644 (file)
--- 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 47ecf5a9b017d2533a82711c7915ece9e5534555..050929d3db57e1b393856feec30a9e8086f16a99 100644 (file)
--- 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()