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

[ADP-2565] Improve performance of insertMany with prepared statement #4699

Merged
merged 1 commit into from
Jul 25, 2024

Conversation

HeinrichApfelmus
Copy link
Contributor

This pull request improves the performance of insertMany by using a single prepared statement and setting new parameters multiple times.

Comments

  • I have skipped extensive property testing for now, as I want to stabilize the API first. The module Demo.Database provides a basic functionality check.

Issue Number

ADP-2565

@HeinrichApfelmus HeinrichApfelmus self-assigned this Jul 24, 2024
Copy link
Member

@Anviking Anviking left a comment

Choose a reason for hiding this comment

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

👍 Obviously some tests would be nice, but seems to work

ReaderT $ \conn -> do
-- The 'query' string contains '?' which will be substituted
-- for the values in the row.
let query = fst $ renderStmt $ Stmt.insertOne proxy
Copy link
Member

Choose a reason for hiding this comment

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

If there shouldn't be any bindings, then perhaps

Suggested change
let query = fst $ renderStmt $ Stmt.insertOne proxy
let (query, _noBindings) = $ renderStmt $ Stmt.insertOne proxy

unless we even want it to crash if there are?

Suggested change
let query = fst $ renderStmt $ Stmt.insertOne proxy
let (query, []) = $ renderStmt $ Stmt.insertOne proxy

Copy link
Contributor Author

Choose a reason for hiding this comment

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

_noBindings sounds good to me.

I don't think that let (query, []) = … actually crashes — let bindings introduce irrefutable pattern.

@HeinrichApfelmus HeinrichApfelmus force-pushed the HeinrichApfelmus/ADP-2565/sql-insertMany branch from 2bea6ec to 5206b45 Compare July 25, 2024 11:41
@HeinrichApfelmus HeinrichApfelmus force-pushed the HeinrichApfelmus/ADP-2565/sql-insertMany branch from 5206b45 to c638f11 Compare July 25, 2024 11:41
@HeinrichApfelmus HeinrichApfelmus added this pull request to the merge queue Jul 25, 2024
Merged via the queue into master with commit 5628dc7 Jul 25, 2024
23 checks passed
@HeinrichApfelmus HeinrichApfelmus deleted the HeinrichApfelmus/ADP-2565/sql-insertMany branch July 25, 2024 14:21
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.

2 participants