Prepared statement deallocation

[jakajancar]

Reading the README, it seems we have two options:

• don't use prepared statements, and manually escape the parameter values, or
• use prepared statements and keep them alive forever

I would like to prepare a statement, execute it and deallocate it.

[mauricio]

That looks cool, would you have a proposed interface for this?

[jakajancar]

I would have:

def sendQuery(query: String, values: Seq[Any] = List())             // prepares and closes statement if values.nonEmpty
def withPreparedStatement[A](sql: String)(f : Statement => A) : A   // loan pattern

then you can do

db.sendQuery("INSERT INTO foo VALUES ('abc')")
db.sendQuery("INSERT INTO foo VALUES (?)", "abc")
db.withPreparedStatement("INSERT INTO foo VALUES (?)") { stmt =>
  stmt.execute("abc")
  stmt.execute("def")
  // ...
}

In any case the current "leaking" sendPreparedStatement() seems weird to me.

[mauricio]

Leaking?

[jakajancar]

If I understand correctly when you do sendPreparedStatement() a statement is prepared on the server and never released (until you disconnect). Isn't that the case?

[mauricio]

Yes, that's how prepared statements are supposed to be used. That's what the JDBC drivers do as well.

It doesn't make much sense to be going through the cost of preparing the statement and then throwing it away.

[jakajancar]

JDBC PreparedStatement::close():

Releases this Statement object's database and JDBC resources immediately instead of waiting for this to happen when it is automatically closed. It is generally good practice to release resources as soon as you are finished with them to avoid tying up database resources.

See also DEALLOCATE. Certainly having them alive for the duration of the entire session is not the only way how they're "supposed to be used", and I would argue with long lived and especially pooled connections, it's even less so.

Also, I'm not sure that the cost of PREPARE + EXECUTE + RELEASE is that much higher than just running the query...

[dylex]

Both are reasonable uses. The current behavior is useful exactly with pooled connections as you may have a query you run for every request that comes in, and you want it to be efficient. Prepared statements with scoped lifetimes may be useful for doing bulk inserts (though it's hard to think of other use cases: repeated selects should probably be converted to a single query, and the cost of three round trips for prepare/execute/release is certainly higher). On the other hand, less common queries you may not want to prepare at all (these are quite common for us, just substituting SQL literals for placeholders).

[dylex]

The easiest way to add this functionality to the existing API might be a releasePreparedStatement(query : String) : Future[QueryResult] that just removes the query from parsedStatements and sends the release. Other interfaces could then be built on top of this.

[mauricio]

:+1:

This would just use the local prepared statements cache every connection has and fire the release messages.

[jakajancar]

So you have to list the same query twice and get a cache involved:

sendPreparedStatement("SELECT ...", args)
releasePreparedStatement("SELECT ...")

Kind of convoluted IMO. The lowest level interface should separate PREPARE from EXECUTE, and the prepare operation should return a Closeable PreparedStatement-like object. No cache involved anywhere.

But I can live with it.

[mauricio]

If you don't want to cache prepared statements, why are you using prepared statements?

[jakajancar]

To not insert large binary cruft into the SQL string.

[mauricio]

Ah, makes sense. The main query interface doesn't have parameter placement.

[mst-appear]

What I am implementing for https://github.com/mauricio/postgresql-async/issues/109 requires a prepared statement, so in that case it makes sense use it once and then throw away.

Prepared statements also gives you proper escaping of strings, to aviod SQL injection security holes.

[farico]

Any update on this ticket? I also need a way to release statements. I have a dynamic query which does INSERT INTO table (a, b, c) VALUES (1, 2, "user input"), (3, 5, "sql_injection_vuln"), ... , N ON DUPLICATE KEY UPDATE SET c = VALUES(c); my values list is always dynamic and I believe a lot of prepared statements will use up MySQL memory. How to avoid this (and SQL injection)?

[mauricio]

@farico still on the plans, but contributions for it as welcome. Just providing a method that compiles and returns the compiled prepared statement would be good enough I think.

[dylex]

I wrote most of a releasePreparedStatement (really just parsedStatements.remove(query) plus Close) for postgres, but don't know enough about mysql to help there. I can add some tests and send a PR for that part if it's useful, though.

[lpfeup]

Any update on this?

[mauricio]

Yes, there's a branch with work on it, I need to finish the MySQL part: https://github.com/mauricio/postgresql-async/tree/closing-prepared-statements

[kyleu]

We're getting demolished by this issue (MySQL). Our app creates a near-infinite set of potential queries, but we need placeholders and strongly typed parameters. Is there a safe way to inject placeholders without hitting the stored procedure cache? Something like forPlaceholders(sql: String, values: Any*): String. If we can't get past this, I'll have to waste a week converting to JDBC, so I'm very motivated to get this resolved. Does the fix described in https://github.com/jilen/postgresql-async/blob/0.2.20-DRIP/mysql-async/src/main/scala/com/github/mauricio/async/db/mysql/codec/MySQLConnectionHandler.scala#L63 fix the unbounded statement cache?

[vitalif]

Hi. Another issue caused by this is mixing DDL and DML. PostgreSQL gives "Cached plan must not change result type" error when you try to use the same prepared statement (select *) with a modified table (for example, when a column has been added). The error itself is idiotic (I think DBMS must recompile the statement automatically in such case), but to workaround it we also need to deallocate prepared queries...

[oshai]

I merged the fix in jasync fork if someone interested: https://github.com/jasync-sql/jasync-sql/issues/82

[eugenemiretsky]

@KyleU How did you solve this issue? We are facing the same problem.

[kyleu]

I switched to JDBC :|