Avoid revisit table

Author: CBroz1

datajoint/table.py

@@ -486,7 +486,7 @@ def delete(
         transaction: bool = True,
         safemode: Union[bool, None] = None,
         force_parts: bool = False,
-        include_master: bool = True,
+        include_parts: bool = True,
     ) -> int:
         """
         Deletes the contents of the table and its dependent tables, recursively.
@@ -498,7 +498,8 @@ def delete(
             safemode: If `True`, prohibit nested transactions and prompt to confirm. Default
                 is `dj.config['safemode']`.
             force_parts: Delete from parts even when not deleting from their masters.
-            include_master: If `True`, delete from the master table as well. Default is `True`.
+            include_parts: If `True`, include part/master pairs in the cascade.
+                Default is `True`.
 
         Returns:
             Number of deleted rows (excluding those from dependent tables).
@@ -509,6 +510,7 @@ def delete(
             DataJointError: Deleting a part table before its master.
         """
         deleted = set()
+        visited_masters = set()
 
         def cascade(table):
             """service function to perform cascading deletes recursively."""
@@ -568,25 +570,30 @@ def cascade(table):
                     else:
                         child &= table.proj()
 
-                    master = get_master(child.full_table_name)
-                    if include_master and master and master not in deleted:
-                        master_table = FreeTable(table.connection, master)
-                        master_table._restriction = [
-                            make_condition(
-                                master_table,
-                                (master_table & child).proj().fetch(),
-                                set(),
+                    master_name = get_master(child.full_table_name)
+                    if (
+                        include_parts
+                        and master_name
+                        and master_name != table.full_table_name
+                        and master_name not in visited_masters
+                    ):
+                        master = FreeTable(table.connection, master_name)
+                        master._restriction_attributes = set()
+                        master._restriction = [
+                            make_condition(  # &= may cause in target tables in subquery
+                                master,
+                                (master.proj() & child.proj()).fetch(),
+                                master._restriction_attributes,
                             )
                         ]
-
-                    cascade(child)
-
-                    if include_master and master and master not in deleted:
-                        cascade(master_table)
+                        visited_masters.add(master_name)
+                        cascade(master)
+                    else:
+                        cascade(child)
                 else:
                     deleted.add(table.full_table_name)
                     logger.info(
-                        "Deleting {count} rows from {table}".format(
+                        "Deleting: {count} rows from {table}".format(
                             count=delete_count, table=table.full_table_name
                         )
                     )

tests/conftest.py

@@ -317,6 +317,7 @@ def schema_simp(connection_test, prefix):
     schema(schema_simple.E)
     schema(schema_simple.F)
     schema(schema_simple.F)
+    schema(schema_simple.G)
     schema(schema_simple.DataA)
     schema(schema_simple.DataB)
     schema(schema_simple.Website)

tests/schema_simple.py

@@ -111,17 +111,36 @@ class F(dj.Part):
         -> B.C
         """
 
+    class G(dj.Part):
+        definition = """ # test secondary fk reference
+        -> E
+        id_g :int
+        ---
+        -> L
+        """
+
+    class H(dj.Part):
+        definition = """ # test no additional fk reference
+        -> E
+        id_h :int
+        """
+
     def make(self, key):
         random.seed(str(key))
-        self.insert1(dict(key, **random.choice(list(L().fetch("KEY")))))
-        sub = E.F()
-        references = list((B.C() & key).fetch("KEY"))
-        random.shuffle(references)
-        sub.insert(
+        l_contents = list(L().fetch("KEY"))
+        part_f, part_g, part_h = E.F(), E.G(), E.H()
+        bc_references = list((B.C() & key).fetch("KEY"))
+        random.shuffle(bc_references)
+
+        self.insert1(dict(key, **random.choice(l_contents)))
+        part_f.insert(
             dict(key, id_f=i, **ref)
-            for i, ref in enumerate(references)
+            for i, ref in enumerate(bc_references)
             if random.getrandbits(1)
         )
+        g_inserts = [dict(key, id_g=i, **ref) for i, ref in enumerate(l_contents)]
+        part_g.insert(g_inserts)
+        part_h.insert(dict(key, id_h=i) for i in range(4))
 
 
 class F(dj.Manual):
@@ -132,6 +151,15 @@ class F(dj.Manual):
     """
 
 
+class G(dj.Computed):
+    definition = """ # test downstream of complex master/parts
+    -> E
+    """
+
+    def make(self, key):
+        self.insert1(key)
+
+
 class DataA(dj.Lookup):
     definition = """
     idx     : int

tests/test_cascading_delete.py

@@ -1,6 +1,6 @@
 import pytest
 import datajoint as dj
-from .schema_simple import A, B, D, E, L, Website, Profile
+from .schema_simple import A, B, D, E, G, L, Website, Profile
 from .schema import ComplexChild, ComplexParent
 
 
@@ -11,6 +11,7 @@ def schema_simp_pop(schema_simp):
     B().populate()
     D().populate()
     E().populate()
+    G().populate()
     yield schema_simp
 
 
@@ -96,7 +97,7 @@ def test_delete_complex_keys(schema_any):
         **{
             "child_id_{}".format(i + 1): (i + parent_key_count)
             for i in range(child_key_count)
-        }
+        },
     )
     assert len(ComplexParent & restriction) == 1, "Parent record missing"
     assert len(ComplexChild & restriction) == 1, "Child record missing"
@@ -114,13 +115,20 @@ def test_delete_parts_error(schema_simp_pop):
     """test issue #151"""
     with pytest.raises(dj.DataJointError):
         Profile().populate_random()
-        Website().delete(include_master=False)
+        Website().delete(include_parts=False)
 
 
 def test_delete_parts(schema_simp_pop):
     """test issue #151"""
     Profile().populate_random()
-    Website().delete(include_master=True)
+    Website().delete(include_parts=True)
+
+
+def test_delete_parts_complex(schema_simp_pop):
+    """test issue #151 with complex master/part. PR #1158."""
+    prev_len = len(G())
+    (A() & "id_a=1").delete()
+    assert prev_len - len(G()) == 16, "Failed to delete parts"
 
 
 def test_drop_part(schema_simp_pop):