Flask-SQLAlchemy does not behave like SQLAlchemy with the same classes, causing a runtime error

[ibraheemalayan]

The Single Table Inheritance setup ( the first code below which is very self explanatory ) the following error is emmitted

sqlalchemy.exc.ArgumentError: Column 'bid' on class ChildB conflicts with existing column 'parent.bid'.  If using Declarative, consider using the use_existing_column parameter of mapped_column() to resolve conflicts.

The same exact code works if Flask-SQLAlchemy was replaced with SQLAlchemy as in the second code block

Reproduce Error

To replicate it just run the following:

from sqlalchemy import Enum, ForeignKey, Integer, Column
from sqlalchemy.orm import Mapped, mapped_column

from enum import IntEnum

from flask_sqlalchemy import SQLAlchemy

db = SQLAlchemy()


class TType(IntEnum):
    a = 1
    b = 2
    c = 3


class Parent(db.Model):
    __tablename__ = "parent"

    id = Column(Integer, primary_key=True, index=True, nullable=False)

    ttype: Mapped[TType] = Column(Enum(TType), nullable=False, index=True)

    __mapper_args__ = {
        "polymorphic_identity": "parent",
        "polymorphic_on": ttype,
    }


class TableB(db.Model):
    __tablename__ = "tableb"
    id: Mapped[int] = mapped_column(primary_key=True)


class TableC(db.Model):
    __tablename__ = "tablec"
    id: Mapped[int] = mapped_column(primary_key=True)


class HasBID:
    bid = mapped_column(
        Integer,
        ForeignKey(
            "tableb.id",
            ondelete="RESTRICT",
            name="tableb_id_fk",
        ),
        index=True,
        use_existing_column=True,
    )


class HasCID:
    cid = mapped_column(
        Integer,
        ForeignKey(
            "tablec.id",
            ondelete="RESTRICT",
            name="tablec_id_fk",
        ),
        index=True,
        use_existing_column=True,
    )


class ChildA(HasBID, HasCID, Parent):
    __mapper_args__ = {"polymorphic_identity": TType.a}
    # other columns


class ChildB(HasBID, Parent):
    __mapper_args__ = {"polymorphic_identity": TType.b}
    # other columns


class ChildC(HasCID, Parent):
    __mapper_args__ = {"polymorphic_identity": TType.c}
    # other columns

Expected Result

The code should work similar to the same code that uses SQLAlchemy instead of Flask-SQLAlchemy below

from sqlalchemy import Enum, ForeignKey, Integer, Column, create_engine
from sqlalchemy.orm import Mapped, mapped_column, DeclarativeBase

from enum import IntEnum


class Base(DeclarativeBase):
    pass


class TType(IntEnum):
    a = 1
    b = 2
    c = 3


class Parent(Base):
    __tablename__ = "parent"

    id = Column(Integer, primary_key=True, index=True, nullable=False)

    ttype: Mapped[TType] = Column(Enum(TType), nullable=False, index=True)

    __mapper_args__ = {
        "polymorphic_identity": "parent",
        "polymorphic_on": ttype,
    }


class TableB(Base):
    __tablename__ = "tableb"
    id: Mapped[int] = mapped_column(primary_key=True)


class TableC(Base):
    __tablename__ = "tablec"
    id: Mapped[int] = mapped_column(primary_key=True)


class HasBID:
    bid = mapped_column(
        Integer,
        ForeignKey(
            "tableb.id",
            ondelete="RESTRICT",
            name="tableb_id_fk",
        ),
        index=True,
        use_existing_column=True,
    )


class HasCID:
    cid = mapped_column(
        Integer,
        ForeignKey(
            "tablec.id",
            ondelete="RESTRICT",
            name="tablec_id_fk",
        ),
        index=True,
        use_existing_column=True,
    )


class ChildA(HasBID, HasCID, Parent):
    __mapper_args__ = {"polymorphic_identity": TType.a}
    # other columns


class ChildB(HasBID, Parent):
    __mapper_args__ = {"polymorphic_identity": TType.b}
    # other columns


class ChildC(HasCID, Parent):
    __mapper_args__ = {"polymorphic_identity": TType.c}
    # other columns


e = create_engine("sqlite://", echo=True)
Base.metadata.create_all(e)

Environment:

Python version: 3.9.16
Flask-SQLAlchemy version: 3.1.1
SQLAlchemy version: 2.0.21

[ibraheemalayan]

@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

[zzzeek]

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.

[zzzeek]

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

flask-sqlalchemy/src/flask_sqlalchemy/model.py

Lines 244 to 262 in d349bdb

	# If a primary key is found, create a table for joined-table inheritance.
	for arg in args:
	if (isinstance(arg, sa.Column) and arg.primary_key) or isinstance(
	arg, sa.PrimaryKeyConstraint
	):
	return sa.Table(*args, **kwargs)
	# If no base classes define a table, return one that's missing a primary key
	# so SQLAlchemy shows the correct error.
	for base in cls.__mro__[1:-1]:
	if "__table__" in base.__dict__:
	break
	else:
	return sa.Table(*args, **kwargs)
	# Single-table inheritance, use the parent table name. __init__ will unset
	# __table__ based on this.
	if "__tablename__" in cls.__dict__:
	del cls.__tablename__

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.

[davidism]

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.

[zzzeek]

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.

[davidism]

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.