ImmutableColumnCollection creates a separate _col_set on getstate

[n1ywb]

Testcase: httpshttps://gist.github.com/anonymous/e343baf76747d104a15e

If you comment out the pickling line, it works. Otherwise, it fails with

sqlalchemy.exc.ArgumentError: Could not locate any simple equality expressions involving locally mapped foreign key columns for primary join condition 'server.id = site.server' on relationship Site.server_.  Ensure that referencing columns are associated with a ForeignKey or ForeignKeyConstraint, or are annotated in the join condition with the foreign() annotation. To allow comparison operators other than '==', the relationship can be marked as viewonly=True.

The only difference it makes is whether or not the metadata has been pickled.

You can also make it work by commenting out the class declarations but I thought automap was supposed to permit you to gracefully override reflected metadata, even if it's been pickled.

Python 2.7.11 SQLAlchemy==1.0.10 PyMySQL==0.6.7

Thanks, Jeff

[Mike Bayer]

here's simplification one:

import pickle
from sqlalchemy import MetaData, Column, Integer
from sqlalchemy.ext.declarative import declarative_base
from sqlalchemy.orm import relationship
from sqlalchemy.orm import configure_mappers
from sqlalchemy import Table, ForeignKey

metadata = MetaData()

Table(
    'server', metadata,
    Column('id', Integer, primary_key=True))
Table(
    'site', metadata,
    Column('id', Integer, primary_key=True),
    Column('server_id', ForeignKey('server.id')))

m2 = metadata

metadata = pickle.loads(pickle.dumps(m2))

Base = declarative_base(metadata=metadata)
Base.__table_args__ = {'extend_existing': True}


class Server(Base):
    __tablename__ = 'server'
    id = Column('id', Integer, primary_key=True)
    sites = relationship("Site")


class Site(Base):
    __tablename__ = 'site'
    id = Column('id', Integer, primary_key=True)


configure_mappers()

then here's more specifically:

from sqlalchemy import Table, MetaData, Integer, Column

m = MetaData()

a = Table(
    'a', m, Column('id', Integer, primary_key=True)
)

import pickle

m = pickle.loads(pickle.dumps(m))

a = m.tables['a']

new_c = Column('id', Integer, primary_key=True)

print "-----------------"
a.append_column(new_c)

assert new_c.table is a
assert a.c.id is new_c

assert a.c.contains_column(new_c)

and the fix is not 100% backwards compatible, as it requires changing the pickle format of either ColumnCollection or ImmutableColumnCollection. Here's the better version:

diff --git a/lib/sqlalchemy/sql/base.py b/lib/sqlalchemy/sql/base.py
index eed0792..cace0dc 100644
--- a/lib/sqlalchemy/sql/base.py
+++ b/lib/sqlalchemy/sql/base.py
@@ -592,15 +592,18 @@ class ColumnCollection(util.OrderedProperties):
         return col in self._all_col_set

     def as_immutable(self):
-        return ImmutableColumnCollection(
-            self._data, self._all_col_set, self._all_columns)
+        return ImmutableColumnCollection(self)


 class ImmutableColumnCollection(util.ImmutableProperties, ColumnCollection):
-    def __init__(self, data, colset, all_columns):
-        util.ImmutableProperties.__init__(self, data)
-        object.__setattr__(self, '_all_col_set', colset)
-        object.__setattr__(self, '_all_columns', all_columns)
+    def __init__(self, source):
+        object.__setattr__(self, '_source', source)
+        util.ImmutableProperties.__init__(self, source._data)
+        object.__setattr__(self, '_all_col_set', source._all_col_set)
+        object.__setattr__(self, '_all_columns', source._all_columns)
+
+    def __reduce__(self):
+        return (ImmutableColumnCollection, (self._source, ))

     extend = remove = util.ImmutableProperties._immutable

a workaround for your immediate case is:

metadata = pickle.loads(pickle.dumps(m2))
for t in metadata.tables.values():
    del t.__dict__['columns']

can you wait til 1.1? it won't be for months.

[n1ywb]

Hi Mike,

I added the workaround to my project and it works perfectly, I can now load my serialized metadata. I'm all set for now.

Thanks so much for the quick response! Jeff

[Mike Bayer]

I do think I can make a fix that works this out for 1.0, basically not using the column set anymore as it doesn't seem to be needed

[Mike Bayer]

• 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

→ <<cset 8163de4cc9e0>>

[Mike Bayer]

• 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

(cherry picked from commit 8163de4cc9e01460d3476b9fb3ed14a5b3e70bae)

→ <<cset c7ddf26dd22a>>

[Mike Bayer]

Also note the adjustment to the patch in 5742e321b261c0c1303835b80 and b11bfd5b0c7262bd207bbaf6be2d1b025119abe7, "column.key" is not sufficient here so we use an ad-hoc set