⬆️ UPGRADE: sqlalchemy v1.4 (v1 API) #5103
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Eurhhhhhh
____________ test_delete_object_hard[get_disk_object_store_backend] ____________
repository = <aiida.repository.repository.Repository object at 0x7fb2a7db03d0>
generate_directory = <function generate_directory.<locals>._generate_directory at 0x7fb2a7d734d0>
def test_delete_object_hard(repository, generate_directory):
"""Test the ``Repository.delete_object`` method with ``hard_delete=True``."""
directory = generate_directory({'file_a': None})
with open(directory / 'file_a', 'rb') as handle:
repository.put_object_from_filelike(handle, 'file_a')
assert repository.has_object('file_a')
key = repository.get_object('file_a').key
> repository.delete_object('file_a', hard_delete=True)
tests/repository/test_repository.py:490:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
aiida/repository/repository.py:415: in delete_object
self.backend.delete_object(file_object.key)
aiida/repository/backend/disk_object_store.py:101: in delete_object
self.container.delete_objects([key])
/opt/hostedtoolcache/Python/3.7.11/x64/lib/python3.7/site-packages/disk_objectstore/container.py:2215: in delete_objects
query.delete(synchronize_session=False)
/opt/hostedtoolcache/Python/3.7.11/x64/lib/python3.7/site-packages/sqlalchemy/orm/query.py:3135: in delete
delete_ = sql.delete(*self._raw_columns)
<string>:2: in delete
???
<string>:2: in __init__
???
/opt/hostedtoolcache/Python/3.7.11/x64/lib/python3.7/site-packages/sqlalchemy/util/deprecations.py:298: in warned
return fn(*args, **kwargs)
/opt/hostedtoolcache/Python/3.7.11/x64/lib/python3.7/site-packages/sqlalchemy/sql/dml.py:1431: in __init__
roles.DMLTableRole, table, apply_propagate_attrs=self
/opt/hostedtoolcache/Python/3.7.11/x64/lib/python3.7/site-packages/sqlalchemy/sql/coercions.py:213: in expect
original_element, resolved, argname=argname, **kw
/opt/hostedtoolcache/Python/3.7.11/x64/lib/python3.7/site-packages/sqlalchemy/sql/coercions.py:255: in _implicit_coercions
self._raise_for_expected(element, argname, resolved)
/opt/hostedtoolcache/Python/3.7.11/x64/lib/python3.7/site-packages/sqlalchemy/sql/coercions.py:443: in _raise_for_expected
**kw
/opt/hostedtoolcache/Python/3.7.11/x64/lib/python3.7/site-packages/sqlalchemy/sql/coercions.py:283: in _raise_for_expected
util.raise_(exc.ArgumentError(msg, code=code), replace_context=err)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
def raise_(
exception, with_traceback=None, replace_context=None, from_=False
):
r"""implement "raise" with cause support.
:param exception: exception to raise
:param with_traceback: will call exception.with_traceback()
:param replace_context: an as-yet-unsupported feature. This is
an exception object which we are "replacing", e.g., it's our
"cause" but we don't want it printed. Basically just what
``__suppress_context__`` does but we don't want to suppress
the enclosing context, if any. So for now we make it the
cause.
:param from\_: the cause. this actually sets the cause and doesn't
hope to hide it someday.
"""
if with_traceback is not None:
exception = exception.with_traceback(with_traceback)
if from_ is not False:
exception.__cause__ = from_
elif replace_context is not None:
# no good solution here, we would like to have the exception
# have only the context of replace_context.__context__ so that the
# intermediary exception does not change, but we can't figure
# that out.
exception.__cause__ = replace_context
try:
> raise exception
E sqlalchemy.exc.ArgumentError: subject table for an INSERT, UPDATE or DELETE expected, got Column('hashkey', String(), table=<db_object>, nullable=False). |
chrisjsewell
force-pushed
the
sqla-1.4
branch
from
de1ae25
to
dcf10d5
Compare
Codecov Report
@@ Coverage Diff @@
## develop #5103 +/- ##
===========================================
+ Coverage 80.76% 80.81% +0.06%
===========================================
Files 534 534
Lines 36962 36946 -16
===========================================
+ Hits 29847 29856 +9
+ Misses 7115 7090 -25
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
|
Ok this is ready for review |
sphuber
reviewed
Sep 8, 2021
|
|
||
| :return: boolean, True if the model is saved in the database, False otherwise | ||
| """ | ||
| return self._model.id is not None | ||
| with self._model.session.no_autoflush: |
There was a problem hiding this comment.
Why would the session be flushed when we request an attribute of a model instance?
There was a problem hiding this comment.
If you have already tried to (incorrectly) set an attribute on the model.
This failed in tests/orm/test_groups.py::TestGroups::test_rename_existing
with self.assertRaises(exceptions.IntegrityError):
group_b.label = label_group_aInstead of being caught below, by save(), and being converted to an aiida IntegrityError, it was failing earlier here
There was a problem hiding this comment.
How was it failing though? Does this have to do with the ModelWrapper we add? I though flushing would only be performed when pushing data from memory to database and not the reverse. So why would setting some attribute on the model (incorrect or not) lead to the self._model.id call resulting in a flush?
There was a problem hiding this comment.
Oops just removed again in f8ea014, because I thought I could no longer reproduce, but then realised I wasn't using the SQLA backend. So you should see it in the results of that CI run, but basically without no_autoflush, this is the error you get:
py37-sqla run-test: commands[0] | pytest tests/orm/test_groups.py::TestGroups::test_rename_existing
==================================================================================== test session starts ====================================================================================
platform darwin -- Python 3.7.10, pytest-6.2.4, py-1.10.0, pluggy-0.13.1
cachedir: .tox/py37-sqla/.pytest_cache
benchmark: 3.4.1 (defaults: timer=time.perf_counter disable_gc=False min_rounds=5 min_time=0.000005 max_time=1.0 calibration_precision=10 warmup=False warmup_iterations=100000)
rootdir: /Users/chrisjsewell/Documents/GitHub/aiida-core-origin, configfile: pyproject.toml
plugins: benchmark-3.4.1, datadir-1.3.1, asyncio-0.15.1, rerunfailures-9.1.1, regressions-2.2.0, cov-2.10.1, timeout-1.4.2
collected 1 item
tests/orm/test_groups.py FE [100%]
========================================================================================== ERRORS ===========================================================================================
___________________________________________________________________ ERROR at teardown of TestGroups.test_rename_existing ____________________________________________________________________
cls = <class 'tests.orm.test_groups.TestGroups'>
@classmethod
def tearDownClass(cls):
"""Tear down test class.
Note: Also cleans file repository.
"""
# Double check for double security to avoid to run the tearDown
# if this is not a test profile
check_if_tests_can_run()
if orm.autogroup.CURRENT_AUTOGROUP is not None:
orm.autogroup.CURRENT_AUTOGROUP.clear_group_cache()
> cls.clean_db()
aiida/backends/testbase.py:96:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
aiida/backends/testbase.py:131: in clean_db
cls.__backend_instance.clean_db()
aiida/backends/sqlalchemy/testbase.py:36: in clean_db
with self.backend.transaction() as session:
.tox/py37-sqla/lib/python3.7/contextlib.py:112: in __enter__
return next(self.gen)
aiida/orm/implementation/sqlalchemy/backend.py:88: in transaction
session.begin_nested()
.tox/py37-sqla/lib/python3.7/site-packages/sqlalchemy/orm/session.py:1365: in begin_nested
return self.begin(nested=True)
.tox/py37-sqla/lib/python3.7/site-packages/sqlalchemy/util/deprecations.py:298: in warned
return fn(*args, **kwargs)
.tox/py37-sqla/lib/python3.7/site-packages/sqlalchemy/orm/session.py:1320: in begin
trans = self._transaction._begin(nested=nested)
.tox/py37-sqla/lib/python3.7/site-packages/sqlalchemy/orm/session.py:629: in _begin
self._assert_active()
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
self = <sqlalchemy.orm.session.SessionTransaction object at 0x7f9bd3474ad0>, prepared_ok = False, rollback_ok = False, deactive_ok = False, closed_msg = 'This transaction is closed'
def _assert_active(
self,
prepared_ok=False,
rollback_ok=False,
deactive_ok=False,
closed_msg="This transaction is closed",
):
if self._state is COMMITTED:
raise sa_exc.InvalidRequestError(
"This session is in 'committed' state; no further "
"SQL can be emitted within this transaction."
)
elif self._state is PREPARED:
if not prepared_ok:
raise sa_exc.InvalidRequestError(
"This session is in 'prepared' state; no further "
"SQL can be emitted within this transaction."
)
elif self._state is DEACTIVE:
if not deactive_ok and not rollback_ok:
if self._rollback_exception:
raise sa_exc.PendingRollbackError(
"This Session's transaction has been rolled back "
"due to a previous exception during flush."
" To begin a new transaction with this Session, "
"first issue Session.rollback()."
" Original exception was: %s"
% self._rollback_exception,
> code="7s2a",
E sqlalchemy.exc.PendingRollbackError: This Session's transaction has been rolled back due to a previous exception during flush. To begin a new transaction with this Session, first issue Session.rollback(). Original exception was: (raised as a result of Query-invoked autoflush; consider using a session.no_autoflush block if this flush is occurring prematurely)
E (psycopg2.errors.UniqueViolation) duplicate key value violates unique constraint "db_dbgroup_label_type_string_key"
E DETAIL: Key (label, type_string)=(group_a, core) already exists.
E
E [SQL: UPDATE db_dbgroup SET label=%(label)s WHERE db_dbgroup.id = %(db_dbgroup_id)s]
E [parameters: {'label': 'group_a', 'db_dbgroup_id': 3}]
E (Background on this error at: https://sqlalche.me/e/14/gkpj) (Background on this error at: https://sqlalche.me/e/14/7s2a)
.tox/py37-sqla/lib/python3.7/site-packages/sqlalchemy/orm/session.py:608: PendingRollbackError
------------------------------------------------------------------------------------ Captured log setup -------------------------------------------------------------------------------------
DEBUG pgsu:__init__.py:141 Trying to connect via "psycopg2"...
========================================================================================= FAILURES ==========================================================================================
______________________________________________________________________________ TestGroups.test_rename_existing ______________________________________________________________________________
def _execute_context(
self,
dialect,
constructor,
statement,
parameters,
execution_options,
*args,
**kw
):
"""Create an :class:`.ExecutionContext` and execute, returning
a :class:`_engine.CursorResult`."""
branched = self
if self.__branch_from:
# if this is a "branched" connection, do everything in terms
# of the "root" connection, *except* for .close(), which is
# the only feature that branching provides
self = self.__branch_from
try:
conn = self._dbapi_connection
if conn is None:
conn = self._revalidate_connection()
context = constructor(
dialect, self, conn, execution_options, *args, **kw
)
except (exc.PendingRollbackError, exc.ResourceClosedError):
raise
except BaseException as e:
self._handle_dbapi_exception(
e, util.text_type(statement), parameters, None, None
)
if (
self._transaction
and not self._transaction.is_active
or (
self._nested_transaction
and not self._nested_transaction.is_active
)
):
self._invalid_transaction()
elif self._trans_context_manager:
TransactionalContext._trans_ctx_check(self)
if self._is_future and self._transaction is None:
self._autobegin()
context.pre_exec()
if dialect.use_setinputsizes:
context._set_input_sizes()
cursor, statement, parameters = (
context.cursor,
context.statement,
context.parameters,
)
if not context.executemany:
parameters = parameters[0]
if self._has_events or self.engine._has_events:
for fn in self.dispatch.before_cursor_execute:
statement, parameters = fn(
self,
cursor,
statement,
parameters,
context,
context.executemany,
)
if self._echo:
self._log_info(statement)
stats = context._get_cache_stats()
if not self.engine.hide_parameters:
self._log_info(
"[%s] %r",
stats,
sql_util._repr_params(
parameters, batches=10, ismulti=context.executemany
),
)
else:
self._log_info(
"[%s] [SQL parameters hidden due to hide_parameters=True]"
% (stats,)
)
evt_handled = False
try:
if context.executemany:
if self.dialect._has_events:
for fn in self.dialect.dispatch.do_executemany:
if fn(cursor, statement, parameters, context):
evt_handled = True
break
if not evt_handled:
self.dialect.do_executemany(
cursor, statement, parameters, context
)
elif not parameters and context.no_parameters:
if self.dialect._has_events:
for fn in self.dialect.dispatch.do_execute_no_params:
if fn(cursor, statement, context):
evt_handled = True
break
if not evt_handled:
self.dialect.do_execute_no_params(
cursor, statement, context
)
else:
if self.dialect._has_events:
for fn in self.dialect.dispatch.do_execute:
if fn(cursor, statement, parameters, context):
evt_handled = True
break
if not evt_handled:
self.dialect.do_execute(
> cursor, statement, parameters, context
)
.tox/py37-sqla/lib/python3.7/site-packages/sqlalchemy/engine/base.py:1772:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
def do_execute(self, cursor, statement, parameters, context=None):
> cursor.execute(statement, parameters)
E psycopg2.errors.UniqueViolation: duplicate key value violates unique constraint "db_dbgroup_label_type_string_key"
E DETAIL: Key (label, type_string)=(group_a, core) already exists.
.tox/py37-sqla/lib/python3.7/site-packages/sqlalchemy/engine/default.py:717: UniqueViolation
The above exception was the direct cause of the following exception:
self = <tests.orm.test_groups.TestGroups testMethod=test_rename_existing>
def test_rename_existing(self):
"""Test that renaming to an already existing name is not permitted."""
label_group_a = 'group_a'
label_group_b = 'group_b'
orm.Group(label=label_group_a, description='I am the Original G').store()
# Before storing everything should be fine
group_b = orm.Group(label=label_group_a, description='They will try to rename me')
# Storing for duplicate group name should trigger UniquenessError
with self.assertRaises(exceptions.IntegrityError):
group_b.store()
# Reverting to unique name before storing
group_b.label = label_group_b
group_b.store()
# After storing name change to existing should raise
with self.assertRaises(exceptions.IntegrityError):
> group_b.label = label_group_a
tests/orm/test_groups.py:255:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
aiida/orm/groups.py:177: in label
self._backend_entity.label = label
aiida/orm/implementation/sqlalchemy/groups.py:67: in label
self._dbmodel.label = label
aiida/orm/implementation/sqlalchemy/utils.py:78: in __setattr__
if self.is_saved() and self._is_mutable_model_field(key):
aiida/orm/implementation/sqlalchemy/utils.py:87: in is_saved
return self._model.id is not None
.tox/py37-sqla/lib/python3.7/site-packages/sqlalchemy/orm/attributes.py:481: in __get__
return self.impl.get(state, dict_)
.tox/py37-sqla/lib/python3.7/site-packages/sqlalchemy/orm/attributes.py:926: in get
value = self._fire_loader_callables(state, key, passive)
.tox/py37-sqla/lib/python3.7/site-packages/sqlalchemy/orm/attributes.py:957: in _fire_loader_callables
return state._load_expired(state, passive)
.tox/py37-sqla/lib/python3.7/site-packages/sqlalchemy/orm/state.py:710: in _load_expired
self.manager.expired_attribute_loader(self, toload, passive)
.tox/py37-sqla/lib/python3.7/site-packages/sqlalchemy/orm/loading.py:1483: in load_scalar_attributes
no_autoflush=no_autoflush,
.tox/py37-sqla/lib/python3.7/site-packages/sqlalchemy/orm/loading.py:411: in load_on_ident
execution_options=execution_options,
.tox/py37-sqla/lib/python3.7/site-packages/sqlalchemy/orm/loading.py:561: in load_on_pk_identity
bind_arguments=bind_arguments,
.tox/py37-sqla/lib/python3.7/site-packages/sqlalchemy/orm/session.py:1643: in execute
_parent_execute_state is not None,
.tox/py37-sqla/lib/python3.7/site-packages/sqlalchemy/orm/context.py:290: in orm_pre_session_exec
session._autoflush()
.tox/py37-sqla/lib/python3.7/site-packages/sqlalchemy/orm/session.py:2204: in _autoflush
util.raise_(e, with_traceback=sys.exc_info()[2])
.tox/py37-sqla/lib/python3.7/site-packages/sqlalchemy/util/compat.py:207: in raise_
raise exception
.tox/py37-sqla/lib/python3.7/site-packages/sqlalchemy/orm/session.py:2193: in _autoflush
self.flush()
.tox/py37-sqla/lib/python3.7/site-packages/sqlalchemy/orm/session.py:3298: in flush
self._flush(objects)
.tox/py37-sqla/lib/python3.7/site-packages/sqlalchemy/orm/session.py:3438: in _flush
transaction.rollback(_capture_exception=True)
.tox/py37-sqla/lib/python3.7/site-packages/sqlalchemy/util/langhelpers.py:72: in __exit__
with_traceback=exc_tb,
.tox/py37-sqla/lib/python3.7/site-packages/sqlalchemy/util/compat.py:207: in raise_
raise exception
.tox/py37-sqla/lib/python3.7/site-packages/sqlalchemy/orm/session.py:3398: in _flush
flush_context.execute()
.tox/py37-sqla/lib/python3.7/site-packages/sqlalchemy/orm/unitofwork.py:456: in execute
rec.execute(self)
.tox/py37-sqla/lib/python3.7/site-packages/sqlalchemy/orm/unitofwork.py:633: in execute
uow,
.tox/py37-sqla/lib/python3.7/site-packages/sqlalchemy/orm/persistence.py:239: in save_obj
update,
.tox/py37-sqla/lib/python3.7/site-packages/sqlalchemy/orm/persistence.py:999: in _emit_update_statements
statement, multiparams, execution_options=execution_options
.tox/py37-sqla/lib/python3.7/site-packages/sqlalchemy/engine/base.py:1583: in _execute_20
return meth(self, args_10style, kwargs_10style, execution_options)
.tox/py37-sqla/lib/python3.7/site-packages/sqlalchemy/sql/elements.py:324: in _execute_on_connection
self, multiparams, params, execution_options
.tox/py37-sqla/lib/python3.7/site-packages/sqlalchemy/engine/base.py:1462: in _execute_clauseelement
cache_hit=cache_hit,
.tox/py37-sqla/lib/python3.7/site-packages/sqlalchemy/engine/base.py:1815: in _execute_context
e, statement, parameters, cursor, context
.tox/py37-sqla/lib/python3.7/site-packages/sqlalchemy/engine/base.py:1996: in _handle_dbapi_exception
sqlalchemy_exception, with_traceback=exc_info[2], from_=e
.tox/py37-sqla/lib/python3.7/site-packages/sqlalchemy/util/compat.py:207: in raise_
raise exception
.tox/py37-sqla/lib/python3.7/site-packages/sqlalchemy/engine/base.py:1772: in _execute_context
cursor, statement, parameters, context
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
def do_execute(self, cursor, statement, parameters, context=None):
> cursor.execute(statement, parameters)
E sqlalchemy.exc.IntegrityError: (raised as a result of Query-invoked autoflush; consider using a session.no_autoflush block if this flush is occurring prematurely)
E (psycopg2.errors.UniqueViolation) duplicate key value violates unique constraint "db_dbgroup_label_type_string_key"
E DETAIL: Key (label, type_string)=(group_a, core) already exists.
E
E [SQL: UPDATE db_dbgroup SET label=%(label)s WHERE db_dbgroup.id = %(db_dbgroup_id)s]
E [parameters: {'label': 'group_a', 'db_dbgroup_id': 3}]
E (Background on this error at: https://sqlalche.me/e/14/gkpj)
.tox/py37-sqla/lib/python3.7/site-packages/sqlalchemy/engine/default.py:717: IntegrityErrorThere was a problem hiding this comment.
Yeah, but I think that the solution should be different. The test is expecting to raise, specifically, it is expecting an exception related to a uniqueness error because it is trying to store a group with a label that already exists. I think what changed is simply the type of exception that is being thrown. It is checking for a exceptions.IntegrityError but now a UniqueViolation seems to be thrown. I think we should simply change the expected exception. And maybe understand why the exception has changed, but if that is to be expected after the upgrade, than that would make sense.
There was a problem hiding this comment.
It is not expecting to raise in this part of the code though; is_saved() should never except,
and it is not now raising a UniqueViolation; it is raising an sqlalchemy.exc.IntegrityError rather than an aiida.exceptions.IntegrityError, which is directly because it is excepting in is_saved(), rather than returning False, then failing later in save(), where the exception gets "swapped" (and the session is correctly rolled back):
try:
commit = not self._in_transaction()
self._model.save(commit=commit)
except IntegrityError as exception:
self._model.session.rollback()
raise exceptions.IntegrityError(str(exception))I feel, this is definitely the correct solution
There was a problem hiding this comment.
it basically goes __setattr__ -> is_saved (False)-> _flush -> save, and should only flush/except when it gets to save
There was a problem hiding this comment.
is_saved() should never except, and it is not now raising a UniqueViolation
That is my point though. I realize this is not your doing but sqlalchemy's design, but to me it seems really weird that simply accessing a models attribute causes the session to be flushed. I am sure they have a reasoning that I am not seeing but clearly this results in weird behavior. Fine to keep the change but please add a comment why this no_autoflush is needed here because it is not at all obvious
There was a problem hiding this comment.
added comment
realize this is not your doing but sqlalchemy's design, but to me it seems really weird that simply accessing a models attribute causes the session to be flushed.
well this is likely more to do with our overrides of the __getattr__, __setattr__ methods, which force this behaviour; remember this is all actually happening during the (incorrect) setting of an attribute
chrisjsewell
commented
Sep 8, 2021
|
addressed all your comments thanks @sphuber |
|
back to you @sphuber |
chrisjsewell
force-pushed
the
sqla-1.4
branch
from
05c790b
to
a8b3265
Compare
This was referenced Sep 8, 2021
|
If you add a comment at the |
chrisjsewell
commented
Sep 9, 2021
sphuber
reviewed
Sep 9, 2021
Comment on lines
+87
to
+88
| # we should not flush here since it may lead to IntegrityErrors | ||
| # which are handled later in the save method |
There was a problem hiding this comment.
Suggested change
| # we should not flush here since it may lead to IntegrityErrors | |
| # which are handled later in the save method | |
| # We should disable the automatic flush here, which is enabled by default in SqlAlchemy | |
| # and each query (including just fetching an attribute) will flush the session. However, here | |
| # a flush may lead to `IntegrityError`'s, which should instead be handled in the `save` method. |
There was a problem hiding this comment.
including just fetching an attribute
as I've mentioned above, this is not case; it is because of our overrides of the __getattr__, __setattr__ methods, and the fact we are calling this fetch "within" a setattr action
There was a problem hiding this comment.
Are you sure though? Since the id is an immutable property, it should not result in a database hit. We have an exception for this in the ModelWrapper. So if what I am saying is incorrect, then why is it flushing? That is the thing that I want to add here.
There was a problem hiding this comment.
Anyway, I will approve now and let you improve the message as you see fit. I am still extremely befuddled why exactly this is happening.
There was a problem hiding this comment.
Are you sure though? Since the id is an immutable property, it should not result in a database hit.
yep pretty sure (having recently watched a 3 hour video on sqla lol, which I highly recommend https://youtu.be/1Va493SMTcY); in the failing test line group_b.label = label_group_a, the first thing we do (before reaching is_saved) is set the attribute on the model (setattr(self._model, key, value)). Sqlachemy has now marked the model as "dirty" and is primed to flush on the next getattr, which we then do with self._model.id.
Note you could add: sessionmaker(bind=engine, autoflush=False), but I would be wary of changing this
Very simply, this is what is happening:
In [0]: import sqlalchemy as sqla
...: @mapper_registry.mapped
...: class User:
...: __tablename__ = "user"
...: id = sqla.Column(sqla.Integer, primary_key=True)
...: name = sqla.Column(sqla.String)
...:
In [0]: engine = sqla.create_engine("sqlite://", echo=True)
In [0]: with engine.begin() as conn:
...: mapper_registry.metadata.create_all(conn)
...:
In [0]: Session = sqla.orm.sessionmaker(bind=engine)
In [0]: session = Session()
In [0]: user = User()
In [0]: session.add(user)
In [1]: session.commit()
2021-09-09 12:24:15,193 INFO sqlalchemy.engine.Engine BEGIN (implicit)
2021-09-09 12:24:15,198 INFO sqlalchemy.engine.Engine INSERT INTO user (name) VALUES (?)
2021-09-09 12:24:15,199 INFO sqlalchemy.engine.Engine [cached since 1559s ago] (None,)
2021-09-09 12:24:15,205 INFO sqlalchemy.engine.Engine COMMIT
In [2]: session.dirty
Out[2]: IdentitySet([])
In [3]: user.name = object()
In [4]: session.dirty
Out[4]: IdentitySet([<__main__.User object at 0x7f859fd25d00>])
In [5]: user.id
2021-09-09 12:13:25,521 INFO sqlalchemy.engine.Engine BEGIN (implicit)
2021-09-09 12:13:25,522 INFO sqlalchemy.engine.Engine SELECT user.id AS user_id
FROM user
WHERE user.id = ?
2021-09-09 12:13:25,523 INFO sqlalchemy.engine.Engine [cached since 527.9s ago] (1,)
2021-09-09 12:13:25,523 INFO sqlalchemy.engine.Engine UPDATE user SET name=? WHERE user.id = ?
2021-09-09 12:13:25,524 INFO sqlalchemy.engine.Engine [cached since 654.2s ago] (<object object at 0x7f85a2f51bf0>, 1)
2021-09-09 12:13:25,524 INFO sqlalchemy.engine.Engine ROLLBACK
---------------------------------------------------------------------------
InterfaceError Traceback (most recent call last)
~/Documents/GitHub/disk-objectstore/.tox/py38/lib/python3.8/site-packages/sqlalchemy/engine/base.py in _execute_context(self, dialect, constructor, statement, parameters, execution_options, *args, **kw)
1770 if not evt_handled:
-> 1771 self.dialect.do_execute(
1772 cursor, statement, parameters, context
~/Documents/GitHub/disk-objectstore/.tox/py38/lib/python3.8/site-packages/sqlalchemy/engine/default.py in do_execute(self, cursor, statement, parameters, context)
716 def do_execute(self, cursor, statement, parameters, context=None):
--> 717 cursor.execute(statement, parameters)
718
InterfaceError: Error binding parameter 0 - probably unsupported type.
The above exception was the direct cause of the following exception:
InterfaceError Traceback (most recent call last)
<ipython-input-85-63cd6cde9a85> in <module>
----> 1 user.id
~/Documents/GitHub/disk-objectstore/.tox/py38/lib/python3.8/site-packages/sqlalchemy/orm/attributes.py in __get__(self, instance, owner)
479 replace_context=err,
480 )
--> 481 return self.impl.get(state, dict_)
482
483
~/Documents/GitHub/disk-objectstore/.tox/py38/lib/python3.8/site-packages/sqlalchemy/orm/attributes.py in get(self, state, dict_, passive)
924 return PASSIVE_NO_RESULT
925
--> 926 value = self._fire_loader_callables(state, key, passive)
927
928 if value is PASSIVE_NO_RESULT or value is NO_VALUE:
~/Documents/GitHub/disk-objectstore/.tox/py38/lib/python3.8/site-packages/sqlalchemy/orm/attributes.py in _fire_loader_callables(self, state, key, passive)
955 and key in state.expired_attributes
956 ):
--> 957 return state._load_expired(state, passive)
958 elif key in state.callables:
959 callable_ = state.callables[key]
~/Documents/GitHub/disk-objectstore/.tox/py38/lib/python3.8/site-packages/sqlalchemy/orm/state.py in _load_expired(self, state, passive)
708 )
709
--> 710 self.manager.expired_attribute_loader(self, toload, passive)
711
712 # if the loader failed, or this
~/Documents/GitHub/disk-objectstore/.tox/py38/lib/python3.8/site-packages/sqlalchemy/orm/loading.py in load_scalar_attributes(mapper, state, attribute_names, passive)
1443 return
1444
-> 1445 result = load_on_ident(
1446 session,
1447 future.select(mapper).set_label_style(
~/Documents/GitHub/disk-objectstore/.tox/py38/lib/python3.8/site-packages/sqlalchemy/orm/loading.py in load_on_ident(session, statement, key, load_options, refresh_state, with_for_update, only_load_props, no_autoflush, bind_arguments, execution_options)
399 ident = identity_token = None
400
--> 401 return load_on_pk_identity(
402 session,
403 statement,
~/Documents/GitHub/disk-objectstore/.tox/py38/lib/python3.8/site-packages/sqlalchemy/orm/loading.py in load_on_pk_identity(session, statement, primary_key_identity, load_options, refresh_state, with_for_update, only_load_props, identity_token, no_autoflush, bind_arguments, execution_options)
522 )
523 result = (
--> 524 session.execute(
525 q,
526 params=params,
~/Documents/GitHub/disk-objectstore/.tox/py38/lib/python3.8/site-packages/sqlalchemy/orm/session.py in execute(self, statement, params, execution_options, bind_arguments, _parent_execute_state, _add_event, **kw)
1635 statement,
1636 execution_options,
-> 1637 ) = compile_state_cls.orm_pre_session_exec(
1638 self,
1639 statement,
~/Documents/GitHub/disk-objectstore/.tox/py38/lib/python3.8/site-packages/sqlalchemy/orm/context.py in orm_pre_session_exec(cls, session, statement, params, execution_options, bind_arguments, is_reentrant_invoke)
290
291 if load_options._autoflush:
--> 292 session._autoflush()
293
294 return statement, execution_options
~/Documents/GitHub/disk-objectstore/.tox/py38/lib/python3.8/site-packages/sqlalchemy/orm/session.py in _autoflush(self)
2202 "flush is occurring prematurely"
2203 )
-> 2204 util.raise_(e, with_traceback=sys.exc_info()[2])
2205
2206 def refresh(self, instance, attribute_names=None, with_for_update=None):
~/Documents/GitHub/disk-objectstore/.tox/py38/lib/python3.8/site-packages/sqlalchemy/util/compat.py in raise_(***failed resolving arguments***)
205
206 try:
--> 207 raise exception
208 finally:
209 # credit to
~/Documents/GitHub/disk-objectstore/.tox/py38/lib/python3.8/site-packages/sqlalchemy/orm/session.py in _autoflush(self)
2191 if self.autoflush and not self._flushing:
2192 try:
-> 2193 self.flush()
2194 except sa_exc.StatementError as e:
2195 # note we are reraising StatementError as opposed to
~/Documents/GitHub/disk-objectstore/.tox/py38/lib/python3.8/site-packages/sqlalchemy/orm/session.py in flush(self, objects)
3296 try:
3297 self._flushing = True
-> 3298 self._flush(objects)
3299 finally:
3300 self._flushing = False
~/Documents/GitHub/disk-objectstore/.tox/py38/lib/python3.8/site-packages/sqlalchemy/orm/session.py in _flush(self, objects)
3436 except:
3437 with util.safe_reraise():
-> 3438 transaction.rollback(_capture_exception=True)
3439
3440 def bulk_save_objects(
~/Documents/GitHub/disk-objectstore/.tox/py38/lib/python3.8/site-packages/sqlalchemy/util/langhelpers.py in __exit__(self, type_, value, traceback)
68 self._exc_info = None # remove potential circular references
69 if not self.warn_only:
---> 70 compat.raise_(
71 exc_value,
72 with_traceback=exc_tb,
~/Documents/GitHub/disk-objectstore/.tox/py38/lib/python3.8/site-packages/sqlalchemy/util/compat.py in raise_(***failed resolving arguments***)
205
206 try:
--> 207 raise exception
208 finally:
209 # credit to
~/Documents/GitHub/disk-objectstore/.tox/py38/lib/python3.8/site-packages/sqlalchemy/orm/session.py in _flush(self, objects)
3396 self._warn_on_events = True
3397 try:
-> 3398 flush_context.execute()
3399 finally:
3400 self._warn_on_events = False
~/Documents/GitHub/disk-objectstore/.tox/py38/lib/python3.8/site-packages/sqlalchemy/orm/unitofwork.py in execute(self)
454 else:
455 for rec in topological.sort(self.dependencies, postsort_actions):
--> 456 rec.execute(self)
457
458 def finalize_flush_changes(self):
~/Documents/GitHub/disk-objectstore/.tox/py38/lib/python3.8/site-packages/sqlalchemy/orm/unitofwork.py in execute(self, uow)
628 @util.preload_module("sqlalchemy.orm.persistence")
629 def execute(self, uow):
--> 630 util.preloaded.orm_persistence.save_obj(
631 self.mapper,
632 uow.states_for_mapper_hierarchy(self.mapper, False, False),
~/Documents/GitHub/disk-objectstore/.tox/py38/lib/python3.8/site-packages/sqlalchemy/orm/persistence.py in save_obj(base_mapper, states, uowtransaction, single)
232 )
233
--> 234 _emit_update_statements(
235 base_mapper,
236 uowtransaction,
~/Documents/GitHub/disk-objectstore/.tox/py38/lib/python3.8/site-packages/sqlalchemy/orm/persistence.py in _emit_update_statements(base_mapper, uowtransaction, mapper, table, update, bookkeeping)
996 )
997
--> 998 c = connection._execute_20(
999 statement, multiparams, execution_options=execution_options
1000 )
~/Documents/GitHub/disk-objectstore/.tox/py38/lib/python3.8/site-packages/sqlalchemy/engine/base.py in _execute_20(self, statement, parameters, execution_options)
1581 )
1582 else:
-> 1583 return meth(self, args_10style, kwargs_10style, execution_options)
1584
1585 def exec_driver_sql(
~/Documents/GitHub/disk-objectstore/.tox/py38/lib/python3.8/site-packages/sqlalchemy/sql/elements.py in _execute_on_connection(self, connection, multiparams, params, execution_options, _force)
321 ):
322 if _force or self.supports_execution:
--> 323 return connection._execute_clauseelement(
324 self, multiparams, params, execution_options
325 )
~/Documents/GitHub/disk-objectstore/.tox/py38/lib/python3.8/site-packages/sqlalchemy/engine/base.py in _execute_clauseelement(self, elem, multiparams, params, execution_options)
1450 linting=self.dialect.compiler_linting | compiler.WARN_LINTING,
1451 )
-> 1452 ret = self._execute_context(
1453 dialect,
1454 dialect.execution_ctx_cls._init_compiled,
~/Documents/GitHub/disk-objectstore/.tox/py38/lib/python3.8/site-packages/sqlalchemy/engine/base.py in _execute_context(self, dialect, constructor, statement, parameters, execution_options, *args, **kw)
1812
1813 except BaseException as e:
-> 1814 self._handle_dbapi_exception(
1815 e, statement, parameters, cursor, context
1816 )
~/Documents/GitHub/disk-objectstore/.tox/py38/lib/python3.8/site-packages/sqlalchemy/engine/base.py in _handle_dbapi_exception(self, e, statement, parameters, cursor, context)
1993 util.raise_(newraise, with_traceback=exc_info[2], from_=e)
1994 elif should_wrap:
-> 1995 util.raise_(
1996 sqlalchemy_exception, with_traceback=exc_info[2], from_=e
1997 )
~/Documents/GitHub/disk-objectstore/.tox/py38/lib/python3.8/site-packages/sqlalchemy/util/compat.py in raise_(***failed resolving arguments***)
205
206 try:
--> 207 raise exception
208 finally:
209 # credit to
~/Documents/GitHub/disk-objectstore/.tox/py38/lib/python3.8/site-packages/sqlalchemy/engine/base.py in _execute_context(self, dialect, constructor, statement, parameters, execution_options, *args, **kw)
1769 break
1770 if not evt_handled:
-> 1771 self.dialect.do_execute(
1772 cursor, statement, parameters, context
1773 )
~/Documents/GitHub/disk-objectstore/.tox/py38/lib/python3.8/site-packages/sqlalchemy/engine/default.py in do_execute(self, cursor, statement, parameters, context)
715
716 def do_execute(self, cursor, statement, parameters, context=None):
--> 717 cursor.execute(statement, parameters)
718
719 def do_execute_no_params(self, cursor, statement, context=None):
InterfaceError: (raised as a result of Query-invoked autoflush; consider using a session.no_autoflush block if this flush is occurring prematurely)
(sqlite3.InterfaceError) Error binding parameter 0 - probably unsupported type.
[SQL: UPDATE user SET name=? WHERE user.id = ?]
[parameters: (<object object at 0x7f85a2f51bf0>, 1)]
(Background on this error at: https://sqlalche.me/e/14/rvf5)whereas we want:
In [87]: session.commit()
In [88]: session.dirty
Out[88]: IdentitySet([])
In [89]: user.name = object()
In [90]: session.dirty
Out[90]: IdentitySet([<__main__.User object at 0x7f859fd25d00>])
In [91]: with session.no_autoflush:
...: print(user.id)
...:
2021-09-09 12:17:07,045 INFO sqlalchemy.engine.Engine BEGIN (implicit)
2021-09-09 12:17:07,045 INFO sqlalchemy.engine.Engine SELECT user.id AS user_id
FROM user
WHERE user.id = ?
2021-09-09 12:17:07,045 INFO sqlalchemy.engine.Engine [cached since 749.4s ago] (1,)
1
In [92]: session.commit()
2021-09-09 12:17:13,939 INFO sqlalchemy.engine.Engine UPDATE user SET name=? WHERE user.id = ?
2021-09-09 12:17:13,939 INFO sqlalchemy.engine.Engine [cached since 882.6s ago] (<object object at 0x7f85a2f51b00>, 1)
2021-09-09 12:17:13,940 INFO sqlalchemy.engine.Engine ROLLBACK
---------------------------------------------------------------------------
InterfaceError Traceback (most recent call last)
~/Documents/GitHub/disk-objectstore/.tox/py38/lib/python3.8/site-packages/sqlalchemy/engine/base.py in _execute_context(self, dialect, constructor, statement, parameters, execution_options, *args, **kw)
1770 if not evt_handled:
-> 1771 self.dialect.do_execute(
1772 cursor, statement, parameters, context
~/Documents/GitHub/disk-objectstore/.tox/py38/lib/python3.8/site-packages/sqlalchemy/engine/default.py in do_execute(self, cursor, statement, parameters, context)
716 def do_execute(self, cursor, statement, parameters, context=None):
--> 717 cursor.execute(statement, parameters)
718
InterfaceError: Error binding parameter 0 - probably unsupported type.
The above exception was the direct cause of the following exception:
InterfaceError Traceback (most recent call last)
<ipython-input-92-83a1dbdbd92a> in <module>
----> 1 session.commit()
~/Documents/GitHub/disk-objectstore/.tox/py38/lib/python3.8/site-packages/sqlalchemy/orm/session.py in commit(self)
1426 raise sa_exc.InvalidRequestError("No transaction is begun.")
1427
-> 1428 self._transaction.commit(_to_root=self.future)
1429
1430 def prepare(self):
~/Documents/GitHub/disk-objectstore/.tox/py38/lib/python3.8/site-packages/sqlalchemy/orm/session.py in commit(self, _to_root)
827 self._assert_active(prepared_ok=True)
828 if self._state is not PREPARED:
--> 829 self._prepare_impl()
830
831 if self._parent is None or self.nested:
~/Documents/GitHub/disk-objectstore/.tox/py38/lib/python3.8/site-packages/sqlalchemy/orm/session.py in _prepare_impl(self)
806 if self.session._is_clean():
807 break
--> 808 self.session.flush()
809 else:
810 raise exc.FlushError(
~/Documents/GitHub/disk-objectstore/.tox/py38/lib/python3.8/site-packages/sqlalchemy/orm/session.py in flush(self, objects)
3296 try:
3297 self._flushing = True
-> 3298 self._flush(objects)
3299 finally:
3300 self._flushing = False
~/Documents/GitHub/disk-objectstore/.tox/py38/lib/python3.8/site-packages/sqlalchemy/orm/session.py in _flush(self, objects)
3436 except:
3437 with util.safe_reraise():
-> 3438 transaction.rollback(_capture_exception=True)
3439
3440 def bulk_save_objects(
~/Documents/GitHub/disk-objectstore/.tox/py38/lib/python3.8/site-packages/sqlalchemy/util/langhelpers.py in __exit__(self, type_, value, traceback)
68 self._exc_info = None # remove potential circular references
69 if not self.warn_only:
---> 70 compat.raise_(
71 exc_value,
72 with_traceback=exc_tb,
~/Documents/GitHub/disk-objectstore/.tox/py38/lib/python3.8/site-packages/sqlalchemy/util/compat.py in raise_(***failed resolving arguments***)
205
206 try:
--> 207 raise exception
208 finally:
209 # credit to
~/Documents/GitHub/disk-objectstore/.tox/py38/lib/python3.8/site-packages/sqlalchemy/orm/session.py in _flush(self, objects)
3396 self._warn_on_events = True
3397 try:
-> 3398 flush_context.execute()
3399 finally:
3400 self._warn_on_events = False
~/Documents/GitHub/disk-objectstore/.tox/py38/lib/python3.8/site-packages/sqlalchemy/orm/unitofwork.py in execute(self)
454 else:
455 for rec in topological.sort(self.dependencies, postsort_actions):
--> 456 rec.execute(self)
457
458 def finalize_flush_changes(self):
~/Documents/GitHub/disk-objectstore/.tox/py38/lib/python3.8/site-packages/sqlalchemy/orm/unitofwork.py in execute(self, uow)
628 @util.preload_module("sqlalchemy.orm.persistence")
629 def execute(self, uow):
--> 630 util.preloaded.orm_persistence.save_obj(
631 self.mapper,
632 uow.states_for_mapper_hierarchy(self.mapper, False, False),
~/Documents/GitHub/disk-objectstore/.tox/py38/lib/python3.8/site-packages/sqlalchemy/orm/persistence.py in save_obj(base_mapper, states, uowtransaction, single)
232 )
233
--> 234 _emit_update_statements(
235 base_mapper,
236 uowtransaction,
~/Documents/GitHub/disk-objectstore/.tox/py38/lib/python3.8/site-packages/sqlalchemy/orm/persistence.py in _emit_update_statements(base_mapper, uowtransaction, mapper, table, update, bookkeeping)
996 )
997
--> 998 c = connection._execute_20(
999 statement, multiparams, execution_options=execution_options
1000 )
~/Documents/GitHub/disk-objectstore/.tox/py38/lib/python3.8/site-packages/sqlalchemy/engine/base.py in _execute_20(self, statement, parameters, execution_options)
1581 )
1582 else:
-> 1583 return meth(self, args_10style, kwargs_10style, execution_options)
1584
1585 def exec_driver_sql(
~/Documents/GitHub/disk-objectstore/.tox/py38/lib/python3.8/site-packages/sqlalchemy/sql/elements.py in _execute_on_connection(self, connection, multiparams, params, execution_options, _force)
321 ):
322 if _force or self.supports_execution:
--> 323 return connection._execute_clauseelement(
324 self, multiparams, params, execution_options
325 )
~/Documents/GitHub/disk-objectstore/.tox/py38/lib/python3.8/site-packages/sqlalchemy/engine/base.py in _execute_clauseelement(self, elem, multiparams, params, execution_options)
1450 linting=self.dialect.compiler_linting | compiler.WARN_LINTING,
1451 )
-> 1452 ret = self._execute_context(
1453 dialect,
1454 dialect.execution_ctx_cls._init_compiled,
~/Documents/GitHub/disk-objectstore/.tox/py38/lib/python3.8/site-packages/sqlalchemy/engine/base.py in _execute_context(self, dialect, constructor, statement, parameters, execution_options, *args, **kw)
1812
1813 except BaseException as e:
-> 1814 self._handle_dbapi_exception(
1815 e, statement, parameters, cursor, context
1816 )
~/Documents/GitHub/disk-objectstore/.tox/py38/lib/python3.8/site-packages/sqlalchemy/engine/base.py in _handle_dbapi_exception(self, e, statement, parameters, cursor, context)
1993 util.raise_(newraise, with_traceback=exc_info[2], from_=e)
1994 elif should_wrap:
-> 1995 util.raise_(
1996 sqlalchemy_exception, with_traceback=exc_info[2], from_=e
1997 )
~/Documents/GitHub/disk-objectstore/.tox/py38/lib/python3.8/site-packages/sqlalchemy/util/compat.py in raise_(***failed resolving arguments***)
205
206 try:
--> 207 raise exception
208 finally:
209 # credit to
~/Documents/GitHub/disk-objectstore/.tox/py38/lib/python3.8/site-packages/sqlalchemy/engine/base.py in _execute_context(self, dialect, constructor, statement, parameters, execution_options, *args, **kw)
1769 break
1770 if not evt_handled:
-> 1771 self.dialect.do_execute(
1772 cursor, statement, parameters, context
1773 )
~/Documents/GitHub/disk-objectstore/.tox/py38/lib/python3.8/site-packages/sqlalchemy/engine/default.py in do_execute(self, cursor, statement, parameters, context)
715
716 def do_execute(self, cursor, statement, parameters, context=None):
--> 717 cursor.execute(statement, parameters)
718
719 def do_execute_no_params(self, cursor, statement, context=None):
InterfaceError: (sqlite3.InterfaceError) Error binding parameter 0 - probably unsupported type.
[SQL: UPDATE user SET name=? WHERE user.id = ?]
[parameters: (<object object at 0x7f85a2f51b00>, 1)]
(Background on this error at: https://sqlalche.me/e/14/rvf5)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR updates sqlalchemy to version 1.4, aldjemy to v2, disk-objectstore to v0.6, and adds the mypy plugin for sqlalchemy.
It will also allow for a move to the v2 API, but this will be done in a subsequent PR.
The existing
SqlaQueryBuilder._buildimplementation included a number of "hacks" on protected attributes, which no longer work in v1.4 and had to be changed:Firstly, the query must be initialised with an entity/column, however, the first entity in the query path may not actually have any associated projections (e.g.
qb.append(Node).append(Node)only projects the second entity).Previously, this was addressed by removing it from the entity list (
self._query._entities.pop(0)), after constructing the query._entitiesis no longer an attribute of the query and, as shown in #5069 (comment), trying something equivalent in 1.4 leads to malformed SQL.Instead, we only add the
idcolumn as the initial column, then discard this when returning the results. This leads to a much cleaner solution, which should not break in the futureSecondly, the query must be set to turn-off de-duplication (sqlalchemy/sqlalchemy#4395 (comment)).
This again no longer works in 1.4. With the v2 API this is off by default, but for this PR I have added a patch on
ORMCompileStateto achieve this.Not ideal, but I would not get too hung up about it, since it will be removed in a follow-up v2 API PR.
A new warning is being emitted (and currently silenced), for both Django and SQLAlchemy backends, which could point to an existing bug in our DB models:
(will address this outside this PR)
The upgrade to aldjemy v2 required one change: aldjemy/aldjemy#188.
It was hoped that it would fix the aliased creation warning above (aldjemy/aldjemy#159), but this is not the case.
Supersedes #5069