GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — use execute_many from backends directly by Mng-dev-ai · Pull Request #468 · encode/databases

This repository was archived by the owner on Aug 19, 2025. It is now read-only.

Conversation

Mng-dev-ai commented Feb 22, 2022 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Contributor

Fixes #284

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions


          use execute_many from backends directly

2871fdb

Mng-dev-ai mentioned this pull request

Feb 22, 2022

add support for bulk_update encode/orm#148

Open


          fix issues

2107de3

Mng-dev-ai force-pushed the use-execute_many-from-backends branch from 8392da6 to 2107de3 Compare

February 22, 2022 17:59

Mng-dev-ai commented Mar 4, 2022

Copy link

Copy Markdown

Contributor Author

@aminalaee Any updates?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.


          Merge branch 'master' into use-execute_many-from-backends

bea2365

zanieb reviewed

Jul 12, 2023

View reviewed changes

Comment thread

databases/core.py

Comment on lines 279 to +281

		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)

zanieb Jul 12, 2023

Copy link

Copy Markdown

Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Should we avoid building the queries here if you're going to compile them later?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Mng-dev-ai Sep 10, 2023

Copy link

Copy Markdown

Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

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)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

zanieb Sep 10, 2023

Copy link

Copy Markdown

Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

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?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Mng-dev-ai Sep 10, 2023

Copy link

Copy Markdown

Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

I can work on it tmr for a few.
it should improve the performance but i didn't make any benchmarks tbh

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

zanieb reviewed

Jul 12, 2023

View reviewed changes

Comment thread

databases/backends/postgres.py

Comment on lines +219 to +221

		async def execute_many(
		self, queries: typing.List[ClauseElement], values: typing.List[dict]
		) -> None:

zanieb Jul 12, 2023

Copy link

Copy Markdown

Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

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?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Mng-dev-ai Sep 10, 2023

Copy link

Copy Markdown

Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

zanieb Sep 10, 2023

Copy link

Copy Markdown

Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Yep that's the question — I want to gauge if changing this function signature is a breaking change.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Mng-dev-ai Sep 10, 2023

Copy link

Copy Markdown

Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

yea I don't think it should break anything

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

zanieb mentioned this pull request

Jul 12, 2023

Inserting to database using execute_many much slower than using fetch_all #284

Open

This branch has not been deployed

No deployments

Reviewers

1 more reviewer

zanieb

zanieb left review comments

Reviewers whose approvals may not affect merge requirements

At least 1 approving review is required to merge this pull request.

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Inserting to database using execute_many much slower than using fetch_all

Uh oh!

There was an error while loading. Please reload this page.

3 participants

Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.

Conversation

Mng-dev-ai commented Feb 22, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Mng-dev-ai commented Mar 4, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Milestone

Development

Uh oh!

3 participants

Mng-dev-ai commented Feb 22, 2022 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.