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

[Add] Multiple insert with logging implemented #302

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Xpshnik
Copy link

@Xpshnik Xpshnik commented Aug 21, 2022

The logged_query function doesn't particularly fit for cases where the query is a multiple rows insertion statement, as one would have to call it for each row insertion separetely.

Recently I had to populate a new table based on the rows from already existing table, and such function would come in handy, especially as it takes exactly the type of argument (sequence of tuples) that a fetch after select from another table can return. Pure SQL INSERT INTO SELECT is the perfect solution if you don't have to handle those values before inserting, but you may have to construct a new list of tuples with somehow modified values or additional values that couldn't be added to the tuples via SELECT. So it probably makes sense to have a function like this for such cases.

I used the cr.mogrify() + cr.execute() approach instead of cr.executemany() as the former is generally believed to perform better.

@Xpshnik Xpshnik force-pushed the add-logged_multiple_insert-function branch 9 times, most recently from 98c42ec to 5c36ffe Compare April 28, 2023 01:20
@Xpshnik Xpshnik force-pushed the add-logged_multiple_insert-function branch from 5c36ffe to 8b9fb6b Compare April 28, 2023 01:23
@legalsylvain
Copy link
Contributor

@OCA/openupgrade-maintainers an opinion on that proposal ?

@hbrunn
Copy link
Member

hbrunn commented May 16, 2023

I'd find this more interesting if there was an OCA/Openupgrade migration script using this function. Code looks fine though

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.

3 participants