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

Improved Compatibility Around LAST_INSERT_ID #17369

Closed
wants to merge 26 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
962d7b9
proto: add fetch_last_insert_id to ExecuteOptions
systay Dec 11, 2024
3df040a
test: clean up test
systay Dec 16, 2024
22ec89f
feat: fetch last_insert_id when asked for it
systay Dec 16, 2024
5244d62
feat: update the ExecuteMultiShard interface method
systay Dec 16, 2024
49dcbe1
feat: add semantic query signature for last_insert_id with arguments
systay Dec 16, 2024
5ada428
feat: pass on the FetchLastInsertID to the engine primitives
systay Dec 16, 2024
fbc3dbb
bugfix: check if options exist before using
systay Dec 17, 2024
0db81eb
added insertID changed boolean to indicate if last_insert id value is…
harshit-gangal Dec 17, 2024
bc26f2f
feat: pass down FetchLastInsertID bool
systay Dec 17, 2024
5b5be41
feat: handle last_insert_id with arguments in more situations
systay Dec 17, 2024
a872d14
feat: pass it along for streaming queries
systay Dec 17, 2024
69175c3
test: add transactions into the test mix
systay Dec 18, 2024
6c1a3d2
result append: remove the short circuit code
harshit-gangal Dec 18, 2024
880284d
feat: make sure to check last insert id in transactions, and don't fi…
systay Dec 18, 2024
128b3d8
feat: stop receiving data from tablet when we know we don't need it
systay Dec 18, 2024
6fd09eb
feat: handle last_insert_id with arguments in the evalengine
systay Dec 18, 2024
f102d99
test: add evalengine tests for last_insert_id
systay Dec 18, 2024
323cf2b
codegen
systay Dec 18, 2024
94054f3
test: add plan-test showing when we need to exhaust the input stream
systay Dec 18, 2024
1e7fbe3
test: implement the needed interface
systay Dec 18, 2024
3abdf60
bugfix: protect against nil exceptions
systay Dec 18, 2024
3fb795b
evalengine: handle NULL in last_insert_id correctly
systay Dec 18, 2024
5f48f05
refactor: clean up compiler code
systay Dec 18, 2024
a1c2707
bugfix: actually use the jump
systay Dec 18, 2024
b0b5bd7
test: add errors to test log
systay Dec 18, 2024
bd81c88
feat: abort early once we reach count = 0
systay Dec 18, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion go/mysql/conn.go
Original file line number Diff line number Diff line change
Expand Up @@ -399,7 +399,7 @@ func (c *Conn) readHeaderFrom(r io.Reader) (int, error) {
return 0, vterrors.Wrapf(err, "io.ReadFull(header size) failed")
}

sequence := uint8(c.header[3])
sequence := c.header[3]
if sequence != c.sequence {
return 0, vterrors.Errorf(vtrpcpb.Code_INTERNAL, "invalid sequence, expected %v got %v", c.sequence, sequence)
}
Expand Down
1 change: 1 addition & 0 deletions go/mysql/query.go
Original file line number Diff line number Diff line change
Expand Up @@ -404,6 +404,7 @@ func (c *Conn) ReadQueryResult(maxrows int, wantfields bool) (*sqltypes.Result,
return &sqltypes.Result{
RowsAffected: packetOk.affectedRows,
InsertID: packetOk.lastInsertID,
InsertIDChanged: packetOk.lastInsertID > 0,
SessionStateChanges: packetOk.sessionStateData,
StatusFlags: packetOk.statusFlags,
Info: packetOk.info,
Expand Down
3 changes: 3 additions & 0 deletions go/sqltypes/proto3.go
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,7 @@ func ResultToProto3(qr *Result) *querypb.QueryResult {
Fields: qr.Fields,
RowsAffected: qr.RowsAffected,
InsertId: qr.InsertID,
InsertIdChanged: qr.InsertIDChanged,
Rows: RowsToProto3(qr.Rows),
Info: qr.Info,
SessionStateChanges: qr.SessionStateChanges,
Expand All @@ -119,6 +120,7 @@ func Proto3ToResult(qr *querypb.QueryResult) *Result {
Fields: qr.Fields,
RowsAffected: qr.RowsAffected,
InsertID: qr.InsertId,
InsertIDChanged: qr.InsertIdChanged,
Rows: proto3ToRows(qr.Fields, qr.Rows),
Info: qr.Info,
SessionStateChanges: qr.SessionStateChanges,
Expand All @@ -136,6 +138,7 @@ func CustomProto3ToResult(fields []*querypb.Field, qr *querypb.QueryResult) *Res
Fields: qr.Fields,
RowsAffected: qr.RowsAffected,
InsertID: qr.InsertId,
InsertIDChanged: qr.InsertIdChanged,
Rows: proto3ToRows(fields, qr.Rows),
Info: qr.Info,
SessionStateChanges: qr.SessionStateChanges,
Expand Down
16 changes: 10 additions & 6 deletions go/sqltypes/result.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ type Result struct {
Fields []*querypb.Field `json:"fields"`
RowsAffected uint64 `json:"rows_affected"`
InsertID uint64 `json:"insert_id"`
InsertIDChanged bool `json:"insert_id_changed"`
Rows []Row `json:"rows"`
SessionStateChanges string `json:"session_state_changes"`
StatusFlags uint16 `json:"status_flags"`
Expand Down Expand Up @@ -92,6 +93,7 @@ func (result *Result) Copy() *Result {
out := &Result{
RowsAffected: result.RowsAffected,
InsertID: result.InsertID,
InsertIDChanged: result.InsertIDChanged,
SessionStateChanges: result.SessionStateChanges,
StatusFlags: result.StatusFlags,
Info: result.Info,
Expand All @@ -116,6 +118,7 @@ func (result *Result) ShallowCopy() *Result {
return &Result{
Fields: result.Fields,
InsertID: result.InsertID,
InsertIDChanged: result.InsertIDChanged,
RowsAffected: result.RowsAffected,
Info: result.Info,
SessionStateChanges: result.SessionStateChanges,
Expand All @@ -129,6 +132,7 @@ func (result *Result) Metadata() *Result {
return &Result{
Fields: result.Fields,
InsertID: result.InsertID,
InsertIDChanged: result.InsertIDChanged,
RowsAffected: result.RowsAffected,
Info: result.Info,
SessionStateChanges: result.SessionStateChanges,
Expand All @@ -153,6 +157,7 @@ func (result *Result) Truncate(l int) *Result {

out := &Result{
InsertID: result.InsertID,
InsertIDChanged: result.InsertIDChanged,
RowsAffected: result.RowsAffected,
Info: result.Info,
SessionStateChanges: result.SessionStateChanges,
Expand Down Expand Up @@ -198,6 +203,7 @@ func (result *Result) Equal(other *Result) bool {
return FieldsEqual(result.Fields, other.Fields) &&
result.RowsAffected == other.RowsAffected &&
result.InsertID == other.InsertID &&
result.InsertIDChanged == other.InsertIDChanged &&
slices.EqualFunc(result.Rows, other.Rows, func(a, b Row) bool {
return RowEqual(a, b)
})
Expand Down Expand Up @@ -324,16 +330,14 @@ func (result *Result) StripMetadata(incl querypb.ExecuteOptions_IncludedFields)
// to another result.Note currently it doesn't handle cases like
// if two results have different fields.We will enhance this function.
func (result *Result) AppendResult(src *Result) {
if src.RowsAffected == 0 && len(src.Rows) == 0 && len(src.Fields) == 0 {
return
result.RowsAffected += src.RowsAffected
if src.InsertID != 0 || src.InsertIDChanged {
result.InsertID = src.InsertID
}
result.InsertIDChanged = result.InsertIDChanged || src.InsertIDChanged
if result.Fields == nil {
result.Fields = src.Fields
}
result.RowsAffected += src.RowsAffected
if src.InsertID != 0 {
result.InsertID = src.InsertID
}
result.Rows = append(result.Rows, src.Rows...)
}

Expand Down
7 changes: 7 additions & 0 deletions go/test/endtoend/utils/cmp.go
Original file line number Diff line number Diff line change
Expand Up @@ -313,3 +313,10 @@ func (mcmp *MySQLCompare) ExecAllowError(query string) (*sqltypes.Result, error)
}
return vtQr, vtErr
}

func (mcmp *MySQLCompare) VExplain(query string) string {
mcmp.t.Helper()
vtQr, vtErr := mcmp.VtConn.ExecuteFetch("vexplain plan "+query, 1, true)
require.NoError(mcmp.t, vtErr, "[Vitess Error] for query: "+query)
return vtQr.Rows[0][0].ToString()
}
56 changes: 56 additions & 0 deletions go/test/endtoend/vtgate/queries/misc/misc_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,62 @@ func TestCast(t *testing.T) {
mcmp.AssertMatches("select cast('3.2' as unsigned)", `[[UINT64(3)]]`)
}

// TestSetAndGetLastInsertID tests that the last_insert_id function works as intended when used with different arguments.
func TestSetAndGetLastInsertID(t *testing.T) {
notZero := 1
checkQuery := func(i string, workload string, tx bool, mcmp utils.MySQLCompare) {
for _, val := range []int{0, notZero} {
query := fmt.Sprintf(i, val)
name := fmt.Sprintf("%s - %s", workload, query)
if tx {
name = "tx - " + name
}
mcmp.Run(name, func(mcmp *utils.MySQLCompare) {
mcmp.Exec(query)
mcmp.Exec("select last_insert_id()")
t := mcmp.AsT()
if t.Failed() {
t.Log(mcmp.VExplain(query))
}
})
}
// we need this value to be not zero, and then we keep changing it so different queries don't interact with each other
notZero++
}

queries := []string{
"select last_insert_id(%d)",
"select last_insert_id(%d), id1, id2 from t1 limit 1",
"select last_insert_id(%d), id1, id2 from t1 where 1 = 2",
"select 12 from t1 where last_insert_id(%d)",
"update t1 set id2 = last_insert_id(%d) where id1 = 1",
"update t1 set id2 = last_insert_id(%d) where id1 = 2",
"update t1 set id2 = 88 where id1 = last_insert_id(%d)",
"delete from t1 where id1 = last_insert_id(%d)",
"select id2, last_insert_id(count(*)) from t1 where %d group by id2",
}

for _, workload := range []string{"olap", "oltp"} {
for _, tx := range []bool{true, false} {
mcmp, closer := start(t)
_, err := mcmp.VtConn.ExecuteFetch(fmt.Sprintf("set workload = %s", workload), 1000, false)
require.NoError(t, err)
if tx {
_, err := mcmp.VtConn.ExecuteFetch("begin", 1000, false)
require.NoError(t, err)
}

// Insert a few rows for UPDATE tests
mcmp.Exec("insert into t1 (id1, id2) values (1, 10)")

for _, query := range queries {
checkQuery(query, workload, tx, mcmp)
}
closer()
}
}
}

// TestVindexHints tests that vindex hints work as intended.
func TestVindexHints(t *testing.T) {
mcmp, closer := start(t)
Expand Down
Loading
Loading