Visitar URL original
order post_update FK clear after child save, before parent delete · sqlalchemy/sqlalchemy@4e974a0 · GitHub
Skip to content

Commit 4e974a0

Browse files
zzzeekzzzeek
committed
order post_update FK clear after child save, before parent delete
Fixed issue where deleting the parent object of a one-to-many relationship configured with relationship.post_update, such as the one-to-many side of a bidirectional post_update relationship, would emit the UPDATE that sets the foreign key of the remaining related objects to NULL twice: once within the regular UPDATE for those objects and again as the post-update. Only the post-update UPDATE is now emitted, and it is explicitly ordered to run before the DELETE of the parent row. Fix approach courtesy 18Abhinav07, with thanks to canblmz1 for the initial analysis. Fixes: #10936 Change-Id: I83b2cc61559f05a37372e2b7610b861800be0c15
1 parent cd2eccb commit 4e974a0

3 files changed

Lines changed: 165 additions & 0 deletions

File tree

‎doc/build/changelog/unreleased_20/10936.rst‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
.. change::
2+
:tags: bug, orm
3+
:tickets: 10936
4+
:versions: 2.1.9
5+
6+
Fixed issue where deleting the parent object of a one-to-many
7+
relationship configured with :paramref:`_orm.relationship.post_update`,
8+
such as the one-to-many side of a bidirectional ``post_update``
9+
relationship, would emit the UPDATE that sets the foreign key of the
10+
remaining related objects to NULL twice: once within the regular UPDATE
11+
for those objects and again as the post-update. Only the post-update
12+
UPDATE is now emitted, and it is explicitly ordered to run before the
13+
DELETE of the parent row. Fix approach courtesy 18Abhinav07, with
14+
thanks to canblmz1 for the initial analysis.

‎lib/sqlalchemy/orm/dependency.py‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -357,6 +357,9 @@ def per_property_dependencies(
357357
(child_saves, after_save),
358358
(parent_saves, after_save),
359359
(after_save, child_post_updates),
360+
(child_saves, before_delete),
361+
(before_delete, child_post_updates),
362+
(child_post_updates, parent_deletes),
360363
(before_delete, child_pre_updates),
361364
(child_pre_updates, parent_deletes),
362365
(child_pre_updates, child_deletes),

‎test/orm/test_cycles.py‎

Lines changed: 148 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1250,6 +1250,104 @@ def test_post_update_m2o_detect_none(self):
12501250

12511251
is_(b1.person, None)
12521252

1253+
def _bidirectional_post_update_fixture(self):
1254+
person, ball, Ball, Person = (
1255+
self.tables.person,
1256+
self.tables.ball,
1257+
self.classes.Ball,
1258+
self.classes.Person,
1259+
)
1260+
1261+
self.mapper_registry.map_imperatively(Ball, ball)
1262+
self.mapper_registry.map_imperatively(
1263+
Person,
1264+
person,
1265+
properties=dict(
1266+
favorite=relationship(
1267+
Ball,
1268+
primaryjoin=person.c.favorite_ball_id == ball.c.id,
1269+
post_update=True,
1270+
backref="favorites",
1271+
_legacy_inactive_history_style=(
1272+
self._legacy_inactive_history_style
1273+
),
1274+
)
1275+
),
1276+
)
1277+
b = Ball(data="some data")
1278+
p1 = Person(data="p1")
1279+
p2 = Person(data="p2")
1280+
p1.favorite = b
1281+
p2.favorite = b
1282+
sess = fixture_session()
1283+
sess.add_all([b, p1, p2])
1284+
sess.flush()
1285+
return sess, b, p1, p2
1286+
1287+
def test_no_double_update_on_delete_bidirectional_post_update(self):
1288+
"""test #10936"""
1289+
1290+
sess, b, p1, p2 = self._bidirectional_post_update_fixture()
1291+
1292+
sess.delete(b)
1293+
1294+
self.assert_sql_execution(
1295+
testing.db,
1296+
sess.flush,
1297+
CompiledSQL(
1298+
"UPDATE person SET favorite_ball_id=:favorite_ball_id "
1299+
"WHERE person.id = :person_id",
1300+
lambda ctx: [
1301+
{"favorite_ball_id": None, "person_id": p1.id},
1302+
{"favorite_ball_id": None, "person_id": p2.id},
1303+
],
1304+
),
1305+
CompiledSQL(
1306+
"DELETE FROM ball WHERE ball.id = :id",
1307+
lambda ctx: {"id": b.id},
1308+
),
1309+
)
1310+
1311+
def test_no_double_update_on_delete_bidirectional_post_update_dirty(
1312+
self,
1313+
):
1314+
"""test #10936 where a child has other changes; the regular UPDATE
1315+
should not include the post_update column"""
1316+
1317+
Person = self.classes.Person
1318+
1319+
sess, b, p1, p2 = self._bidirectional_post_update_fixture()
1320+
1321+
p1.data = "p1 new"
1322+
sess.delete(b)
1323+
1324+
self.assert_sql_execution(
1325+
testing.db,
1326+
sess.flush,
1327+
CompiledSQL(
1328+
"UPDATE person SET data=:data WHERE person.id = :person_id",
1329+
lambda ctx: {"data": "p1 new", "person_id": p1.id},
1330+
),
1331+
CompiledSQL(
1332+
"UPDATE person SET favorite_ball_id=:favorite_ball_id "
1333+
"WHERE person.id = :person_id",
1334+
lambda ctx: [
1335+
{"favorite_ball_id": None, "person_id": p1.id},
1336+
{"favorite_ball_id": None, "person_id": p2.id},
1337+
],
1338+
),
1339+
CompiledSQL(
1340+
"DELETE FROM ball WHERE ball.id = :id",
1341+
lambda ctx: {"id": b.id},
1342+
),
1343+
)
1344+
eq_(
1345+
sess.query(Person.data, Person.favorite_ball_id)
1346+
.order_by(Person.id)
1347+
.all(),
1348+
[("p1 new", None), ("p2", None)],
1349+
)
1350+
12531351

12541352
class SelfReferentialPostUpdateTest(fixtures.MappedTest):
12551353
"""Post_update on a single self-referential mapper."""
@@ -1432,6 +1530,56 @@ def remove_child(parent, child):
14321530
cats.prev_sibling = None
14331531
session.flush()
14341532

1533+
def test_o2m_post_update_delete_parent(self):
1534+
"""test #10936 for a self-referential one-to-many; the children are
1535+
updated once, before the parent is deleted"""
1536+
1537+
node, Node = self.tables.node, self.classes.Node
1538+
1539+
self.mapper_registry.map_imperatively(
1540+
Node,
1541+
node,
1542+
properties={
1543+
"children": relationship(
1544+
Node,
1545+
primaryjoin=node.c.id == node.c.parent_id,
1546+
post_update=True,
1547+
),
1548+
},
1549+
)
1550+
1551+
session = fixture_session()
1552+
root = Node("root")
1553+
c1 = Node("c1")
1554+
c2 = Node("c2")
1555+
root.children = [c1, c2]
1556+
session.add(root)
1557+
session.flush()
1558+
1559+
c2.path = "c2 new"
1560+
session.delete(root)
1561+
1562+
self.assert_sql_execution(
1563+
testing.db,
1564+
session.flush,
1565+
CompiledSQL(
1566+
"UPDATE node SET path=:path WHERE node.id = :node_id",
1567+
lambda ctx: {"path": "c2 new", "node_id": c2.id},
1568+
),
1569+
CompiledSQL(
1570+
"UPDATE node SET parent_id=:parent_id "
1571+
"WHERE node.id = :node_id",
1572+
lambda ctx: [
1573+
{"parent_id": None, "node_id": c1.id},
1574+
{"parent_id": None, "node_id": c2.id},
1575+
],
1576+
),
1577+
CompiledSQL(
1578+
"DELETE FROM node WHERE node.id = :id",
1579+
lambda ctx: {"id": root.id},
1580+
),
1581+
)
1582+
14351583

14361584
class SelfReferentialPostUpdateTest2(fixtures.MappedTest):
14371585
@classmethod

0 commit comments

Comments
 (0)