Repository navigation
model_copy(deep=False) on a persisted table instance shares SQLAlchemy InstanceState — mutations silently lost on commit, no error #2087
Description
Activity
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.
yes
Reacted by Jagadeep MamidiThanks 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=Truedoesn'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=Falseanddeep=True: pydantic'smodel_copy(update=...)writescopied.__dict__.update(update)directly (pydantic/main.py:423), which never goes throughSQLModel.__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'sInstanceStateobject 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 thatmodel_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:
-
Docs only. Add a note next to
sqlmodel_update()'s docs (and/or the "Update Data"/PATCH-endpoint tutorial section) calling out explicitly thatmodel_copy()/model_copy(update=...)on a persisted table instance silently does not persist the change, in either copy mode, and pointing atsqlmodel_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. -
Guard rail. Override
model_copyon table models to raise (or at minimum warn) when called on a session-attached/persistent instance, pointing atsqlmodel_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. -
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 stillsqlmodel_update()'s job) but it does makemodel_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=Falseanddeep=Truecases if they're useful to attach anywhere.-
@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 bothdeep=Falseanddeep=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.- locked and limited conversation to collaborators
on Sep 7, 2026
First check
Commit to Help
Example Code
Description
Calling
model_copy(deep=False)on atable=Truemodel instance that isalready attached to a
Sessionproduces an object that looks independent(different
id(), mutatingcopy.namedoes not touchhero.nameinPython), but is not independent from SQLAlchemy's point of view: the copy
and the original share the exact same
_sa_instance_state(
InstanceState) object.Because the shared
InstanceState.obj()still resolves tohero, notcopy, everything downstream keys offhero:session.add(copy)does not register a second pending/dirty object —session.newstays empty.session.dirtyshowsheroas the modified object (notcopy).session.commit()succeeds with no exception.Deadpond, notRENAMED) — theedit made through
copyis 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.namereallydoes read
"RENAMED"right up until commit, and there is no way to tellfrom 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_stateattribute that SQLAlchemy's instrumentation storesthere, instead of giving the copy a fresh
InstanceStatebound to itself.model_copy(deep=True)also shares the same_sa_instance_stateobject(deep-copying a
weakref-bearing SQLAlchemy internal doesn't produce anindependent one either), so
deep=Trueis 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:
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
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 freshInstanceStaterather than sharing the original's, but I wanted toconfirm with maintainers whether this is considered a bug in
model_copy()'s interaction with the ORM state, a SQLAlchemy-levellimitation to document, or something to solve with a "use
session.merge/ re-fetch instead" recommendation in the docs.