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

last_insert_ID() not served from connection-cache when used in prepared statement #4621

Open
sbstnpl1 opened this issue Aug 21, 2024 · 2 comments

Comments

@sbstnpl1
Copy link

sbstnpl1 commented Aug 21, 2024

Good day,

we've noticed, ProxySQL's feature to return LAST_INSERT_ID() from the Connection-Cache instead of querying it from db-node (to allow Multipexing in conjunction with LAST_INSERT_ID) does not work, if SELECT LAST_INSERT_ID() is queried as a prepared statement (binary protocol).

Steps to reproduce:

I used a python3 script below to reproduce. The SELECT LAST_INSERT_ID() Statement does show up in the audit log of the MySQL Backend node. When setting prepared=True to prepared=False for stmt2, it's not beeing forwarded to Backend-Node and served from Connection Cache es expected.

#!/usr/bin/python3
import mysql.connector

conn = mysql.connector.connect(
host="10.0.10.10",
port=6033,
user="testuser",
password="testuser",
database="schema1"
)
conn.autocommit = True

cursor = conn.cursor(prepared=True)
stmt = "INSERT INTO `table` (`col`) VALUES (%s)"
value = "2"
cursor.execute(stmt, (value))
cursor.close()

cursor = conn.cursor(prepared=True)
stmt2 = "SELECT LAST_INSERT_ID()"
cursor.execute(stmt2)
results = cursor.fetchall()
cursor.close()

conn.close()

Impact:

As multiplexing won't be disabled and last_insert_id won't be served from connection cache, this inevitably leads to last_insert_id not working reliable and wrong IDs to be returned.

(im)possible Workarounds:

regular statements
Of course an easy solution would be to simply use regular statements instead of prepare. However, we use MyBatis, which useses prepared statements by default and we can't change that easily.

convert them with a query rule
We tried to use proxysql query rules to convert those prepared statements to regular statements. Surprisingly, this generally seems to work, but the response won't be accepted by the client as it expects a binary OK packet:
erfaceError('Expected Binary OK packet')

use mysql-auto_increment_delay_multiplex
this is of course a working workaround, but not very nice and also not 100% reliable.

Fix:

The LAST_INSERT_ID should also be returned from connection cache when used in a prepared statement.

@sbstnpl1 sbstnpl1 changed the title last_insert_ID() not served from ProxySQL when used in prepared statement last_insert_ID() not served from connection-cache when used in prepared statement Aug 21, 2024
@renecannao
Copy link
Contributor

Hi @sbstnpl1 .
I feel your pain in this issue.
The issue is actually quite complex...

First of all, I think that any application using SELECT LAST_INSERT_ID() is wasting 1 query: in fact, the client already knows the last insert id: it is returned to the client as part of the OK packet after the INSERT statement.
If an application is using prepared statements to run SELECT LAST_INSERT_ID() , they are actually wasting 3 commands.
It is extremely sub-optimal , it should be avoided as much as possible , as it creates not justifiable latency.

Of course an easy solution would be to simply use regular statements instead of prepare.

That would work. The use of text query for SELECT LAST_INSERT_ID() literally wastes a network round-trip, but it is better than wasting 3 network round trips.

However, we use MyBatis, which useses prepared statements by default and we can't change that easily.

It seems to be Java related. In JDBC you can use executeUpdate() to completely avoid the use of SELECT LAST_INSERT_ID() , that as I mention is a bad idea:
https://docs.oracle.com/javase/6/docs/api/java/sql/Statement.html#executeUpdate%28java.lang.String,%20int%29

We tried to use proxysql query rules to convert those prepared statements to regular statements.

No , this doesn't work.

use mysql-auto_increment_delay_multiplex

If you want to use SELECT LAST_INSERT_ID() (bad idea) with prepared statements (very bad idea) , I think mysql-auto_increment_delay_multiplex is your only option at the moment.

Should ProxySQL reply to SELECT LAST_INSERT_ID() with its metadata also for prepared statement?
It is possible, but requires a lot of changes. In case of prepared statement, the client will execute 3 commands to execute SELECT LAST_INSERT_ID() , ProxySQL would need to generate a statement_id for a statement that would never be prepare on backend and track it to intercept the com_stmt_execute, or actually prepare SELECT LAST_INSERT_ID()on backend but never execute it, or ... (other possible list of complex solution).
I mean, it is a lot of code overhead to get along with applications that use bad query patterns.

Again, possible to implement, but I don't think the effort is justified.
Thoughts?

@sbstnpl1
Copy link
Author

Hi @renecannao , thank you for your fast and detailed clarification.

We totally agree with all your points. Use of LAST_INSERT_ID() is not really necessary and is absolutely nonesense to use it in prepared statement.
We thought this might be easy to implement. As it seems to be complex and time-consuming, we will of course try to find a solution on our application side.

You might want to add this information to documentation. The feature is described here. It might be useful to mention here that this does not work with prepared statements. Cause blindly relying on it (like we did), will lead do data/ID inconsistencies.

again, thanks for you support. Enjoy your weekend!
Sebastian

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