]> git.ipfire.org Git - thirdparty/sqlalchemy/sqlalchemy.git/commitdiff
Reword delete-orphan on many error message and document
authorMike Bayer <mike_mp@zzzcomputing.com>
Sat, 16 May 2020 17:05:00 +0000 (13:05 -0400)
committerMike Bayer <mike_mp@zzzcomputing.com>
Sat, 16 May 2020 23:45:24 +0000 (19:45 -0400)
For many years we have encountered users making use of the
"single_parent" flag in response to the error message for
"delete-orphan" expressing this as a means to cancel the current
error.   However, the actual issue here is usually a misuse
of the delete-orphan cascade setting.  Reword the error message to
be much more descriptive about what this means and add new
error link sections describing the situation in as much detail
as possible.

Fixes: #5329
Change-Id: I7ba710378b2935479ab22ff9a0a79c692dbf69a6
(cherry picked from commit 0d611b88911217727a174916b83e296e8c7105b1)

doc/build/errors.rst
doc/build/orm/cascades.rst
lib/sqlalchemy/orm/relationships.py
lib/sqlalchemy/orm/strategies.py
test/orm/test_cascade.py

index 9f628817974eb337c673c8acc59069f4ea0c644d..2df9d12ba255831db279f54a96e147fcea154fff 100644 (file)
@@ -623,7 +623,228 @@ application that doesn't yet have correct "framing" around its
 :class:`.Session` operations. Further detail is described in the FAQ at
 :ref:`faq_session_rollback`.
 
+.. _error_bbf0:
 
+For relationship <relationship>, delete-orphan cascade is normally configured only on the "one" side of a one-to-many relationship, and not on the "many" side of a many-to-one or many-to-many relationship.
+------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
+
+
+This error arises when the "delete-orphan" :ref:`cascade <unitofwork_cascades>`
+is set on a many-to-one or many-to-many relationship, such as::
+
+
+    class A(Base):
+        __tablename__ = "a"
+
+        id = Column(Integer, primary_key=True)
+
+        bs = relationship("B", back_populates="a")
+
+
+    class B(Base):
+        __tablename__ = "b"
+        id = Column(Integer, primary_key=True)
+        a_id = Column(ForeignKey("a.id"))
+
+        # this will emit the error message when the mapper
+        # configuration step occurs
+        a = relationship("A", back_populates="bs", cascade="all, delete-orphan")
+
+    configure_mappers()
+
+Above, the "delete-orphan" setting on ``B.a`` indicates the intent that
+when every ``B`` object that refers to a particular ``A`` is deleted, that the
+``A`` should then be deleted as well.   That is, it expresses that the "orphan"
+which is being deleted would be an ``A`` object, and it becomes an "orphan"
+when every ``B`` that refers to it is deleted.
+
+The "delete-orphan" cascade model does not support this functionality.   The
+"orphan" consideration is only made in terms of the deletion of a single object
+which would then refer to zero or more objects that are now "orphaned" by
+this single deletion, which would result in those objects being deleted as
+well.  In other words, it is designed only to track the creation of "orphans"
+based on the removal of one and only one "parent" object per orphan,  which is
+the natural case in a one-to-many relationship where a deletion of the
+object on the "one" side results in the subsequent deletion of the related
+items on the "many" side.
+
+The above mapping in support of this functionality would instead place the
+cascade setting on the one-to-many side, which looks like::
+
+    class A(Base):
+        __tablename__ = "a"
+
+        id = Column(Integer, primary_key=True)
+
+        bs = relationship("B", back_populates="a", cascade="all, delete-orphan")
+
+
+    class B(Base):
+        __tablename__ = "b"
+        id = Column(Integer, primary_key=True)
+        a_id = Column(ForeignKey("a.id"))
+
+        a = relationship("A", back_populates="bs")
+
+Where the intent is expressed that when an ``A`` is deleted, all of the
+``B`` objects to which it refers are also deleted.
+
+The error message then goes on to suggest the usage of the
+:paramref:`_orm.relationship.single_parent` flag.    This flag may be used
+to enforce that a relationship which is capable of having many objects
+refer to a particular object will in fact have only **one** object referring
+to it at a time.   It is used for legacy or other less ideal
+database schemas where the foreign key relationships suggest a "many"
+collection, however in practice only one object would actually refer
+to a given target object at at time.  This uncommon scenario
+can be demonstrated in terms of the above example as follows::
+
+    class A(Base):
+        __tablename__ = "a"
+
+        id = Column(Integer, primary_key=True)
+
+        bs = relationship("B", back_populates="a")
+
+
+    class B(Base):
+        __tablename__ = "b"
+        id = Column(Integer, primary_key=True)
+        a_id = Column(ForeignKey("a.id"))
+
+        a = relationship(
+            "A",
+            back_populates="bs",
+            single_parent=True,
+            cascade="all, delete-orphan",
+        )
+
+The above configuration will then install a validator which will enforce
+that only one ``B`` may be associated with an ``A`` at at time, within
+the scope of the ``B.a`` relationship::
+
+    >>> b1 = B()
+    >>> b2 = B()
+    >>> a1 = A()
+    >>> b1.a = a1
+    >>> b2.a = a1
+    sqlalchemy.exc.InvalidRequestError: Instance <A at 0x7eff44359350> is
+    already associated with an instance of <class '__main__.B'> via its
+    B.a attribute, and is only allowed a single parent.
+
+Note that this validator is of limited scope and will not prevent multiple
+"parents" from being created via the other direction.  For example, it will
+not detect the same setting in terms of ``A.bs``:
+
+.. sourcecode:: pycon+sql
+
+    >>> a1.bs = [b1, b2]
+    >>> session.add_all([a1, b1, b2])
+    >>> session.commit()
+    {opensql}
+    INSERT INTO a DEFAULT VALUES
+    ()
+    INSERT INTO b (a_id) VALUES (?)
+    (1,)
+    INSERT INTO b (a_id) VALUES (?)
+    (1,)
+
+However, things will not go as expected later on, as the "delete-orphan" cascade
+will continue to work in terms of a **single** lead object, meaning if we
+delete **either** of the ``B`` objects, the ``A`` is deleted.   The other ``B`` stays
+around, where the ORM will usually be smart enough to set the foreign key attribute
+to NULL, but this is usually not what's desired:
+
+.. sourcecode:: pycon+sql
+
+    >>> session.delete(b1)
+    >>> session.commit()
+    {opensql}
+    UPDATE b SET a_id=? WHERE b.id = ?
+    (None, 2)
+    DELETE FROM b WHERE b.id = ?
+    (1,)
+    DELETE FROM a WHERE a.id = ?
+    (1,)
+    COMMIT
+
+For all the above examples, similar logic applies to the calculus of a
+many-to-many relationship; if a many-to-many relationship sets single_parent=True
+on one side, that side can use the "delete-orphan" cascade, however this is
+very unlikely to be what someone actually wants as the point of a many-to-many
+relationship is so that there can be many objects referring to an object
+in either direction.
+
+Overall, "delete-orphan" cascade is usually applied
+on the "one" side of a one-to-many relationship so that it deletes objects
+in the "many" side, and not the other way around.
+
+.. versionchanged:: 1.3.18  The text of the "delete-orphan" error message
+   when used on a many-to-one or many-to-many relationship has been updated
+   to be more descriptive.
+
+
+.. seealso::
+
+    :ref:`unitofwork_cascades`
+
+    :ref:`cascade_delete_orphan`
+
+    :ref:`error_bbf1`
+
+
+
+.. _error_bbf1:
+
+Instance <instance> is already associated with an instance of <instance> via its <attribute> attribute, and is only allowed a single parent.
+---------------------------------------------------------------------------------------------------------------------------------------------
+
+
+This error is emited when the :paramref:`_orm.relationship.single_parent` flag
+is used, and more than one object is assigned as the "parent" of an object at
+once.
+
+Given the following mapping::
+
+    class A(Base):
+        __tablename__ = "a"
+
+        id = Column(Integer, primary_key=True)
+
+
+    class B(Base):
+        __tablename__ = "b"
+        id = Column(Integer, primary_key=True)
+        a_id = Column(ForeignKey("a.id"))
+
+        a = relationship(
+            "A",
+            single_parent=True,
+            cascade="all, delete-orphan",
+        )
+
+The intent indicates that no more than a single ``B`` object may refer
+to a particular ``A`` object at once::
+
+    >>> b1 = B()
+    >>> b2 = B()
+    >>> a1 = A()
+    >>> b1.a = a1
+    >>> b2.a = a1
+    sqlalchemy.exc.InvalidRequestError: Instance <A at 0x7eff44359350> is
+    already associated with an instance of <class '__main__.B'> via its
+    B.a attribute, and is only allowed a single parent.
+
+When this error occurs unexpectedly, it is usually because the
+:paramref:`_orm.relationship.single_parent` flag was applied in response
+to the error message described at :ref:`error_bbf0`, and the issue is in
+fact a misunderstanding of the "delete-orphan" cascade setting.  See that
+message for details.
+
+
+.. seealso::
+
+    :ref:`error_bbf0`
 
 Core Exception Classes
 ======================
index d6b1be1873d138e8af8e799b5eced8ca9e661bfa..d7cddc09c225f4013e85c67aaf84ad58d8f8eca5 100644 (file)
@@ -290,13 +290,20 @@ so that removal of the item from the parent collection results
 in its deletion.
 
 ``delete-orphan`` cascade implies that each child object can only
-have one parent at a time, so is configured in the vast majority of cases
-on a one-to-many relationship.   Setting it on a many-to-one or
-many-to-many relationship is more awkward; for this use case,
-SQLAlchemy requires that the :func:`~sqlalchemy.orm.relationship`
-be configured with the :paramref:`_orm.relationship.single_parent` argument,
-establishes Python-side validation that ensures the object
-is associated with only one parent at a time.
+have one parent at a time, and in the **vast majority of cases is configured
+only on a one-to-many relationship.**   For the much less common
+case of setting it on a many-to-one or
+many-to-many relationship, the "many" side can be forced to allow only
+a single object at a time by configuring the :paramref:`_orm.relationship.single_parent` argument,
+which establishes Python-side validation that ensures the object
+is associated with only one parent at a time, however this greatly limits
+the functionality of the "many" relationship and is usually not what's
+desired.
+
+.. seealso::
+
+  :ref:`error_bbf0` - background on a common error scenario involving delete-orphan
+  cascade.
 
 .. _cascade_merge:
 
index a262d120dffe62f2a4484c8c18d428f5b31ea6a3..8e4bcd63ca3f396d5155691d48f8ea055f02bcf0 100644 (file)
@@ -2235,11 +2235,30 @@ class RelationshipProperty(StrategizedProperty):
             and (self.direction is MANYTOMANY or self.direction is MANYTOONE)
         ):
             raise sa_exc.ArgumentError(
-                "On %s, delete-orphan cascade is not supported "
-                "on a many-to-many or many-to-one relationship "
-                "when single_parent is not set.   Set "
-                "single_parent=True on the relationship()." % self
+                "For %(direction)s relationship %(rel)s, delete-orphan "
+                "cascade is normally "
+                'configured only on the "one" side of a one-to-many '
+                "relationship, "
+                'and not on the "many" side of a many-to-one or many-to-many '
+                "relationship.  "
+                "To force this relationship to allow a particular "
+                '"%(relatedcls)s" object to be referred towards by only '
+                'a single "%(clsname)s" object at a time via the '
+                "%(rel)s relationship, which "
+                "would allow "
+                "delete-orphan cascade to take place in this direction, set "
+                "the single_parent=True flag."
+                % {
+                    "rel": self,
+                    "direction": "many-to-one"
+                    if self.direction is MANYTOONE
+                    else "many-to-many",
+                    "clsname": self.parent.class_.__name__,
+                    "relatedcls": self.mapper.class_.__name__,
+                },
+                code="bbf0",
             )
+
         if self.direction is MANYTOONE and self.passive_deletes:
             util.warn(
                 "On %s, 'passive_deletes' is normally configured "
index 8a97bf6c9f3193d92eeb07fbdebf3ce70505fa64..f21be23d6cff5d513b073049cf14abf847149c57 100644 (file)
@@ -2496,7 +2496,8 @@ def single_parent_validator(desc, prop):
                     "Instance %s is already associated with an instance "
                     "of %s via its %s attribute, and is only allowed a "
                     "single parent."
-                    % (orm_util.instance_str(value), state.class_, prop)
+                    % (orm_util.instance_str(value), state.class_, prop),
+                    code="bbf1",
                 )
         return value
 
index 611f6e176359b645e56c6e2288a2892f58022252..e726551ab7510120beb8eeb2dc92225441b378d4 100644 (file)
@@ -2233,7 +2233,7 @@ class M2MCascadeTest(fixtures.MappedTest):
         mapper(B, b)
         assert_raises_message(
             sa_exc.ArgumentError,
-            "On A.bs, delete-orphan cascade is not supported",
+            "For many-to-many relationship A.bs, delete-orphan cascade",
             configure_mappers,
         )