[Sqlalchemy-tickets] Issue #4353: raiseload with sql_only=False is overly aggressive (zzzeek/sqlalc
Brought to you by:
zzzeek
From: Alex R. <iss...@bi...> - 2018-11-01 07:03:51
|
New issue 4353: raiseload with sql_only=False is overly aggressive https://bitbucket.org/zzzeek/sqlalchemy/issues/4353/raiseload-with-sql_only-false-is-overly Alex Rothberg: I believe that I am seeing a case where `raiseload` with `sql_only=False` is triggering an exception when it should not be. While setting `sql_only=True` avoids the error for reasons outlined below this is not an ideal workaround. Further, exception is being generated in a case where the look up is entirely not needed (explanation below). The stack trace is: ``` #!python /Users/alex/.pyenv/versions/app/lib/python3.7/site-packages/sqlalchemy/orm/attributes.py:229: in __set__ instance_dict(instance), value, None) /Users/alex/.pyenv/versions/app/lib/python3.7/site-packages/sqlalchemy/orm/attributes.py:1079: in set initiator=evt) /Users/alex/.pyenv/versions/app/lib/python3.7/site-packages/sqlalchemy/orm/collections.py:768: in bulk_replace existing_adapter.fire_remove_event(member, initiator=initiator) /Users/alex/.pyenv/versions/app/lib/python3.7/site-packages/sqlalchemy/orm/collections.py:698: in fire_remove_event item, initiator) /Users/alex/.pyenv/versions/app/lib/python3.7/site-packages/sqlalchemy/orm/attributes.py:962: in fire_remove_event fn(state, value, initiator or self._remove_token) /Users/alex/.pyenv/versions/app/lib/python3.7/site-packages/sqlalchemy/orm/attributes.py:1255: in emit_backref_from_collection_remove_event passive=PASSIVE_NO_FETCH) /Users/alex/.pyenv/versions/app/lib/python3.7/site-packages/sqlalchemy/orm/attributes.py:634: in pop passive=passive, check_old=value, pop=True) /Users/alex/.pyenv/versions/app/lib/python3.7/site-packages/sqlalchemy/orm/attributes.py:800: in set LOAD_AGAINST_COMMITTED) /Users/alex/.pyenv/versions/app/lib/python3.7/site-packages/sqlalchemy/orm/attributes.py:599: in get value = callable_(state, passive) /Users/alex/.pyenv/versions/app/lib/python3.7/site-packages/sqlalchemy/orm/strategies.py:834: in __call__ return strategy._load_for_state(state, passive) /Users/alex/.pyenv/versions/app/lib/python3.7/site-packages/sqlalchemy/orm/strategies.py:589: in _load_for_state self._invoke_raise_load(state, passive, "raise") _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ self = <sqlalchemy.orm.strategies.LazyLoader object at 0x1176b2f20> state = <sqlalchemy.orm.state.InstanceState object at 0x1162462b0>, passive = 53 lazy = 'raise' ``` I believe that the problem is that an exception is thrown unconditionally here: https://github.com/zzzeek/sqlalchemy/blob/7d372da7385be6a9817a20b6b62f7c4237af7b26/lib/sqlalchemy/orm/strategies.py#L588-L589 even if this escape hatch would have been hit: https://github.com/zzzeek/sqlalchemy/blob/7d372da7385be6a9817a20b6b62f7c4237af7b26/lib/sqlalchemy/orm/strategies.py#L626-L628 I think in the cases where the load is entirely unneeded (as it is in my case), the exception should not be raised. While a workaround is to set `sql_only=True`, I want to avoid that as it can lead to differences is results between testing and production (due to the presence of items in the identity map avoiding exceptions). The reason I say the load us unneeded is that sqla is attempting to load the old value in a soon to be replaced backref (ie this is part of the emit). |