Broken compatibility on generation of joined table inheritance with abstract models

[krassowski]

First of all many thanks for releasing 2.2!

Here is the test case for my problem:

import flask
import flask_sqlalchemy as fsa
from sqlalchemy.ext.declarative import declared_attr


app = flask.Flask(__name__)
db = fsa.SQLAlchemy(app)


class CustomBaseForJoinedInheritance(db.Model):
    __abstract__ = True

    @declared_attr
    def __tablename__(cls):
        return cls.__name__.lower()

    @declared_attr
    def id(cls):
        return db.Column('id', db.Integer, primary_key=True)


class Employee(CustomBaseForJoinedInheritance):
    name = db.Column(db.String(50))
    type = db.Column(db.String(50))

    __mapper_args__ = {
        'polymorphic_identity': 'employee',
        'polymorphic_on': type
    }


class Engineer(Employee):
    id = db.Column(db.Integer, db.ForeignKey('employee.id'), primary_key=True)
    engineer_name = db.Column(db.String(30))

    __mapper_args__ = {
        'polymorphic_identity': 'engineer',
    }


class BadEmployee(CustomBaseForJoinedInheritance):
    name = db.Column(db.String(50))
    type = db.Column(db.String(50))

    __mapper_args__ = {
        'polymorphic_identity': 'employee',
        'polymorphic_on': type
    }


class BadEngineer(BadEmployee):
    id = db.Column(db.Integer, db.ForeignKey('bademployee.id'), primary_key=True)
    engineer_name = db.Column(db.String(30))

    __mapper_args__ = {
        'polymorphic_identity': 'engineer',
    }


assert Employee.__tablename__ == 'employee'
assert Engineer.__tablename__ == 'engineer'
assert BadEmployee.__tablename__ == 'bademployee'
assert BadEngineer.__tablename__ == 'badengineer'

# works until here
db.create_all()
assert True

It's a bit long but please bear with me; all equality assertions pass greatly but calling db.create_all() raises following exception:

Traceback (most recent call last):
  File "test_case.py", line 51, in <module>
    class BadEngineer(BadEmployee):
  File "/usr/lib64/python3.5/site-packages/flask_sqlalchemy/__init__.py", line 602, in __init__
    DeclarativeMeta.__init__(self, name, bases, d)
  File "/usr/lib64/python3.5/site-packages/sqlalchemy/ext/declarative/api.py", line 64, in __init__
    _as_declarative(cls, classname, cls.__dict__)
  File "/usr/lib64/python3.5/site-packages/sqlalchemy/ext/declarative/base.py", line 88, in _as_declarative
    _MapperConfig.setup_mapping(cls, classname, dict_)
  File "/usr/lib64/python3.5/site-packages/sqlalchemy/ext/declarative/base.py", line 103, in setup_mapping
    cfg_cls(cls_, classname, dict_)
  File "/usr/lib64/python3.5/site-packages/sqlalchemy/ext/declarative/base.py", line 135, in __init__
    self._early_mapping()
  File "/usr/lib64/python3.5/site-packages/sqlalchemy/ext/declarative/base.py", line 138, in _early_mapping
    self.map()
  File "/usr/lib64/python3.5/site-packages/sqlalchemy/ext/declarative/base.py", line 530, in map
    **self.mapper_args
  File "<string>", line 2, in mapper
  File "/usr/lib64/python3.5/site-packages/sqlalchemy/orm/mapper.py", line 671, in __init__
    self._configure_inheritance()
  File "/usr/lib64/python3.5/site-packages/sqlalchemy/orm/mapper.py", line 978, in _configure_inheritance
    self.local_table)
  File "<string>", line 2, in join_condition
  File "/usr/lib64/python3.5/site-packages/sqlalchemy/sql/selectable.py", line 976, in _join_condition
    (a.description, b.description, hint))
sqlalchemy.exc.NoForeignKeysError: Can't find any foreign key relationships between 'bad_employee' and 'bad_engineer'.

Interestingly we have: bad_employee and bad_engineer, not bademployee and badengineer. It was working in 2.1, stopped in 2.2 (probably after #467). Do you plan to support such test case?

[davidism]

Yes, that should work, this is a bug. Probably related to popping the tablename attribute without checking if it's a declared attr.

[ThiefMaster]

The sole existence of the declared_attr called __tablename__ in the base Model class apparently causes this problem (even if it returns None).

[vToMy]

It's related to test case test_manual_name - if you have a base class defining __tablename__ as a string, you don't want that inherited.
However if you declare a __tablename__ as a declared_attr you do want to inherit it.
The solution is simple - only pop the __tablename__ if it's not a declared attribute:

        if '__tablename__' in d:
            table = d['__tablename__']
            if not isinstance(table, declared_attr):
                d.pop('__tablename__')
            d['_cached_tablename'] = table

I suggest adding a test case as well.

[davidism]

This test from #541 should cover this case. If a declared attr name is found, a new name is not set.

Note that using BadEmployee.id instead of 'bademployee.id' would have worked since SQLAlchemy would pull the correct reference string out of the attribute (but the name would still have been wrong behind the scenes).