delete/insert into update doesn't handle relationship changes

[Denis Otkidach]

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).

[Mike Bayer]

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.

[Mike Bayer]

patch is attached. 0.9 OK ? (or even 1.0...) tinkering with UOW mechanics not something I do lightly.

[Denis Otkidach]

Works fine for out case. Thank you for quick response!

[Mike Bayer]

are you using 0.9 and do you have any workarounds in place for this issue or is it a blocker?

[Denis Otkidach]

We are using 0.9 and we have a workaround.

[Mike Bayer]

• 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>>

[Mike Bayer]

• 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>>

[Mike Bayer]

OK this led pretty deep down a rabbit hole of a different variety and I might look into some other changes for 1.0.

[Mike Bayer]

okey doke, well this issue just broke all of openstack. so let's look into it.

[Mike Bayer]

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.

[Mike Bayer]

ah yes. this is fixed differently in 1.0. that might be the ticket.

[Mike Bayer]

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

[Mike Bayer]

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.

[Mike Bayer]

so this is so this is 080ae2f15f8d74b225ed

[Mike Bayer]

Issue #3209 was marked as a duplicate of this issue.