- edited description
declared_attr.cascading doesn't work for __tablename__
I noticed this while fixing tablename generation in Flask-SQLAlchemy. The reason I use the metaclass instead of declared_attr
is because the cascading
option doesn't work for __tablename__
. Instead, it's treated like a normal declared attr, where once an intermediate class sets a value, that's used going forward.
You also showed declared_attr.cascading
for __tablename__
in the docs for __table_cls__
, which won't work as intended: https://gerrit.sqlalchemy.org/plugins/gitiles/zzzeek/sqlalchemy/+/refs/changes/27/527/2/doc/build/orm/extensions/declarative/api.rst#137
import sqlalchemy as sa
from sqlalchemy.ext.declarative import declarative_base, declared_attr
class Base:
@declared_attr.cascading
def __tablename__(cls):
return cls.__name__.lower()
Base = declarative_base(cls=Base)
class User(Base):
id = sa.Column(sa.Integer, primary_key=True)
class User2(User):
__tablename__ = 'user2'
id = sa.Column(sa.ForeignKey(User.id), primary_key=True)
class User3(User2):
# I would expect __tablename__ = 'user3' here
# but sqla keeps 'user2'
id = sa.Column(sa.ForeignKey(User2.id), primary_key=True)
engine = sa.create_engine('sqlite://', echo=True)
Base.metadata.create_all(engine)
Traceback (most recent call last):
File "/home/david/Projects/flask-sqlalchemy/example.py", line 23, in <module>
class User3(User2):
File "/home/david/.virtualenvs/flask-sqlalchemy/lib/python3.6/site-packages/sqlalchemy/ext/declarative/api.py", line 64, in __init__
_as_declarative(cls, classname, cls.__dict__)
File "/home/david/.virtualenvs/flask-sqlalchemy/lib/python3.6/site-packages/sqlalchemy/ext/declarative/base.py", line 88, in _as_declarative
_MapperConfig.setup_mapping(cls, classname, dict_)
File "/home/david/.virtualenvs/flask-sqlalchemy/lib/python3.6/site-packages/sqlalchemy/ext/declarative/base.py", line 103, in setup_mapping
cfg_cls(cls_, classname, dict_)
File "/home/david/.virtualenvs/flask-sqlalchemy/lib/python3.6/site-packages/sqlalchemy/ext/declarative/base.py", line 131, in __init__
self._setup_table()
File "/home/david/.virtualenvs/flask-sqlalchemy/lib/python3.6/site-packages/sqlalchemy/ext/declarative/base.py", line 395, in _setup_table
**table_kw)
File "/home/david/.virtualenvs/flask-sqlalchemy/lib/python3.6/site-packages/sqlalchemy/sql/schema.py", line 421, in __new__
"existing Table object." % key)
sqlalchemy.exc.InvalidRequestError: Table 'user2' is already defined for this MetaData instance. Specify 'extend_existing=True' to redefine options and columns on an existing Table object.
Comments (7)
-
reporter -
repo owner the glitch in my doc is that you don't actually need the .cascading part: http://docs.sqlalchemy.org/en/latest/orm/extensions/declarative/mixins.html#controlling-table-inheritance-with-mixins "Declarative will always invoke declared_attr for the special names
__tablename__
,__mapper_args__
and__table_args__
function for each mapped class in the hierarchy. ".in your example, you are replacing
__tablename__
with a fixed string value on User2 which is why it isn't getting propagated. remove that and it works, doesn't matter if .cascading is present or not.we don't have the option to "ignore" that a class overrode
__tablename__
, I think people would expect that if you override an attribute in a subclass, that's the new behavior going forward. that it's a straight string and not a new function signals that this is likely not what the user really wants here but i don't think declarative should be trying to guess the intent.i dont think there's a bug so far.
-
reporter This and #4091 are connected. Since
cascading
applies to further subclasses even if an intermediate class sets the attr manually, I'd expect that behavior with__tablename__
too. What I'm observing seems to go against "Declarative will always invoke declared_attr for the special names ... for each mapped class."cascading
seems to fit that description. -
reporter If this worked the way I want (tablename attr can cascade like other attrs, skipping manually set attrs), I could remove the tablename generation from the metaclass init entirely. The
_should_set_tablename
detection would go away completely and I'd just have a simple cascading declared attr tablename. (Although I could only drop it if it was backported to 1.0, since that's the lowest version I support.) -
repo owner This and #4091 are connected. Since cascading applies to further subclasses even if an intermediate class sets the attr manually, I'd expect that behavior with tablename too.
in the case of #4091, that is a limitation of @declared_attr.cascading that I don't have the energy to fix right now. I don't see why you'd expect
__tablename__
to act this way if ".cascading" is not present. if it is present, then yes, it's hard to know this has no effect.So fix here would be just another warning. "You used @cascading with tablename and this has no effect".
"Declarative will always invoke declared_attr for the special names ... for each mapped class."
then we change that doc too.
If this worked the way I want (tablename attr can cascade like other attrs, skipping manually set attrs)
I think to as much a degree as possible, the normal behavior of "the most recent class attribute in the
__mro__
takes precedence" should be enforced. Once you override an attribute in Python, that's it, you don't have a previous version of the attribute hopping over the one you have. If you want flask-sqlalchemy to have arbitrary__tablename__
on any class in the hierarchy, but if the attribute is not immediately present and then it would use the superclass__tablename__
function, then you should tell users to not use__tablename__
and use something like__flask_tablename__
.put another way, I think
__tablename__
does it right and@declared_attr.cascading
does it wrong. I just have it warning in #4091 that hey, this doesn't really work, sorry. -
repo owner same gerrit https://gerrit.sqlalchemy.org/530
-
reporter OK, fair enough, as long as the behavior is clear I'm ok with it.
- Log in to comment