-
Notifications
You must be signed in to change notification settings - Fork 1.5k
fix(mysql): RFC Handle missing server-side prepared statements #3995
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -183,7 +183,11 @@ impl MySqlConnection { | |
loop { | ||
// query response is a meta-packet which may be one of: | ||
// Ok, Err, ResultSet, or (unhandled) LocalInfileRequest | ||
let mut packet = self.inner.stream.recv_packet().await?; | ||
let mut packet = self.inner.stream.recv_packet().await.inspect_err(|_| { | ||
// if a prepared statement vanished on the server side, we get an error here | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It would be good to link to the specific issue for context, if you created one (I didn't look). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This PR is also fine. |
||
// clear the statement cache in case the connection got reset to cause re-preparing | ||
self.inner.cache_statement.clear(); | ||
})?; | ||
|
||
if packet[0] == 0x00 || packet[0] == 0xff { | ||
// first packet in a query response is OK or ERR | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -57,6 +57,19 @@ impl MySqlConnection { | |
.status_flags | ||
.intersects(Status::SERVER_STATUS_IN_TRANS) | ||
} | ||
|
||
pub async fn nuke_cached_statements(&mut self) -> Result<(), Error> { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Of course, this shouldn't just be a public method on the connection. Users might confuse it for the method that they're supposed to use to clear the prepared statement cache. The easiest route would be to just mark this |
||
for (statement_id, _) in self.inner.cache_statement.iter() { | ||
self.inner | ||
.stream | ||
.send_packet(StmtClose { | ||
statement: *statement_id, | ||
}) | ||
.await?; | ||
} | ||
|
||
Ok(()) | ||
} | ||
} | ||
|
||
impl Debug for MySqlConnection { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, if you could find the exact error code that's returned, that would probably be better. This is a huge pessimization.
Keep in mind we also get errors here for "normal" errors like constraint violations.
This would leak any valid prepared statements on the server side.