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

idea as fixed bulk update/insert and mogrify #550

Closed
vir-mir opened this issue Mar 30, 2019 · 7 comments
Closed

idea as fixed bulk update/insert and mogrify #550

vir-mir opened this issue Mar 30, 2019 · 7 comments
Assignees

Comments

@vir-mir
Copy link
Member

vir-mir commented Mar 30, 2019

Hey all.

We can implement a bulk insert, but we need to decide what to do with mogrify.

Im test psycopg2.extras.execute_batch in asynchronous style and the result impressed me.
we replace "executemany" with "mogrify and execute"

Recently, the argument was use_batch_mode added to sqlalchemy. psycopg/psycopg2#491 (comment)

use_batch_mode: This flag allows psycopg2.extras.execute_batch for cursor.executemany() calls performed by the Engine. It is currently experimental but may well become True by default as it is critical for executemany performance.

psycopg documentation: http://initd.org/psycopg/docs/extras.html#psycopg2.extras.execute_batch
psycopg implementation: https://github.com/psycopg/psycopg2/blob/master/lib/extras.py#L1185
significant speedup: psycopg/psycopg2#491 (comment)

This allows us to make mass requests using use_batch_mode=True in Engine and will resolve of issues: #112, #546.

however, cursor.mogrify is an asynchronous function.
although is doesn't need #186, #352

I propose to change this behavior in version 1.0.0 without backward compatibility.

@aio-libs-bot
Copy link

GitMate.io thinks possibly related issues are #112 (aiopg.sa bulk inserts / executemany), #546 (Bulk update values with SQLAlchemy), #87 (Returning after update), and #352 (Should cursor.mogrify be coroutine?).

@vir-mir
Copy link
Member Author

vir-mir commented Mar 30, 2019

I propose to discuss @jettify @asvetlov @thehesiod @webknjaz

@vir-mir vir-mir mentioned this issue Apr 1, 2019
@webknjaz
Copy link
Member

webknjaz commented Apr 8, 2019

I think mogrify is like this just for interface consistency. Following your logic none of the other coroutine methods in that class need async. But using async def means that there will also be a control flow yield to the event loop while with sync methods it wouldn't happen.

@thehesiod
Copy link
Contributor

leave this up to jettify/asvetlov :]

@vir-mir
Copy link
Member Author

vir-mir commented Apr 13, 2019

I am ready to wait a week more. but after I collect the new release candidate, removing async before this function

@vir-mir vir-mir mentioned this issue Sep 20, 2019
@vir-mir
Copy link
Member Author

vir-mir commented Sep 20, 2019

preparing release

@vir-mir
Copy link
Member Author

vir-mir commented Dec 5, 2019

started implementing #632

@vir-mir vir-mir closed this as completed Dec 5, 2019
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

6 participants