Skip to content

Conversation

@mchataigner
Copy link

Description

This is a working example describing the issue regarding models mapped as dataclass with relationship.
Currently it’s not possible to provide the relationship with only the foreign_key as it will be overriden by the relationship object.

I already put a comment on an existing discussion here #9383 (comment)

Checklist

This pull request is:

  • A documentation / typographical / small typing error fix
    • Good to go, no issue or tests are needed
  • A short code fix
    • please include the issue number, and create an issue if none exists, which
      must include a complete example of the issue. one line code fixes without an
      issue and demonstration will not be accepted.
    • Please include: Fixes: #<issue number> in the commit message
    • please include tests. one line code fixes without tests will not be accepted.
  • A new feature implementation
    • please include the issue number, and create an issue if none exists, which must
      include a complete example of how the feature would look.
    • Please include: Fixes: #<issue number> in the commit message
    • please include tests.

Have a nice day!

@CaselIT
Copy link
Member

CaselIT commented Jul 24, 2024

Hi,

Thanks for the effort, but I would rather implement the NO_VALUE change what was suggested in other places.

Or maybe this could be merged in the meantime and updated later to make use of the no-value thing once it's merged?
Thoughts on this @zzzeek ?

@mchataigner
Copy link
Author

Yes, I was not expecting you to merge this as it. It was mainly to help you have an easy example to setup for the development as this discussion #9410 was marked as PR with tests welcome. But writing that I figure that you were probably expecting an implementation for NO_VALUE with tests.

Imran-imtiaz48

This comment was marked as spam.

@sqlalchemy sqlalchemy deleted a comment from benedikt-bartscher Oct 10, 2024
Copy link
Member

@zzzeek zzzeek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would call the directory here "dataclasses", where we can accumulate a series of dataclass-related examples. then we would need to have introductory documentation in the __init__.py file, see other directories in the examples/ folder for how this looks. The example script itself also needs introductory docstrings at the top. The examples get picked up by the documentation generation into sphinx docs which you can browse at https://docs.sqlalchemy.org/en/20/orm/examples.html to see what the final product looks like.

@jgourinda
Copy link

jgourinda commented Oct 17, 2024

Hi,

I don't know if this is the right place to report or it should be in the original discussion so let me know if I need to repost.

I tried integrating the solution but I have AttributeError: 'LoaderCallableStatus' object has no attribute '_sa_instance_state'

The difference with the example above is that I have a model that involves Joined Table Inheritance (if I am not wrong)

Compared to the example this the diff to reproduce the error

--- as_is.py	2024-10-17 21:41:47
+++ with_inheritance.py	2024-10-17 21:40:07
@@ -1,4 +1,5 @@
 from datetime import datetime
+from enum import Enum
 from typing import Any
 
 import sqlalchemy as sa
@@ -37,15 +38,61 @@
     )
 
 
+class BalanceType(Enum):
+    user = "user"
+    company = "company"
+
+
+class Balance(Base):
+    __tablename__ = "balance"
+    __mapper_args__ = {
+        "polymorphic_identity": "__notallowed__",
+        "polymorphic_on": "type",
+    }
+
+    id: Mapped[int] = mapped_column(primary_key=True, init=False)
+    type: Mapped[BalanceType] = mapped_column(init=False)
+
+
+class UserBalance(Balance):
+    __tablename__ = "user_balance"
+    __mapper_args__ = {"polymorphic_identity": BalanceType.user}
+
+    id: Mapped[int] = mapped_column(
+        ForeignKey("balance.id"), primary_key=True, init=False
+    )
+
+    user: Mapped["User"] = relationship(back_populates="balance", default=NEVER_SET)
+
+
+class CompanyBalance(Balance):
+    __tablename__ = "company_balance"
+    __mapper_args__ = {"polymorphic_identity": BalanceType.company}
+
+    id: Mapped[int] = mapped_column(
+        ForeignKey("balance.id"), primary_key=True, init=False
+    )
+
+    company: Mapped["Company"] = relationship(
+        back_populates="balance", default=NEVER_SET
+    )
+
+
 class Company(Base, TimestampsMixin):
     __tablename__ = "companies"
 
     id: Mapped[int] = mapped_column(primary_key=True, autoincrement=True, default=None)
     name: Mapped[str] = mapped_column(nullable=False, default=None)
+    balance_id: Mapped[int] = mapped_column(
+        ForeignKey("company_balance.id"), nullable=False, default=None
+    )
 
     users: Mapped[list["User"]] = relationship(
         back_populates="company", default_factory=list
     )
+    balance: Mapped[CompanyBalance] = relationship(
+        back_populates="company", default=NEVER_SET
+    )
 
 
 class User(Base, TimestampsMixin):
@@ -59,7 +106,13 @@
     company_id: Mapped[int] = mapped_column(
         ForeignKey("companies.id"), nullable=False, default=None
     )
+    balance_id: Mapped[int] = mapped_column(
+        ForeignKey("user_balance.id"), nullable=False, default=None
+    )
     company: Mapped[Company] = relationship(back_populates="users", default=NEVER_SET)
+    balance: Mapped[UserBalance] = relationship(
+        back_populates="user", default=NEVER_SET
+    )
 
     applications: Mapped[list["Application"]] = relationship(
         back_populates="owner", default_factory=list
@@ -84,7 +137,7 @@
 
     session = Session(engine)
 
-    company = Company(name="Company")
+    company = Company(name="Company", balance=CompanyBalance())
     session.add(company)
     session.commit()

It fails at Company(name="Company", balance=CompanyBalance()) and it seems to happen when instantiating CompanyBalance()

Complete error trace
Traceback (most recent call last):
  File "/Users/xxxxxx/Work/with_inheritance.py", line 140, in <module>
    company = Company(name="Company", balance=CompanyBalance())
                                              ^^^^^^^^^^^^^^^^
  File "<string>", line 4, in __init__
  File "/Users/xxxxxx/Work/venv/lib/python3.12/site-packages/sqlalchemy/orm/state.py", line 571, in _initialize_instance
    with util.safe_reraise():
         ^^^^^^^^^^^^^^^^^^^
  File "/Users/xxxxxx/Work/venv/lib/python3.12/site-packages/sqlalchemy/util/langhelpers.py", line 146, in __exit__
    raise exc_value.with_traceback(exc_tb)
  File "/Users/xxxxxx/Work/venv/lib/python3.12/site-packages/sqlalchemy/orm/state.py", line 569, in _initialize_instance
    manager.original_init(*mixed[1:], **kwargs)
  File "<string>", line 3, in __init__
  File "/Users/xxxxxx/Work/venv/lib/python3.12/site-packages/sqlalchemy/orm/attributes.py", line 537, in __set__
    self.impl.set(
  File "/Users/xxxxxx/Work/venv/lib/python3.12/site-packages/sqlalchemy/orm/attributes.py", line 1467, in set
    value = self.fire_replace_event(state, dict_, value, old, initiator)
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/xxxxxx/Work/venv/lib/python3.12/site-packages/sqlalchemy/orm/attributes.py", line 1514, in fire_replace_event
    self.sethasparent(instance_state(value), state, True)
                      ^^^^^^^^^^^^^^^^^^^^^
AttributeError: 'LoaderCallableStatus' object has no attribute '_sa_instance_state'
with_inheritance.py
from datetime import datetime
from enum import Enum
from typing import Any

import sqlalchemy as sa
from sqlalchemy import ForeignKey, create_engine
from sqlalchemy.orm import (
    DeclarativeBase,
    Mapped,
    MappedAsDataclass,
    Session,
    mapped_column,
    relationship,
)
from sqlalchemy.orm.base import NEVER_SET


class Base(MappedAsDataclass, DeclarativeBase):
    __allow_unmapped__ = True

    def __post_init__(self, **kwargs: Any) -> None:
        for _name, infos in sa.inspect(self.__class__).relationships.items():
            if getattr(self, infos.key) == NEVER_SET:
                # we have to trick the instance here otherwise it will refuse
                # to delete the attribute
                setattr(self, infos.key, None)
                # removing the attribute in this case will avoid trashing
                # the foreign_key attribute
                delattr(self, infos.key)


class TimestampsMixin(MappedAsDataclass):
    created_at: Mapped[datetime] = mapped_column(
        default=None, server_default=sa.func.now()
    )
    updated_at: Mapped[datetime] = mapped_column(
        default=None, server_default=sa.func.now(), onupdate=sa.func.now()
    )


class BalanceType(Enum):
    user = "user"
    company = "company"


class Balance(Base):
    __tablename__ = "balance"
    __mapper_args__ = {
        "polymorphic_identity": "__notallowed__",
        "polymorphic_on": "type",
    }

    id: Mapped[int] = mapped_column(primary_key=True, init=False)
    type: Mapped[BalanceType] = mapped_column(init=False)


class UserBalance(Balance):
    __tablename__ = "user_balance"
    __mapper_args__ = {"polymorphic_identity": BalanceType.user}

    id: Mapped[int] = mapped_column(
        ForeignKey("balance.id"), primary_key=True, init=False
    )

    user: Mapped["User"] = relationship(back_populates="balance", default=NEVER_SET)


class CompanyBalance(Balance):
    __tablename__ = "company_balance"
    __mapper_args__ = {"polymorphic_identity": BalanceType.company}

    id: Mapped[int] = mapped_column(
        ForeignKey("balance.id"), primary_key=True, init=False
    )

    company: Mapped["Company"] = relationship(
        back_populates="balance", default=NEVER_SET
    )


class Company(Base, TimestampsMixin):
    __tablename__ = "companies"

    id: Mapped[int] = mapped_column(primary_key=True, autoincrement=True, default=None)
    name: Mapped[str] = mapped_column(nullable=False, default=None)
    balance_id: Mapped[int] = mapped_column(
        ForeignKey("company_balance.id"), nullable=False, default=None
    )

    users: Mapped[list["User"]] = relationship(
        back_populates="company", default_factory=list
    )
    balance: Mapped[CompanyBalance] = relationship(
        back_populates="company", default=NEVER_SET
    )


class User(Base, TimestampsMixin):
    __tablename__ = "users"

    id: Mapped[int] = mapped_column(primary_key=True, autoincrement=True, default=None)

    name: Mapped[str] = mapped_column(nullable=False, default=None)
    email: Mapped[str] = mapped_column(nullable=False, default=None)

    company_id: Mapped[int] = mapped_column(
        ForeignKey("companies.id"), nullable=False, default=None
    )
    balance_id: Mapped[int] = mapped_column(
        ForeignKey("user_balance.id"), nullable=False, default=None
    )
    company: Mapped[Company] = relationship(back_populates="users", default=NEVER_SET)
    balance: Mapped[UserBalance] = relationship(
        back_populates="user", default=NEVER_SET
    )

    applications: Mapped[list["Application"]] = relationship(
        back_populates="owner", default_factory=list
    )


class Application(Base, TimestampsMixin):
    __tablename__ = "applications"

    id: Mapped[int] = mapped_column(primary_key=True, autoincrement=True, default=None)
    name: Mapped[str] = mapped_column(nullable=False, default=None)

    owner_id: Mapped[int] = mapped_column(
        ForeignKey("users.id"), nullable=False, default=None
    )
    owner: Mapped[User] = relationship(back_populates="applications", default=None)


if __name__ == "__main__":
    engine = create_engine("sqlite://")
    Base.metadata.create_all(engine)

    session = Session(engine)

    company = Company(name="Company", balance=CompanyBalance())
    session.add(company)
    session.commit()

    user1 = User(name="User", email="user@example.com", company=company)  # works
    session.add(user1)
    session.commit()

    user2 = User(name="User", email="user2@example.com", company_id=company.id)  # works
    session.add(user2)
    session.commit()

    application1 = Application(name="Application", owner=user1)  # works
    session.add(application1)
    session.commit()

    try:
        application2 = Application(
            name="Application2", owner_id=user2.id
        )  # does not work
        session.add(application2)
        session.commit()
    except sa.exc.IntegrityError as e:
        print(e)

pip freeze

SQLAlchemy==2.0.36
typing_extensions==4.12.2

@zzzeek
Copy link
Member

zzzeek commented Oct 17, 2024

you can't use NEVER_SET as a default value like that, that's why the error is being raised, that's not a valid value to assign to a mapped attribute

i see a user is suggesting this as a "hacky workaround" at #9383 (comment) but this is not SQLAlchemy stuff, that's not supported

looks like the discussion was looking for a new constant DONT_SET, no work has been done for that

@zzzeek
Copy link
Member

zzzeek commented Oct 17, 2024

specifically it's the backref / back_populates handler that does not recognize this constant in that context

@jgourinda
Copy link

Ok I think that I got my mistake.

The NEVER_SET workaround is only useful when on the same model you have both a column referencing a Foreign Key and the associated relationship. With the proposed workaround you can do any of Child(parent_id=1) or Child(parent=my_parent) and it works.

But the problem in my case was that neither of CompanyBalance or UserBalance have a column referencing a foreign key. So the use case does not even exist. I changed default=NEVER_SET to default=None for both CompanyBalance.company and UserBalance.user and it works now.

I was a bit too quick at Find / Replace all default=None to default=NEVER_SET in all relationships.

@jgourinda
Copy link

jgourinda commented Oct 17, 2024

But there is something I might have misunderstood

i see a user is suggesting this as a "hacky workaround" at #9383 (comment) but this is not SQLAlchemy stuff, that's not supported

Isn't that "hacky workaround" exactly what is in the example proposed for this PR ?

When I read your reply I got under the impression that you were endorsing it

@zzzeek
Copy link
Member

zzzeek commented Oct 17, 2024

But there is something I might have misunderstood

i see a user is suggesting this as a "hacky workaround" at #9383 (comment) but this is not SQLAlchemy stuff, that's not supported

Isn't that "hacky workaround" exactly what is in the example proposed for this PR ?

When I read your reply I got under the impression that you were endorsing it

it looks like the example was meant to be a template for the feature itself, once implemented.

Im not sure why this has to be that complicated. you can pass None and just search for that:

from __future__ import annotations

from typing import Any

from sqlalchemy import create_engine
from sqlalchemy import ForeignKey
from sqlalchemy import inspect
from sqlalchemy.orm import DeclarativeBase
from sqlalchemy.orm import Mapped
from sqlalchemy.orm import mapped_column
from sqlalchemy.orm import MappedAsDataclass
from sqlalchemy.orm import relationship
from sqlalchemy.orm import Session


class Base(MappedAsDataclass, DeclarativeBase):
    def __post_init__(self, **kwargs: Any) -> None:
        for name in inspect(self.__class__).relationships.keys():
            if self.__dict__.get(name, False) is None:
                delattr(self, name)


class A(Base):
    __tablename__ = "a"

    id: Mapped[int] = mapped_column(primary_key=True, init=False)
    data: Mapped[str]
    b_id: Mapped[int] = mapped_column(ForeignKey("b.id"))
    b: Mapped[B] = relationship("B", default=None)


class B(Base):
    __tablename__ = "b"
    id: Mapped[int] = mapped_column(primary_key=True)
    data: Mapped[str]

e = create_engine("sqlite://", echo=True)
Base.metadata.create_all(e)
a1 = A(data='x', b_id=5)

s = Session(e)
b = B(id=5,data='b1')
s.add(b)
a1 = A(data='x', b_id=5)
s.add(a1)
s.commit()

@jgourinda
Copy link

Okay thank you even better

@zzzeek
Copy link
Member

zzzeek commented Oct 17, 2024

the "DONT_SET_ME" constant still keeps seeming like a good idea here but I dont like how it works against both the philosophy of dataclasses as well as SQLAlchemy's own philosophy that you shouldn't be trying to populate objects in two different ways (fk id vs. object)

@mchataigner
Copy link
Author

mchataigner commented Jan 2, 2025

Thanks for the replies and sorry for the delay.

So your example works fine but if you try to pass the foreign object and not the foreign key it does not:

a2 = A(data='x', b=B(id=6, data='b2'))
s.add(a2)
s.commit()

will fail

---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
Cell In[2], line 1
----> 1 a2 = A(data="pouet", b=B(id=6, data="b2"))

TypeError: __init__() missing 1 required positional argument: 'b_id'

My wish is to be able to use either the foreign key or the foreign object entirely with no error and being wired correctly, and potentially raise an error if both are passed to the init function with different id.

It is currently possible to do the following though:

a2 = A(data="pouet", b_id=6, b=B(id=6, data="b2"))
s.add(a2)
s.commit()

but it feels wrong and error prone having to repeat b and b_id as:

a3 = A(data="toto", b_id=7, b=B(id=8, data="b3"))
s.add(a3)
s.commit()

after the commit, a3.b_id will have value 8, so it correctly ignored the param b_id and wired the id from the object b.
So to me, b_id should not be required.

But if I put init = False to b_id then I cannot pass it as param ever.
and adding default = None to b_id has a side effect of overriding any value passed as b.
It actually works as expected.

So solution is to use:

class Base(MappedAsDataclass, DeclarativeBase):
    def __post_init__(self, **kwargs: Any) -> None:
        for name in inspect(self.__class__).relationships.keys():
            if self.__dict__.get(name, False) is None:
                delattr(self, name)

and set default=None to all fields.

But it still feels weird to add this to non-nullable fields, just to accommodate types.

@mchataigner mchataigner requested a review from zzzeek January 2, 2025 10:38
@zzzeek
Copy link
Member

zzzeek commented Jan 2, 2025

this example is not necessary going forward as the overall issue will be fixed in 2.1 with #12168

@zzzeek zzzeek closed this Jan 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants