Skip to content

SQLAlchemy version 1.4.36 breaks SQLModel relationships #315

Closed

Description

@archydeberker

First Check

  • I added a very descriptive title to this issue.
  • I used the GitHub search to find a similar issue and didn't find it.
  • I searched the SQLModel documentation, with the integrated search.
  • I already searched in Google "How to X in SQLModel" and didn't find any information.
  • I already read and followed all the tutorial in the docs and didn't find an answer.
  • I already checked if it is not related to SQLModel but to Pydantic.
  • I already checked if it is not related to SQLModel but to SQLAlchemy.

👆 Not quite true - this is definitely related to SQLAlchemy!

Commit to Help

  • I commit to help with one of those options 👆

Example Code

from typing import Optional

from sqlmodel import Field, Relationship, SQLModel


class City(SQLModel, table=True):
    name: str = Field(primary_key=True)
    heroes: "Hero" = Relationship(back_populates="city")

class Hero(SQLModel, table=True):
    name: str = Field(primary_key=True)
    city_name: Optional[str] = Field(default=None,foreign_key="city.name")
    city: Optional[City] = Relationship(back_populates="heroes",
                              sa_relationship_kwargs=dict(cascade="all,delete")
                              )


if __name__ == "__main__":

    gotham = City(name="Gotham")
    batman = Hero(name="Batman", city=gotham)

    assert batman.name == 'Batman' # This is fine
    assert batman.city == gotham # This now breaks

Description

Our CI suddenly started failing, despite local SQLModel working fine. The issues turns out to be the transitive dependency on SQLAlchemy, which is weakly pinned: Github Actions pulled the latest version (1.4.36) and most of our tests started failing.

https://github.com/sqlalchemy/sqlalchemy/releases/tag/rel_1_4_36

The problem seems to be related to how relationships are defined, but I haven't yet dug into the SQLAlchemy changes enough to understand why that is.

I'm opening this issue chiefly to help anybody else who is confused by why suddenly their tests are failing. I'm happy to help fix it if it's affecting others too.

For the time being we have just pinned SQLAlchemy==1.4.34 in our requirements.txt.

Operating System

Linux, macOS

Operating System Details

Replicated locally and on Github Actions, both running in Docker

SQLModel Version

0.0.6

Python Version

3.9.10

Additional Context

We were previously running SQLAlchemy 1.4.34 locally and that works fine. Pinning to 1.4.36 breaks SQLModel.

Activity

  1. antont commented on Apr 27, 2022

    @antont

    We pinned to 1.4.35, in our fork of sqlmodel. Happened to have that locally as upgraded recently and hadn't had problems. Ran into this same this afternoon. Thanks for reporting!

  2. JLHasson commented on Apr 27, 2022

    @JLHasson

    Same issue here, pinning to 1.4.35 resolved

  3. byrman commented on Apr 28, 2022

    @byrman
    Contributor

    It seems related to this recent change:

    class DeclarativeMeta(
        _DynamicAttributesType, inspection.Inspectable["Mapper[Any]"]
    ):
        metadata: MetaData
        registry: "RegistryType"
    
        def __init__(
            cls, classname: Any, bases: Any, dict_: Any, **kw: Any
        ) -> None:
            # use cls.__dict__, which can be modified by an
            # __init_subclass__() method (#7900)
            dict_ = cls.__dict__  # This line is the culprit?!
    

    https://github.com/sqlalchemy/sqlalchemy/blob/main/lib/sqlalchemy/orm/decl_api.py#L114

  4. jossefaz commented on Apr 28, 2022

    @jossefaz

    As @byrman wrote, the change of DeclarativeMeta seems to be the breaking change here.

    [orm] [declarative] [bug] Modified the DeclarativeMeta metaclass to pass cls.dict
    into the declarative scanning process to look for attributes, rather than
    the separate dictionary passed to the type’s init() method. This
    allows user-defined base classes that add attributes within an
    init_subclass() to work as expected, as init_subclass() can
    only affect the cls.dict itself and not the other dictionary. This
    is technically a regression from 1.3 where dict was being used.

  5. byrman commented on Apr 28, 2022

    @byrman
    Contributor

    Not sure if this is a sustainable fix, I don't know how to leverage __init_subclass__, but adding 1 line after here makes things work again:

    ...
    dict_used[rel_name] = rel_value
    setattr(cls, rel_name, rel_value)  # Quick fix?
    
  6. jossefaz commented on Apr 30, 2022

    @jossefaz

    The SQLAlchemy maintainers confirmed that this is the issue, and also suggested a fix : sqlalchemy/sqlalchemy#7972 (reply in thread)

  7. byrman commented on May 1, 2022

    @byrman
    Contributor

    The fix I suggested is backwards compatible. Shall I make a pull request or is there more to it than setting rel_name on the cls?

  8. byrman commented on May 1, 2022

    @byrman
    Contributor

    Let me know if this can be improved upon: #322.

  9. maresb commented on May 6, 2022

    @maresb

    For reference, the actual error message is:

    AttributeError: 'SomeModelWithRelationshipAttribute' object has no attribute 'the_relationship_attribute'
  10. eddywee commented on May 18, 2022

    @eddywee

    thank you @archydeberker !! after hours of debugging i found this issue pinned the version and it works now! <3 thank you!

  11. 14 remaining items

  12. tiangolo commented on Aug 27, 2022

    @tiangolo
    Member

    Thanks for the report @archydeberker! 🤓

    And thanks for the discussion everyone! This was solved by @byrman in #322.

    It will be available in the next version, SQLModel 0.0.7, released in the next hours. 🚀

  13. added
    bugSomething isn't working
    and removed
    questionFurther information is requested
    on Aug 27, 2022
  14. github-actions commented on Sep 7, 2022

    @github-actions
    Contributor

    Assuming the original need was handled, this will be automatically closed now. But feel free to add more comments or create new issues or PRs.

  15. mathieu-lemay commented on Oct 7, 2022

    @mathieu-lemay

    I can still reproduce the issue with SQLModel==0.0.8 and SQLAlchemy==1.4.41

    from sqlalchemy import ForeignKeyConstraint
    from sqlalchemy.orm import RelationshipProperty, selectinload
    from sqlmodel import Field, Relationship, SQLModel, select
    
    
    class Parent(SQLModel, table=True):
        __tablename__ = "parents"
    
        # The keys are set up this way
        parent_id: int = Field(primary_key=True)
        name: str
    
        children: list["Child"] = Relationship(
            sa_relationship=RelationshipProperty(
                "Child",
                back_populates="parent",
            )
        )
    
    
    class Child(SQLModel, table=True):
        __tablename__ = "children"
        __table_args__ = (
            ForeignKeyConstraint(
                ["parent_id"],
                [f"{Parent.__tablename__}.parent_id"],
                ondelete="CASCADE",
            ),
        )
    
        child_id: int = Field(primary_key=True)
        parent_id: int
        name: str
    
        parent: Parent = Relationship(
            sa_relationship=RelationshipProperty(
                "Parent",
                cascade="all, delete",
                back_populates="children",
            )
        )
    
    
    stmt = select(Parent).options(selectinload(Parent.children))

    Running it gives the following error:

    Traceback (most recent call last):
      File "/home/mathieu/sqlmodelbug/main2.py", line 46, in <module>
        stmt = select(Parent).options(selectinload(Parent.children))
    AttributeError: type object 'Parent' has no attribute 'children'
    

    It works fine with SLQAlchemy==1.4.35

  16. byrman commented on Oct 8, 2022

    @byrman
    Contributor

    Indeed, if sa_relationship is defined, my naive fix is left untouched due to these lines:

                for rel_name, rel_info in cls.__sqlmodel_relationships__.items():
                    if rel_info.sa_relationship:
                        # There's a SQLAlchemy relationship declared, that takes precedence
                        # over anything else, use that and continue with the next attribute
                        dict_used[rel_name] = rel_info.sa_relationship
                        continue  # Will not reach fix!
    

    Sorry I missed that path! It needs to be addressed, of course, but perhaps you can use this as a workaround:

    class Parent(SQLModel, table=True):
        children: list["Child"] = Relationship(back_populates="parent")
    
    class Child(SQLModel, table=True):
        parent: Parent = Relationship(back_populates="children")
    
  17. byrman commented on Oct 8, 2022

    @byrman
    Contributor

    @mathieu-lemay, I made a pull request that covers your case as well: #461. I still have to add a test though.

  18. mathieu-lemay commented on Oct 11, 2022

    @mathieu-lemay

    @byrman The workaround works so I'm gonna use that. Thanks for the fix!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    answeredbugSomething isn't working

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions