_sa_instance_state is dropped when SQLModels are nested. Is this expected?

[davepeck]

First Check

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

Commit to Help

• I commit to help with one of those options 👆

Example Code

from sqlmodel import SQLModel, Session, create_engine, Field

engine = create_engine("sqlite:///:memory:")


class A(SQLModel, table=True):
    id: int | None = Field(primary_key=True)
    x: int

    def blow_up(self, session: Session):
        self.x = 42
        session.add(self)


class B(SQLModel):
    a: A

    def blow_up(self, session: Session):
        self.a.blow_up(session)


if __name__ == "__main__":
    SQLModel.metadata.create_all(engine)
    with Session(engine) as session:
        a = A(x=1)
        session.add(a)
        # Placing `a` inside a `B` instance appears to copy the `a` instance,
        # stripping `_sa_instance_state` from it. Is this intentional?
        b = B(a=a)
        b.blow_up(session)

Description

• Create a SQLModel instance (a)
• Create a non-table SQLModel (b) and nest a inside it
• Note that b.a._sa_instance_state is not present, whereas a._instance_state is
• The call to b.blow_up() will result in SQLAlchemy failing to find instance state

  File "/Users/me/.virtualenvs/sqlmodel-issue-u6SafBdE/lib/python3.10/site-packages/sqlalchemy/orm/attributes.py", line 2254, in set_attribute
    state, dict_ = instance_state(instance), instance_dict(instance)
AttributeError: 'A' object has no attribute '_sa_instance_state'

Operating System

macOS

Operating System Details

Monterey 12.4

SQLModel Version

0.0.6

Python Version

Python 3.10.4

Additional Context

My question is: what should the expected behavior here be?

Stepping back: nesting pydantic models like this is pretty natural; that's why I found this behavior surprising. But: perhaps nesting is not-so-natural when working with SQLModels? I'm not sure. The upshot is that you can't perform database operations on nested instances, for example, by calling b.a.some_method_that_does_database_stuff().

(The behavior is unchanged if B derives directly from pydantic.BaseModel, too.)

If the behavior we're seeing is not the desired behavior, I'm happy to contribute a PR, provided we have a clear understanding of what the right behavior should be.

Thanks for all the hard work on FastAPI, Typer, and SQLModel!

[varneyo]

Did this every get anywhere, just come across it today. Any suggestions at all?

[davepeck]

@varneyo

Doesn't look like it, but I know that tiangolo must be super busy!

I think it's an interesting issue, in the sense that it seems to get to the heart of expected behavior and there seem to be a few ways it could be resolved.

Stepping back:

SQLAlchemy models and Pydantic models are often tantalizingly similar... but I think nesting is a good example of where it's not clear that they can or even should be unified. Nesting in relational databases is special, y'know?

On the other hand, mature web frameworks tend to have stories about how data models and database models relate. For instance, Django has its Model class (database) and its Form class (data). Yes, Django's Form is a ball of yarn that does a bunch of other stuff, but we can ignore that here. Django also provides a way to take a database Model and create a Form from it, but this requires the developer to be explicit about which fields they want to carry over. The solution is intentionally not two-way: there's no built-in way to go from a Form back to a Model.

After dwelling on it a bit, this separation feels right to me; it's a separation I want in the code I write. Data models tend to sit at the API interface; database models sit one layer beneath. The User in my database is very much not the User exposed by my API, and I think that's a good thing.

[Baghdady92]

Any update on this ?

[redb0]

I am also interested in this question

[YuriiMotov]

In current version (0.0.24) the provided code example doesn't result in error. And, if we add session.commit() we can see that updated value is persisted in the DB:

INFO sqlalchemy.engine.Engine INSERT INTO a (x) VALUES (?)
INFO sqlalchemy.engine.Engine [generated in 0.00008s] (42,)

I suppose we can close this issue now. @davepeck, could you please check it?

[davepeck]

Thanks @YuriiMotov !

In current version (0.0.24) the provided code example doesn't result in error.

I can confirm this.

I went back through the full version history and determined that the underlying issue was fixed with v0.0.14 in December of 2023. (v0.0.13 was the last version to exhibit the error.)

I suppose we can close this issue now. @davepeck

I'm not sure!

Version 0.0.14 was a huge change that, among other things, added support for Pydantic v2.

It's not clear to me what, specifically, altered the behavior of the demo code I wrote for this issue.

More to the point: it's still not clear to me what the desired behavior should be. It's good that we no longer crash. But what I mean by this is that nesting of SQL models (including a mixture of table=True and otherwise) seems... tricky. Nesting is a concept that's very natural for Pydantic models but much less natural for relational databases. I suppose I'd personally like to see a clear description (here in this thread, or in the docs) of what nesting means, semantically, and how it is treated in practice. (Absent such a description, it feels to me like the underlying crash may have been fixed by accident, not intent.)

[YuriiMotov]

So, as I understand, adding this test would resolve this issue:

from sqlmodel import SQLModel, Session, create_engine, Field

class A(SQLModel, table=True):
    id: int = Field(primary_key=True)
    foo: str

class B(SQLModel):
    a: A

engine = create_engine("sqlite:///:memory:")


def test_copy_sa_instance_state_of_nested_models():
    with Session(engine) as session:
        a = A(id=1, foo="a")
        session.add(a)
        b = B(a=a)
        assert hasattr(b.a, "_sa_instance_state")
        # assert b.a is a

Right?

Interestingly, the last (commented) assertion passes with Pydantic V2 and fails with V1

[iloveitaly]

Not exactly tied to this specific issue, but I ran into a ton of frusteration managing sessions and added helpers (fastapi, celery, and pytest) to make this easy in activemodel.