]> 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:43:46 +0000 (19:43 -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

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 599b91e2630b2cd99d1531f9de5c74d2f489e7ec..9e7b21318cd362ca5cba464f98b058284fa17b67 100644 (file)
@@ -757,7 +757,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 3aa761bd2bc0a8992551f29fbbdb94733ccddc69..f539e968fa90f8f14a377ecf6d0989b3e5ae089e 100644 (file)
@@ -2248,11 +2248,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 63293d656c554494bebf85a3e9814e5f220e6143..7edac8990e8b2770ba1e32190f017e98e4803522 100644 (file)
@@ -2564,7 +2564,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 7b54f3bc1c5e40f36ba992d5973b6012137ac080..72d71635b1e383046befbdd5c80c6fc332a1c9a6 100644 (file)
@@ -2247,7 +2247,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,
         )