Repository navigation
🐛 Fix allowing using a ForeignKey directly, remove repeated column construction from SQLModelMetaclass.__init__ and upgrade minimum SQLAlchemy to >=1.4.36 - #443
Conversation
Sorry, something went wrong.
have `get_column_from_field` be called once for each field (in `SQLModelMetaclass.__new__`)
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files @@ Coverage Diff @@
## main #443 +/- ##
==========================================
- Coverage 98.49% 97.76% -0.74%
==========================================
Files 185 187 +2
Lines 5856 6266 +410
==========================================
+ Hits 5768 6126 +358
- Misses 88 140 +52
... and 1 file with indirect coverage changes ☔ View full report in Codecov by Sentry. |
Sorry, something went wrong.
|
📝 Docs preview for commit 5ad7c6a at: https://631b234f16d4cf27863512be--sqlmodel.netlify.app |
Sorry, something went wrong.
|
📝 Docs preview for commit eee8e80 at: https://639cdfc1a7c2ed00568a9bc6--sqlmodel.netlify.app |
Sorry, something went wrong.
|
Hey is there a workaround for cascade FK definition now while this fix is being investigated? 😊 UPD, probably something like this: from sqlmodel import Field, SQLModel, create_engine
from sqlalchemy import Column, Integer
from sqlalchemy.sql.schema import ForeignKey
class User(SQLModel, table=True):
id: int = Field(primary_key=True)
class Post(SQLModel, table=True):
id: int = Field(primary_key=True)
user_id: int = Field(
sa_column=Column("user_id", Integer, ForeignKey("user.id", ondelete="CASCADE")),)
) |
Sorry, something went wrong.
|
📝 Docs preview for commit 50a3f76 at: https://7b43a033.sqlmodel.pages.dev |
Sorry, something went wrong.
|
Awesome, thanks a lot for the thorough explanation @daniil-berg! 🚀 I tweaked it and updated the internals a bit. This will be available in the next version, 0.0.9. 🎉 |
Sorry, something went wrong.
# What This updates SQLModel to version 0.0.11, which solves an [issue](fastapi/sqlmodel#443) with foreign key declaration, and another [issue](fastapi/sqlmodel#58) with AsyncSession type annotations. # Why Prior to this update, attempting to delete a document with an associated index record would throw a foreign key violation error. # Test plan Add a document which results in an index record being created. Delete the same document and verify that - The document is deleted without error - The index records associated with the document are also deleted
Summary
There was a bug in the SQLModelMetaclass whereby the SQLAlchemy Column objects were constructed twice for each field defined on a table model: First in the meta class' __new__ method and then again in its __init__ method.
With these changes the get_column_from_field function is called only once for each field, namely in SQLModelMetaclass.__new__.
Example
Construct a table model with a foreign key explicitly constructed via the SQLAlchemy ForeignKey class:
Without the proposed changes, even just running these class definitions causes an error:
Traceback (most recent call last): File "/home/daniil/coding/sqlmodel/experiments/__init__.py", line 12, in <module> class Post(SQLModel, table=True): File "/home/daniil/coding/sqlmodel/sqlmodel/main.py", line 329, in __init__ dict_used[field_name] = get_column_from_field(field_value) File "/home/daniil/coding/sqlmodel/sqlmodel/main.py", line 457, in get_column_from_field return Column(sa_type, *args, **kwargs) # type: ignore File "/home/daniil/.cache/pypoetry/virtualenvs/sqlmodel-TV15XYpK-py3.10/lib/python3.10/site-packages/sqlalchemy/sql/schema.py", line 1765, in __init__ self._init_items(*args) File "/home/daniil/.cache/pypoetry/virtualenvs/sqlmodel-TV15XYpK-py3.10/lib/python3.10/site-packages/sqlalchemy/sql/schema.py", line 144, in _init_items spwd(self, **kw) File "/home/daniil/.cache/pypoetry/virtualenvs/sqlmodel-TV15XYpK-py3.10/lib/python3.10/site-packages/sqlalchemy/sql/base.py", line 1047, in _set_parent_with_dispatch self._set_parent(parent, **kw) File "/home/daniil/.cache/pypoetry/virtualenvs/sqlmodel-TV15XYpK-py3.10/lib/python3.10/site-packages/sqlalchemy/sql/schema.py", line 2559, in _set_parent raise exc.InvalidRequestError( sqlalchemy.exc.InvalidRequestError: This ForeignKey already has a parent !This is fixed with this PR.
Add the following to the example:
There is no more error and the SQL statements are as expected:
PS
The codecov bot is weird... I really don't see how coverage could possibly have decreased. The statement below is also inconsistent with the report, if you click on it. Oh well.