diff --git a/CHANGELOG.md b/CHANGELOG.md index ea721e2b3..57d639fbd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,9 +11,10 @@ - Added - Missing tests for set_password - PR [#1106](https://github.com/datajoint/datajoint-python/pull/1106) - Changed - Returning success count after the .populate() call - PR [#1050](https://github.com/datajoint/datajoint-python/pull/1050) - Fixed - `Autopopulate.populate` excludes `reserved` jobs in addition to `ignore` and `error` jobs -- Fixed - Issue [#1159]((https://github.com/datajoint/datajoint-python/pull/1159) (cascading delete) - PR [#1160](https://github.com/datajoint/datajoint-python/pull/1160) +- Fixed - Issue [#1159](https://github.com/datajoint/datajoint-python/pull/1159) (cascading delete) - PR [#1160](https://github.com/datajoint/datajoint-python/pull/1160) - Changed - Minimum Python version for Datajoint-Python is now 3.8 PR #1163 - Fixed - `docker compose` commands in CI [#1164](https://github.com/datajoint/datajoint-python/pull/1164) +- Changed - Default delete behavior now includes masters of part tables - PR [#1158](https://github.com/datajoint/datajoint-python/pull/1158) ### 0.14.1 -- Jun 02, 2023 - Fixed - Fix altering a part table that uses the "master" keyword - PR [#991](https://github.com/datajoint/datajoint-python/pull/991) diff --git a/datajoint/table.py b/datajoint/table.py index 53b34923e..96e380823 100644 --- a/datajoint/table.py +++ b/datajoint/table.py @@ -486,6 +486,7 @@ def delete( transaction: bool = True, safemode: Union[bool, None] = None, force_parts: bool = False, + force_masters: bool = False, ) -> int: """ Deletes the contents of the table and its dependent tables, recursively. @@ -497,6 +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. + force_masters: If `True`, include part/master pairs in the cascade. + Default is `False`. Returns: Number of deleted rows (excluding those from dependent tables). @@ -507,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.""" @@ -566,7 +570,27 @@ def cascade(table): ) else: child &= table.proj() - cascade(child) + + master_name = get_master(child.full_table_name) + if ( + force_masters + 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, + ) + ] + visited_masters.add(master_name) + cascade(master) + else: + cascade(child) else: deleted.add(table.full_table_name) logger.info( diff --git a/docs/src/manipulation/delete.md b/docs/src/manipulation/delete.md index 533be6abd..83eb0125b 100644 --- a/docs/src/manipulation/delete.md +++ b/docs/src/manipulation/delete.md @@ -27,4 +27,5 @@ consequence of deleting the master table. To enforce this workflow, calling `delete` directly on a part table produces an error. In some cases, it may be necessary to override this behavior. -To remove entities from a part table without calling `delete` master, use the argument `force=True`. +To remove entities from a part table without calling `delete` master, use the argument `force_parts=True`. +To include the corresponding entries in the master table, use the argument `force_masters=True`. diff --git a/tests/conftest.py b/tests/conftest.py index 46f59acb3..65d68268b 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -339,6 +339,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) diff --git a/tests/schema_simple.py b/tests/schema_simple.py index f0faeea4b..9e3113c9a 100644 --- a/tests/schema_simple.py +++ b/tests/schema_simple.py @@ -111,17 +111,46 @@ 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 + """ + + class M(dj.Part): + definition = """ # test force_masters revisit + -> E + id_m :int + --- + -> E.H + """ + 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, part_m = E.F(), E.G(), E.H(), E.M() + 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) + h_inserts = [dict(key, id_h=i) for i in range(4)] + part_h.insert(h_inserts) + part_m.insert(dict(key, id_m=m, **random.choice(h_inserts)) for m in range(4)) class F(dj.Manual): @@ -132,6 +161,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 diff --git a/tests/test_cascading_delete.py b/tests/test_cascading_delete.py index e4cb5a955..b4adb31c2 100644 --- a/tests/test_cascading_delete.py +++ b/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" @@ -110,11 +111,24 @@ def test_delete_master(schema_simp_pop): Profile().delete() -def test_delete_parts(schema_simp_pop): +def test_delete_parts_error(schema_simp_pop): """test issue #151""" with pytest.raises(dj.DataJointError): Profile().populate_random() - Website().delete() + Website().delete(force_masters=False) + + +def test_delete_parts(schema_simp_pop): + """test issue #151""" + Profile().populate_random() + Website().delete(force_masters=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(force_masters=True) + assert prev_len - len(G()) == 16, "Failed to delete parts" def test_drop_part(schema_simp_pop): diff --git a/tests/test_erd.py b/tests/test_erd.py index 8a2d1d3ac..1cdd936b0 100644 --- a/tests/test_erd.py +++ b/tests/test_erd.py @@ -1,5 +1,5 @@ import datajoint as dj -from .schema_simple import LOCALS_SIMPLE, A, B, D, E, L, OutfitLaunch +from .schema_simple import LOCALS_SIMPLE, A, B, D, E, G, L, OutfitLaunch from .schema_advanced import * @@ -20,7 +20,7 @@ def test_dependencies(schema_simp): assert set(D().parents(primary=True)) == set([A.full_table_name]) assert set(D().parents(primary=False)) == set([L.full_table_name]) assert set(deps.descendants(L.full_table_name)).issubset( - cls.full_table_name for cls in (L, D, E, E.F) + cls.full_table_name for cls in (L, D, E, E.F, E.G, E.H, E.M, G) ) @@ -38,10 +38,14 @@ def test_erd_algebra(schema_simp): erd3 = erd1 * erd2 erd4 = (erd0 + E).add_parts() - B - E assert erd0.nodes_to_show == set(cls.full_table_name for cls in [B]) - assert erd1.nodes_to_show == set(cls.full_table_name for cls in (B, B.C, E, E.F)) + assert erd1.nodes_to_show == set( + cls.full_table_name for cls in (B, B.C, E, E.F, E.G, E.H, E.M, G) + ) assert erd2.nodes_to_show == set(cls.full_table_name for cls in (A, B, D, E, L)) assert erd3.nodes_to_show == set(cls.full_table_name for cls in (B, E)) - assert erd4.nodes_to_show == set(cls.full_table_name for cls in (B.C, E.F)) + assert erd4.nodes_to_show == set( + cls.full_table_name for cls in (B.C, E.F, E.G, E.H, E.M) + ) def test_repr_svg(schema_adv): diff --git a/tests/test_schema.py b/tests/test_schema.py index d9e220892..6407cacab 100644 --- a/tests/test_schema.py +++ b/tests/test_schema.py @@ -184,7 +184,7 @@ def test_list_tables(schema_simp): """ https://github.com/datajoint/datajoint-python/issues/838 """ - assert set( + expected = set( [ "reserved_word", "#l", @@ -194,6 +194,10 @@ def test_list_tables(schema_simp): "__b__c", "__e", "__e__f", + "__e__g", + "__e__h", + "__e__m", + "__g", "#outfit_launch", "#outfit_launch__outfit_piece", "#i_j", @@ -207,7 +211,9 @@ def test_list_tables(schema_simp): "profile", "profile__website", ] - ) == set(schema_simp.list_tables()) + ) + actual = set(schema_simp.list_tables()) + assert actual == expected, f"Missing from list_tables(): {expected - actual}" def test_schema_save_any(schema_any):