Skip to content

Commit

Permalink
This resolves aiidateam#1600, by forcing SQLAlchemy to not de-duplica…
Browse files Browse the repository at this point in the history
…te rows. Therefore, the backend (psql) behavior is more closely reproduced.
  • Loading branch information
lekah committed Dec 4, 2018
1 parent 6c6b890 commit 5a50ddb
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 2 deletions.
25 changes: 23 additions & 2 deletions aiida/backends/tests/query.py
Original file line number Diff line number Diff line change
Expand Up @@ -987,9 +987,11 @@ def test_query_path(self):

class TestConsistency(AiidaTestCase):
def test_create_node_and_query(self):
"""
Testing whether creating nodes within a iterall iteration changes the results.
"""
from aiida.orm import Node
from aiida.orm.querybuilder import QueryBuilder

import random

for i in range(100):
Expand All @@ -998,12 +1000,31 @@ def test_create_node_and_query(self):

for idx, item in enumerate(QueryBuilder().append(Node, project=['id', 'label']).iterall(batch_size=10)):
if idx % 10 == 10:
print("creating new node")
n = Node()
n.store()
self.assertEqual(idx, 99)
self.assertTrue(len(QueryBuilder().append(Node, project=['id', 'label']).all(batch_size=10)) > 99)

def test_len_results(self):
"""
Test whether the len of results matches the count returned.
See also https://github.com/aiidateam/aiida_core/issues/1600
SQLAlchemy has a deduplication strategy that leads to strange behavior, tested against here
"""
from aiida.orm import Data
from aiida.orm.node.process import CalculationNode
from aiida.orm.querybuilder import QueryBuilder

parent = CalculationNode().store()
# adding 5 links going out:
for inode in range(5):
output_node = Data().store()
output_node.add_incoming(parent, link_type=LinkType.CREATE, link_label='link-{}'.format(inode))
for projection in ('id', '*'):
qb = QueryBuilder()
qb.append(CalculationNode, filters={'id':parent.id}, tag='parent', project=projection)
qb.append(Data, with_incoming='parent')
self.assertEqual(len(qb.all()), qb.count())

class TestManager(AiidaTestCase):
def test_statistics(self):
Expand Down
11 changes: 11 additions & 0 deletions aiida/orm/querybuilder.py
Original file line number Diff line number Diff line change
Expand Up @@ -1746,6 +1746,17 @@ def _build(self):
# pop the entity that I added to start the query
self._query._entities.pop(0)

# Dirty solution coming up:
# Sqlalchemy is by default de-duplicating results if possible.
# This can lead to strange results, as shown in:
# https://github.com/aiidateam/aiida_core/issues/1600
# essentially qb.count() != len(qb.all()) in some cases.
# We also addressed this with sqlachemy:
# https://github.com/sqlalchemy/sqlalchemy/issues/4395#event-2002418814
# where the following solution was sanctioned:
self._query._has_mapper_entities = False
# We should monitor SQLAlchemy, for when a solution is officially supported by the API!

# Make a list that helps the projection postprocessing
self._attrkeys_as_in_sql_result = {
index_in_sql_result: attrkey for tag, projected_entities_dict in self.tag_to_projected_entity_dict.items()
Expand Down

0 comments on commit 5a50ddb

Please sign in to comment.