Flask-SQLAlchemy does not behave like SQLAlchemy with the same classes, causing a runtime error about flask-sqlalchemy HOT 6 OPEN

the issue is flask-sqlalchemy is autonaming tables so that the subclasses are not single table inheritance.

defining like this:

db = SQLAlchemy(disable_autonaming=True)

or using __tablename__ = None allows it to work.

what's not clear yet is why sqlalchemy is complaining about these columns if it sees joined table inheritance.

from flask-sqlalchemy.

oh OK, it's because flask-sqlalchemy does this

where it then deletes the tablename so that the mapping ends up being single inheritance. This is changing the model in the middle of the declarative scan to be single inheritance, so that's why SQLAlchemy is both not seeing the model as single inheritance, but then later thinks it is single inheritance.

so this is a bug for...someone. I think I had to change when single inheritance is first determined in order for the use_existing_column parameter to work, so that is a change in our flow. that is, we need to know about single inheritance while we are scanning. flask-sqlalchemy is holding off the decision about inheritance until after scanning since it's looking for primary key columns. I am ...not yet seeing any way to reconcile these two flows.

from flask-sqlalchemy.

To be clear, I'm not blaming SQLAlchemy for any of this, we're clearly the ones hacking around things. It already worked like this when I inherited the project, then I did a bunch of work to clean it up and make it work "correctly" in more cases. I was definitely diving into the internals, tracing how the metaclass and table_cls callable interacted, etc; it was a pretty difficult task to work on.

I do want to keep the auto table naming behavior, as I think that's pretty useful and results in a good/consistent naming scheme. But if I remember correctly, one of the reasons for detecting primary keys and table inheritance is because if we don't, then a tablename is always generated and causes some other sort of inheritance. Either way it's a bit inconvenient.

So I'm open to deprecating then removing our current behavior, especially on the advice/request of SQLAlchemy. The main problem is figuring out how to deprecate it, and what to instruct users to do instead. It's been years since I really looked at this code I wrote. If you have any suggestions for what to do, I'm happy to discuss it and figure it out with you.

from flask-sqlalchemy.

@davidism Here is a minimal code to reproduce the issue and here is the relevant discussion link if interests you to participate in the discussion there.

sqlalchemy/sqlalchemy#10461

from flask-sqlalchemy.

I don’t see a way to address this in Flask-SQLAlchemy. Yes, we’re doing weird things with table creation, but that’s because there’s no better hooks to accomplish this in SQLAlchemy itself. If there were appropriate extension points for controlling the table name and inheritance during definition, we would use it. I’ve also discussed a bit of what we do with SQLAlchemy devs in the past due to other changes.

from flask-sqlalchemy.

this is really not about a lack of events. we have tons of events. what is happening here is flask is assuming something about the exact order of steps which occur within the declarative process, assuming that the determination if the class should be set up as single table inheritance happens after the Table object is created, which is therefore after the columns have been scanned. From this, the feature that Flask presents is that it changes the single-table-inheritance detection from whether or not __tablename__ is present, to whether the calculated Table has a local set of primary key columns; it then flips __tablename__ late in the process to change declarative's mind that the class should be single inheritance.

in SQLAlchemy 2.0, 99% of the code still works this way but an important part of the "single" inheritance question is now determined before the columns are scanned, which at the moment is something really small, whether the all-new mapped_column() construct should ignore the new use_existing_column parameter.

We can very easily "make it work the old way" by just removing this ".single" attribute and taking the one test we have right now which tests that use_existing_column is ignored and just remove the test, or assume it fails; if someone uses use_existing_column with non-single inheritance (where the parameter should have no meaning), they will get bad results. Or even more hacky, Flask could "trick" declarative into seeing single-table inheritance at this early point by temporarily erasing __tablename__ so that the issue goes away.

these approaches are of course bad ideas because we are still just having dueling non-defined behaviors.

I dont yet see a way to support flask without some extremely flask-specific feature added to declarative, because as it stands flask's assumptions create a dependency cycle:

• the creation of the Table, depends on scanning all the columns, which depends on:

• mapped_column, which with use_existing_column=True, depends on:

• Whether or not the class intends to be single inheritance, which depends on

• if __tablename__ is non-None, which in Flask-SQLAlchemy depends on:

• the creation of the Table

So there's no event alone that can allow this to work; Flask's current assumptions are slightly incompatible with ours. mapped_column() would need to have use_existing_column somehow work without knowing if the class intends to be single inheritance, and we'd then have to add basically a guarantee throughout declarative that is basically hardcoded to this particular convention that flask-sqlalchemy uses.

At the core of this, Flask-sqlalchemy has a feature where you dont have to explicitly state __tablename__ = None in order to indicate single-table inheritance. you can just put no primary key columns on the class, allowing the decision to be made implicitly. So this mapping:

class Base(DeclarativeBase):
    @declared_attr.directive
    def __tablename__(cls):
        return cls.__name__.lower()


class Person(Base):
    id = mapped_column(Integer, primary_key=True)

class Engineer(Person):
    name = mapped_column(String)

SQLAlchemy would see this as an error, the user forgot to put a primary key on their "Engineer" joined table. Flask sees it as a directive that Engineer has no table and assumes single inheritance.

There's a bit of a clash of design choices happening here. SQLAlchemy especially in 2.0 tries in all cases where we can decide to never assume in the face of ambiguity and to raise an error instead. We of course started out in the early 2000's making a whole slate of implicit choices and assumptions but that's really where a lot of user confusion comes from and that's why the Zen of Python has an opinion on this.

I kind of think Flask-SQLAlchemy's "guess" here is a bit dated and should move towards encouraging the user to explicitly state whether a class intends to be single- or joined- inheritance. class mapping is confusing enough without guesses happening.

from flask-sqlalchemy.