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

  1. jagadeepmamidi commented on Sep 5, 2026

    @jagadeepmamidi

    Hi @tritsystem, are you still planning to work on this? If not, I’d be interested in taking it up once the maintainers confirm the expected approach.

  2. tritsystem commented on Sep 5, 2026

    @tritsystem
    Author

    yes

  3. tritsystem commented on Sep 5, 2026

    @tritsystem
    Author

    Thanks for the interest in this, @jagadeepmamidi — while sitting down to work on it I dug a bit further and the root cause turned out to be a little sharper than my original report, so sharing that first in case it changes which fix direction makes the most sense to you.

    deep=True doesn't actually fix it either — it fails a different way. Verified on sqlmodel 0.0.42 / SQLAlchemy 2.0.52:

    deep = hero.model_copy(update={"age": 31}, deep=True)
    sa_inspect(hero).key == sa_inspect(deep).key
    # True — deepcopy faithfully copies InstanceState.key too, so the "new"
    # state still claims the SAME identity as the already-persistent original
    
    session.add(deep)
    session.commit()
    # "succeeds", no exception — but session.new and session.dirty are both
    # empty throughout, and the DB row is never touched

    The common root cause for both deep=False and deep=True: pydantic's model_copy(update=...) writes copied.__dict__.update(update) directly (pydantic/main.py:423), which never goes through SQLModel.__setattr__ — the method that actually fires SQLAlchemy's attribute-instrumentation events. Since __setattr__ is never called, SQLAlchemy never sees a change to mark dirty, regardless of whether the copy's InstanceState object happens to be shared or distinct. The aliasing I originally reported is a real, visible symptom of this, but not the only way the update goes missing.

    The tool that already exists in SQLModel and handles this correctly is hero.sqlmodel_update({"age": 31}) — it goes through __setattr__, correctly marks the instance dirty, and persists. So the sharp edge here is specifically that model_copy() looks like a reasonable, idiomatic way to do a partial update on a table instance (very natural pydantic instinct, and it's what plain pydantic models are for) — and silently isn't one, in either copy mode, with no error raised either way.

    A few directions this could go, each with different scope/tradeoffs — happy to send a PR for whichever fits best, or if you'd rather take it from here yourselves that's completely understood, just let me know which way (if any) you'd like it to go:

    1. Docs only. Add a note next to sqlmodel_update()'s docs (and/or the "Update Data"/PATCH-endpoint tutorial section) calling out explicitly that model_copy() / model_copy(update=...) on a persisted table instance silently does not persist the change, in either copy mode, and pointing at sqlmodel_update() as the correct tool. Smallest possible change, zero behavior risk, ships immediately — but relies on the user finding the docs before hitting the trap.

    2. Guard rail. Override model_copy on table models to raise (or at minimum warn) when called on a session-attached/persistent instance, pointing at sqlmodel_update() in the message. Closes the silent-data-loss hole directly at the moment it would occur, with a small, well-contained code change and no change to working behavior — it only ever fires in the case that's already broken today.

    3. Behavioral fix. Override model_copy/__copy__/__deepcopy__ on table models so copying a persistent instance always returns a clean, fully-detached transient object (state and identity key reset), matching ordinary Python copy semantics of "an independent object" rather than a silently entangled one. Broadest scope — this doesn't give you "patch this same row" (that's still sqlmodel_update()'s job) but it does make model_copy() behave predictably and safely for the case where someone genuinely wants an independent, insertable-as-a-new-row clone, which today is also silently broken the same way.

    (1) and (2) combine well and are both low-risk if you'd rather ship something quickly while leaving (3)'s behavior change for more discussion. Happy to scope a PR to just one of these, any combination, or none if you'd prefer to handle it internally — whatever's easiest on your end. I have repro scripts for both the deep=False and deep=True cases if they're useful to attach anywhere.

  4. tritsystem commented on Sep 5, 2026

    @tritsystem
    Author

    @tiangolo tagging you directly since this touches model_copy() semantics on table models specifically — figured you'd want visibility given you're SQLModel's creator/maintainer. The analysis above narrows this down to a silent-data-loss trap that exists in both deep=False and deep=True, with three possible fix directions laid out (docs-only, a guard rail, or a behavioral fix) — happy to send a PR for whichever fits your preference for the project's scope, or step back entirely if you'd rather handle it yourselves. No rush, just wanted to make sure this had a chance to reach you rather than sit in the comment thread.

  5. locked and limited conversation to collaborators on Sep 7, 2026
  6. converted this issue into a discussion #2089 on Sep 7, 2026
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