0.6: Extension mapper no longer fires for children in relation

[Former user]

There's a change in behavior between 0.5.8 and 0.6 which breaks my application. I have ShoppingList and ShoppingListItems tables, each of which uses a MapperExtension to automatically increment a version field.

In 0.6, if I update a ShoppingListItem, its version no longer gets automatically incremented. A repro is attached.

[Former user]

Repro case

[Mike Bayer]

Your test program relies upon a bug in SQLA 0.5's merge() method and makes a mistake with regards to session state, which this bug concealed. It has nothing to do with mapper extensions.

First, when tracking down any SQLA behavior we need to turn on SQL echoing. This is the best way to see what's going on:

engine = create_engine('sqlite:///:memory:', echo=True)

Then, let's look at the test. Add these assertions to update_shopping_item():

def update_shopping_item(session, updated_item):
    existing_list = get_shopping_list(session, updated_item.shopping_list_id)
    for i, item in enumerate(existing_list.children):
        if item.item_id == updated_item.item_id:
            existing_list.children[i](i) = updated_item
            break
    assert updated_item in session
    new = session.merge(updated_item)
    assert new is updated_item
    return new

Above, we illustrate that the call to session.merge(updated_item) is entirely superfluous - merge() is handed an object that is already in the session, and the identical instance is returned. By attaching updated_item to a collection within the session, the object is added based on the default cascade rules on children. updated_item has already been placed in the session via add().

So in theory, nothing should be accomplished by calling merge().

However, there are two important things about this operation. The first is that merge() triggers autoflush before doing anything. As you watch the SQL go by, you can note that everything has been flushed by the time merge() begins to do its real work, including all the state of updated_item.

updated_item got persisted to the database via an UPDATE. But from an ORM perspective, the state change is that orig_item has been deleted (via delete-orphan) and updated_item has been added (via collection assignment). It's not an update - its a delete + insert of the same primary key. SQLAlchemy turns this into a SQL UPDATE at the last minute, but from a mapper extension perspective, its an insert. No version id is updated. This is the correct behavior and is why the program does not work correctly (on either 0.5 or 0.6).

The second important thing is how merge() conceals the issue in 0.5. merge() in 0.5 has a usually harmless bug whereby it doesn't recognize that it's been handed exactly the object that it would be returning anyway, and goes along copying all the state of updated_item onto itself. This is a pointless and wasteful operation that has been fixed in 0.6. However, it also has the effect here of tripping the "dirty" flag on updated_item - this because any attribute set operation does this, even if there's no net change in the values of the attributes. Below, asserting that updated_item is in the dirty list after the create+add+pointless-merge operation will only pass on 0.5:

updated_item = update_shopping_item(session, updated_item)

# only on 0.5 !  
assert updated_item in session.dirty

The fact that there's no actual net change on the attributes updated_item would then be detected during the flush operation, and no UPDATE statement would occur.

Except ! You have a mapper extension that is receiving the net-unchanged pointlessly-dirty-marked updated_item and changing the value of the version attribute. So then there is an UPDATE - a second UPDATE distinct from the first one. You can see this in the SQL - this operation makes no sense:

UPDATE shopping_list_items SET version=? WHERE shopping_list_items.shopping_list_id = ? AND shopping_list_items.item_id = ?

why is an UPDATE occuring just of version ? Where's the other attributes changed on the row ? There are none. They were already flushed.

The extension makes the mistake of assuming that just because before_update is called, an UPDATE will actually occur. The documentation for this method and all the others makes it very clear that this assumption cannot be made:

Note that this method is called for all instances that are marked as "dirty", even those which have no net changes to their column-based attributes. An object is marked as dirty when any of its column-based attributes have a "set attribute" operation called or when any of its collections are modified. If, at update time, no column-based attributes have any net changes, no UPDATE statement will be issued. This means that an instance being sent to before_update is not a guarantee that an UPDATE statement will be issued (although you can affect the outcome here).

The versioning example that comes with the distro in examples/versioning/history_meta.py illustrates a versioning extension that checks for changed state on the incoming instance, before determining that the version number should be bumped. Using that approach will prevent empty udpates from erroneously bumping the version number up.

[Mike Bayer]

Removing milestone: 0.6.1 (automated comment)