-
-
Notifications
You must be signed in to change notification settings - Fork 781
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? |
…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 |
…enhance association proxy handling
…perties and enhance setter handling
…s for fallback behavior
…fallback values in deferred properties; add comprehensive tests for async scenarios and edge cases
…d tests to verify fallback behavior after refresh and loading
…y; add basic tests for fallback behavior and undefer functionality
… typing tests for compatibility
|
any updates on this? |
|
This pull request has a merge conflict that needs to be resolved. |
|
I think I can go a bit further than what this pr had in mind. I was thinking about combining computed_feilds and hybrid_property column_property and declared_attr together. Does anyone have any tips and suggestions? I'll try and use what this pr had in mind as a blueprint. |
…configuration and utility functions for SQLModel
|
@50Bytes-dev Glad to see your pr is still alive, mind if I help you review it? |