onupdate runs for post_update updates

[Adrian]

I'm inserting a new row where I specify an explicit value for a column that usually has a value set via onupdate in the model.

This works fine except when a relationship with post_update=True is involved. In this case the UPDATE used for the relationship also uses the value from onupdate.

[Mike Bayer]

unfortunately I don't see any way this would be changed. the contract of "onupdate" as well as "post_update" is clear, you are free to not use either one and use alternate means of setting the values as needed.

[Adrian]

Well, the main use case for post_update is to support foreign keys which relationships that go in both directions and thus require either the ID to be known before the INSERT or the UPDATE.

I don't see why this should affect onupdate behavior. Sure, it's an update, but should it be considered an update as in "the application changed data" when it's executed because of post_update?

Also, after a session flush the object I just added still had the value i initially set instead of the one that was actually saved in the db via onupdate... or is it intended that the onupdate value is not exposed on the model object until an expire?

[Mike Bayer]

post_update emits an UPDATE statement at the ORM level, 'onupdate' runs whenever there is an UPDATE statement at the Core level and the value is otherwise not modified. Trying to chase down obscure edge cases like this with significant additional complexity of behavior, implementation, testing, not to mention documentation "usually unrelated features X and Y have an exception case when exactly Q, P, R are present, such that the documented behavior changes entirely to suit this very particular use case, and there's no way to make the features work as they usually do when this happens", is just not worth it.

I would not use an "onupdate" rule for a column that I occasionally intend to populate with a very important value that cannot change; I'd remove the "onupdate" rule (which is intended to act mostly like a database-level trigger) and replace it with logic within my ORM model (probably using events, like before_update(), before_flush(), or similar) that puts the value that is desired here correctly in all cases.

[Adrian]

ah ok, i'll just use a workaround in that case.. it's only in a script that imports legacy data where i need to specify a value for the onupdate column anyway, in the actual application it always uses the implicit value

[Mike Bayer]

Also, after a session flush the object I just added still had the value i initially set instead of the one that was actually saved in the db via onupdate... or is it intended that the onupdate value is not exposed on the model object until an expire?

OK, that is true, and that is more of an issue.

[Mike Bayer]

see #3472

[Mike Bayer]

it's stupid easy to do it so might as well

[Mike Bayer]

test case:

from sqlalchemy import *
from sqlalchemy.orm import *
from sqlalchemy.ext.declarative import declarative_base

Base = declarative_base()

from itertools import count

value = count()


class A(Base):
    __tablename__ = 'a'
    id = Column(Integer, primary_key=True)
    favorite_b_id = Column(ForeignKey('b.id'))
    bs = relationship("B", primaryjoin="A.id == B.a_id")
    favorite_b = relationship(
        "B", primaryjoin="A.favorite_b_id == B.id", post_update=True)
    updated = Column(Integer, onupdate=lambda: next(value))


class B(Base):
    __tablename__ = 'b'
    id = Column(Integer, primary_key=True)
    a_id = Column(ForeignKey('a.id'))

e = create_engine("sqlite://", echo=True)
Base.metadata.create_all(e)

s = Session(e)
a1 = A()
b1 = B()

a1.bs.append(b1)
a1.favorite_b = b1
a1.updated = 5
s.add(a1)
s.flush()
assert a1.updated == 5

patch:

diff --git a/lib/sqlalchemy/orm/persistence.py b/lib/sqlalchemy/orm/persistence.py
index 0bfee2e..736be7c 100644
--- a/lib/sqlalchemy/orm/persistence.py
+++ b/lib/sqlalchemy/orm/persistence.py
@@ -552,7 +552,7 @@ def _collect_post_update_commands(base_mapper, uowtransaction, table,
                         state,
                         state_dict, col, passive=attributes.PASSIVE_OFF)

-            elif col in post_update_cols:
+            elif col in post_update_cols or col.onupdate is not None:
                 prop = mapper._columntoproperty[col]
                 history = state.manager[prop.key].impl.get_history(
                     state, state_dict,

[Mike Bayer]

this is still a major backwards incompat-change so will have to make it prominent, also this has to be done with #3472 or not at all

[Mike Bayer]

basically this needs to be described as: "when post_update is used, values that are explicitly set for columns that feature onupdate or server_onupdate are sent an additional time within the UPDATE statement to ensure defaults don't overwrite the value". need to look at server_onupdate as well:

diff --git a/lib/sqlalchemy/orm/persistence.py b/lib/sqlalchemy/orm/persistence.py
index 0bfee2e..2c68b04 100644
--- a/lib/sqlalchemy/orm/persistence.py
+++ b/lib/sqlalchemy/orm/persistence.py
@@ -552,7 +552,9 @@ def _collect_post_update_commands(base_mapper, uowtransaction, table,
                         state,
                         state_dict, col, passive=attributes.PASSIVE_OFF)

-            elif col in post_update_cols:
+            elif col in post_update_cols or \
+                    col.onupdate is not None or \
+                    col.server_onupdate is not None:
                 prop = mapper._columntoproperty[col]
                 history = state.manager[prop.key].impl.get_history(
                     state, state_dict,

[Mike Bayer]

starting to wrap up scope for 1.1, remaining behavioral changes being pushed out

[Mike Bayer]

Re-send column value w/ onupdate default during post-update

Adjusted the behavior of post_update such that if a column with an "onupdate" default has received an explicit value for INSERT, re-send the same data during a post-update UPDATE so that the value remains in effect, rather than an onupdate overwriting it.

Change-Id: I26bccb6f957dcad07a2bcbda2dd9e14c60b92b06 Fixes: #3471

→ <<cset 6b68a70b5f90>>