Conversation
…urned as pointers
…GetLastQueryInfoInt now that we store strings and ints
zachmu
left a comment
There was a problem hiding this comment.
I think this needs a little work before it's ready to ship, see comments. I might be missing something obvious for why the row solution I proposed isn't workable.
…/go-mysql-server into fulghum/last_insert_uuid
…/go-mysql-server into fulghum/last_insert_uuid
max-hoffman
left a comment
There was a problem hiding this comment.
Mostly LGTM! Having the plan fixed before execution makes it easier to understand what's happening and why. You also covered a lot of cases I didn't think of, looks very generalizable. One issue I maybe see is the stateful expression behaving unpredictably on re-execution, which is probably an easy fix with sql.Dispose
| // read this value too early. We should verify this isn't how MySQL behaves, and then could fix | ||
| // by setting a PENDING_LAST_INSERT_UUID value in the session query info, then moving it to | ||
| // LAST_INSERT_UUID in the session query info at the end of execution. | ||
| ctx.Session.SetLastQueryInfoString(sql.LastInsertUuid, uuidValue) |
There was a problem hiding this comment.
I was expecting most of the state to be handled by the session. For example, if we have a prepared statement INSERT with an autoUuid node (probably common?), would only the first execution update the LastInsertUuid? We get away with stateful nodes and expressions (see Subquery) with the sql.Dispose interface, but that's probably also an anti pattern in the longterm
There was a problem hiding this comment.
Ahh... great catch on the prepared statement case. Thank you! I'll test that and add in the sql.Dispose fix if I can't find another way to address it.
There was a problem hiding this comment.
I added tests for prepared statements in 38b7c52. It looks like they didn't need the sql.Dispose implementation in order to execute correctly – I believe this is because we do a smaller re-analysis and must generate a new plan tree, so the value of foundUuid never leaks into other executions of the prepared statement.
I'm happy to add the sql.Dispose implementation if you think it'd still be safer though.
There was a problem hiding this comment.
ok cool, the latest changes to prepareds might only saved the post-binding plan, resolveInsert is going to be after that step. Might become relevant in the future, but thanks for checking!
…ptures don't prevent capture in future execution
…atement returns different results through the server engine tests
UUIDs are often used in place of
auto_incrementIDs, but MySQL doesn't provide an easy way to get the last generated UUID that was used in an insert. This change introduces a new function,last_insert_uuid()that operates similarly tolast_insert_id(). For a column identified as a UUID column, callers can uselast_insert_uuid()to retrieve the last generated UUID value that was inserted into that column. In order to be considered a UUID column, a column must be part of the primary key and it must meet one of the following type signatures:VARCHAR(36)orCHAR(36)with a default value expression ofUUID()VARBINARY(16)orBINARY(16)with a default value expression ofUUID_to_bin(UUID())(optionally, theswap_flagforUUID_to_binmay also be specified)Example usage:
Related to dolthub/dolt#7547