From 9d1b063cffa96ed183e2df05124b22a9741ed092 Mon Sep 17 00:00:00 2001 From: Charles Leifer Date: Tue, 12 Nov 2024 10:49:37 -0600 Subject: [PATCH] Fix regression in delete_instance() with nullable foreign-keys. Refs #2952 --- peewee.py | 11 ++++++++--- tests/regressions.py | 43 +++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 49 insertions(+), 5 deletions(-) diff --git a/peewee.py b/peewee.py index ef6bae103..4c35ac445 100644 --- a/peewee.py +++ b/peewee.py @@ -6967,7 +6967,7 @@ def is_dirty(self): def dirty_fields(self): return [f for f in self._meta.sorted_fields if f.name in self._dirty] - def dependencies(self, search_nullable=True): + def dependencies(self, search_nullable=True, exclude_null_children=False): model_class = type(self) stack = [(type(self), None)] queries = {} @@ -6987,7 +6987,12 @@ def dependencies(self, search_nullable=True): .where(node)) if not fk.null or search_nullable: queries.setdefault(rel_model, []).append((node, fk)) - stack.append((rel_model, subquery)) + if fk.null and exclude_null_children: + # Do not process additional children of this node, but + # include it in the list of dependencies. + seen.add(rel_model) + else: + stack.append((rel_model, subquery)) for m in reversed(sort_models(seen)): for sq, q in queries.get(m, ()): @@ -6995,7 +7000,7 @@ def dependencies(self, search_nullable=True): def delete_instance(self, recursive=False, delete_nullable=False): if recursive: - for query, fk in self.dependencies(): + for query, fk in self.dependencies(exclude_null_children=not delete_nullable): model = fk.model if fk.null and not delete_nullable: model.update(**{fk.name: None}).where(query).execute() diff --git a/tests/regressions.py b/tests/regressions.py index c066f58d4..5372a7baa 100644 --- a/tests/regressions.py +++ b/tests/regressions.py @@ -1849,9 +1849,47 @@ class O(TestModel): class OX(TestModel): o = ForeignKeyField(O, null=True) +class Character(TestModel): + name = TextField() +class Shape(TestModel): + character = ForeignKeyField(Character, null=True) +class ShapeDetail(TestModel): + shape = ForeignKeyField(Shape) + class TestDeleteInstanceDFS(ModelTestCase): - requires = [I, S, P, PS, PP, O, OX] + @requires_models(Character, Shape, ShapeDetail) + def test_delete_instance_dfs_nullable(self): + c1, c2 = [Character.create(name=name) for name in ('c1', 'c2')] + for c in (c1, c2): + s = Shape.create(character=c) + ShapeDetail.create(shape=s) + + # Update nullables. + with self.assertQueryCount(2): + c1.delete_instance(True) + + self.assertHistory(2, [ + ('UPDATE "shape" SET "character_id" = ? WHERE ' + '("shape"."character_id" = ?)', [None, c1.id]), + ('DELETE FROM "character" WHERE ("character"."id" = ?)', [c1.id])]) + + self.assertEqual(Shape.select().count(), 2) + + # Delete nullables as well. + with self.assertQueryCount(3): + c2.delete_instance(True, True) + + self.assertHistory(3, [ + ('DELETE FROM "shape_detail" WHERE ' + '("shape_detail"."shape_id" IN ' + '(SELECT "t1"."id" FROM "shape" AS "t1" WHERE ' + '("t1"."character_id" = ?)))', [c2.id]), + ('DELETE FROM "shape" WHERE ("shape"."character_id" = ?)', [c2.id]), + ('DELETE FROM "character" WHERE ("character"."id" = ?)', [c2.id])]) + + self.assertEqual(Shape.select().count(), 1) + @requires_models(I, S, P, PS, PP, O, OX) def test_delete_instance_dfs(self): i1, i2 = [I.create(name=n) for n in ('i1', 'i2')] for i in (i1, i2): @@ -1893,6 +1931,7 @@ def test_delete_instance_dfs(self): ('DELETE FROM "i" WHERE ("i"."id" = ?)', [i1.id]), ]) + models = [I, S, P, PS, PP, O, OX] counts = {OX: 2} - for m in self.requires: + for m in models: self.assertEqual(m.select().count(), counts.get(m, 1))