Skip to content

Commit

Permalink
⬆️ UPGRADE: sqlalchemy v1.4 (v1 API) (#5103)
Browse files Browse the repository at this point in the history
This commit 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](https://docs.sqlalchemy.org/en/14/glossary.html#term-1),
but this will be done in a subsequent PR.

The existing `SqlaQueryBuilder._build` implementation 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. `_entities` is no longer an attribute of the query and
trying something equivalent in 1.4 leads to malformed SQL.
Instead, we only add the `id` column as the initial column,
then discard this when returning the results.
This leads to a much cleaner solution, which should not break in the future

Secondly, the query must be set to turn-off de-duplication (sqlalchemy/sqlalchemy#4395 (comment)).
This again no longer works in 1.4.
For this commit, a patch on `ORMCompileState` has been added to achieve this.
Not ideal, but with the v2 API this is off by default and so the patch can be eventually removed.

The upgrade to aldjemy v2 required one change: see aldjemy/aldjemy#188.
  • Loading branch information
chrisjsewell authored Sep 9, 2021
1 parent 42c5ab1 commit d33ae8a
Show file tree
Hide file tree
Showing 25 changed files with 247 additions and 247 deletions.
1 change: 1 addition & 0 deletions .github/workflows/ci-code.yml
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ jobs:
- name: Run test suite
env:
AIIDA_TEST_BACKEND: ${{ matrix.backend }}
SQLALCHEMY_WARN_20: 1
run:
.github/workflows/tests.sh

Expand Down
4 changes: 2 additions & 2 deletions aiida/backends/sqlalchemy/manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import os
import contextlib

import sqlalchemy
from sqlalchemy.orm.exc import NoResultFound

from aiida.backends.sqlalchemy import get_scoped_session
Expand Down Expand Up @@ -149,8 +150,7 @@ def validate_table_existence(self):
:raises: `~aiida.common.exceptions.NotExistent` if the settings table does not exist
"""
from sqlalchemy.engine import reflection
inspector = reflection.Inspector.from_engine(get_scoped_session().bind)
inspector = sqlalchemy.inspect(get_scoped_session().bind)
if self.table_name not in inspector.get_table_names():
raise NotExistent('the settings table does not exist')

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
"""
from alembic import op
from sqlalchemy.engine.reflection import Inspector
import sqlalchemy

# revision identifiers, used by Alembic.
revision = '59edaf8a8b79'
Expand All @@ -29,7 +29,7 @@ def upgrade():
"""Migrations for the upgrade."""
# Check if constraint uix_dbnode_id_dbgroup_id of migration 7a6587e16f4c
# is there and if yes, drop it
insp = Inspector.from_engine(op.get_bind())
insp = sqlalchemy.inspect(op.get_bind())
for constr in insp.get_unique_constraints('db_dbgroup_dbnodes'):
if constr['name'] == 'uix_dbnode_id_dbgroup_id':
op.drop_constraint('uix_dbnode_id_dbgroup_id', 'db_dbgroup_dbnodes')
Expand Down
2 changes: 1 addition & 1 deletion aiida/backends/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ def create_sqlalchemy_engine(profile, **kwargs):
name=profile.database_name
)
return create_engine(
engine_url, json_serializer=json.dumps, json_deserializer=json.loads, encoding='utf-8', **kwargs
engine_url, json_serializer=json.dumps, json_deserializer=json.loads, future=False, encoding='utf-8', **kwargs
)


Expand Down
4 changes: 1 addition & 3 deletions aiida/orm/implementation/django/querybuilder.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,6 @@
# For further information please visit http://www.aiida.net #
###########################################################################
"""Django query builder implementation"""
from aldjemy import core

from aiida.backends.djsite.db import models
from aiida.orm.implementation.sqlalchemy.querybuilder import SqlaQueryBuilder

Expand Down Expand Up @@ -58,7 +56,7 @@ def Log(self):

@property
def table_groups_nodes(self):
return core.Cache.meta.tables['db_dbgroup_dbnodes']
return models.DbGroup.sa.table.metadata.tables['db_dbgroup_dbnodes'] # pylint: disable=no-member

def modify_expansions(self, alias, expansions):
"""
Expand Down
10 changes: 0 additions & 10 deletions aiida/orm/implementation/querybuilder.py
Original file line number Diff line number Diff line change
Expand Up @@ -105,16 +105,6 @@ def __init__(self, backend: 'Backend'):
type_check(backend, backends.Backend)
self._backend = backend

@abc.abstractmethod
def yield_per(self, data: QueryDictType, batch_size: int):
"""
:param int batch_size: Number of rows to yield per step
Yields *count* rows at a time
:returns: a generator
"""

@abc.abstractmethod
def count(self, data: QueryDictType) -> int:
"""Return the number of results of the query"""
Expand Down
8 changes: 4 additions & 4 deletions aiida/orm/implementation/sqlalchemy/querybuilder/joiner.py
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ def _join_descendants_recursive(
selection_walk_list = [
link1.input_id.label('ancestor_id'),
link1.output_id.label('descendant_id'),
type_cast(0, Integer).label('depth'),
type_cast(0, Integer).label('depth'), # type: ignore[type-var]
]
if expand_path:
selection_walk_list.append(array((link1.input_id, link1.output_id)).label('path'))
Expand All @@ -207,7 +207,7 @@ def _join_descendants_recursive(
selection_union_list = [
aliased_walk.c.ancestor_id.label('ancestor_id'),
link2.output_id.label('descendant_id'),
(aliased_walk.c.depth + type_cast(1, Integer)).label('current_depth')
(aliased_walk.c.depth + type_cast(1, Integer)).label('current_depth') # type: ignore[type-var]
]
if expand_path:
selection_union_list.append((aliased_walk.c.path + array((link2.output_id,))).label('path'))
Expand Down Expand Up @@ -254,7 +254,7 @@ def _join_ancestors_recursive(
selection_walk_list = [
link1.input_id.label('ancestor_id'),
link1.output_id.label('descendant_id'),
type_cast(0, Integer).label('depth'),
type_cast(0, Integer).label('depth'), # type: ignore[type-var]
]
if expand_path:
selection_walk_list.append(array((link1.output_id, link1.input_id)).label('path'))
Expand All @@ -268,7 +268,7 @@ def _join_ancestors_recursive(
selection_union_list = [
link2.input_id.label('ancestor_id'),
aliased_walk.c.descendant_id.label('descendant_id'),
(aliased_walk.c.depth + type_cast(1, Integer)).label('current_depth'),
(aliased_walk.c.depth + type_cast(1, Integer)).label('current_depth'), # type: ignore[type-var]
]
if expand_path:
selection_union_list.append((aliased_walk.c.path + array((link2.input_id,))).label('path'))
Expand Down
Loading

0 comments on commit d33ae8a

Please sign in to comment.