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 :ticket:`3061`. fixes #3060
.. changelog::
:version: 0.9.5
+ .. change::
+ :tags: bug, orm
+ :tickets: 3060
+ :versions: 1.0.0
+
+ 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 :ticket:`3061`.
+
.. change::
:tags: bug, orm
:tickets: 3057
# if history present, don't load
key = self.key
if key not in state.committed_state or \
- state.committed_state[key] is NEVER_SET:
+ state.committed_state[key] is NEVER_SET:
if not passive & CALLABLES_OK:
return PASSIVE_NO_RESULT
if self.dispatch._active_history:
old = self.get(state, dict_, passive=PASSIVE_ONLY_PERSISTENT | NO_AUTOFLUSH)
else:
+ # would like to call with PASSIVE_NO_FETCH ^ INIT_OK. However,
+ # we have a long-standing behavior that a "get()" on never set
+ # should implicitly set the value to None. Leaving INIT_OK
+ # set here means we are consistent whether or not we did a get
+ # first.
+ # see test_use_object_set_None vs. test_use_object_get_first_set_None
+ # in test_attributes.py
old = self.get(state, dict_, passive=PASSIVE_NO_FETCH)
if check_old is not None and \
state_str(state),
self.key
))
+
value = self.fire_replace_event(state, dict_, value, old, initiator)
dict_[self.key] = value
def fire_replace_event(self, state, dict_, value, previous, initiator):
if self.trackparent:
if (previous is not value and
- previous is not None and
- previous is not PASSIVE_NO_RESULT):
+ previous not in (None, PASSIVE_NO_RESULT, NEVER_SET)):
self.sethasparent(instance_state(previous), state, False)
for fn in self.dispatch.set:
def emit_backref_from_scalar_set_event(state, child, oldchild, initiator):
if oldchild is child:
return child
- if oldchild is not None and oldchild is not PASSIVE_NO_RESULT:
+ if oldchild is not None and oldchild not in (PASSIVE_NO_RESULT, NEVER_SET):
# With lazy=None, there's no guarantee that the full collection is
# present when updating via a backref.
old_state, old_dict = instance_state(oldchild),\
return not bool(
(self.added or self.deleted)
- or self.unchanged and self.unchanged != [None]
+ or self.unchanged
)
def sum(self):
self.key,
attributes.PASSIVE_NO_INITIALIZE)
if history:
- for child in history.added:
- self._synchronize(state, child, None, False,
- uowcommit, "add")
-
+ if history.added:
+ for child in history.added:
+ self._synchronize(state, child, None, False,
+ uowcommit, "add")
+ elif history.unchanged == [None]:
+ # this is to appease the case where our row
+ # here is in fact going to be a so-called "row switch",
+ # where an INSERT becomes an UPDATE. See #3060.
+ self._synchronize(state, None, None, True, uowcommit)
if self.post_update:
self._post_update(state, uowcommit, history.sum())
if value is None:
hasnull = True
params[col._label] = value
+
+ # see #3060. Need to consider an "unchanged" None
+ # as potentially history for now.
+ elif row_switch and history.unchanged == [None]:
+ params[col.key] = None
+ hasdata = True
if hasdata:
if hasnull:
raise orm_exc.FlushError(
def clear(dest, dest_mapper, synchronize_pairs):
for l, r in synchronize_pairs:
- if r.primary_key:
+ if r.primary_key and \
+ dest_mapper._get_state_attr_by_column(
+ dest, dest.dict, r) is not None:
+
raise AssertionError(
"Dependency rule tried to blank-out primary key "
"column '%s' on instance '%s'" %
Foo, Bar = self._two_obj_fixture(uselist=False)
f = Foo()
f.someattr = None
+ # we'd expect ([None], (), ()), however because
+ # we set to None w/o setting history if we were to "get" first,
+ # it is more consistent that this doesn't set history.
+ eq_(self._someattr_history(f), ((), [None], ()))
+
+ def test_use_object_get_first_set_None(self):
+ Foo, Bar = self._two_obj_fixture(uselist=False)
+ f = Foo()
+ assert f.someattr is None
+ f.someattr = None
eq_(self._someattr_history(f), ((), [None], ()))
def test_use_object_set_dict_set_None(self):
Foo, Bar = self._two_obj_fixture(uselist=False)
f = Foo()
- hi =Bar(name='hi')
+ hi = Bar(name='hi')
f.__dict__['someattr'] = hi
f.someattr = None
eq_(self._someattr_history(f), ([None], (), [hi]))
eq_(sess.scalar(self.tables.parent.count()), 0)
+class RowswitchM2OTest(fixtures.MappedTest):
+ # tests for #3060 and related issues
+ @classmethod
+ def define_tables(cls, metadata):
+ Table(
+ 'a', metadata,
+ Column('id', Integer, primary_key=True),
+ )
+ Table(
+ 'b', metadata,
+ Column('id', Integer, primary_key=True),
+ Column('aid', ForeignKey('a.id')),
+ Column('cid', ForeignKey('c.id')),
+ Column('data', String(50))
+ )
+ Table(
+ 'c', metadata,
+ Column('id', Integer, primary_key=True),
+ )
+
+ def _fixture(self):
+ a, b, c = self.tables.a, self.tables.b, self.tables.c
+
+ class A(fixtures.BasicEntity):
+ pass
+ class B(fixtures.BasicEntity):
+ pass
+ class C(fixtures.BasicEntity):
+ pass
+
+
+ mapper(A, a, properties={
+ 'bs': relationship(B, cascade="all, delete-orphan")
+ })
+ mapper(B, b, properties={
+ 'c': relationship(C)
+ })
+ mapper(C, c)
+ return A, B, C
+
+ def test_set_none_replaces_m2o(self):
+ # we have to deal here with the fact that a
+ # get of an unset attribute implicitly sets it to None
+ # with no history. So while we'd like "b.x = None" to
+ # record that "None" was added and we can then actively set it,
+ # a simple read of "b.x" ruins that; we'd have to dramatically
+ # alter the semantics of get() such that it creates history, which
+ # would incur extra work within the flush process to deal with
+ # change that previously showed up as nothing.
+
+ A, B, C = self._fixture()
+ sess = Session()
+
+ sess.add(
+ A(id=1, bs=[B(id=1, c=C(id=1))])
+ )
+ sess.commit()
+
+ a1 = sess.query(A).first()
+ a1.bs = [B(id=1, c=None)]
+ sess.commit()
+ assert a1.bs[0].c is None
+
+ def test_set_none_w_get_replaces_m2o(self):
+ A, B, C = self._fixture()
+ sess = Session()
+
+ sess.add(
+ A(id=1, bs=[B(id=1, c=C(id=1))])
+ )
+ sess.commit()
+
+ a1 = sess.query(A).first()
+ b2 = B(id=1)
+ assert b2.c is None
+ b2.c = None
+ a1.bs = [b2]
+ sess.commit()
+ assert a1.bs[0].c is None
+
+ def test_set_none_replaces_scalar(self):
+ # this case worked before #3060, because a straight scalar
+ # set of None shows up. Howver, as test_set_none_w_get
+ # shows, we can't rely on this - the get of None will blow
+ # away the history.
+ A, B, C = self._fixture()
+ sess = Session()
+
+ sess.add(
+ A(id=1, bs=[B(id=1, data='somedata')])
+ )
+ sess.commit()
+
+ a1 = sess.query(A).first()
+ a1.bs = [B(id=1, data=None)]
+ sess.commit()
+ assert a1.bs[0].data is None
+
+ def test_set_none_w_get_replaces_scalar(self):
+ A, B, C = self._fixture()
+ sess = Session()
+
+ sess.add(
+ A(id=1, bs=[B(id=1, data='somedata')])
+ )
+ sess.commit()
+
+ a1 = sess.query(A).first()
+ b2 = B(id=1)
+ assert b2.data is None
+ b2.data = None
+ a1.bs = [b2]
+ sess.commit()
+ assert a1.bs[0].data is None
+
+
class BasicStaleChecksTest(fixtures.MappedTest):
@classmethod