- edited description
delete/insert into update doesn't handle relationship changes
New object with the same ID as existing one implicitly merged via cascade rules has old value of relationship property after the commit when the new value is None.
In short:
doc.doc_links = [DocLink(id=doc_link_id, link=None)]
session.commit()
assert doc.doc_links[0].link is None # fails
Full test is attached (test3 is the test case that fails).
Comments (18)
-
reporter -
repo owner okey doke. well first thing to note, that "merge" is less than ideal. the test passes if we apply #2501 and then map:
class DocLink(Base): __tablename__ = 'doc_link' id = Column(Integer, primary_key=True, autoincrement=True) doc_id = Column(Integer, ForeignKey(Doc.id), nullable=False) doc = relationship(Doc) link_id = Column(Integer, ForeignKey(Link.id)) link = relationship(Link) __mapper_args__ = {"maintain_unique_attributes": [("id", )]}
the output is then:
DELETE FROM doc_link WHERE doc_link.id = ? (1,) INSERT INTO doc_link (id, doc_id, link_id) VALUES (?, ?, ?)
the maintain_unique_attributes feature introduces other caveats as far as the order of things so it's unlikely at this point it can be made a default for primary keys. it would be a good idea to use in cases like these where you expect rows to be switching on the same primary and/or unique keys.
as far as the delete->insert->update taking into account simultaneous changes in relationships, not really sure on that yet.
-
repo owner - changed title to delete/insert into update doesn't handle relationship changes
-
repo owner - changed component to orm
- attached 3060.patch
patch is attached. 0.9 OK ? (or even 1.0...) tinkering with UOW mechanics not something I do lightly.
-
repo owner - marked as critical
-
reporter Works fine for out case. Thank you for quick response!
-
repo owner are you using 0.9 and do you have any workarounds in place for this issue or is it a blocker?
-
reporter We are using 0.9 and we have a workaround.
-
repo owner - changed status to resolved
- Fixed a few edge cases which arise in the so-called "row switch"
scenario, where an INSERT/DELETE can be turned into an UPDATE.
In this situation, a many-to-one relationship set to None, or
in some cases a scalar attribute set to None, may not be detected
as a net change in value, and therefore the UPDATE would not reset
what was on the previous row. This is due to some as-yet
unresovled side effects of the way attribute history works in terms
of implicitly assuming None isn't really a "change" for a previously
un-set attribute. See also
3061
. fixes#3060
→ <<cset bc08ee902925>>
-
repo owner - Fixed a few edge cases which arise in the so-called "row switch"
scenario, where an INSERT/DELETE can be turned into an UPDATE.
In this situation, a many-to-one relationship set to None, or
in some cases a scalar attribute set to None, may not be detected
as a net change in value, and therefore the UPDATE would not reset
what was on the previous row. This is due to some as-yet
unresovled side effects of the way attribute history works in terms
of implicitly assuming None isn't really a "change" for a previously
un-set attribute. See also
3061
. fixes#3060
→ <<cset 16b7f1816d54>>
- Fixed a few edge cases which arise in the so-called "row switch"
scenario, where an INSERT/DELETE can be turned into an UPDATE.
In this situation, a many-to-one relationship set to None, or
in some cases a scalar attribute set to None, may not be detected
as a net change in value, and therefore the UPDATE would not reset
what was on the previous row. This is due to some as-yet
unresovled side effects of the way attribute history works in terms
of implicitly assuming None isn't really a "change" for a previously
un-set attribute. See also
-
repo owner OK this led pretty deep down a rabbit hole of a different variety and I might look into some other changes for 1.0.
-
repo owner - changed status to open
okey doke, well this issue just broke all of openstack. so let's look into it.
-
repo owner here's the test:
diff --git a/test/orm/test_unitofworkv2.py b/test/orm/test_unitofworkv2.py index a76f928..25fd11d 100644 --- a/test/orm/test_unitofworkv2.py +++ b/test/orm/test_unitofworkv2.py @@ -1387,6 +1387,19 @@ class RowswitchM2OTest(fixtures.MappedTest): sess.commit() assert a1.bs[0].data is None + def test_dont_blow_away_partial_relationship(self): + A, B, C = self._fixture() + sess = Session() + + c1 = C() + sess.add(c1) + sess.flush() + + b1 = B(cid=c1.id) + assert b1.c is None + sess.add(b1) + sess.flush() + assert b1.cid == c1.id class BasicStaleChecksTest(fixtures.MappedTest):
not sure what I'm doing yet. this is a serious regression and i might have to revert this whole thing and re-release. not sure.
-
repo owner ah yes. this is fixed differently in 1.0. that might be the ticket.
-
repo owner OK that test is not quite it, in 1.0 we are expecting "b1.c" accessor which sets up None to actually generate an event. what's happening in openstack is this:
def test_joinedload_doesnt_produce_bogus_event(self): A, B, C = self._fixture() sess = Session() c1 = C() sess.add(c1) sess.flush() b1 = B() sess.add(b1) sess.commit() # test that was broken by #3060 from sqlalchemy.orm import joinedload b1 = sess.query(B).options(joinedload("c")).first() b1.cid = c1.id sess.flush() assert b1.cid == c1.id
-
repo owner in openstack, the joinedload in question is nova/db/sqlalchemy/api.py -> _floating_ip_get_by_address, there's a joinedload() in there of fixed_ip.instance. The API there is otherwise manipulating FloatingIP/FixedIP using the foreign key values directly.
-
repo owner - changed status to resolved
so this is so this is 080ae2f15f8d74b225ed
-
repo owner Issue
#3209was marked as a duplicate of this issue. - Log in to comment