sqlmodel icon indicating copy to clipboard operation
sqlmodel copied to clipboard

Delete cascade not working in AsyncSession

Open farahats9 opened this issue 3 years ago • 7 comments

First Check

  • [X] I added a very descriptive title to this issue.
  • [X] I used the GitHub search to find a similar issue and didn't find it.
  • [X] I searched the SQLModel documentation, with the integrated search.
  • [X] I already searched in Google "How to X in SQLModel" and didn't find any information.
  • [X] I already read and followed all the tutorial in the docs and didn't find an answer.
  • [X] I already checked if it is not related to SQLModel but to Pydantic.
  • [X] I already checked if it is not related to SQLModel but to SQLAlchemy.

Commit to Help

  • [X] I commit to help with one of those options 👆

Example Code

from typing import List, Optional
import sqlalchemy as sa
from sqlmodel import Field, Relationship, SQLModel


class Address(SQLModel, table=True):
    __tablename__ = 'addresses'

    id: Optional[int] = Field(default=None, primary_key=True, nullable=False)
    user_id: int = Field(foreign_key='users.id', nullable=False)
    user: 'User' = Relationship(back_populates='addresses', sa_relationship_kwargs={'lazy':'noload'})

class User(SQLModel, table=True):
    __tablename__ = 'users'

    id: Optional[int] = Field(default=None, primary_key=True, nullable=False)
    name: str = Field(max_length=100, nullable=False)
    addresses: List['Address'] = Relationship(back_populates='user', sa_relationship_kwargs={'cascade': 'all, delete', 'lazy':'noload'})


# when doing a delete

async def remove_user(db: AsyncSession, *, id: int):
    obj = await db.get(User, id)
    await db.delete(obj)
    await db.commit()

Description

When using lazy='noload' in the relationship arguments it loads the user object with addresses: [] so there are no addresses to apply the cascade to, you always have to selectinload or joinedload so that when deleting the object it works.

Am I misunderstanding or doing it wrong because that looks too much work just to delete an object, specially if it has too many relations.

Operating System

Linux, Windows

Operating System Details

No response

SQLModel Version

0.0.8

Python Version

3.11

Additional Context

No response

farahats9 avatar Oct 25 '22 05:10 farahats9

For relationship cascades to work your related entities should be loaded first You might want to confidure on delete on your foreign keys for db-level cascades https://docs.sqlalchemy.org/en/14/core/constraints.html#sqlalchemy.schema.ForeignKey.params.ondelete

async def main():
    async with engine.connect() as connection:
        await connection.run_sync(SQLModel.metadata.drop_all)
        await connection.run_sync(SQLModel.metadata.create_all)
        await connection.commit()


    # Seed the data
    async with async_sessionmaker.begin() as session:
        session.add(
            User(
                name="Doctor",
                addresses=[Address(), Address()],
            )
        )

    async with async_sessionmaker.begin() as session:
        stmt = select(User).options(joinedload(User.addresses))
        user = await session.scalar(stmt)
        await session.delete(user)

notypecheck avatar Nov 12 '22 16:11 notypecheck

@ThirVondukr I am aware that we need to load the relation manually but I am asking if there is another way, since usually you may have many relations it is not practical to load each of them. Also keep in mind that these children will have other children under them that need to be loaded as well to apply the cascades.

I think using the on_delete='CASCADE' seems to be the only practical option but I read that it is not a good practice to keep the logic in the database and it should be on the ORM side.

farahats9 avatar Nov 12 '22 17:11 farahats9

Your data integrity should be handled on DB side as much as possible 😉

notypecheck avatar Nov 12 '22 23:11 notypecheck

maybe you can try to add passive_deletes=True to delete children by db cascade delete natively, also it's has better performance than sqlalchemy core.

wangxin688 avatar Dec 27 '22 06:12 wangxin688

@wangxin688 thanks I didn't know about this flag, but it still won't fix the problem because you would need to add a CASCADE on the database side. I have opted to do this for my project but everywhere I read says its not best practice.

farahats9 avatar Dec 27 '22 16:12 farahats9

@wangxin688 thanks I didn't know about this flag, but it still won't fix the problem because you would need to add a CASCADE on the database side. I have opted to do this for my project but everywhere I read says its not best practice.

That's true, you need define cascade action in Column level, db natively action is always a better choice for me.

wangxin688 avatar Dec 29 '22 11:12 wangxin688

This really looks a bit unexpected. session.delete() is run asynchronously and it could load nested objects before iterating through them.

But, there is the same issue when pure SQLAlchemy is used (see in the details):

import asyncio
from typing import List

from sqlalchemy import ForeignKey, String, select
from sqlalchemy.ext.asyncio import AsyncSession, create_async_engine
from sqlalchemy.orm import DeclarativeBase, Mapped, Relationship, mapped_column


class Base(DeclarativeBase):
    pass


class Address(Base):
    __tablename__ = "addresses"

    id: Mapped[int] = mapped_column(primary_key=True)
    user_id: Mapped[int] = mapped_column(ForeignKey("users.id"), nullable=False)
    user: Mapped["User"] = Relationship(back_populates="addresses", lazy="noload")


class User(Base):
    __tablename__ = "users"

    id: Mapped[int] = mapped_column(primary_key=True)
    name: Mapped[str] = mapped_column(String(100), nullable=False)
    addresses: Mapped[List["Address"]] = Relationship(
        back_populates="user", cascade="all", lazy="noload"
    )


engine = create_async_engine("sqlite+aiosqlite:///")


async def main():
    async with engine.connect() as connection:
        await connection.run_sync(Base.metadata.create_all)

    async with AsyncSession(engine) as session:
        u = User(name="user 1", addresses=[Address(), Address()])
        session.add(u)
        await session.commit()

    async with AsyncSession(engine) as session:
        res = await session.execute(select(Address))
        assert len(res.all()) == 2

    async with AsyncSession(engine) as session:
        u = await session.get(User, 1)
        # u = await session.get(User, 1, options=[selectinload(User.addresses)])
        await session.delete(u)
        await session.commit()

    async with AsyncSession(engine) as session:
        res = await session.execute(select(Address))
        addresses = list(res.all())
        assert len(addresses) == 0, len(addresses)  # AssertionError: 2


if __name__ == "__main__":
    asyncio.run(main())

YuriiMotov avatar Aug 26 '25 08:08 YuriiMotov