Skip to content

No need to find statement in cached statement map? #1198

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

Closed
kingluo opened this issue Apr 29, 2021 · 1 comment
Closed

No need to find statement in cached statement map? #1198

kingluo opened this issue Apr 29, 2021 · 1 comment

Comments

@kingluo
Copy link

kingluo commented Apr 29, 2021

To execute each query, sqlx needs to find the cached statement even for statement object already provided.

For example:

Statement provided:

let stmt = conn.prepare_with("select 1", &[]);
let stmt = stmt.await.unwrap();
let row: (i32,) = stmt.query_as().fetch_one(&mut conn).await.unwrap();

Statement not provided:

let row: (i32,) = sqlx::query_as("SELECT 1")
    .fetch_one(&mut conn)
    .await
    .unwrap();

And see get_or_prepare:
https://github.com/launchbadge/sqlx/blob/master/sqlx-core/src/postgres/connection/executor.rs#L173

If the map is big, then the find would be an unnecessary overhead.

@abonander
Copy link
Collaborator

Unless you're actually running into a situation where the hashmap lookup for the prepared statement is a significant source of overhead, I don't think it's worth the effort. Being a hashmap, lookups are going to be quite fast (relatively speaking) up to probably several hundred thousand elements.

Where a significant overhead is likely to occur on maps of that size is on insert, when the map needs to resize itself and thus rehash all its elements.

However, you'd need to execute that many unique queries on a single connection for this to become an issue, which is only likely to come up if you're dynamically generating queries. In that case I think we just need a way to say "prepare this statement, but don't cache it" which I already have in mind for implementing #875 and we can expose that for users to set as well.

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

No branches or pull requests

2 participants