The query string function is called on each invocation, even for prepared queries.

[mefyl]

When preparing a query, a function Dialect.t -> string is passed to Request.create to compute the query string depending on the database backend. I would expect this function to be called only once to prepare the query, but it is actually called each time the query is used. This is due to Request_cache.find_and_promote recalculating it each time to lookup its internal hash table of queries.

While not critical in itself, it seems a bit wasteful and can duplicate side effects - in our case we log the prepared queries, so we are getting spammed by this log.

Amongst solutions I could imagine:

• Have a string Hashtbl.Make(Dialect) in queries that cache the actual query per dialect. Very simple, but it hurts my feelings a bit to have to lookup a hashtbl every time.
• All queries actually already have a unique integer id, it seems to me that indexing the cache by this id would be much simpler and way faster? Currently it has to recompute the query string every time, but also compare the input and output types. But maybe I'm being naive and missing something?

If there is a consensus that this is indeed desirable and on which solution is the best one, I'd be happy to submit a PR.

[paurkedal]

First, I didn't think of this as an issue, since it's meant to be an pure environment function which should be fast compared to the execution of the query, though the multiple invocations should be documented. That said, I'm open to alternatives.

I'm not quite done with the caqti.template library, and one thing I have considered is to replace the function call with a case-construct in the template language itself. Your suggestion may be an alternative, but I'd expect a few challenges, if I recall correctly. I'll revisit the code, as well, to see if it or what can work.

[mefyl]

I indeed don't think it's not unreasonable that the function is invoked every time. Still, if it can be avoided that seems like a plus to me.

Regarding my question for indexing queries by their unique id, not by their string representation + additional discrimination down the line on the argument types, it seems to me that it would make the code much simpler, more efficient memory/CPU wise and it would solve my multi-invocation grievance. What do you think? Is it worth that I take a stab at it, or am I missing something?

[paurkedal]

I forgot to mention that I'm phasing out the unique ID because it doesn't work for the dynamic prepare policy which will be provided with the upcoming API. Each generated request template will have a new ID,

[mefyl]

Got it, then that's definitely not an angle. I'll wait for caqti.template to reach maturity then to reassess, thanks.