Inconsistent behavior when updating foreign key

[pkyosx]

Suppose we have Parent and Child classes as One-To-Many relationship. If I move child from one parent to another, the order of changing foreign_key and foreign_object result in different behavior. Following are the briefing of reproducing steps. Attachments are runnable reproducing scripts. It still happens in 1.1.0b1.dev0 build.

// Models
class Parent(Base):
    __tablename__ = 'parent'
    id = Column(Integer, primary_key=True)
    children = relationship("Child", back_populates="parent")

class Child(Base):
    __tablename__ = 'child'
    id = Column(Integer, primary_key=True)
    parent_id = Column(Integer, ForeignKey('parent.id'))
    parent = relationship("Parent", back_populates="children")

// Initial Setting
p1 = Parent()
p2 = Parent()
c = Child()
c.parent = p1
s.commit()

// Scenario1: Move c from p1 to p2
c.parent_id = p2.id
c.parent = p2
assert c in p2.children, "this will fail"

// Scenario 2: Move c from p1 to p2
c.parent = p2
c.parent_id = p2.id
assert c in p2.children, "this will success"

[Mike Bayer]

well there is a known reason this happens and there are even tests that assert it happens, although they don't insist this is what it should do. In general, it's advised that you either work with relationship-bound attributes and not the foreign key attributes, letting the ORM handle the latter, or you work with foreign key attributes and then it's expected that you flush() and expire() your Session as needed in order to have your relationships reflect those changes.

So in this case, there is a very specific thread-the-needle thing going on where it looks like you don't actually need to flush/expire to see the change, but you would. However, it's possibly not reasonable that this is expected.

Basically, even though backrefs work in memory without using the database:

some_b.a = some_a
assert some_b in some_a.bs   # no DB needed

that event wont do anything if "some_b.a" already points to some_a. Otherwise you'd get duplicate entries in some_a.bs.

in your test, the part where it checks if "c.parent" is already "p2" is emitting a lazyload for the "old" value of "c.parent". This lazyload is a many-to-one so it ends up loading up either p1 or p2 from the local identity map. However, it's using the current value of c.parent_id, not the database-committed value of c.parent_id. That is, because you're setting c.parent_id to something that the ORM expects to be doing itself, the "c.parent" operation is seeing the wrong history.

That's a bug and while I thought I must have seen this one before I think it's possible that I have not. The fix breaks those tests that are currently testing that this behavior does occur when the "load_on_pending" and "enable_relationship_loading" features are used, but those shouldn't make a difference here either. We'll have to see if this change breaks anything. https://gerrit.sqlalchemy.org/72

[pkyosx]

Thanks for the prompt response and clear explanation. In fact, I was helping an old project to migrate its sqlalchemy from 0.5.8 to the latest version. The migration breaks some of the test cases. If you have ever seen this one, it would probably be a long time ago.

[Mike Bayer]

Use the "committed" values when extracting many-to-one lazyload value

The scalar object set() method calls upon the lazy loader to get at the "old" value of the attriute, however doesn't ensure that the "committed" value of the foreign key attributes is used. If the user has manipulated these attributes and they themselves have pending, non committed changes, we get the "new" value which these attributes would have set up if they were flushed. "old" vs "new" value is always about how the value has changed since the load, so we always have to use the DB-persisted values for everything when looking for "old".

Change-Id: I82bdc40ad0cf033c3a98f3361776cf3161542cd6 Fixes: #3708

→ <<cset c99fc44e170b>>