From 89afc8ab6b0d88b38e2e3a9f20e4ccdb67ee997a Mon Sep 17 00:00:00 2001 From: Dimitri Yatsenko Date: Sat, 17 Oct 2026 17:07:14 -0500 Subject: [PATCH 1/3] fix(cascade): seed a delete restricted by another query with its keys A delete restricted by another query, such as `(Subject & (Session & "quality = 0")).delete()`, failed. The cascade applies the seed's compiled conditions again to a fresh copy of the table through `restrict()`, which collects column names from the whole condition text, subquery included, and rejects every name the seed table lacks. The delete raised `Attribute ... is not found in query` before deleting anything. On MySQL the backquoted schema name in every subquery tripped the same check, so even `(Subject & Session).delete()` raised. On PostgreSQL, a seed whose subquery names only the seed's columns got past the check but still did nothing. The cascade deletes children before parents, so by the time it reached the seed, the rows its subquery reads were gone. `(Subject & Session).delete()` matched no subject, returned 0 and rolled back. A seed whose conditions contain a subquery is now replaced by the primary keys it selects, as a `dj.Top` seed already is. The traversal and the delete then work from the rows the restriction selected when the delete started. The new tests delete subjects restricted by a query on their sessions, by its negation, and by the session table itself. They check the preview counts and the rows left afterwards, on both backends. All six cases fail on the previous code. --- src/datajoint/diagram.py | 26 +++++++++--- tests/integration/test_cascade_overdelete.py | 44 ++++++++++++++++++++ 2 files changed, 65 insertions(+), 5 deletions(-) diff --git a/src/datajoint/diagram.py b/src/datajoint/diagram.py index 96f4145db..ff35f8c51 100644 --- a/src/datajoint/diagram.py +++ b/src/datajoint/diagram.py @@ -20,6 +20,7 @@ import inspect import io import logging +import re import networkx as nx @@ -154,16 +155,31 @@ def stroke_rule(light_hex, dark_hex, opacity=None): return "" +def _has_subquery(condition): + """Whether a compiled restriction condition contains a subquery.""" + if not isinstance(condition, str): + return False + text = re.sub(r"(\\\")|(\\\')", "", condition) # escaped quotes + text = re.sub(r"'[^']*'|\"[^\"]*\"", "", text) # quoted text and identifiers + return re.search(r"\bselect\b", text, re.IGNORECASE) is not None + + def _seed_restriction(table_expr): """ Return a traversal seed's restriction and the attributes it names. - A ``dj.Top`` restriction is an ORDER BY ... LIMIT, not part of the WHERE - clause, so a seed restricted by ``Top`` is replaced by the primary keys - it selects. Otherwise the traversal would start from every row that - matches the seed's other conditions. + Two kinds of seed are replaced by the primary keys they select: + + - A ``dj.Top`` restriction is an ORDER BY ... LIMIT, not part of the WHERE + clause. Otherwise the traversal would start from every row that matches + the seed's other conditions. + - A restriction by another query, such as ``Subject & (Session & "quality = 0")``, + compiles to a subquery. The traversal applies the seed's conditions again + to a fresh copy of the table, which rejects a subquery that names other + tables' columns. A delete would also remove the rows the subquery reads, + when they are downstream, before it deletes the seed itself. """ - if table_expr._top is not None: + if table_expr._top is not None or any(_has_subquery(c) for c in table_expr.restriction): return AndList([table_expr.keys()]), set(table_expr.primary_key) return AndList(table_expr.restriction), set(table_expr.restriction_attributes) diff --git a/tests/integration/test_cascade_overdelete.py b/tests/integration/test_cascade_overdelete.py index 516b9e849..35e3e6bcc 100644 --- a/tests/integration/test_cascade_overdelete.py +++ b/tests/integration/test_cascade_overdelete.py @@ -246,3 +246,47 @@ class Downstream(dj.Manual): assert Master.to_arrays("master_id").tolist() == [2] assert len(Master.P()) == 0 assert Downstream.to_arrays("master_id").tolist() == [2] + + +SUBQUERY_SEEDS = { + # seed -> (subjects deleted, sessions deleted) + "restricted by a query": (lambda S, T: S & (T & "quality = 0"), {1, 3}, {(1, 1), (1, 2), (3, 1)}), + "anti-restricted by a query": (lambda S, T: S - (T & "quality = 0"), {2, 4}, {(2, 1)}), + "restricted by a table": (lambda S, T: S & T, {1, 2, 3}, {(1, 1), (1, 2), (2, 1), (3, 1)}), +} + + +@pytest.mark.parametrize("seed_kind", list(SUBQUERY_SEEDS)) +def test_seed_restricted_by_a_downstream_query(schema_by_backend, seed_kind): + """A seed restricted by a query on a downstream table deletes the rows that + query selected when the delete started, with all their descendants.""" + + @schema_by_backend + class Subject(dj.Manual): + definition = """ + subject_id : int32 + """ + + @schema_by_backend + class Session(dj.Manual): + definition = """ + -> Subject + session_id : int32 + --- + quality : int32 + """ + + Subject.insert([(1,), (2,), (3,), (4,)]) + Session.insert([(1, 1, 1), (1, 2, 0), (2, 1, 1), (3, 1, 0)]) + all_sessions = {(1, 1), (1, 2), (2, 1), (3, 1)} + + make_seed, subjects, sessions = SUBQUERY_SEEDS[seed_kind] + seed = make_seed(Subject, Session) + + counts = dj.Diagram.cascade(seed).counts() + assert counts[Subject.full_table_name] == len(subjects) + assert counts[Session.full_table_name] == len(sessions) + + seed.delete(prompt=False) + assert set(Subject.to_arrays("subject_id").tolist()) == {1, 2, 3, 4} - subjects + assert set(zip(*Session.to_arrays("subject_id", "session_id"))) == all_sessions - sessions From 05335d1a2e9c12c49145dfaa7d5994670ea8556d Mon Sep 17 00:00:00 2001 From: Dimitri Yatsenko Date: Sat, 17 Oct 2026 19:31:32 -0500 Subject: [PATCH 2/3] fix(cascade): keep a % in a delete's restriction A delete restricted by a condition containing %, such as `(Label & "name = '50%'").delete()` or a dict `{"name": "50%"}`, deleted nothing and returned 0. The seed's conditions are compiled SQL, in which each % is already escaped as %% for the database driver. The cascade applies them again through `restrict()`, which escaped every % a second time, so the condition looked for the literal text `50%%` and matched no row. A LIKE pattern kept working only because `%%` and `%` match the same strings there. The seed's conditions now have that first escape undone before the cascade applies them again, so they select the same rows as the restriction on its own. The new test checks the preview counts for a string condition, a dict and a condition holding `%%`, and deletes with a LIKE pattern, on both backends. It fails before this change. --- src/datajoint/diagram.py | 6 +++- tests/integration/test_cascade_overdelete.py | 35 ++++++++++++++++++++ 2 files changed, 40 insertions(+), 1 deletion(-) diff --git a/src/datajoint/diagram.py b/src/datajoint/diagram.py index ff35f8c51..12e59eac5 100644 --- a/src/datajoint/diagram.py +++ b/src/datajoint/diagram.py @@ -181,7 +181,11 @@ def _seed_restriction(table_expr): """ if table_expr._top is not None or any(_has_subquery(c) for c in table_expr.restriction): return AndList([table_expr.keys()]), set(table_expr.primary_key) - return AndList(table_expr.restriction), set(table_expr.restriction_attributes) + # The seed's conditions are compiled SQL, with each % already escaped as %% + # for the driver. The traversal applies them again through restrict(), which + # escapes again, so the escape is undone here. + restriction = AndList(c.replace("%%", "%") if isinstance(c, str) else c for c in table_expr.restriction) + return restriction, set(table_expr.restriction_attributes) class Diagram(nx.MultiDiGraph): # noqa: C901 diff --git a/tests/integration/test_cascade_overdelete.py b/tests/integration/test_cascade_overdelete.py index 35e3e6bcc..632443507 100644 --- a/tests/integration/test_cascade_overdelete.py +++ b/tests/integration/test_cascade_overdelete.py @@ -290,3 +290,38 @@ class Session(dj.Manual): seed.delete(prompt=False) assert set(Subject.to_arrays("subject_id").tolist()) == {1, 2, 3, 4} - subjects assert set(zip(*Session.to_arrays("subject_id", "session_id"))) == all_sessions - sessions + + +def test_seed_condition_with_a_percent_sign(schema_by_backend): + """A seed condition containing % selects the same rows in the cascade as + on its own.""" + + @schema_by_backend + class Label(dj.Manual): + definition = """ + label_id : int32 + --- + name : varchar(16) + """ + + @schema_by_backend + class Use(dj.Manual): + definition = """ + -> Label + use_id : int32 + """ + + Label.insert([(1, "50%"), (2, "other"), (3, "5%%")]) + Use.insert([(1, 1), (2, 1), (3, 1)]) + + for seed, label in [ + (Label & "name = '50%'", 1), + (Label & {"name": "50%"}, 1), + (Label & "name = '5%%'", 3), + ]: + assert len(seed) == 1 + assert dj.Diagram.cascade(seed).counts()[Use.full_table_name] == 1 + + (Label & "name LIKE '50%'").delete(prompt=False) + assert sorted(Label.to_arrays("label_id").tolist()) == [2, 3] + assert sorted(Use.to_arrays("label_id").tolist()) == [2, 3] From 94d11055f0e2eaf418932ca01cfacab360ece1da Mon Sep 17 00:00:00 2001 From: Dimitri Yatsenko Date: Tue, 20 Oct 2026 18:48:48 -0500 Subject: [PATCH 3/3] fix(cascade): ignore backquoted identifiers in the subquery check; assert the matched row Review follow-up. The subquery check strips quoted text before looking for SELECT; it now strips MySQL's backquoted identifiers too, as it already did PostgreSQL's double-quoted ones, so a column named `select` no longer makes a seed look like a subquery (that case was safe, since the seed's keys select the same rows). The percent-sign test now checks which row each seed selects, not only how many. The seed helper's docstring notes that replacing a seed by its keys reads them all into one condition. --- src/datajoint/diagram.py | 6 +++++- tests/integration/test_cascade_overdelete.py | 2 +- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/datajoint/diagram.py b/src/datajoint/diagram.py index 12e59eac5..cd30e1e0a 100644 --- a/src/datajoint/diagram.py +++ b/src/datajoint/diagram.py @@ -160,7 +160,7 @@ def _has_subquery(condition): if not isinstance(condition, str): return False text = re.sub(r"(\\\")|(\\\')", "", condition) # escaped quotes - text = re.sub(r"'[^']*'|\"[^\"]*\"", "", text) # quoted text and identifiers + text = re.sub(r"'[^']*'|\"[^\"]*\"|`[^`]*`", "", text) # quoted text and identifiers return re.search(r"\bselect\b", text, re.IGNORECASE) is not None @@ -178,6 +178,10 @@ def _seed_restriction(table_expr): to a fresh copy of the table, which rejects a subquery that names other tables' columns. A delete would also remove the rows the subquery reads, when they are downstream, before it deletes the seed itself. + + Replacing a seed by its keys reads every selected primary key into Python + and sends them back as one condition, so a very large seed makes a large + query; restrict by the keys in batches if that becomes a problem. """ if table_expr._top is not None or any(_has_subquery(c) for c in table_expr.restriction): return AndList([table_expr.keys()]), set(table_expr.primary_key) diff --git a/tests/integration/test_cascade_overdelete.py b/tests/integration/test_cascade_overdelete.py index 632443507..e63ce1d6d 100644 --- a/tests/integration/test_cascade_overdelete.py +++ b/tests/integration/test_cascade_overdelete.py @@ -319,7 +319,7 @@ class Use(dj.Manual): (Label & {"name": "50%"}, 1), (Label & "name = '5%%'", 3), ]: - assert len(seed) == 1 + assert seed.to_arrays("label_id").tolist() == [label] assert dj.Diagram.cascade(seed).counts()[Use.full_table_name] == 1 (Label & "name LIKE '50%'").delete(prompt=False)