Skip to content

model_copy(deep=False) on a persisted table instance shares SQLAlchemy InstanceState — mutations silently lost on commit, no error #2087

Description

@tritsystem

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 typing import Optional
from sqlmodel import Field, SQLModel, Session, create_engine, select


class Hero(SQLModel, table=True):
    id: Optional[int] = Field(default=None, primary_key=True)
    name: str


engine = create_engine("sqlite://")
SQLModel.metadata.create_all(engine)

with Session(engine) as session:
    hero = Hero(name="Deadpond")
    session.add(hero)
    session.commit()
    session.refresh(hero)

    # "Independent" copy, the documented way to snapshot/rename a model
    copy = hero.model_copy(deep=False)
    copy.name = "RENAMED"

    session.add(copy)
    session.commit()  # no exception

with Session(engine) as session2:
    rows = session2.exec(select(Hero)).all()
    print(rows)  # -> [Hero(id=1, name='Deadpond')]   *** the rename is gone ***

Description

Calling model_copy(deep=False) on a table=True model instance that is
already attached to a Session produces an object that looks independent
(different id(), mutating copy.name does not touch hero.name in
Python), but is not independent from SQLAlchemy's point of view: the copy
and the original share the exact same _sa_instance_state
(InstanceState) object.

copy = hero.model_copy(deep=False)
copy.__dict__["_sa_instance_state"] is hero.__dict__["_sa_instance_state"]
# -> True
copy.__dict__["_sa_instance_state"].obj() is hero
# -> True   (the shared state's weakref still points at the ORIGINAL object)

Because the shared InstanceState.obj() still resolves to hero, not
copy, everything downstream keys off hero:

  • session.add(copy) does not register a second pending/dirty object —
    session.new stays empty.
  • session.dirty shows hero as the modified object (not copy).
  • session.commit() succeeds with no exception.
  • The row in the database is unchanged (Deadpond, not RENAMED) — the
    edit made through copy is silently discarded.

This is not simple field-aliasing (like a shared mutable list) — it's a
silent lost update: no error is raised anywhere, copy.name really
does read "RENAMED" right up until commit, and there is no way to tell
from the copy alone that the edit will not persist.

I'd guess the root cause is that model_copy()'s shallow copy of
__dict__ (Pydantic's mechanism) also shallow-copies the private
_sa_instance_state attribute that SQLAlchemy's instrumentation stores
there, instead of giving the copy a fresh InstanceState bound to itself.
model_copy(deep=True) also shares the same _sa_instance_state object
(deep-copying a weakref-bearing SQLAlchemy internal doesn't produce an
independent one either), so deep=True is not a workaround.

This seems like a real correctness hazard for any code that treats
model_copy() as "make an independent snapshot I can edit and save" —
e.g. an update/PATCH endpoint pattern like:

def update_hero(hero_id: int, hero_update: HeroUpdate, session: Session):
    db_hero = session.get(Hero, hero_id)
    hero_data = hero_update.model_dump(exclude_unset=True)
    updated_hero = db_hero.model_copy(update=hero_data)
    session.add(updated_hero)
    session.commit()
    return updated_hero

which silently no-ops instead of updating the row or raising.

Operating System

Windows

Operating System Details

Windows 11

SQLModel Version

0.0.42

Python Version

3.12.10

Additional Context

sqlalchemy==2.0.52
pydantic==2.13.5

Happy to open a PR — the most surgical fix I can see is having
model_copy() (or a documented safe pattern) give the copy a fresh
InstanceState rather than sharing the original's, but I wanted to
confirm with maintainers whether this is considered a bug in
model_copy()'s interaction with the ORM state, a SQLAlchemy-level
limitation to document, or something to solve with a "use session.merge
/ re-fetch instead" recommendation in the docs.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions