diff options
| -rw-r--r-- | doc/build/changelog/unreleased_14/4696.rst | 9 | ||||
| -rw-r--r-- | lib/sqlalchemy/orm/attributes.py | 61 | ||||
| -rw-r--r-- | lib/sqlalchemy/orm/base.py | 19 | ||||
| -rw-r--r-- | lib/sqlalchemy/orm/mapper.py | 6 | ||||
| -rw-r--r-- | test/orm/test_attributes.py | 47 | ||||
| -rw-r--r-- | test/orm/test_lazy_relations.py | 10 |
6 files changed, 78 insertions, 74 deletions
diff --git a/doc/build/changelog/unreleased_14/4696.rst b/doc/build/changelog/unreleased_14/4696.rst new file mode 100644 index 000000000..c4629db36 --- /dev/null +++ b/doc/build/changelog/unreleased_14/4696.rst @@ -0,0 +1,9 @@ +.. change:: + :tags: bug, orm + :tickets: 4696 + + The internal attribute symbols NO_VALUE and NEVER_SET have been unified, as + there was no meaningful difference between these two symbols, other than a + few codepaths where they were differentiated in subtle and undocumented + ways, these have been fixed. + diff --git a/lib/sqlalchemy/orm/attributes.py b/lib/sqlalchemy/orm/attributes.py index 01f19d991..321ab7d6f 100644 --- a/lib/sqlalchemy/orm/attributes.py +++ b/lib/sqlalchemy/orm/attributes.py @@ -28,7 +28,7 @@ from .base import instance_state from .base import instance_str from .base import LOAD_AGAINST_COMMITTED from .base import manager_of_class -from .base import NEVER_SET +from .base import NEVER_SET # noqa from .base import NO_AUTOFLUSH from .base import NO_CHANGE # noqa from .base import NO_RAISE @@ -40,7 +40,7 @@ from .base import PASSIVE_NO_INITIALIZE from .base import PASSIVE_NO_RESULT from .base import PASSIVE_OFF from .base import PASSIVE_ONLY_PERSISTENT -from .base import PASSIVE_RETURN_NEVER_SET +from .base import PASSIVE_RETURN_NO_VALUE from .base import RELATED_OBJECT_OK # noqa from .base import SQL_OK # noqa from .base import state_str @@ -677,7 +677,7 @@ class AttributeImpl(object): key = self.key if ( key not in state.committed_state - or state.committed_state[key] is NEVER_SET + or state.committed_state[key] is NO_VALUE ): if not passive & CALLABLES_OK: return PASSIVE_NO_RESULT @@ -692,7 +692,7 @@ class AttributeImpl(object): else: value = ATTR_EMPTY - if value is PASSIVE_NO_RESULT or value is NEVER_SET: + if value is PASSIVE_NO_RESULT or value is NO_VALUE: return value elif value is ATTR_WAS_SET: try: @@ -708,7 +708,7 @@ class AttributeImpl(object): return self.set_committed_value(state, dict_, value) if not passive & INIT_OK: - return NEVER_SET + return NO_VALUE else: # Return a new, empty value return self.initialize(state, dict_) @@ -749,7 +749,7 @@ class AttributeImpl(object): if self.key in state.committed_state: value = state.committed_state[self.key] - if value in (NO_VALUE, NEVER_SET): + if value is NO_VALUE: return None else: return value @@ -782,7 +782,7 @@ class ScalarAttributeImpl(AttributeImpl): def delete(self, state, dict_): if self.dispatch._active_history: - old = self.get(state, dict_, PASSIVE_RETURN_NEVER_SET) + old = self.get(state, dict_, PASSIVE_RETURN_NO_VALUE) else: old = dict_.get(self.key, NO_VALUE) @@ -802,6 +802,8 @@ class ScalarAttributeImpl(AttributeImpl): def get_history(self, state, dict_, passive=PASSIVE_OFF): if self.key in dict_: return History.from_scalar_attribute(self, state, dict_[self.key]) + elif self.key in state.committed_state: + return History.from_scalar_attribute(self, state, NO_VALUE) else: if passive & INIT_OK: passive ^= INIT_OK @@ -822,7 +824,7 @@ class ScalarAttributeImpl(AttributeImpl): pop=False, ): if self.dispatch._active_history: - old = self.get(state, dict_, PASSIVE_RETURN_NEVER_SET) + old = self.get(state, dict_, PASSIVE_RETURN_NO_VALUE) else: old = dict_.get(self.key, NO_VALUE) @@ -920,7 +922,7 @@ class ScalarObjectAttributeImpl(ScalarAttributeImpl): if ( current is not None and current is not PASSIVE_NO_RESULT - and current is not NEVER_SET + and current is not NO_VALUE ): ret = [(instance_state(current), current)] else: @@ -931,7 +933,7 @@ class ScalarObjectAttributeImpl(ScalarAttributeImpl): if ( original is not None and original is not PASSIVE_NO_RESULT - and original is not NEVER_SET + and original is not NO_VALUE and original is not current ): @@ -998,7 +1000,7 @@ class ScalarObjectAttributeImpl(ScalarAttributeImpl): if previous is not value and previous not in ( None, PASSIVE_NO_RESULT, - NEVER_SET, + NO_VALUE, ): self.sethasparent(instance_state(previous), state, False) @@ -1109,7 +1111,7 @@ class CollectionAttributeImpl(AttributeImpl): if self.key in state.committed_state: original = state.committed_state[self.key] - if original not in (NO_VALUE, NEVER_SET): + if original is not NO_VALUE: current_states = [ ((c is not None) and instance_state(c) or None, c) for c in current @@ -1142,7 +1144,7 @@ class CollectionAttributeImpl(AttributeImpl): for fn in self.dispatch.append: value = fn(state, value, initiator or self._append_token) - state._modified_event(dict_, self, NEVER_SET, True) + state._modified_event(dict_, self, NO_VALUE, True) if self.trackparent and value is not None: self.sethasparent(instance_state(value), state, True) @@ -1158,7 +1160,7 @@ class CollectionAttributeImpl(AttributeImpl): operations (even though set.pop is the one where it is really needed). """ - state._modified_event(dict_, self, NEVER_SET, True) + state._modified_event(dict_, self, NO_VALUE, True) def fire_remove_event(self, state, dict_, value, initiator): if self.trackparent and value is not None: @@ -1167,13 +1169,13 @@ class CollectionAttributeImpl(AttributeImpl): for fn in self.dispatch.remove: fn(state, value, initiator or self._remove_token) - state._modified_event(dict_, self, NEVER_SET, True) + state._modified_event(dict_, self, NO_VALUE, True) def delete(self, state, dict_): if self.key not in dict_: return - state._modified_event(dict_, self, NEVER_SET, True) + state._modified_event(dict_, self, NO_VALUE, True) collection = self.get_collection(state, state.dict) collection.clear_with_event() @@ -1386,7 +1388,7 @@ def backref_listeners(attribute, key, uselist): if ( oldchild is not None and oldchild is not PASSIVE_NO_RESULT - and oldchild is not NEVER_SET + and oldchild is not NO_VALUE ): # With lazy=None, there's no guarantee that the full collection is # present when updating via a backref. @@ -1481,7 +1483,7 @@ def backref_listeners(attribute, key, uselist): if ( child is not None and child is not PASSIVE_NO_RESULT - and child is not NEVER_SET + and child is not NO_VALUE ): child_state, child_dict = ( instance_state(child), @@ -1549,9 +1551,7 @@ def backref_listeners(attribute, key, uselist): _NO_HISTORY = util.symbol("NO_HISTORY") -_NO_STATE_SYMBOLS = frozenset( - [id(PASSIVE_NO_RESULT), id(NO_VALUE), id(NEVER_SET)] -) +_NO_STATE_SYMBOLS = frozenset([id(PASSIVE_NO_RESULT), id(NO_VALUE)]) History = util.namedtuple("History", ["added", "unchanged", "deleted"]) @@ -1637,12 +1637,15 @@ class History(History): original = state.committed_state.get(attribute.key, _NO_HISTORY) if original is _NO_HISTORY: - if current is NEVER_SET: + if current is NO_VALUE: return cls((), (), ()) else: return cls((), [current], ()) # don't let ClauseElement expressions here trip things up - elif attribute.is_equal(current, original) is True: + elif ( + current is not NO_VALUE + and attribute.is_equal(current, original) is True + ): return cls((), [current], ()) else: # current convention on native scalars is to not @@ -1658,7 +1661,7 @@ class History(History): current = None else: deleted = [original] - if current is NEVER_SET: + if current is NO_VALUE: return cls((), (), deleted) else: return cls([current], (), deleted) @@ -1668,11 +1671,11 @@ class History(History): original = state.committed_state.get(attribute.key, _NO_HISTORY) if original is _NO_HISTORY: - if current is NO_VALUE or current is NEVER_SET: + if current is NO_VALUE: return cls((), (), ()) else: return cls((), [current], ()) - elif current is original and current is not NEVER_SET: + elif current is original and current is not NO_VALUE: return cls((), [current], ()) else: # current convention on related objects is to not @@ -1688,7 +1691,7 @@ class History(History): current = None else: deleted = [original] - if current is NO_VALUE or current is NEVER_SET: + if current is NO_VALUE: return cls((), (), deleted) else: return cls([current], (), deleted) @@ -1697,11 +1700,11 @@ class History(History): def from_collection(cls, attribute, state, current): original = state.committed_state.get(attribute.key, _NO_HISTORY) - if current is NO_VALUE or current is NEVER_SET: + if current is NO_VALUE: return cls((), (), ()) current = getattr(current, "_sa_adapter") - if original in (NO_VALUE, NEVER_SET): + if original is NO_VALUE: return cls(list(current), (), ()) elif original is _NO_HISTORY: return cls((), list(current), ()) diff --git a/lib/sqlalchemy/orm/base.py b/lib/sqlalchemy/orm/base.py index f809d5891..4d308d26b 100644 --- a/lib/sqlalchemy/orm/base.py +++ b/lib/sqlalchemy/orm/base.py @@ -45,13 +45,12 @@ NO_VALUE = util.symbol( and flags indicated we were not to load it. """, ) +NEVER_SET = NO_VALUE +""" +Synonymous with NO_VALUE -NEVER_SET = util.symbol( - "NEVER_SET", - """Symbol which may be placed as the 'previous' value of an attribute - indicating that the attribute had not been assigned to previously. - """, -) +.. versionchanged:: 1.4 NEVER_SET was merged with NO_VALUE +""" NO_CHANGE = util.symbol( "NO_CHANGE", @@ -126,15 +125,15 @@ PASSIVE_OFF = util.symbol( RELATED_OBJECT_OK | NON_PERSISTENT_OK | INIT_OK | CALLABLES_OK | SQL_OK ), ) -PASSIVE_RETURN_NEVER_SET = util.symbol( - "PASSIVE_RETURN_NEVER_SET", +PASSIVE_RETURN_NO_VALUE = util.symbol( + "PASSIVE_RETURN_NO_VALUE", """PASSIVE_OFF ^ INIT_OK""", canonical=PASSIVE_OFF ^ INIT_OK, ) PASSIVE_NO_INITIALIZE = util.symbol( "PASSIVE_NO_INITIALIZE", - "PASSIVE_RETURN_NEVER_SET ^ CALLABLES_OK", - canonical=PASSIVE_RETURN_NEVER_SET ^ CALLABLES_OK, + "PASSIVE_RETURN_NO_VALUE ^ CALLABLES_OK", + canonical=PASSIVE_RETURN_NO_VALUE ^ CALLABLES_OK, ) PASSIVE_NO_FETCH = util.symbol( "PASSIVE_NO_FETCH", "PASSIVE_OFF ^ SQL_OK", canonical=PASSIVE_OFF ^ SQL_OK diff --git a/lib/sqlalchemy/orm/mapper.py b/lib/sqlalchemy/orm/mapper.py index ccf05a783..3c26f7247 100644 --- a/lib/sqlalchemy/orm/mapper.py +++ b/lib/sqlalchemy/orm/mapper.py @@ -2715,7 +2715,7 @@ class Mapper(InspectionAttr): return self._identity_key_from_state(state, attributes.PASSIVE_OFF) def _identity_key_from_state( - self, state, passive=attributes.PASSIVE_RETURN_NEVER_SET + self, state, passive=attributes.PASSIVE_RETURN_NO_VALUE ): dict_ = state.dict manager = state.manager @@ -2769,7 +2769,7 @@ class Mapper(InspectionAttr): return {prop.key for prop in self._all_pk_props} def _get_state_attr_by_column( - self, state, dict_, column, passive=attributes.PASSIVE_RETURN_NEVER_SET + self, state, dict_, column, passive=attributes.PASSIVE_RETURN_NO_VALUE ): prop = self._columntoproperty[column] return state.manager[prop.key].impl.get(state, dict_, passive=passive) @@ -2790,7 +2790,7 @@ class Mapper(InspectionAttr): ) def _get_committed_state_attr_by_column( - self, state, dict_, column, passive=attributes.PASSIVE_RETURN_NEVER_SET + self, state, dict_, column, passive=attributes.PASSIVE_RETURN_NO_VALUE ): prop = self._columntoproperty[column] diff --git a/test/orm/test_attributes.py b/test/orm/test_attributes.py index 17488f236..8d244c4fa 100644 --- a/test/orm/test_attributes.py +++ b/test/orm/test_attributes.py @@ -1028,31 +1028,27 @@ class GetNoValueTest(fixtures.ORMTest): attributes.PASSIVE_NO_RESULT, ) - def test_passive_no_result_never_set(self): - attr, state, dict_ = self._fixture(attributes.NEVER_SET) + def test_passive_no_result_no_value(self): + attr, state, dict_ = self._fixture(attributes.NO_VALUE) eq_( attr.get(state, dict_, passive=attributes.PASSIVE_NO_INITIALIZE), attributes.PASSIVE_NO_RESULT, ) assert "attr" not in dict_ - def test_passive_ret_never_set_never_set(self): - attr, state, dict_ = self._fixture(attributes.NEVER_SET) + def test_passive_ret_no_value(self): + attr, state, dict_ = self._fixture(attributes.NO_VALUE) eq_( - attr.get( - state, dict_, passive=attributes.PASSIVE_RETURN_NEVER_SET - ), - attributes.NEVER_SET, + attr.get(state, dict_, passive=attributes.PASSIVE_RETURN_NO_VALUE), + attributes.NO_VALUE, ) assert "attr" not in dict_ - def test_passive_ret_never_set_empty(self): + def test_passive_ret_no_value_empty(self): attr, state, dict_ = self._fixture(None) eq_( - attr.get( - state, dict_, passive=attributes.PASSIVE_RETURN_NEVER_SET - ), - attributes.NEVER_SET, + attr.get(state, dict_, passive=attributes.PASSIVE_RETURN_NO_VALUE), + attributes.NO_VALUE, ) assert "attr" not in dict_ @@ -1581,7 +1577,7 @@ class PendingBackrefTest(fixtures.ORMTest): ) eq_(lazy_posts.call_count, 1) - def test_passive_history_collection_never_set(self): + def test_passive_history_collection_no_value(self): Post, Blog, lazy_posts = self._fixture() lazy_posts.return_value = attributes.PASSIVE_NO_RESULT @@ -1594,10 +1590,10 @@ class PendingBackrefTest(fixtures.ORMTest): attributes.instance_dict(b), ) - # this sets up NEVER_SET on b.posts + # this sets up NO_VALUE on b.posts p.blog = b - eq_(state.committed_state, {"posts": attributes.NEVER_SET}) + eq_(state.committed_state, {"posts": attributes.NO_VALUE}) assert "posts" not in dict_ # then suppose the object was made transient again, @@ -1607,7 +1603,7 @@ class PendingBackrefTest(fixtures.ORMTest): p2 = Post("asdf") p2.blog = b - eq_(state.committed_state, {"posts": attributes.NEVER_SET}) + eq_(state.committed_state, {"posts": attributes.NO_VALUE}) eq_(dict_["posts"], [p2]) # then this would fail. @@ -2080,17 +2076,17 @@ class HistoryTest(fixtures.TestBase): assert "someattr" not in f.__dict__ assert "someattr" not in attributes.instance_state(f).committed_state - def test_collection_never_set(self): + def test_collection_no_value(self): Foo = self._fixture(uselist=True, useobject=True, active_history=True) f = Foo() eq_(self._someattr_history(f, passive=True), (None, None, None)) - def test_scalar_obj_never_set(self): + def test_scalar_obj_no_value(self): Foo = self._fixture(uselist=False, useobject=True, active_history=True) f = Foo() eq_(self._someattr_history(f, passive=True), (None, None, None)) - def test_scalar_never_set(self): + def test_scalar_no_value(self): Foo = self._fixture( uselist=False, useobject=False, active_history=True ) @@ -3483,14 +3479,14 @@ class EventPropagateTest(fixtures.TestBase): b = B() b.attrib = "foo" eq_(b.attrib, "foo") - eq_(canary, [("foo", attributes.NEVER_SET)]) + eq_(canary, [("foo", attributes.NO_VALUE)]) c = C() c.attrib = "bar" eq_(c.attrib, "bar") eq_( canary, - [("foo", attributes.NEVER_SET), ("bar", attributes.NEVER_SET)], + [("foo", attributes.NO_VALUE), ("bar", attributes.NO_VALUE)], ) def test_propagate(self): @@ -3531,16 +3527,13 @@ class EventPropagateTest(fixtures.TestBase): d1 = D() b.attrib = d1 is_(b.attrib, d1) - eq_(canary, [(d1, attributes.NEVER_SET)]) + eq_(canary, [(d1, attributes.NO_VALUE)]) c = C() d2 = D() c.attrib = d2 is_(c.attrib, d2) - eq_( - canary, - [(d1, attributes.NEVER_SET), (d2, attributes.NEVER_SET)], - ) + eq_(canary, [(d1, attributes.NO_VALUE), (d2, attributes.NO_VALUE)]) def _test_propagate_fixtures(self, active_history, useobject): classes = [None, None, None, None] diff --git a/test/orm/test_lazy_relations.py b/test/orm/test_lazy_relations.py index 8d1e82fb5..0ababc262 100644 --- a/test/orm/test_lazy_relations.py +++ b/test/orm/test_lazy_relations.py @@ -1096,9 +1096,9 @@ class GetterStateTest(_fixtures.FixtureTest): Address.user.impl.get( attributes.instance_state(a1), attributes.instance_dict(a1), - passive=attributes.PASSIVE_RETURN_NEVER_SET, + passive=attributes.PASSIVE_RETURN_NO_VALUE, ), - attributes.NEVER_SET, + attributes.NO_VALUE, ) assert "user_id" not in a1.__dict__ assert "user" not in a1.__dict__ @@ -1109,7 +1109,7 @@ class GetterStateTest(_fixtures.FixtureTest): Address.user.impl.get_history( attributes.instance_state(a1), attributes.instance_dict(a1), - passive=attributes.PASSIVE_RETURN_NEVER_SET, + passive=attributes.PASSIVE_RETURN_NO_VALUE, ), ((), (), ()), ) @@ -1174,7 +1174,7 @@ class GetterStateTest(_fixtures.FixtureTest): Address.user.impl.get( attributes.instance_state(a1), attributes.instance_dict(a1), - passive=attributes.PASSIVE_RETURN_NEVER_SET, + passive=attributes.PASSIVE_RETURN_NO_VALUE, ), User(name="ed"), ) @@ -1185,7 +1185,7 @@ class GetterStateTest(_fixtures.FixtureTest): Address.user.impl.get_history( attributes.instance_state(a1), attributes.instance_dict(a1), - passive=attributes.PASSIVE_RETURN_NEVER_SET, + passive=attributes.PASSIVE_RETURN_NO_VALUE, ), ((), [User(name="ed")], ()), ) |