Skip to content

add model_dump options to sqlmodel_update#1897

Open
A00474880 wants to merge 12 commits into
fastapi:mainfrom
A00474880:improve_sqlmodel_update
Open

add model_dump options to sqlmodel_update#1897
A00474880 wants to merge 12 commits into
fastapi:mainfrom
A00474880:improve_sqlmodel_update

Conversation

@A00474880
Copy link
Copy Markdown

@A00474880 A00474880 commented May 1, 2026

As per discussion #1838, this PR allows users to use model_dump parameters directly in sqlmodel_update function.

New Usage Pattern

Previous recommend use of sqlmodel_update is as follows:

def update_hero(
    *, session: Session = Depends(get_session), hero_id: int, hero: HeroUpdate
):
    db_hero = session.get(Hero, hero_id)
    ...
    hero_data = hero.model_dump(exclude_unset=True)
    db_hero.sqlmodel_update(hero_data)
    ...

This PR allows the following while keeping backward compatibility with the above:

def update_hero(
    *, session: Session = Depends(get_session), hero_id: int, hero: HeroUpdate
):
    db_hero = session.get(Hero, hero_id)
    ...
    db_hero.sqlmodel_update(hero, exclude_unset=True)
    ...

Tests

This PR also adds assertions in the test_update function that increases test coverage by 2 lines:
before:

Name                                                                                                        Stmts   Miss  Cover   Missing
-----------------------------------------------------------------------------------------------------------------------------------------
sqlmodel/main.py                                                                                              375     16    96%   593, 645, 666, 673, 675, 690, 712, 715, 721, 723, 725, 727, 729, 908, 996, 1004

after:

Name                                                                                                        Stmts   Miss  Cover   Missing
-----------------------------------------------------------------------------------------------------------------------------------------
sqlmodel/main.py                                                                                              368     14    96%   593, 645, 666, 673, 675, 690, 712, 715, 721, 723, 725, 727, 729, 908

@A00474880 A00474880 marked this pull request as draft May 1, 2026 18:39
@A00474880 A00474880 marked this pull request as ready for review May 1, 2026 18:40
@A00474880
Copy link
Copy Markdown
Author

I can't seem to be able to add labels to this PR. So it'd be great if I can get this reviewed

Copy link
Copy Markdown
Member

@YuriiMotov YuriiMotov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@A00474880, thanks for your interest and efforts!

I pointed to a couple of issues with this implementation

Comment thread sqlmodel/main.py Outdated
obj: builtins.dict[str, Any] | BaseModel,
*,
update: builtins.dict[str, Any] | None = None,
**model_dump_kwargs,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

kwargs will not give autocompletion, and it's error-prone

Comment thread sqlmodel/main.py
Comment on lines +994 to +995
if isinstance(obj, BaseModel):
obj = obj.model_dump(**model_dump_kwargs)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will break use cases with models that have fields with exclude=True:

from sqlmodel import Field, SQLModel


class Item(SQLModel):
    id: str
    param: str = Field(exclude=True)


a = Item.model_validate({"id": "1", "param": "1"})
b = Item.model_validate({"id": "1", "param": "2"})


a.sqlmodel_update(b, exclude={"id"})
# a.sqlmodel_update(b)


assert a.param == "2"

.. and probably some other cases when model has settings that change the default serialization schema.

@A00474880
Copy link
Copy Markdown
Author

Hi @YuriiMotov , thank you for the review. My recent commits tries to address your feedback. Please let me know if you need anything else.

Change summary:

  1. Added type support for the model_dump_kwargs with a TypedDict definition
  2. To address the issue with Model fields with serialization settings: Added steps to create temporary UpdateModel schema which removes the extra model settings before using model_dump on an instance of the new UpdateModel obj

@github-actions github-actions Bot removed the waiting label May 7, 2026
@A00474880
Copy link
Copy Markdown
Author

Hi all I've had some time to carefully consider the changes and how it affects use cases. With the current implementation of creating a temporary "clean" UpdateModel, some model dump kwargs are effectively ignored when using them directly in the new sqlmodel_update, while the remaining provide slight convenience. Here's a summary:

Effective (8 — work correctly on clean UpdateModel)

These don't depend on field-level metadata:

Kwarg Mechanism
exclude_unset Checks model_fields_set (preserved via model_construct)
exclude_none Compares value against None
exclude Filters by field name
include Filters by field name
mode Controls serialization format
round_trip Round-trip compatible output
warnings Controls warning behavior
context / fallback / serialize_as_any Passed through to serializer

Ignored (3 — clean UpdateModel lacks the corresponding metadata)

Kwarg Why Impact
by_alias Clean model has no serialization_alias on any field Arguably an improvement. This prevents aliased fields from being ignored in sqlmodel_update(Hero.model_dump(by_alias=True)), with current sqlmodel_update(Hero, by_alias=True), aliased fields are also updated
exclude_defaults Clean model has all fields as required (no defaults) Unexpected but harmless. User expects to filter default-valued fields; they all pass through. Original branch had no exclude_defaults at all, so this is a new-but-broken feature, not a regression.
exclude_computed_fields Clean model has no @computed_field OK. model_fields is the basis; computed fields aren't in it to begin with.

Comparison to main branch (getattr approach)

The original sqlmodel_update used getattr(obj, field_name) — no serialization at all:

Aspect Original (getattr) New (clean model + model_dump)
Field(exclude=True) Ignored ✓ Ignored ✓
Field(serialization_alias) Ignored ✓ Ignored ✓
exclude_unset Not supported Added
exclude_none Not supported Added
exclude / include Not supported Added
by_alias Not supported no-op (same as original)
exclude_defaults Not supported no-op (same as original)
Custom serializer on type Not applied (raw value) Applied (serialized value)

Note

We can actually add support for exclude_defaults by adding defaults to the fields_def definition:

            fields_def = {
                fname: (finfo.annotation, finfo.default)
                for fname, finfo in ObjClass.model_fields.items()
            }

Please let me know if you have any suggestions!

@A00474880 A00474880 requested a review from YuriiMotov May 15, 2026 22:24
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.

3 participants