Skip to content

fix: re-initialize SQLAlchemy InstanceState on table model_copy - #2088

Open
taran-dev4u wants to merge 2 commits into
fastapi:mainfrom
taran-dev4u:fix/model-copy-sqlalchemy-instance-state
Open

fix: re-initialize SQLAlchemy InstanceState on table model_copy#2088
taran-dev4u wants to merge 2 commits into
fastapi:mainfrom
taran-dev4u:fix/model-copy-sqlalchemy-instance-state

Conversation

@taran-dev4u

Copy link
Copy Markdown

Description

When calling model_copy() (or deprecated copy()) on a table model instance (especially one attached to or loaded from a database session), Pydantic's default BaseModel.model_copy performs a shallow copy of __dict__, copying _sa_instance_state by reference.

Because _sa_instance_state is a SQLAlchemy InstanceState containing an internal weak reference to the original model instance (state.obj()), the newly created copy shares the same InstanceState referencing the original instance. This leads to subtle and severe bugs:

  • Mutating fields on the copied object does not trigger change tracking on the copy, or corrupts the session state of the original object.
  • Adding the copied object to a database session fails or causes duplicate key / state collisions.

Solution

  1. Overrode SQLModel.model_copy(...) to detect when is_table_model_class(self.__class__) is true.
  2. Stripped the copied _sa_instance_state from new_copy.__dict__ and initialized a fresh SQLAlchemy InstanceState via with partial_init(): self.__class__.__init__(new_copy).
  3. Added SQLModel.copy(...) marked with @deprecated forwarding to model_copy(...) for backwards compatibility.
  4. Added comprehensive test coverage in tests/test_pydantic/test_model_copy.py testing non-table models, unpersisted table models, persisted table models with session mutation/saving, and deprecated copy().

Validation

  • uv run pytest: 219 passed (100%).
  • uv run ruff check .: clean.
  • uv run ruff format --check sqlmodel tests: clean.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants