declarative places things inside cls.__mapper_args___, should copy the dict first, use __mapper_args__ from cls.__dict__

[Mike Bayer]

line 487:

    mapper_args = getattr(cls, '__mapper_args__', {})

pulls the mapper_args from a base class even on an inherited class. this should return the args from cls.dict.

then:

            mapper_args['inherit_condition']('inherit_condition') = sql_util.join_condition(
                mapper_args['inherits']('inherits').__table__, table,
                ignore_nonexistent_tables=True)

modifies cls.mapper_args. copy the dict first in any case.

[Former user]

(chrisw) okay, so I understand the bug in the above, in that mapper_args needs to be copied before something is shoved into it, but why does it need to be grabbed from dict rather than getattr(cls,'mapper_args',None)?

[Former user]

So, patch for the 2nd part of the bug. And I think we agreed that the first part of the bug is just fine as it is now ;-)

Index: test/ext/test_declarative.py

--- test/ext/test_declarative.py (revision 6811) +++ test/ext/test_declarative.py (working copy) @@ -918,6 +918,19 @@

class DeclarativeInheritanceTest(DeclarativeTestBase): +
+ def test_we_must_copy_mapper_args(self): + class Person(Base): + tablename = 'people' + id = Column(Integer, primary_key=True) + discriminator = Column('type', String(50)) + mapper_args = {'polymorphic_on': discriminator} +
+ class Engineer(Person): + primary_language = Column(String(50)) + + assert 'inherits' not in Person.mapper_args +
def test_custom_join_condition(self): class Foo(Base): tablename = 'foo' Index: lib/sqlalchemy/ext/declarative.py =================================================================== --- lib/sqlalchemy/ext/declarative.py (revision 6811) +++ lib/sqlalchemy/ext/declarative.py (working copy) @@ -500,7 +500,7 @@ raise exceptions.ArgumentError( "Can't add additional column %r when specifying table" % key)

• mapper_args = getattr(cls, 'mapper_args', {})
• mapper_args = dict(getattr(cls, 'mapper_args', {})) if 'inherits' not in mapper_args: for c in cls.bases: if _is_mapped_class(c):

[Mike Bayer]

this is c2060e8943e628404d9620ad60787c6af0a12b07, minus the __dict__ part, which is backwards incompatible for users that expect "extensions" to propagate. We'll look into #1701 to do the work of dealing with __mapper_args__, __table_args__ comprehensively.

[Mike Bayer]

der, "extensions" propagates anyway via mapper inheritance which is implicit in declarative inheritance. so the __dict__ change is still good for now.

[Mike Bayer]

bee34dfcd61106ee8eecebd7913efe1904ce5a4a

[Mike Bayer]

Removing milestone: 0.6.0 (automated comment)