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

🐛 FIX: SQLite: apply escape \ in QueryBuilder like and ilike #5553

Merged
merged 3 commits into from
Jun 2, 2022

Conversation

zhubonan
Copy link
Contributor

@zhubonan zhubonan commented Jun 2, 2022

For like and ilike, the special characters _ and % may need to be escaped if they have literal meanings (LIKE .. ESCAPE in SQL) . However, the sqlite backends do not implicity treat \ as escape sequence (unlike Postgres). https://sqlite.org/lang_expr.html

I discovered this bug when using the SQLite backend (in memory) with aiida-vasp where one of the entry point has name with an underscore. Querying such node gets escaped like:

QueryBuilder(path=[{'entity_type': 'data.vasp.potcar_file.PotcarFileData.', 'orm_base': 'node', 'tag': 'PotcarFileData_1', 'joining_keyword': None, 'joining_value': None, 'edge_tag': None, 'outerjoin': False}], filters={'PotcarFileData_1': {'node_type': {'like': 'data.vasp.potcar\\_file.%'}}}, project={'PotcarFileData_1': []}, order_by=[], limit=None, offset=None, distinct=False)

Here the _ is automatically escaped, but the query only works fine with the psql backend, with sqlite it does not match any existing objects.

This PR makes \ to pass explicity as escaping character for sqlite backends. The use of \ for escapign is already hard-coded aiida.

Here is an example test script:

from aiida_vasp.data.potcar import PotcarFileData
from aiida import orm


from aiida import load_profile, engine, orm, plugins
from aiida.storage.sqlite_temp import SqliteTempBackend

profile = load_profile(
    SqliteTempBackend.create_profile(
        'myprofile',
        sandbox_path='_sandbox',
        options={
            'warnings.development_version': False,
            'runner.poll.interval': 1
        },
        debug=False
    ),
    allow_switch=True
)

d = PotcarFileData()
d._store()
q = orm.QueryBuilder()

# Query the PoatcarData
q.append(PotcarFileData)
print(q)
# Should get one node
print(q.all())

d1 = orm.Data()
d1.set_attribute('a', 'a_b')
print('d1', ' ', d1.store())

d2 = orm.Data()
d2.set_attribute('a', 'acb')
print('d2', ' ',d2.store())

d3 = orm.Data()
d3.set_attribute('a', 'a\_b')
print('d3', ' ',d3.store())

q = orm.QueryBuilder()
q.append(orm.Data, filters={'attributes.a': {'like': 'a\\_b'}})
# Should match to d1
print(q.all())


q = orm.QueryBuilder()
q.append(orm.Data, filters={'attributes.a': {'like': 'a_b'}})
# Should match to d1 and d2 but not d3
print(q.all())

q = orm.QueryBuilder()
q.append(orm.Data, filters={'attributes.a': {'like': 'a\\\\_b'}})
# Should match to d3
print(q.all())

p1 = orm.WorkflowNode()
p1.set_process_type('aiida.test.a_b')
print(p1.store())
p2 = orm.WorkflowNode()
p2.set_process_type('aiida.test.abb')
print(p2.store())

q = orm.QueryBuilder()
q.append(orm.WorkflowNode, filters={'process_type': {'like': 'aiida.test.a_b%'}})
# Should match to p1 and p2
print(q.all())

q = orm.QueryBuilder()
q.append(orm.WorkflowNode, filters={'process_type': {'like': 'aiida.test.a\\_b%'}})
# Should match to p1 only
print(q.all())

The '\' is used for escaping special characters such as '*' and '_'.
This is hard-coded in QueryBuilder but not passed on when constructing
SQL clause, hence replying on the database using '\' for escaping
by default.
For PostgreSQL this works, but not for SQLite where the ESCAPE clause
is needed. Now, we enforce the existence of ESCAPE in all cases.
The PostgreSQL backend does not like it (translates into '\\' instead).
Also added tests for the SQLite backend.
Copy link
Member

@chrisjsewell chrisjsewell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep looks good cheers

@chrisjsewell chrisjsewell changed the title Fix sqlite backends not applying \ for escaping in query builder like and ilike 🐛 Fix: sqlite: apply escape \ in QueryBuilder like and ilike Jun 2, 2022
@chrisjsewell chrisjsewell changed the title 🐛 Fix: sqlite: apply escape \ in QueryBuilder like and ilike 🐛 FIX: sqlite: apply escape \ in QueryBuilder like and ilike Jun 2, 2022
@chrisjsewell chrisjsewell changed the title 🐛 FIX: sqlite: apply escape \ in QueryBuilder like and ilike 🐛 FIX: SQLite: apply escape \ in QueryBuilder like and ilike Jun 2, 2022
@chrisjsewell chrisjsewell merged commit 29a7764 into aiidateam:main Jun 2, 2022
sphuber pushed a commit that referenced this pull request Sep 22, 2022
…5553)

For `like` and `ilike`,
the special characters `_` and `%` may need to be escaped if they have literal meanings (`LIKE .. ESCAPE` in SQL).
However, the SQLite backends do not implicitly treat `\` as escape sequence (unlike Postgres): https://sqlite.org/lang_expr.html

This commit makes `\` explicitly an escaping character for SQLite backends. Note, the use of `\` for escaping is already hard-coded in aiida-core.

Cherry-pick: 29a7764
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants