A part table that references a table downstream of its own master breaks delete, drop, trace and list_tables on the connection #1619

Summary

DataJoint accepts a part table whose reference makes its master-part group depend on itself. After that declaration, every cascading delete, every drop, Diagram.trace() and Schema.list_tables() on the connection fail with networkx.exception.NetworkXUnfeasible. This includes operations on tables that have nothing to do with the cycle, and dropping the part that caused it.

Reproduction

import datajoint as dj

schema = dj.Schema("repro_group_cycle")


@schema
class Session(dj.Manual):
    definition = """
    session_id : int32
    """


@schema
class Analysis(dj.Manual):
    definition = """
    -> Session
    analysis_id : int32
    """


@schema
class Subject(dj.Manual):
    definition = """
    subject_id : int32
    """


# Later, a part is added to Session that references Analysis.
class Session(dj.Manual):
    definition = """
    session_id : int32
    """

    class Summary(dj.Part):
        definition = """
        -> master
        -> Analysis
        """


schema(Session)  # succeeds: session__summary is created

Each of these then raises networkx.exception.NetworkXUnfeasible: Graph contains a cycle or graph changed during iteration:

Session.delete()
Subject.delete()                                 # Subject is unrelated to the cycle
dj.Diagram.trace(Analysis & {"session_id": 1})
schema.list_tables()
Session.Summary.drop(part_integrity="ignore")    # so the part cannot be removed through DataJoint

Observed on MySQL and on PostgreSQL with the current master.

Cause

topo_sort in dependencies.py keeps each part right after its master. To make that possible, it orders a master after every table that its parts reference. Here Session.Summary references Analysis, so Session has to come after Analysis, while Analysis references Session and has to come after it. No such order exists, and nx.topological_sort raises.

In general: treat each master and its parts as one node. The tables can be ordered exactly when these groups do not depend on each other in a cycle. Besides the case above, two masters whose parts reference each other's masters also form a cycle: A.Part references B, and B.Part references A.

Diagram.cascade(), Diagram.trace(), Diagram iteration (used by delete and drop) and Schema.list_tables() each sort the whole dependency graph loaded on the connection. That is why a single cycle breaks them for every table.

topo_sort also decides group membership by the table name alone (master__part). A part that never references its master, or references it only through a secondary or nullable foreign key, is still pulled next to its master. That causes failures in both directions:

• m; x references m; part m__p references x but not m. Nothing here is circular, yet topo_sort raises, because it orders m after x. Even without that, keeping m__p next to m would place it before x, which it references.
• m; part m__p1 references m only through a nullable foreign key; part m__p2 references m in its primary key and also references m__p1. topo_sort treats m__p1 as part of m's group and sorts this. But m__p1 is not tied to a master row: it is a table that depends on m, and m__p2 depends on it.

Expected behavior

• A part belongs to its master's group only when it is owned by the master: a foreign key in the part's primary key references the master, or references another owned part of the same master. The reference may rename the key attributes. Any other part is treated as a separate table when DataJoint orders tables.
• Declaring an owned part whose reference would close a cycle raises a DataJointError before any table is created. The message names the part, the referenced table and the cycle. An owned part may still reference its master, another owned part of the same master, or any table that does not depend on its master.
• A cycle that already exists in a database (declared before the check) raises a DataJointError that names the cycle instead of a networkx error. Only the operations that must order the cycle raise it. Operations on other tables, and dropping the part that closes the cycle, keep working.
• A part that is not owned gets a warning at declaration. It is sorted as a separate table, and delete and trace are expected to treat it the same way in 2.4.

Where

• Table.declare() calls the _declare_check() hook after parsing the definition and before running any DDL. Part can override it.
• topo_sort() in dependencies.py, and its callers: Diagram._propagate_restrictions, Diagram._propagate_restrictions_upstream, Diagram.__iter__ / __reversed__, and Schema.list_tables().