sqlmodel icon indicating copy to clipboard operation
sqlmodel copied to clipboard

✨ Support `sqlmodel_rebuild`

Open adsharma opened this issue 1 year ago • 16 comments

The @sqlmodel decorator introduced by fquery.sqlmodel moves schema definitions and types closer to the object model (references) instead of foreign_keys.

Instead of writing:

class Hero(SQLModel, table=True):
    ...
    team_id: int | None = Field(default=None, foreign_key="team.id")

you could write:

@sqlmodel
class Hero:
    ...
    team: Team | None = foreign_key("team.id")

Everything works the same as before. Only the syntax has changed. However, if Team also has a foreign key to Hero, then one of them will have to use forward declaration so type checkers work.

In those cases we need a two pass solution where in the second pass, with the full type definition available, we can generate the correct code.

Refactor some of the existing code to introduce sqlmodel_rebuild().

Use cases and benefits

  • Model relationships (SQLModel objects) instead of exposing ids - an implementation detail. If someone wants to use UUID instead of int, it could be handled via meta programming instead of having to rewrite all the models.

  • Declarative/Flexible ID generation. Do you want each table to have a different ID space or keep them all in one global ID space where objects are guaranteed to have unique IDs regardless of the type.

  • Multi-model use cases - relational, columnar and graph. Each one could use its own decorator that stacks on top of @sqlmodel. fquery uses the labelled property graph approach (LPG). Such graphs can be further exposed via graphql or related technologies.

  • Startup performance in a large repo containing 1000 sqlmodels. By deferring some of the initialization we get faster load times. The rest of the initialization could happen only when the model is instantiated.

The decorator is in a self contained python file and can be moved to a standalone library independent of fquery or be included as a part of this PR.

adsharma avatar Jan 17 '25 01:01 adsharma

@svlandeg I think there were some merge errors in one of the commits in this thread.

The following should show changes to only sqlmodel/main.py. However, I see other deltas.

git diff 83629f6 7d00768 --stat
 .github/workflows/publish.yml   |  2 +-
 .github/workflows/smokeshow.yml | 12 +-----------
 .github/workflows/test.yml      | 11 ++---------
 docs/databases.md               |  2 +-
 docs/release-notes.md           |  6 ------
 sqlmodel/main.py                | 17 +----------------
 6 files changed, 6 insertions(+), 44 deletions(-)

adsharma avatar Feb 26 '25 21:02 adsharma

@svlandeg I think there were some merge errors in one of the commits in this thread.

The following should show changes to only sqlmodel/main.py. However, I see other deltas.

The diff on this PR only shows changes in sqlmodel/main.py. That said, the CI has not been green at any point throughout the commits of this PR. This should be fixed before the PR is taken out of draft and marked as ready for review.

As you can see, the tests for Pydantic v1 are failing. This needs to be addressed before we can properly review this PR. Thanks! 🙏

svlandeg avatar Feb 27 '25 08:02 svlandeg

The diff on this PR only shows changes in sqlmodel/main.py

That's the inconsistency I'm calling out. What the UI shows and git diff commit1 commit2 show are different and I believe that's causing the test failures.

I'm thinking I'll close this and create a new PR as a workaround.

adsharma avatar Feb 27 '25 18:02 adsharma

I looked into the pydantic_v1 test failures a bit. There are 3 different instances of the test:

~/src/sqlmodel/docs_src/tutorial/fastapi/app_testing$ ls -1d tut*
tutorial001
tutorial001_py310
tutorial001_py39

If I run pytest in each individual directory, they all pass. However, if I try to turn pytest in the parent directory, it tries to load the Hero class from tutorial001 and then again an identically named class in tutorial001_py310 and fails exactly the same way as CI. Please note that this is happening at test collection time, even before tests are executed.

I can reproduce this behavior on the main branch as well. So the question becomes how were these tests passing before my changes?

adsharma avatar Feb 27 '25 19:02 adsharma

I can reproduce this behavior on the main branch as well. So the question becomes how were these tests passing before my changes?

Hm, that's puzzling to me as well, as the tests do seem to succeed just fine on other recent PRs for this repo 🤔

svlandeg avatar Feb 28 '25 10:02 svlandeg

@svlandeg the above test run verifies that the pydantic-v1 breakage was caused by my change.

Looking into the actual cause of the failure, I think there is some __call__ magic happening on the call to __do_init__() that I don't completely understand. The problem is that classname argument is different before and after the change.

Solving this boils down to figuring out how to correctly invoke __do_init__() so the behavior is the same as before.

adsharma avatar Feb 28 '25 18:02 adsharma

@svlandeg tests are all green. Please review.

adsharma avatar Mar 01 '25 04:03 adsharma

@adsharma, are there any other use cases beside the use case with external library you've mentioned? Could it be useful for something else?

YuriiMotov avatar Oct 08 '25 15:10 YuriiMotov

@YuriiMotov here's how I would like this work to be viewed:

The most general case is using meta programming to hide implementation details behind Pydantic and SQLModel.

In a large repository of many thousand SQLModels, you might want to defer work. Otherwise, model loading becomes slow and expensive.

Anytime you want to do multiple passes over the objects, you'll need the rebuild capability. So no, it's not something specific to my pet python library. Pydantic already implements it. But it's not lazy enough.

adsharma avatar Oct 08 '25 16:10 adsharma

Thanks for quick reply!

In a large repository of many thousand SQLModels, you might want to defer work. Otherwise, model loading becomes slow and expensive.

But in current implementation (this PR) initialization is not deferred, right? It just allows to re-initialize model later.

Anytime you want to do multiple passes over the objects, you'll need the rebuild capability. So no, it's not something specific to my pet python library

I'm afraid it's a bit too abstract. Wen need more clear reasoning to move it forward.

I can imagine someone may want to dynamically add fields using decorators (e.g. for Multi-tenancy). Like:

@add_client_specific_fields(client_id)
class Hero(SQLModel, table-True):
    pass

Is it a valid use case you have in mind for this feature? Can you add a few more use cases?

YuriiMotov avatar Oct 08 '25 16:10 YuriiMotov

But in current implementation (this PR) initialization is not deferred, right? It just allows to re-initialize model later.

That's right. But this file works as a standalone library. If you prefer it to be a part of this PR, I'm happy to contribute.

Re-initialize the model later has many good use cases:

  • Model relationships (SQLModel objects) instead of exposing ids - an implementation detail. If someone wants to use UUID instead of int, it could be handled via meta programming instead of having to rewrite all the models.

  • Declarative/Flexible ID generation. Do you want each table to have a different ID space or keep them all in one global ID space where objects are guaranteed to have unique IDs regardless of the type.

Most importantly, on repos like schema-org-python such re-initialize later flow improves startup time. The repo has about 1000 models.

adsharma avatar Oct 08 '25 18:10 adsharma

dynamically add fields using decorators

yes, this is a valid use case. So are stacking other decorators:

@node
@sqlmodel
class SomeModel:
 ...

Would allow this table to be used as a node in a labelled property graph. You can further expose them via graphql or similar.

adsharma avatar Oct 08 '25 19:10 adsharma

Would be nice if you could summarize use cases we discussed and update the description of this PR, so that we could hide some comments as resolved. It would simplify the review process for Sebastian

YuriiMotov avatar Oct 08 '25 20:10 YuriiMotov

Updated the summary. Please review.

adsharma avatar Oct 08 '25 20:10 adsharma

Updated the summary. Please review.

Unfortunately, I think you misunderstood me.. For now the description looks like a promotion of your library. But I think we should focus on the question "Why introducing sqlmodel_rebuild() method is beneficial for SQLModel and its users. Why Sebastian should prioritize this task other hundreds other tasks?

YuriiMotov avatar Oct 09 '25 08:10 YuriiMotov

In the last sentence of the description, I address "is it a promotion of a particular library?". It's certainly a promotion of some of the methods I discuss. I'm agnostic about where the code lives.

Are there other compositional techniques (stacking decorators) possible? Certainly. It's really a question for fastapi user community to come up with use cases.

Here's a more complete end to end example utilizing this approach.

adsharma avatar Oct 09 '25 20:10 adsharma