Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make QueryBuilder backend agnostic #3632

Closed
3 tasks
muhrin opened this issue Dec 10, 2019 · 2 comments
Closed
3 tasks

Make QueryBuilder backend agnostic #3632

muhrin opened this issue Dec 10, 2019 · 2 comments

Comments

@muhrin
Copy link
Contributor

muhrin commented Dec 10, 2019

Currently the QueryBuilder is written with relational databases in mind, and in fact probabaly specifically with using either Django or SQLA. This is at odds with the rest of the entities in aiida.orm which could suppot any backend type which implements the general backend interfaces (found in aiida.orm.implementation).

There are a number of implications to this beyond just not being able to use the current implemetation should a new backend ever be added including that the methods in QueryBuilder are very long and come with a corresponding greater maintenance difficulty. This would be helped by splitting things up into the things general to the QueryBuilder and things related to relational databases.

Given that there is quite a bit of shared implenetation between Django and SQLA (given that SQLA is used for querying in both cases), I would be tempted to to have a superclass for both Django and SQLA that implements aiida.orm.implementation.BackendQueryBuilder.

Amongst other things the final implementation would:

  • Have no sqlalchemy imports in aiida.orm.QueryBuilder
  • Have no sqlalchemy imports in aiida.orm.implementation.BackendQueryBuilder
  • Methods like (and calls to them) get_filter_expr, get_filter_expr_from_column and get_filter_expr_from_attributes would be moved out of aiida.orm.QueryBuilder and down to the implemetation level only because a general QueryBuilder should not even rely on there being an expression that performs the query (it may just ben an API call)
@lekah
Copy link
Contributor

lekah commented Apr 1, 2020

I'm not going to work on this since I see no improvement for the user experience and this issue would require a lot of work and time that I don't have. I've assigned @sphuber in case someone wants to take this up in the next coding week, or to close it at his discretion.

@giovannipizzi
Copy link
Member

I think it's ok to close for now, and the be reopened once we want to extend beyond SQL(alchemy)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants