use execute_many from backends directly #468
Conversation
Mng-dev-ai
force-pushed
the
use-execute_many-from-backends
branch
from
8392da6 to
2107de3
Compare
|
@aminalaee Any updates? |
| queries = [self._build_query(query, values_set) for values_set in values] | ||
| async with self._query_lock: | ||
| await self._connection.execute_many(queries) | ||
| await self._connection.execute_many(queries, values) |
There was a problem hiding this comment.
Should we avoid building the queries here if you're going to compile them later?
There was a problem hiding this comment.
Hmm, it's been a long time since I wrote this PR, but I think we still need to convert the query to ClauseElement and bind params, but maybe we don't need list comprehension here, so maybe something like that is sufficient
async def execute_many(
self, query: typing.Union[ClauseElement, str], values: list
) -> None:
query = self._build_query(query,values[0])
async with self._query_lock:
await self._connection.execute_many(query, values)
There was a problem hiding this comment.
Hm interesting. I'm not sure myself, just wanted to check if you had considered it.
Thanks for chiming in on this after the delay. If you're interested in working on it again I can review and release it. Did you confirm this works and improves performance as expected?
There was a problem hiding this comment.
I can work on it tmr for a few.
it should improve the performance but i didn't make any benchmarks tbh
| async def execute_many( | ||
| self, queries: typing.List[ClauseElement], values: typing.List[dict] | ||
| ) -> None: |
There was a problem hiding this comment.
I presume this it not a breaking change for users because this is an internal method and values is always provided by us in the public method?
There was a problem hiding this comment.
I'm not sure I understand what you mean here, but I think users are only supposed to use execute_many from the core file, not from any of the backends
There was a problem hiding this comment.
Yep that's the question — I want to gauge if changing this function signature is a breaking change.
There was a problem hiding this comment.
yea I don't think it should break anything
Fixes #284