Prepared versions of sql and sql_query functions

[belovaf]

Currently diesel doesn't support prepared statements cache for any sql which contains at least one raw sql fragment.
This really negatively affects performance.
Often I want to write a custom sql fragment or a whole query and I know, that it will be static and can be safely added to a prepared statement cache.
Actually I even think that this should be the default behaviour because this is much more common case.
In my practice 99% of statements can be safely prepared.

I see two possible implementations:

1. Brand new prepared_sql and prepared_sql_query functions.
2. Add bool field to corresponding structs (a little bit weird, because each wrapper will have this field).

Do you like the idea? Can I implement this?

[weiznich]

First of all thanks for investing this in #4049

Not caching prepared statements for sql_query and sql DSL notes was a conscious decision. There is currently no way for the user to to control the statement cache so we fear that it might grow unreasonably large by allowing it to cache all queries. That is documented here: https://docs.diesel.rs/2.2.x/diesel/connection/statement_cache/index.html#a-primer-on-prepared-statement-caching-in-diesel

That written: I can understand that this can lead to unexpected performance results and that someone might want a better way to fix this. I do not have a finished design ready for this but I would like to go for something that has a bit stronger guarantees about that the query string is actually statically known at compile time.

There is currently a rather straightforward method to do that in your code:

1. Define a struct that holds all parameters + derive QueryId
2. Implement QueryFragment for that struct to set the corresponding query
3. Implementation Query to define the sql return type by that query. Use Untyped for mirroring sql_query
4. Implement RunQueryDsl without overwriting any of the default methods

That is a bit more code than just calling sql_query but gives you much greater control about what is actually going on. Maybe we can work on documentation and possibly streamlining that approach instead?

[belovaf]

If you want my opinion, I think that diesel should support prepared statements from strings.
QueryFragment is more unsafe, than sql_query. I can easily write a dynamic unbound sql query with it.
And prepared_sql_query doesn't have to be a static string. For example if I have 3 dynamic filters, then at most 2^3 statements have to be cached.
I think developer should be able to take the responsibility and say, that a concrete statement must be cached, no matter what.
Look to this problem as a user. Who wants to understand all these traits to just write a simple native sql?
And many diesel users currently pay for nothing: they force a database to parse their complex (for simple queries they probably use dsl) queries each time.
I discovered this problem only after reviewing stress test results. I think that most developers (including me) expect that their queries will be cached.

[belovaf]

Also I have some benchmark results.

Axum http2 endpoint with select query (1500 chars) to postgres db with 50 connections pool:

Dsl with sql functions: ~2400 rps.
Dsl with prepared_sql functions: ~6000 rps
Dsl with small custom QueryFragments (HAS_STATIC_QUERY_ID = true): ~6000 rps
Optimal custom QueryFragment for the whole query: ~6000 rps

I have 3 conclusions from these results:

1. A static query id doesn't significantly increase performance. prepared_sql has almost the same speed.
2. Unprepared statements significantly decreases performance. sql slows down queries about 2.5 times.
3. Dsl has almost zero overhead.

[weiznich]

To be clear here: I do not say that this is a bad idea and this shouldn't be done in some way. I just pointed out that there are already tools that can be used to build cached statements from raw sql. I agree that we should somehow simplify the process, but I'm not sold on the two proposed variants. The first one sounds like a large duplication of interfaces, which is bad in terms of maintainability. The second variant sounds more like a hack to me. If you manage to get the first version to work without duplicating everything, but only with adding a single function that would be fine for me. At least as long as there is extensive documentation about what could go wrong if you use that with an unbound number of statements.

As for the benchmarks: It's hard to tell what you exactly measured so it's hard to comment there on details. Generally speaking:

• Using a webframework introduce additional overheads, that makes it really hard to judge finer changes in performance
• The static query id approach does show a noticeable difference for simple queries using the sqlite backend. So it might be relevant to test there.