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 - evalengine #17409

Open
wants to merge 23 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
9a3ee9b
proto: add fetch_last_insert_id to ExecuteOptions
systay Dec 11, 2024
55cddc6
test: clean up test
systay Dec 16, 2024
d7df55f
feat: fetch last_insert_id when asked for it
systay Dec 16, 2024
b2236a6
feat: update the ExecuteMultiShard interface method
systay Dec 16, 2024
fcc3619
feat: add semantic query signature for last_insert_id with arguments
systay Dec 16, 2024
d82f3fb
feat: pass on the FetchLastInsertID to the engine primitives
systay Dec 16, 2024
02309c9
bugfix: check if options exist before using
systay Dec 17, 2024
6037a4a
added insertID changed boolean to indicate if last_insert id value is…
harshit-gangal Dec 17, 2024
fa23fd1
feat: pass down FetchLastInsertID bool
systay Dec 17, 2024
d92546f
feat: handle last_insert_id with arguments in more situations
systay Dec 17, 2024
56624f6
feat: pass it along for streaming queries
systay Dec 17, 2024
e0bb055
test: add transactions into the test mix
systay Dec 18, 2024
598e21f
result append: remove the short circuit code
harshit-gangal Dec 18, 2024
a5145d6
feat: make sure to check last insert id in transactions, and don't fi…
systay Dec 18, 2024
aea2c38
feat: stop receiving data from tablet when we know we don't need it
systay Dec 18, 2024
5b8ed47
test: add plan-test showing when we need to exhaust the input stream
systay Dec 18, 2024
9fdbc25
bugfix: protect against nil exceptions
systay Dec 18, 2024
193e007
test: add errors to test log
systay Dec 18, 2024
6db0700
feat: abort early once we reach count = 0
systay Dec 18, 2024
bea0515
codegen
systay Dec 19, 2024
61c0b64
feat: clean up limit code
systay Dec 19, 2024
e133d2f
feat: handle last_insert_id with arguments in the evalengine
systay Dec 18, 2024
c270b66
evalengine: jump inside instruction
vmg Dec 19, 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