-
-
Notifications
You must be signed in to change notification settings - Fork 740
Support hybrid_property, column_property, declared_attr #801
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
50Bytes-dev
commented
Feb 14, 2024
class Item(SQLModel, table=True):
id: Optional[int] = Field(default=None, primary_key=True)
value: float
hero_id: int = Field(foreign_key="hero.id")
hero: "Hero" = Relationship(back_populates="items")
class Hero(SQLModel, table=True):
id: Optional[int] = Field(default=None, primary_key=True)
name: str
items: List[Item] = Relationship(back_populates="hero")
@declared_attr
def total_items(cls):
return column_property(cls._total_items_expression())
@classmethod
def _total_items_expression(cls):
return (
select(func.coalesce(func.sum(Item.value), 0))
.where(Item.hero_id == cls.id)
.correlate_except(Item)
.label("total_items")
)
@declared_attr
def status(cls):
return column_property(
select(
case(
(cls._total_items_expression() > 0, "active"), else_="inactive"
)
).scalar_subquery()
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just small suggestions from a random outsider :p (some suggestions might be wrong).
All in all, this is clean and adds a lot of functionality, thanks !
You should look into this @tiangolo !
Co-authored-by: Arthur Woimbée <[email protected]>
Co-authored-by: Arthur Woimbée <[email protected]>
Co-authored-by: Arthur Woimbée <[email protected]>
Please merge it asap :3. I need it, lol. |
hey can we get this merged in |
@tiangolo Could you please take a look at this MR. The functionality is very nice. Would be a nice addition. Doesn't seem to clash with overall design |
Following this example I am getting
Even if I do But I think we can get the same result with
|
Any movement to get this functionality in? |
I haven't got this to work with @declared_attr for this #167 , but I am having the same problem as you with pydantic |
A complete stranger here, but as a heads up: pydantic's stance on |
Currently migrating from native sqlalchemy to sqlmodel, and this is one of the blockers. Is there any update? |
…ionship updates - Implement tests for single and list relationships in Pydantic to SQLModel conversion. - Validate mixed assignments of Pydantic and SQLModel instances. - Test database integration to ensure converted models work with database operations. - Add tests for edge cases and performance characteristics of relationship updates. - Ensure proper handling of forward references in relationships. - Create simple tests for basic relationship updates with Pydantic models.
This commit addresses several issues: 1. **Forward Reference Conversion in SQLModel:** I modified `sqlmodel/main.py` in the `_convert_single_pydantic_to_table_model` function to correctly resolve and convert Pydantic models to SQLModel table models when forward references (string type hints) are used in relationship definitions. This ensures that assigning Pydantic objects to such relationships correctly populates or updates the SQLModel instances. 2. **Test State Leakage in `tests/conftest.py`:** I introduced an autouse fixture in `tests/conftest.py` that calls `SQLModel.metadata.clear()` and `default_registry.dispose()` before each test. This prevents SQLAlchemy registry state from leaking between tests, resolving "Table already defined" and "Multiple classes found" errors, leading to more reliable test runs. 3. **Logic in `tests/test_relationships_update.py`:** I corrected the test logic in `tests/test_relationships_update.py` to properly update existing entities. Previously, the test was attempting to add new instances created via `model_validate`, leading to `IntegrityError` (UNIQUE constraint failed). The test now fetches existing entities from the session and updates their attributes before committing, ensuring the update operations are tested correctly. As a result of these changes, `tests/test_relationships_update.py` now passes, and all other tests in the suite also pass successfully, ensuring the stability of the relationship update functionality.
…d-ref Fix: Correct relationship updates with forward references and test logic
…on with support for dicts
Hi, Cheers |
Hi. Just use my repo https://github.com/50Bytes-dev/sqlmodel/tree/main |
Thanks, no offence but I would prefer to stick with the official repro to keep everythin in line. Isn't there a chance for a merge any time soon? |
This repository has long been abandoned, which is why I made a fork and am adding needed functionality there |