[patch] Escape database queries #1735
Conversation
Queries containing underscores take significantly longer when used in LIKE sql queries, often this is the result of filenames or directory paths containing them and not intended. If the sqlite backend is not used, no escaping is done.
for more information, see https://pre-commit.ci
| @@ -880,10 +900,10 @@ def get_items_dict( | |||
| self.conn.connection.create_function("like", 2, self.regexp) | |||
|
|
|||
| result = self.conn.execute(query) | |||
| row = result.fetchall() | |||
| results = [row._asdict() for row in result.fetchall()] | |||
There was a problem hiding this comment.
A slightly faster (~10%) alternative would be
| results = [row._asdict() for row in result.fetchall()] | |
| results = result.mappings().all() |
but the result is then a list of sqlalchemy objects that behave like dicts, not standard dicts. One can also feed result.mappings() directly into a DataFrame directly, but that would change api more dramatically.
There was a problem hiding this comment.
Just to add some numbers for a query on a project with ~2M jobs:
- ~10s for the plain sqlalchemy query
- ~20s for a call to get_items_dict
- ~30s for a call to job_table
There was a problem hiding this comment.
database reports around 3s for your 2024% queries. The rest seems to be sqlalchemy and pyiron.
|
Fixes #1725, but the performance gain is mainly eaten by the need to transform the query result into a list of dicts and then to a DataFrame. |
pmrv
added
integration
github-actions
bot
changed the title
| if c in s: | ||
| s = s.replace(escape_char + c, c) |
There was a problem hiding this comment.
I understand why you are doing it, but I wonder if this case ever happens. Do you have an example when this is the case?
There was a problem hiding this comment.
No, I had just asked ChatGPT to make sure it handles that case and it seemed to work in my testing.
Queries containing underscores take significantly longer when used in LIKE sql queries, often this is the result of filenames or directory paths containing them and not intended.
If the sqlite backend is not used, no escaping is done.