copy mapper_args (and table_args) dictionaries in declarative base? (from mixins only?)

Issue #3062 resolved
Paveł Tyslacki created an issue

I have next example:

from datetime import datetime

from sqlalchemy import Column, Integer, DateTime
from sqlalchemy import create_engine
from sqlalchemy.orm import sessionmaker, scoped_session
from sqlalchemy.ext.declarative import declarative_base


engine = create_engine('sqlite://')
Session = scoped_session(sessionmaker(bind=engine))
Base = declarative_base()


def datetime_utcnow():
    return datetime.utcnow()


class TestBase(Base):
    id = Column(Integer, primary_key=True)
    version_id = Column(Integer, nullable=False)
    create_datetime = Column(DateTime, default=datetime_utcnow, nullable=False)
    update_datetime = Column(DateTime, default=datetime_utcnow, onupdate=datetime_utcnow, nullable=False)

    __abstract__ = True
    __mapper_args__ = {
        'version_id_col': version_id,
    }


class Test1(TestBase):

    __tablename__ = 'test1'

    data = Column(Integer)


class Test2(TestBase):

    __tablename__ = 'test2'

    data = Column(Integer)


if __name__ == '__main__':
    Base.metadata.create_all(engine)
    session = Session()
    session.add(Test1())
    session.commit()
    session.add(Test2())
    session.commit()

This code throw exception on second commit:

/home/tbicr/Project/env/bin/python /home/tbicr/Project/test_sample/main_dev.py
Traceback (most recent call last):
  File "/home/tbicr/Project/env/lib/python3.4/site-packages/sqlalchemy/engine/base.py", line 940, in _execute_context
    context)
  File "/home/tbicr/Project/env/lib/python3.4/site-packages/sqlalchemy/engine/default.py", line 441, in do_execute
    cursor.execute(statement, parameters)
sqlite3.IntegrityError: NOT NULL constraint failed: test2.version_id

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/home/tbicr/Project/test_sample/main_dev.py", line 50, in <module>
    session.commit()
  File "/home/tbicr/Project/env/lib/python3.4/site-packages/sqlalchemy/orm/session.py", line 765, in commit
    self.transaction.commit()
  File "/home/tbicr/Project/env/lib/python3.4/site-packages/sqlalchemy/orm/session.py", line 370, in commit
    self._prepare_impl()
  File "/home/tbicr/Project/env/lib/python3.4/site-packages/sqlalchemy/orm/session.py", line 350, in _prepare_impl
    self.session.flush()
  File "/home/tbicr/Project/env/lib/python3.4/site-packages/sqlalchemy/orm/session.py", line 1903, in flush
    self._flush(objects)
  File "/home/tbicr/Project/env/lib/python3.4/site-packages/sqlalchemy/orm/session.py", line 2021, in _flush
    transaction.rollback(_capture_exception=True)
  File "/home/tbicr/Project/env/lib/python3.4/site-packages/sqlalchemy/util/langhelpers.py", line 57, in __exit__
    compat.reraise(exc_type, exc_value, exc_tb)
  File "/home/tbicr/Project/env/lib/python3.4/site-packages/sqlalchemy/util/compat.py", line 168, in reraise
    raise value
  File "/home/tbicr/Project/env/lib/python3.4/site-packages/sqlalchemy/orm/session.py", line 1985, in _flush
    flush_context.execute()
  File "/home/tbicr/Project/env/lib/python3.4/site-packages/sqlalchemy/orm/unitofwork.py", line 370, in execute
    rec.execute(self)
  File "/home/tbicr/Project/env/lib/python3.4/site-packages/sqlalchemy/orm/unitofwork.py", line 523, in execute
    uow
  File "/home/tbicr/Project/env/lib/python3.4/site-packages/sqlalchemy/orm/persistence.py", line 64, in save_obj
    mapper, table, insert)
  File "/home/tbicr/Project/env/lib/python3.4/site-packages/sqlalchemy/orm/persistence.py", line 594, in _emit_insert_statements
    execute(statement, params)
  File "/home/tbicr/Project/env/lib/python3.4/site-packages/sqlalchemy/engine/base.py", line 720, in execute
    return meth(self, multiparams, params)
  File "/home/tbicr/Project/env/lib/python3.4/site-packages/sqlalchemy/sql/elements.py", line 317, in _execute_on_connection
    return connection._execute_clauseelement(self, multiparams, params)
  File "/home/tbicr/Project/env/lib/python3.4/site-packages/sqlalchemy/engine/base.py", line 817, in _execute_clauseelement
    compiled_sql, distilled_params
  File "/home/tbicr/Project/env/lib/python3.4/site-packages/sqlalchemy/engine/base.py", line 947, in _execute_context
    context)
  File "/home/tbicr/Project/env/lib/python3.4/site-packages/sqlalchemy/engine/base.py", line 1108, in _handle_dbapi_exception
    exc_info
  File "/home/tbicr/Project/env/lib/python3.4/site-packages/sqlalchemy/util/compat.py", line 174, in raise_from_cause
    reraise(type(exception), exception, tb=exc_tb, cause=exc_value)
  File "/home/tbicr/Project/env/lib/python3.4/site-packages/sqlalchemy/util/compat.py", line 167, in reraise
    raise value.with_traceback(tb)
  File "/home/tbicr/Project/env/lib/python3.4/site-packages/sqlalchemy/engine/base.py", line 940, in _execute_context
    context)
  File "/home/tbicr/Project/env/lib/python3.4/site-packages/sqlalchemy/engine/default.py", line 441, in do_execute
    cursor.execute(statement, parameters)
sqlalchemy.exc.IntegrityError: (IntegrityError) NOT NULL constraint failed: test2.version_id 'INSERT INTO test2 (version_id, create_datetime, update_datetime, data) VALUES (?, ?, ?, ?)' (None, '2014-05-30 09:29:16.697519', '2014-05-30 09:29:16.697548', None)

Process finishe

Look like version_id column mapped to Test1 on first definition and on Test2 definition check that it is instance of Test1 and skip it: https://bitbucket.org/zzzeek/sqlalchemy/src/28fbecaaa00ac6039d6c6b7e5abd594160f74104/lib/sqlalchemy/orm/persistence.py?at=master#cl-251.

Comments (9)

  1. Mike Bayer repo owner

    OK seems like there is logic that copies the version_id_col, so not sure why it doesnt work out of the gate, do it like this:

    class TestBase(Base):
       # ...
        __abstract__ = True
    
        @declared_attr
        def __mapper_args__(cls):
            return {
                'version_id_col': cls.version_id,
            }
    
  2. Mike Bayer repo owner
        def _prepare_mapper_arguments(self):
            properties = self.properties
            if self.mapper_args_fn:
                mapper_args = self.mapper_args_fn()
            else:
                mapper_args = {}
    
    
            for k in ('version_id_col', 'polymorphic_on',):
                if k in mapper_args:
                    v = mapper_args[k]
                    # the problem is that TestBase.version_id isn't 
                    # in the dictionary here, seems odd
                    mapper_args[k] = self.column_copies.get(v, v)
    
  3. Mike Bayer repo owner

    oh, that's not the problem. the problem is the mapper_args dictionary is mutated in place. Not sure if we can just make a copy of that dictionary...

  4. Mike Bayer repo owner

    hm ok this actually does that, so we can go in sooner for this

    diff --git a/lib/sqlalchemy/ext/declarative/base.py b/lib/sqlalchemy/ext/declarative/base.py
    index eb66f12..b110456 100644
    --- a/lib/sqlalchemy/ext/declarative/base.py
    +++ b/lib/sqlalchemy/ext/declarative/base.py
    @@ -351,6 +351,7 @@ class _MapperConfig(object):
             properties = self.properties
             if self.mapper_args_fn:
                 mapper_args = self.mapper_args_fn()
    +            mapper_args = dict(mapper_args)
             else:
                 mapper_args = {}
    
  5. Mike Bayer repo owner
    • The __mapper_args__ dictionary is copied from a declarative mixin or abstract class when accessed, so that modifications made to this dictionary by declarative itself won't conflict with that of other mappings. The dictionary is modified regarding the version_id_col and polymorphic_on arguments, replacing the column within with the one that is officially mapped to the local class/table. fixes #3062

    → <<cset 8daa6ccfb0be>>

  6. Mike Bayer repo owner
    • The __mapper_args__ dictionary is copied from a declarative mixin or abstract class when accessed, so that modifications made to this dictionary by declarative itself won't conflict with that of other mappings. The dictionary is modified regarding the version_id_col and polymorphic_on arguments, replacing the column within with the one that is officially mapped to the local class/table. fixes #3062

    → <<cset 1f88b16a0fc0>>

  7. Mike Bayer repo owner
    • The __mapper_args__ dictionary is copied from a declarative mixin or abstract class when accessed, so that modifications made to this dictionary by declarative itself won't conflict with that of other mappings. The dictionary is modified regarding the version_id_col and polymorphic_on arguments, replacing the column within with the one that is officially mapped to the local class/table. fixes #3062

    → <<cset 01eb52b516f1>>

  8. Log in to comment