From d009e81d43f1c7d380e26abca9824c72e0e5eb21 Mon Sep 17 00:00:00 2001 From: Andres Taylor Date: Wed, 18 Dec 2024 12:36:30 +0100 Subject: [PATCH 01/13] feat: handle last_insert_id with arguments in the evalengine Signed-off-by: Andres Taylor --- .../endtoend/vtgate/queries/misc/misc_test.go | 3 +- go/vt/vtgate/engine/fake_vcursor_test.go | 3 + go/vt/vtgate/engine/primitive.go | 2 + go/vt/vtgate/evalengine/compiler_asm.go | 21 +++++ go/vt/vtgate/evalengine/compiler_test.go | 93 +++++++++++++++++++ go/vt/vtgate/evalengine/expr_env.go | 2 + go/vt/vtgate/evalengine/fn_misc.go | 40 ++++++++ .../evalengine/integration/comparison_test.go | 4 + go/vt/vtgate/evalengine/translate_builtin.go | 5 + go/vt/vtgate/executorcontext/vcursor_impl.go | 6 ++ 10 files changed, 178 insertions(+), 1 deletion(-) diff --git a/go/test/endtoend/vtgate/queries/misc/misc_test.go b/go/test/endtoend/vtgate/queries/misc/misc_test.go index 9f9860bd0e0..7ad9e911686 100644 --- a/go/test/endtoend/vtgate/queries/misc/misc_test.go +++ b/go/test/endtoend/vtgate/queries/misc/misc_test.go @@ -163,6 +163,7 @@ func TestSetAndGetLastInsertID(t *testing.T) { "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"} { @@ -175,7 +176,7 @@ func TestSetAndGetLastInsertID(t *testing.T) { require.NoError(t, err) } - // Insert a row for UPDATE tests + // Insert a few rows for UPDATE tests mcmp.Exec("insert into t1 (id1, id2) values (1, 10)") for _, query := range queries { diff --git a/go/vt/vtgate/engine/fake_vcursor_test.go b/go/vt/vtgate/engine/fake_vcursor_test.go index f27ca380876..72422193ee8 100644 --- a/go/vt/vtgate/engine/fake_vcursor_test.go +++ b/go/vt/vtgate/engine/fake_vcursor_test.go @@ -893,6 +893,9 @@ func (t *loggingVCursor) RecordMirrorStats(sourceExecTime, targetExecTime time.D } } +func (t *loggingVCursor) SetLastInsertID(id uint64) {} +func (t *noopVCursor) SetLastInsertID(id uint64) {} + func (t *noopVCursor) VExplainLogging() {} func (t *noopVCursor) DisableLogging() {} func (t *noopVCursor) GetVExplainLogs() []ExecuteEntry { diff --git a/go/vt/vtgate/engine/primitive.go b/go/vt/vtgate/engine/primitive.go index e6fa102581e..7734dd81a6b 100644 --- a/go/vt/vtgate/engine/primitive.go +++ b/go/vt/vtgate/engine/primitive.go @@ -147,6 +147,8 @@ type ( // RecordMirrorStats is used to record stats about a mirror query. RecordMirrorStats(time.Duration, time.Duration, error) + + SetLastInsertID(uint64) } // SessionActions gives primitives ability to interact with the session state diff --git a/go/vt/vtgate/evalengine/compiler_asm.go b/go/vt/vtgate/evalengine/compiler_asm.go index dfb1a30bffc..9099f8711c0 100644 --- a/go/vt/vtgate/evalengine/compiler_asm.go +++ b/go/vt/vtgate/evalengine/compiler_asm.go @@ -5138,3 +5138,24 @@ func (asm *assembler) Introduce(offset int, t sqltypes.Type, col collations.Type return 1 }, "INTRODUCE (SP-1)") } + +func (asm *assembler) Fn_LAST_INSERT_ID() { + asm.emit(func(env *ExpressionEnv) int { + arg := env.vm.stack[env.vm.sp-1].(*evalUint64) + env.VCursor().SetLastInsertID(arg.u) + return 1 + }, "FN LAST_INSERT_ID UINT64(SP-1)") +} + +func (asm *assembler) Fn_LAST_INSERT_ID_NULL() { + asm.emit(func(env *ExpressionEnv) int { + env.VCursor().SetLastInsertID(0) + return 1 + }, "FN LAST_INSERT_ID NULL") +} + +func (asm *assembler) addJump(end *jump) { + asm.emit(func(env *ExpressionEnv) int { + return end.offset() + }, "JUMP") +} diff --git a/go/vt/vtgate/evalengine/compiler_test.go b/go/vt/vtgate/evalengine/compiler_test.go index 88c13a479ed..7396529dcf1 100644 --- a/go/vt/vtgate/evalengine/compiler_test.go +++ b/go/vt/vtgate/evalengine/compiler_test.go @@ -903,6 +903,99 @@ func TestBindVarLiteral(t *testing.T) { } } +type testVcursor struct { + lastInsertID *uint64 + env *vtenv.Environment +} + +func (t *testVcursor) TimeZone() *time.Location { + return time.UTC +} + +func (t *testVcursor) GetKeyspace() string { + return "apa" +} + +func (t *testVcursor) SQLMode() string { + return "oltp" +} + +func (t *testVcursor) Environment() *vtenv.Environment { + return t.env +} + +func (t *testVcursor) SetLastInsertID(id uint64) { + t.lastInsertID = &id +} + +var _ evalengine.VCursor = (*testVcursor)(nil) + +func TestLastInsertID(t *testing.T) { + var testCases = []struct { + expression string + result uint64 + missing bool + }{ + { + expression: `last_insert_id(1)`, + result: 1, + }, { + expression: `12`, + missing: true, + }, { + expression: `last_insert_id(666)`, + result: 666, + }, { + expression: `last_insert_id(null)`, + result: 0, + }, + } + + venv := vtenv.NewTestEnv() + for _, tc := range testCases { + t.Run(tc.expression, func(t *testing.T) { + expr, err := venv.Parser().ParseExpr(tc.expression) + require.NoError(t, err) + + cfg := &evalengine.Config{ + Collation: collations.CollationUtf8mb4ID, + NoConstantFolding: true, + NoCompilation: false, + Environment: venv, + } + t.Run("eval", func(t *testing.T) { + cfg.NoCompilation = true + runTest(t, expr, cfg, tc) + }) + t.Run("compiled", func(t *testing.T) { + cfg.NoCompilation = false + runTest(t, expr, cfg, tc) + }) + }) + } +} + +func runTest(t *testing.T, expr sqlparser.Expr, cfg *evalengine.Config, tc struct { + expression string + result uint64 + missing bool +}) { + converted, err := evalengine.Translate(expr, cfg) + require.NoError(t, err) + + vc := &testVcursor{env: vtenv.NewTestEnv()} + env := evalengine.NewExpressionEnv(context.Background(), nil, vc) + + _, err = env.Evaluate(converted) + require.NoError(t, err) + if tc.missing { + require.Nil(t, vc.lastInsertID) + } else { + require.NotNil(t, vc.lastInsertID) + require.Equal(t, tc.result, *vc.lastInsertID) + } +} + func TestCompilerNonConstant(t *testing.T) { var testCases = []struct { expression string diff --git a/go/vt/vtgate/evalengine/expr_env.go b/go/vt/vtgate/evalengine/expr_env.go index 38a65f9b4e0..4a7f9849ab0 100644 --- a/go/vt/vtgate/evalengine/expr_env.go +++ b/go/vt/vtgate/evalengine/expr_env.go @@ -35,6 +35,7 @@ type VCursor interface { GetKeyspace() string SQLMode() string Environment() *vtenv.Environment + SetLastInsertID(id uint64) } type ( @@ -140,6 +141,7 @@ func (e *emptyVCursor) GetKeyspace() string { func (e *emptyVCursor) SQLMode() string { return config.DefaultSQLMode } +func (e *emptyVCursor) SetLastInsertID(_ uint64) {} func NewEmptyVCursor(env *vtenv.Environment, tz *time.Location) VCursor { return &emptyVCursor{env: env, tz: tz} diff --git a/go/vt/vtgate/evalengine/fn_misc.go b/go/vt/vtgate/evalengine/fn_misc.go index 8813b62f823..cb17f8d6560 100644 --- a/go/vt/vtgate/evalengine/fn_misc.go +++ b/go/vt/vtgate/evalengine/fn_misc.go @@ -81,6 +81,10 @@ type ( builtinUUIDToBin struct { CallExpr } + + builtinLastInsertID struct { + CallExpr + } ) var _ IR = (*builtinInetAton)(nil) @@ -95,6 +99,7 @@ var _ IR = (*builtinBinToUUID)(nil) var _ IR = (*builtinIsUUID)(nil) var _ IR = (*builtinUUID)(nil) var _ IR = (*builtinUUIDToBin)(nil) +var _ IR = (*builtinLastInsertID)(nil) func (call *builtinInetAton) eval(env *ExpressionEnv) (eval, error) { arg, err := call.arg1(env) @@ -155,6 +160,7 @@ func (call *builtinInetNtoa) compile(c *compiler) (ctype, error) { c.compileToUint64(arg, 1) col := typedCoercionCollation(sqltypes.VarChar, call.collate) c.asm.Fn_INET_NTOA(col) + c.asm.jumpDestination(skip) return ctype{Type: sqltypes.VarChar, Flag: flagNullable, Col: col}, nil @@ -194,6 +200,40 @@ func (call *builtinInet6Aton) compile(c *compiler) (ctype, error) { return ctype{Type: sqltypes.VarBinary, Flag: flagNullable, Col: collationBinary}, nil } +func (call *builtinLastInsertID) eval(env *ExpressionEnv) (eval, error) { + arg, err := call.arg1(env) + if err != nil { + return nil, err + } + if arg == nil { + env.VCursor().SetLastInsertID(0) + return nil, err + } + insertID := uint64(evalToInt64(arg).i) + env.VCursor().SetLastInsertID(insertID) + return newEvalUint64(insertID), nil +} + +func (call *builtinLastInsertID) compile(c *compiler) (ctype, error) { + arg, err := call.Arguments[0].compile(c) + if err != nil { + return ctype{}, err + } + + setZero := c.compileNullCheck1(arg) + c.compileToUint64(arg, 1) + c.asm.Fn_LAST_INSERT_ID() + end := c.asm.jumpFrom() + c.asm.addJump(end) + + c.asm.jumpDestination(setZero) + c.asm.Fn_LAST_INSERT_ID_NULL() + + c.asm.jumpDestination(end) + + return ctype{Type: sqltypes.Uint64, Flag: flagNullable, Col: collationNumeric}, nil +} + func printIPv6AsIPv4(addr netip.Addr) (netip.Addr, bool) { b := addr.AsSlice() if len(b) != 16 { diff --git a/go/vt/vtgate/evalengine/integration/comparison_test.go b/go/vt/vtgate/evalengine/integration/comparison_test.go index ea327601975..d559cb8ab1d 100644 --- a/go/vt/vtgate/evalengine/integration/comparison_test.go +++ b/go/vt/vtgate/evalengine/integration/comparison_test.go @@ -209,6 +209,10 @@ type vcursor struct { env *vtenv.Environment } +func (vc *vcursor) SetLastInsertID(id uint64) {} + +var _ evalengine.VCursor = (*vcursor)(nil) + func (vc *vcursor) GetKeyspace() string { return "vttest" } diff --git a/go/vt/vtgate/evalengine/translate_builtin.go b/go/vt/vtgate/evalengine/translate_builtin.go index 476ee32483b..1f8bd7798aa 100644 --- a/go/vt/vtgate/evalengine/translate_builtin.go +++ b/go/vt/vtgate/evalengine/translate_builtin.go @@ -662,6 +662,11 @@ func (ast *astCompiler) translateFuncExpr(fn *sqlparser.FuncExpr) (IR, error) { return nil, argError(method) } return &builtinReplace{CallExpr: call, collate: ast.cfg.Collation}, nil + case "last_insert_id": + if len(args) != 1 { + return nil, argError(method) + } + return &builtinLastInsertID{CallExpr: call}, nil default: return nil, translateExprNotSupported(fn) } diff --git a/go/vt/vtgate/executorcontext/vcursor_impl.go b/go/vt/vtgate/executorcontext/vcursor_impl.go index df989fd7a67..3f8d7def797 100644 --- a/go/vt/vtgate/executorcontext/vcursor_impl.go +++ b/go/vt/vtgate/executorcontext/vcursor_impl.go @@ -1594,3 +1594,9 @@ func (vc *VCursorImpl) GetContextWithTimeOut(ctx context.Context) (context.Conte func (vc *VCursorImpl) IgnoreMaxMemoryRows() bool { return vc.ignoreMaxMemoryRows } + +func (vc *VCursorImpl) SetLastInsertID(id uint64) { + vc.SafeSession.mu.Lock() + defer vc.SafeSession.mu.Unlock() + vc.SafeSession.LastInsertId = id +} From 0c6313222b2ad6eac02e9d1405d7b86476156f46 Mon Sep 17 00:00:00 2001 From: Vicent Marti Date: Thu, 19 Dec 2024 10:20:43 +0100 Subject: [PATCH 02/13] evalengine: jump inside instruction Signed-off-by: Vicent Marti --- go/vt/vtgate/evalengine/compiler_asm.go | 24 +++++++++--------------- go/vt/vtgate/evalengine/fn_misc.go | 13 +------------ 2 files changed, 10 insertions(+), 27 deletions(-) diff --git a/go/vt/vtgate/evalengine/compiler_asm.go b/go/vt/vtgate/evalengine/compiler_asm.go index 9099f8711c0..7dda215353f 100644 --- a/go/vt/vtgate/evalengine/compiler_asm.go +++ b/go/vt/vtgate/evalengine/compiler_asm.go @@ -5141,21 +5141,15 @@ func (asm *assembler) Introduce(offset int, t sqltypes.Type, col collations.Type func (asm *assembler) Fn_LAST_INSERT_ID() { asm.emit(func(env *ExpressionEnv) int { - arg := env.vm.stack[env.vm.sp-1].(*evalUint64) - env.VCursor().SetLastInsertID(arg.u) + arg := env.vm.stack[env.vm.sp-1] + if arg == nil { + env.VCursor().SetLastInsertID(0) + } else { + iarg := evalToInt64(arg) + uarg := env.vm.arena.newEvalUint64(uint64(iarg.i)) + env.vm.stack[env.vm.sp-1] = uarg + env.VCursor().SetLastInsertID(uarg.u) + } return 1 }, "FN LAST_INSERT_ID UINT64(SP-1)") } - -func (asm *assembler) Fn_LAST_INSERT_ID_NULL() { - asm.emit(func(env *ExpressionEnv) int { - env.VCursor().SetLastInsertID(0) - return 1 - }, "FN LAST_INSERT_ID NULL") -} - -func (asm *assembler) addJump(end *jump) { - asm.emit(func(env *ExpressionEnv) int { - return end.offset() - }, "JUMP") -} diff --git a/go/vt/vtgate/evalengine/fn_misc.go b/go/vt/vtgate/evalengine/fn_misc.go index cb17f8d6560..6f7d27c1101 100644 --- a/go/vt/vtgate/evalengine/fn_misc.go +++ b/go/vt/vtgate/evalengine/fn_misc.go @@ -219,19 +219,8 @@ func (call *builtinLastInsertID) compile(c *compiler) (ctype, error) { if err != nil { return ctype{}, err } - - setZero := c.compileNullCheck1(arg) - c.compileToUint64(arg, 1) c.asm.Fn_LAST_INSERT_ID() - end := c.asm.jumpFrom() - c.asm.addJump(end) - - c.asm.jumpDestination(setZero) - c.asm.Fn_LAST_INSERT_ID_NULL() - - c.asm.jumpDestination(end) - - return ctype{Type: sqltypes.Uint64, Flag: flagNullable, Col: collationNumeric}, nil + return ctype{Type: sqltypes.Uint64, Flag: arg.Flag & flagNullable, Col: collationNumeric}, nil } func printIPv6AsIPv4(addr netip.Addr) (netip.Addr, bool) { From 8771daeeda9f144b1657a611ce124153fd05c289 Mon Sep 17 00:00:00 2001 From: Andres Taylor Date: Fri, 20 Dec 2024 15:52:31 +0100 Subject: [PATCH 03/13] codegen Signed-off-by: Andres Taylor --- go/vt/vtgate/evalengine/cached_size.go | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/go/vt/vtgate/evalengine/cached_size.go b/go/vt/vtgate/evalengine/cached_size.go index c1ed1f9475c..d51c65c75b4 100644 --- a/go/vt/vtgate/evalengine/cached_size.go +++ b/go/vt/vtgate/evalengine/cached_size.go @@ -1181,6 +1181,18 @@ func (cached *builtinLastDay) CachedSize(alloc bool) int64 { size += cached.CallExpr.CachedSize(false) return size } +func (cached *builtinLastInsertID) CachedSize(alloc bool) int64 { + if cached == nil { + return int64(0) + } + size := int64(0) + if alloc { + size += int64(48) + } + // field CallExpr vitess.io/vitess/go/vt/vtgate/evalengine.CallExpr + size += cached.CallExpr.CachedSize(false) + return size +} func (cached *builtinLeftRight) CachedSize(alloc bool) int64 { if cached == nil { return int64(0) From 40e1bd2bf6210917341735da7b3aa14542e82d16 Mon Sep 17 00:00:00 2001 From: Andres Taylor Date: Fri, 20 Dec 2024 16:17:48 +0100 Subject: [PATCH 04/13] make sure evalengine expressions are not simplified Signed-off-by: Andres Taylor --- .../endtoend/vtgate/queries/misc/misc_test.go | 34 +++++++++++++++++++ go/vt/vtgate/evalengine/fn_misc.go | 4 +++ .../planbuilder/testdata/set_cases.json | 19 +++++++++++ 3 files changed, 57 insertions(+) diff --git a/go/test/endtoend/vtgate/queries/misc/misc_test.go b/go/test/endtoend/vtgate/queries/misc/misc_test.go index 7ad9e911686..88242bc9622 100644 --- a/go/test/endtoend/vtgate/queries/misc/misc_test.go +++ b/go/test/endtoend/vtgate/queries/misc/misc_test.go @@ -164,6 +164,7 @@ func TestSetAndGetLastInsertID(t *testing.T) { "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", + "set @x = last_insert_id(%d)", } for _, workload := range []string{"olap", "oltp"} { @@ -187,6 +188,39 @@ func TestSetAndGetLastInsertID(t *testing.T) { } } +func TestSetAndGetLastInsertIDWithInsert(t *testing.T) { + mcmp, closer := start(t) + defer closer() + + mcmp.Exec("insert into t1(id1, id2) values (last_insert_id(12),0)") + mcmp.Exec("select last_insert_id()") + mcmp.Exec("insert into t1(id1, id2) values (13,last_insert_id(0))") + mcmp.Exec("select last_insert_id()") + + mcmp.Exec("begin") + mcmp.Exec("insert into t1(id1, id2) values (last_insert_id(14),0)") + mcmp.Exec("select last_insert_id()") + mcmp.Exec("insert into t1(id1, id2) values (15,last_insert_id(0))") + mcmp.Exec("select last_insert_id()") + mcmp.Exec("commit") + + _, err := mcmp.VtConn.ExecuteFetch("set workload = olap", 1, false) + require.NoError(t, err) + + mcmp.Exec("insert into t1(id1, id2) values (last_insert_id(16),0)") + mcmp.Exec("select last_insert_id()") + mcmp.Exec("insert into t1(id1, id2) values (17,last_insert_id(0))") + mcmp.Exec("select last_insert_id()") + + mcmp.Exec("begin") + mcmp.Exec("insert into t1(id1, id2) values (last_insert_id(18),0)") + mcmp.Exec("select last_insert_id()") + mcmp.Exec("insert into t1(id1, id2) values (19,last_insert_id(0))") + mcmp.Exec("select last_insert_id()") + mcmp.Exec("commit") + +} + // TestVindexHints tests that vindex hints work as intended. func TestVindexHints(t *testing.T) { mcmp, closer := start(t) diff --git a/go/vt/vtgate/evalengine/fn_misc.go b/go/vt/vtgate/evalengine/fn_misc.go index 6f7d27c1101..8d6db24a2dc 100644 --- a/go/vt/vtgate/evalengine/fn_misc.go +++ b/go/vt/vtgate/evalengine/fn_misc.go @@ -223,6 +223,10 @@ func (call *builtinLastInsertID) compile(c *compiler) (ctype, error) { return ctype{Type: sqltypes.Uint64, Flag: arg.Flag & flagNullable, Col: collationNumeric}, nil } +func (call *builtinLastInsertID) constant() bool { + return false // we don't want this function to be simplified away +} + func printIPv6AsIPv4(addr netip.Addr) (netip.Addr, bool) { b := addr.AsSlice() if len(b) != 16 { diff --git a/go/vt/vtgate/planbuilder/testdata/set_cases.json b/go/vt/vtgate/planbuilder/testdata/set_cases.json index 58cb2fffa75..5528c0e32fe 100644 --- a/go/vt/vtgate/planbuilder/testdata/set_cases.json +++ b/go/vt/vtgate/planbuilder/testdata/set_cases.json @@ -605,5 +605,24 @@ ] } } + }, + { + "QueryType": "SET", + "Original": "set @foo = last_insert_id(1)", + "Instructions": { + "OperatorType": "Set", + "Ops": [ + { + "Type": "UserDefinedVariable", + "Name": "foo", + "Expr": "last_insert_id(1)" + } + ], + "Inputs": [ + { + "OperatorType": "SingleRow" + } + ] + } } ] From 5ca7df052f17a04494d06b5f813de171374c6caa Mon Sep 17 00:00:00 2001 From: Harshit Gangal Date: Mon, 30 Dec 2024 20:40:23 +0530 Subject: [PATCH 05/13] addressed review comments Signed-off-by: Harshit Gangal --- go/vt/vtgate/engine/fake_vcursor_test.go | 31 +++++++++++------------- go/vt/vtgate/evalengine/fn_misc.go | 1 - 2 files changed, 14 insertions(+), 18 deletions(-) diff --git a/go/vt/vtgate/engine/fake_vcursor_test.go b/go/vt/vtgate/engine/fake_vcursor_test.go index 72422193ee8..aac3e9b584c 100644 --- a/go/vt/vtgate/engine/fake_vcursor_test.go +++ b/go/vt/vtgate/engine/fake_vcursor_test.go @@ -400,6 +400,20 @@ func (t *noopVCursor) GetDBDDLPluginName() string { panic("unimplemented") } +func (t *noopVCursor) SetLastInsertID(uint64) {} +func (t *noopVCursor) VExplainLogging() {} +func (t *noopVCursor) DisableLogging() {} +func (t *noopVCursor) GetVExplainLogs() []ExecuteEntry { + return nil +} +func (t *noopVCursor) GetLogs() ([]ExecuteEntry, error) { + return nil, nil +} + +// RecordMirrorStats implements VCursor. +func (t *noopVCursor) RecordMirrorStats(sourceExecTime, targetExecTime time.Duration, targetErr error) { +} + var ( _ VCursor = (*loggingVCursor)(nil) _ SessionActions = (*loggingVCursor)(nil) @@ -893,23 +907,6 @@ func (t *loggingVCursor) RecordMirrorStats(sourceExecTime, targetExecTime time.D } } -func (t *loggingVCursor) SetLastInsertID(id uint64) {} -func (t *noopVCursor) SetLastInsertID(id uint64) {} - -func (t *noopVCursor) VExplainLogging() {} -func (t *noopVCursor) DisableLogging() {} -func (t *noopVCursor) GetVExplainLogs() []ExecuteEntry { - return nil -} - -func (t *noopVCursor) GetLogs() ([]ExecuteEntry, error) { - return nil, nil -} - -// RecordMirrorStats implements VCursor. -func (t *noopVCursor) RecordMirrorStats(sourceExecTime, targetExecTime time.Duration, targetErr error) { -} - func expectResult(t *testing.T, result, want *sqltypes.Result) { t.Helper() fieldsResult := fmt.Sprintf("%v", result.Fields) diff --git a/go/vt/vtgate/evalengine/fn_misc.go b/go/vt/vtgate/evalengine/fn_misc.go index 8d6db24a2dc..2a2119ee6f4 100644 --- a/go/vt/vtgate/evalengine/fn_misc.go +++ b/go/vt/vtgate/evalengine/fn_misc.go @@ -160,7 +160,6 @@ func (call *builtinInetNtoa) compile(c *compiler) (ctype, error) { c.compileToUint64(arg, 1) col := typedCoercionCollation(sqltypes.VarChar, call.collate) c.asm.Fn_INET_NTOA(col) - c.asm.jumpDestination(skip) return ctype{Type: sqltypes.VarChar, Flag: flagNullable, Col: col}, nil From a86bf2c2a3b76c742cf0cfff6648da02324edd0b Mon Sep 17 00:00:00 2001 From: Harshit Gangal Date: Mon, 30 Dec 2024 20:50:05 +0530 Subject: [PATCH 06/13] test: fix set test case Signed-off-by: Harshit Gangal --- go/vt/vtgate/engine/set.go | 17 ++++----- .../planbuilder/testdata/set_cases.json | 36 ++++++++++--------- 2 files changed, 26 insertions(+), 27 deletions(-) diff --git a/go/vt/vtgate/engine/set.go b/go/vt/vtgate/engine/set.go index 95fb5c87a32..f0de330cbed 100644 --- a/go/vt/vtgate/engine/set.go +++ b/go/vt/vtgate/engine/set.go @@ -22,23 +22,18 @@ import ( "fmt" "strings" - "vitess.io/vitess/go/vt/sqlparser" - "vitess.io/vitess/go/vt/sysvars" - - vtgatepb "vitess.io/vitess/go/vt/proto/vtgate" - - "vitess.io/vitess/go/vt/log" - - "vitess.io/vitess/go/vt/srvtopo" - - "vitess.io/vitess/go/vt/vtgate/evalengine" - "vitess.io/vitess/go/sqltypes" "vitess.io/vitess/go/vt/key" + "vitess.io/vitess/go/vt/log" querypb "vitess.io/vitess/go/vt/proto/query" + vtgatepb "vitess.io/vitess/go/vt/proto/vtgate" vtrpcpb "vitess.io/vitess/go/vt/proto/vtrpc" "vitess.io/vitess/go/vt/schema" + "vitess.io/vitess/go/vt/sqlparser" + "vitess.io/vitess/go/vt/srvtopo" + "vitess.io/vitess/go/vt/sysvars" "vitess.io/vitess/go/vt/vterrors" + "vitess.io/vitess/go/vt/vtgate/evalengine" "vitess.io/vitess/go/vt/vtgate/vindexes" ) diff --git a/go/vt/vtgate/planbuilder/testdata/set_cases.json b/go/vt/vtgate/planbuilder/testdata/set_cases.json index 5528c0e32fe..02c5603a03c 100644 --- a/go/vt/vtgate/planbuilder/testdata/set_cases.json +++ b/go/vt/vtgate/planbuilder/testdata/set_cases.json @@ -607,22 +607,26 @@ } }, { - "QueryType": "SET", - "Original": "set @foo = last_insert_id(1)", - "Instructions": { - "OperatorType": "Set", - "Ops": [ - { - "Type": "UserDefinedVariable", - "Name": "foo", - "Expr": "last_insert_id(1)" - } - ], - "Inputs": [ - { - "OperatorType": "SingleRow" - } - ] + "comment": "set last_insert_id with agrument to user defined variable", + "query": "set @foo = last_insert_id(1)", + "plan": { + "QueryType": "SET", + "Original": "set @foo = last_insert_id(1)", + "Instructions": { + "OperatorType": "Set", + "Ops": [ + { + "Type": "UserDefinedVariable", + "Name": "foo", + "Expr": "last_insert_id(1)" + } + ], + "Inputs": [ + { + "OperatorType": "SingleRow" + } + ] + } } } ] From b2ba42fa10e7fc1d3130ded81102487dfafb627e Mon Sep 17 00:00:00 2001 From: Andres Taylor Date: Tue, 7 Jan 2025 10:01:40 +0100 Subject: [PATCH 07/13] test: change to table driven test Signed-off-by: Andres Taylor --- .../endtoend/vtgate/queries/misc/misc_test.go | 53 +++++++++++-------- 1 file changed, 30 insertions(+), 23 deletions(-) diff --git a/go/test/endtoend/vtgate/queries/misc/misc_test.go b/go/test/endtoend/vtgate/queries/misc/misc_test.go index 88242bc9622..d1547e0fc46 100644 --- a/go/test/endtoend/vtgate/queries/misc/misc_test.go +++ b/go/test/endtoend/vtgate/queries/misc/misc_test.go @@ -24,6 +24,8 @@ import ( "testing" "time" + "vitess.io/vitess/go/vt/log" + "vitess.io/vitess/go/mysql" _ "github.com/go-sql-driver/mysql" @@ -192,33 +194,38 @@ func TestSetAndGetLastInsertIDWithInsert(t *testing.T) { mcmp, closer := start(t) defer closer() - mcmp.Exec("insert into t1(id1, id2) values (last_insert_id(12),0)") - mcmp.Exec("select last_insert_id()") - mcmp.Exec("insert into t1(id1, id2) values (13,last_insert_id(0))") - mcmp.Exec("select last_insert_id()") - - mcmp.Exec("begin") - mcmp.Exec("insert into t1(id1, id2) values (last_insert_id(14),0)") - mcmp.Exec("select last_insert_id()") - mcmp.Exec("insert into t1(id1, id2) values (15,last_insert_id(0))") - mcmp.Exec("select last_insert_id()") - mcmp.Exec("commit") + tests := []string{ + "insert into t1(id1, id2) values (last_insert_id(%d),%d)", + "insert into t1(id1, id2) values (%d, last_insert_id(%d))", + } - _, err := mcmp.VtConn.ExecuteFetch("set workload = olap", 1, false) - require.NoError(t, err) + i := 0 + getVal := func() int { + defer func() { i++ }() + return i + } - mcmp.Exec("insert into t1(id1, id2) values (last_insert_id(16),0)") - mcmp.Exec("select last_insert_id()") - mcmp.Exec("insert into t1(id1, id2) values (17,last_insert_id(0))") - mcmp.Exec("select last_insert_id()") + runTests := func(mcmp *utils.MySQLCompare) { + for _, test := range tests { + query := fmt.Sprintf(test, getVal(), getVal()) + log.Errorf("test: %s", query) + mcmp.Exec(query) + mcmp.Exec("select last_insert_id()") + } + } - mcmp.Exec("begin") - mcmp.Exec("insert into t1(id1, id2) values (last_insert_id(18),0)") - mcmp.Exec("select last_insert_id()") - mcmp.Exec("insert into t1(id1, id2) values (19,last_insert_id(0))") - mcmp.Exec("select last_insert_id()") - mcmp.Exec("commit") + for _, workload := range []string{"olap", "oltp"} { + mcmp.Run(workload, func(mcmp *utils.MySQLCompare) { + _, err := mcmp.VtConn.ExecuteFetch("set workload = "+workload, 1, false) + require.NoError(t, err) + runTests(mcmp) + // run the queries again, but inside a transaction this time + mcmp.Exec("begin") + runTests(mcmp) + mcmp.Exec("commit") + }) + } } // TestVindexHints tests that vindex hints work as intended. From 0ea6afdf60795a64ed7a374cafb3466dc3f88f92 Mon Sep 17 00:00:00 2001 From: Andres Taylor Date: Tue, 7 Jan 2025 15:47:05 +0100 Subject: [PATCH 08/13] handle INSERT with last_insert_id() for sharded keyspaces Signed-off-by: Andres Taylor --- go/vt/vtgate/engine/insert.go | 6 +-- go/vt/vtgate/engine/insert_common.go | 2 +- go/vt/vtgate/engine/insert_select.go | 2 +- go/vt/vtgate/planbuilder/insert.go | 11 +++--- .../planbuilder/operator_transformers.go | 6 +-- .../planbuilder/testdata/aggr_cases.json | 38 +++++++++++++++++++ .../planbuilder/testdata/dml_cases.json | 24 ++++++++++++ 7 files changed, 75 insertions(+), 14 deletions(-) diff --git a/go/vt/vtgate/engine/insert.go b/go/vt/vtgate/engine/insert.go index 10a4048572f..5bc206f7465 100644 --- a/go/vt/vtgate/engine/insert.go +++ b/go/vt/vtgate/engine/insert.go @@ -58,11 +58,9 @@ type Insert struct { // Alias represents the row alias with columns if specified in the query. Alias string - - FetchLastInsertID bool } -// newQueryInsert creates an Insert with a query string. +// newQueryInsert creates an Insert with a query string. Used in testing. func newQueryInsert(opcode InsertOpcode, keyspace *vindexes.Keyspace, query string) *Insert { return &Insert{ InsertCommon: InsertCommon{ @@ -73,7 +71,7 @@ func newQueryInsert(opcode InsertOpcode, keyspace *vindexes.Keyspace, query stri } } -// newInsert creates a new Insert. +// newInsert creates a new Insert. Used in testing. func newInsert( opcode InsertOpcode, ignore bool, diff --git a/go/vt/vtgate/engine/insert_common.go b/go/vt/vtgate/engine/insert_common.go index 629d848d978..d4cae045e86 100644 --- a/go/vt/vtgate/engine/insert_common.go +++ b/go/vt/vtgate/engine/insert_common.go @@ -161,7 +161,7 @@ func (ins *InsertCommon) executeUnshardedTableQuery(ctx context.Context, vcursor if err != nil { return nil, err } - qr, err := execShard(ctx, loggingPrimitive, vcursor, query, bindVars, rss[0], true, !ins.PreventAutoCommit /* canAutocommit */, false) + qr, err := execShard(ctx, loggingPrimitive, vcursor, query, bindVars, rss[0], true, !ins.PreventAutoCommit /* canAutocommit */, ins.FetchLastInsertID) if err != nil { return nil, err } diff --git a/go/vt/vtgate/engine/insert_select.go b/go/vt/vtgate/engine/insert_select.go index bccee5f2cf9..af834858175 100644 --- a/go/vt/vtgate/engine/insert_select.go +++ b/go/vt/vtgate/engine/insert_select.go @@ -51,7 +51,7 @@ type ( } ) -// newInsertSelect creates a new InsertSelect. +// newInsertSelect creates a new InsertSelect. Used in testing. func newInsertSelect( ignore bool, keyspace *vindexes.Keyspace, diff --git a/go/vt/vtgate/planbuilder/insert.go b/go/vt/vtgate/planbuilder/insert.go index 80516871623..d3ad5afac72 100644 --- a/go/vt/vtgate/planbuilder/insert.go +++ b/go/vt/vtgate/planbuilder/insert.go @@ -51,7 +51,7 @@ func gen4InsertStmtPlanner(version querypb.ExecuteOptions_PlannerVersion, insStm } if ks != nil { if tables[0].AutoIncrement == nil && !ctx.SemTable.ForeignKeysPresent() { - plan := insertUnshardedShortcut(insStmt, ks, tables) + plan := insertUnshardedShortcut(ctx, insStmt, ks, tables) setCommentDirectivesOnPlan(plan, insStmt) return newPlanResult(plan, operators.QualifiedTables(ks, tables)...), nil } @@ -90,12 +90,13 @@ func errOutIfPlanCannotBeConstructed(ctx *plancontext.PlanningContext, vTbl *vin return ctx.SemTable.NotUnshardedErr } -func insertUnshardedShortcut(stmt *sqlparser.Insert, ks *vindexes.Keyspace, tables []*vindexes.Table) engine.Primitive { +func insertUnshardedShortcut(ctx *plancontext.PlanningContext, stmt *sqlparser.Insert, ks *vindexes.Keyspace, tables []*vindexes.Table) engine.Primitive { eIns := &engine.Insert{ InsertCommon: engine.InsertCommon{ - Opcode: engine.InsertUnsharded, - Keyspace: ks, - TableName: tables[0].Name.String(), + Opcode: engine.InsertUnsharded, + Keyspace: ks, + TableName: tables[0].Name.String(), + FetchLastInsertID: ctx.SemTable.ShouldFetchLastInsertID(), }, } eIns.Query = generateQuery(stmt) diff --git a/go/vt/vtgate/planbuilder/operator_transformers.go b/go/vt/vtgate/planbuilder/operator_transformers.go index bc71c7195b4..b51eac449fc 100644 --- a/go/vt/vtgate/planbuilder/operator_transformers.go +++ b/go/vt/vtgate/planbuilder/operator_transformers.go @@ -190,6 +190,7 @@ func transformInsertionSelection(ctx *plancontext.PlanningContext, op *operators ForceNonStreaming: op.ForceNonStreaming, Generate: autoIncGenerate(ins.AutoIncrement), ColVindexes: ins.ColVindexes, + FetchLastInsertID: ctx.SemTable.ShouldFetchLastInsertID(), }, VindexValueOffset: ins.VindexValueOffset, } @@ -659,9 +660,8 @@ func buildInsertPrimitive( } eins := &engine.Insert{ - InsertCommon: ic, - VindexValues: ins.VindexValues, - FetchLastInsertID: ctx.SemTable.ShouldFetchLastInsertID(), + InsertCommon: ic, + VindexValues: ins.VindexValues, } // we would need to generate the query on the fly. The only exception here is diff --git a/go/vt/vtgate/planbuilder/testdata/aggr_cases.json b/go/vt/vtgate/planbuilder/testdata/aggr_cases.json index 49a03a8f05a..1ecbf3d4ff9 100644 --- a/go/vt/vtgate/planbuilder/testdata/aggr_cases.json +++ b/go/vt/vtgate/planbuilder/testdata/aggr_cases.json @@ -6160,6 +6160,44 @@ ] } }, + { + "comment": "last_insert_id on aggregation calculated at the vtgate level", + "query": "select last_insert_id(count(*)) from user", + "plan": { + "QueryType": "SELECT", + "Original": "select last_insert_id(count(*)) from user", + "Instructions": { + "OperatorType": "Projection", + "Expressions": [ + "last_insert_id(count(*)) as last_insert_id(count(*))" + ], + "Inputs": [ + { + "OperatorType": "Aggregate", + "Variant": "Scalar", + "Aggregates": "sum_count_star(0) AS count(*)", + "Inputs": [ + { + "OperatorType": "Route", + "Variant": "Scatter", + "Keyspace": { + "Name": "user", + "Sharded": true + }, + "FetchLastInsertID": true, + "FieldQuery": "select count(*) from `user` where 1 != 1", + "Query": "select count(*) from `user`", + "Table": "`user`" + } + ] + } + ] + }, + "TablesUsed": [ + "user.user" + ] + } + }, { "comment": "aggregation on top of aggregation works fine", "query": "select distinct count(*) from user, (select distinct count(*) from user) X", diff --git a/go/vt/vtgate/planbuilder/testdata/dml_cases.json b/go/vt/vtgate/planbuilder/testdata/dml_cases.json index 8893b4df0c0..47cbc066aea 100644 --- a/go/vt/vtgate/planbuilder/testdata/dml_cases.json +++ b/go/vt/vtgate/planbuilder/testdata/dml_cases.json @@ -2648,6 +2648,30 @@ }, "skip_e2e": true }, + { + "comment": "insert using last_insert_id with argument", + "query": "insert into unsharded values(last_insert_id(24), 2)", + "plan": { + "QueryType": "INSERT", + "Original": "insert into unsharded values(last_insert_id(24), 2)", + "Instructions": { + "OperatorType": "Insert", + "Variant": "Unsharded", + "Keyspace": { + "Name": "main", + "Sharded": false + }, + "TargetTabletType": "PRIMARY", + "FetchLastInsertID": true, + "Query": "insert into unsharded values (last_insert_id(24), 2)", + "TableName": "unsharded" + }, + "TablesUsed": [ + "main.unsharded" + ] + }, + "skip_e2e": true + }, { "comment": "update vindex value to null with multiple primary keyspace id", "query": "update user set name = null where id in (1, 2, 3)", From 72e2e1e4b7f7d1e0b06bbc35250065b31528ca2d Mon Sep 17 00:00:00 2001 From: Florent Poinsard Date: Tue, 7 Jan 2025 09:52:27 -0600 Subject: [PATCH 09/13] Unsharded tests for last insert id Signed-off-by: Florent Poinsard --- .../endtoend/vtgate/queries/misc/misc_test.go | 56 +++++++++++++++++-- .../planbuilder/testdata/dml_cases.json | 3 +- 2 files changed, 53 insertions(+), 6 deletions(-) diff --git a/go/test/endtoend/vtgate/queries/misc/misc_test.go b/go/test/endtoend/vtgate/queries/misc/misc_test.go index d1547e0fc46..6034ff4a7f5 100644 --- a/go/test/endtoend/vtgate/queries/misc/misc_test.go +++ b/go/test/endtoend/vtgate/queries/misc/misc_test.go @@ -24,8 +24,6 @@ import ( "testing" "time" - "vitess.io/vitess/go/vt/log" - "vitess.io/vitess/go/mysql" _ "github.com/go-sql-driver/mysql" @@ -190,13 +188,64 @@ func TestSetAndGetLastInsertID(t *testing.T) { } } +func TestSetAndGetLastInsertIDWithInsertUnsharded(t *testing.T) { + // in this test we can't use the mcmp, so we need to assert the returned values manually + mcmp, closer := start(t) + defer closer() + + tests := []string{ + "insert into uks.unsharded(id1, id2) values (last_insert_id(%d),12)", + "insert into uks.unsharded(id1, id2) select last_insert_id(%d), 453", + } + + i := 0 + getVal := func() int { + defer func() { i++ }() + return i + } + + runTests := func(mcmp *utils.MySQLCompare) { + for _, test := range tests { + lastInsertID := getVal() + query := fmt.Sprintf(test, lastInsertID) + utils.Exec(mcmp.AsT(), mcmp.VtConn, query) + result := utils.Exec(mcmp.AsT(), mcmp.VtConn, "select last_insert_id()") + uintVal, err := result.Rows[0][0].ToCastUint64() + require.NoError(mcmp.AsT(), err) + require.EqualValues(mcmp.AsT(), lastInsertID, uintVal, query) + } + } + + for _, workload := range []string{"olap", "oltp"} { + mcmp.Run(workload, func(mcmp *utils.MySQLCompare) { + _, err := mcmp.VtConn.ExecuteFetch("set workload = "+workload, 1, false) + require.NoError(t, err) + runTests(mcmp) + + // run the queries again, but inside a transaction this time + mcmp.Exec("begin") + runTests(mcmp) + mcmp.Exec("commit") + }) + } + + // Now test to set the last insert id to 0, see that it has changed correctly even if the value is 0 + utils.Exec(t, mcmp.VtConn, "insert into uks.unsharded(id1, id2) values (last_insert_id(0),12)") + result := utils.Exec(t, mcmp.VtConn, "select last_insert_id()") + uintVal, err := result.Rows[0][0].ToCastUint64() + require.NoError(t, err) + require.Zero(t, uintVal) +} + func TestSetAndGetLastInsertIDWithInsert(t *testing.T) { mcmp, closer := start(t) defer closer() tests := []string{ - "insert into t1(id1, id2) values (last_insert_id(%d),%d)", + "insert into t1(id1, id2) values (last_insert_id(%d) ,%d)", "insert into t1(id1, id2) values (%d, last_insert_id(%d))", + "insert into t1(id1, id2) select last_insert_id(%d), %d", + "insert into t1(id1, id2) select last_insert_id(id1+%d), 12 from t1 where 1 > %d", } i := 0 @@ -208,7 +257,6 @@ func TestSetAndGetLastInsertIDWithInsert(t *testing.T) { runTests := func(mcmp *utils.MySQLCompare) { for _, test := range tests { query := fmt.Sprintf(test, getVal(), getVal()) - log.Errorf("test: %s", query) mcmp.Exec(query) mcmp.Exec("select last_insert_id()") } diff --git a/go/vt/vtgate/planbuilder/testdata/dml_cases.json b/go/vt/vtgate/planbuilder/testdata/dml_cases.json index 47cbc066aea..1efb85f5cdb 100644 --- a/go/vt/vtgate/planbuilder/testdata/dml_cases.json +++ b/go/vt/vtgate/planbuilder/testdata/dml_cases.json @@ -2669,8 +2669,7 @@ "TablesUsed": [ "main.unsharded" ] - }, - "skip_e2e": true + } }, { "comment": "update vindex value to null with multiple primary keyspace id", From c6f615c48302567c6c2129a0e43ad11be8e3f34c Mon Sep 17 00:00:00 2001 From: Florent Poinsard Date: Tue, 7 Jan 2025 10:09:41 -0600 Subject: [PATCH 10/13] Simplify tests and regen code Signed-off-by: Florent Poinsard --- go/test/endtoend/utils/cmp.go | 4 ++-- .../vtgate/plan_tests/plan_e2e_test.go | 2 +- .../endtoend/vtgate/queries/misc/main_test.go | 2 +- .../endtoend/vtgate/queries/misc/misc_test.go | 23 +++++++++++-------- go/vt/sqlparser/ast_test.go | 8 +++++++ go/vt/vtgate/engine/cached_size.go | 2 +- 6 files changed, 26 insertions(+), 15 deletions(-) diff --git a/go/test/endtoend/utils/cmp.go b/go/test/endtoend/utils/cmp.go index 3a05c75c33b..b2e1eca03e9 100644 --- a/go/test/endtoend/utils/cmp.go +++ b/go/test/endtoend/utils/cmp.go @@ -215,8 +215,8 @@ func (mcmp *MySQLCompare) Exec(query string) *sqltypes.Result { return vtQr } -// ExecVitessAndMySQL executes Vitess and MySQL with the queries provided. -func (mcmp *MySQLCompare) ExecVitessAndMySQL(vtQ, mQ string) *sqltypes.Result { +// ExecVitessAndMySQLDifferentQueries executes Vitess and MySQL with the queries provided. +func (mcmp *MySQLCompare) ExecVitessAndMySQLDifferentQueries(vtQ, mQ string) *sqltypes.Result { mcmp.t.Helper() vtQr, err := mcmp.VtConn.ExecuteFetch(vtQ, 1000, true) require.NoError(mcmp.t, err, "[Vitess Error] for query: "+vtQ) diff --git a/go/test/endtoend/vtgate/plan_tests/plan_e2e_test.go b/go/test/endtoend/vtgate/plan_tests/plan_e2e_test.go index ffe712c141c..0068616c3b8 100644 --- a/go/test/endtoend/vtgate/plan_tests/plan_e2e_test.go +++ b/go/test/endtoend/vtgate/plan_tests/plan_e2e_test.go @@ -49,7 +49,7 @@ func TestE2ECases(t *testing.T) { require.NoError(mcmp.AsT(), err) sqlparser.RemoveKeyspaceIgnoreSysSchema(stmt) - mcmp.ExecVitessAndMySQL(test.Query, sqlparser.String(stmt)) + mcmp.ExecVitessAndMySQLDifferentQueries(test.Query, sqlparser.String(stmt)) pd := utils.ExecTrace(mcmp.AsT(), mcmp.VtConn, test.Query) verifyTestExpectations(mcmp.AsT(), pd, test) if mcmp.VtConn.IsClosed() { diff --git a/go/test/endtoend/vtgate/queries/misc/main_test.go b/go/test/endtoend/vtgate/queries/misc/main_test.go index ee9be542634..536dfa7500a 100644 --- a/go/test/endtoend/vtgate/queries/misc/main_test.go +++ b/go/test/endtoend/vtgate/queries/misc/main_test.go @@ -95,7 +95,7 @@ func TestMain(m *testing.M) { vtParams = clusterInstance.GetVTParams(keyspaceName) // create mysql instance and connection parameters - conn, closer, err := utils.NewMySQL(clusterInstance, keyspaceName, schemaSQL) + conn, closer, err := utils.NewMySQL(clusterInstance, keyspaceName, schemaSQL, uschemaSQL) if err != nil { fmt.Println(err) return 1 diff --git a/go/test/endtoend/vtgate/queries/misc/misc_test.go b/go/test/endtoend/vtgate/queries/misc/misc_test.go index 6034ff4a7f5..7ab0fe7ef54 100644 --- a/go/test/endtoend/vtgate/queries/misc/misc_test.go +++ b/go/test/endtoend/vtgate/queries/misc/misc_test.go @@ -25,6 +25,7 @@ import ( "time" "vitess.io/vitess/go/mysql" + "vitess.io/vitess/go/vt/sqlparser" _ "github.com/go-sql-driver/mysql" "github.com/stretchr/testify/assert" @@ -189,7 +190,6 @@ func TestSetAndGetLastInsertID(t *testing.T) { } func TestSetAndGetLastInsertIDWithInsertUnsharded(t *testing.T) { - // in this test we can't use the mcmp, so we need to assert the returned values manually mcmp, closer := start(t) defer closer() @@ -206,13 +206,16 @@ func TestSetAndGetLastInsertIDWithInsertUnsharded(t *testing.T) { runTests := func(mcmp *utils.MySQLCompare) { for _, test := range tests { + lastInsertID := getVal() query := fmt.Sprintf(test, lastInsertID) - utils.Exec(mcmp.AsT(), mcmp.VtConn, query) - result := utils.Exec(mcmp.AsT(), mcmp.VtConn, "select last_insert_id()") - uintVal, err := result.Rows[0][0].ToCastUint64() + + stmt, err := sqlparser.NewTestParser().Parse(query) require.NoError(mcmp.AsT(), err) - require.EqualValues(mcmp.AsT(), lastInsertID, uintVal, query) + sqlparser.RemoveKeyspaceIgnoreSysSchema(stmt) + + mcmp.ExecVitessAndMySQLDifferentQueries(query, sqlparser.String(stmt)) + mcmp.Exec("select last_insert_id()") } } @@ -230,11 +233,11 @@ func TestSetAndGetLastInsertIDWithInsertUnsharded(t *testing.T) { } // Now test to set the last insert id to 0, see that it has changed correctly even if the value is 0 - utils.Exec(t, mcmp.VtConn, "insert into uks.unsharded(id1, id2) values (last_insert_id(0),12)") - result := utils.Exec(t, mcmp.VtConn, "select last_insert_id()") - uintVal, err := result.Rows[0][0].ToCastUint64() - require.NoError(t, err) - require.Zero(t, uintVal) + mcmp.ExecVitessAndMySQLDifferentQueries( + "insert into uks.unsharded(id1, id2) values (last_insert_id(0),12)", + "insert into unsharded(id1, id2) values (last_insert_id(0),12)", + ) + mcmp.Exec("select last_insert_id()") } func TestSetAndGetLastInsertIDWithInsert(t *testing.T) { diff --git a/go/vt/sqlparser/ast_test.go b/go/vt/sqlparser/ast_test.go index f01b47cbd7b..c1484df7cc4 100644 --- a/go/vt/sqlparser/ast_test.go +++ b/go/vt/sqlparser/ast_test.go @@ -917,3 +917,11 @@ func TestCloneComments(t *testing.T) { assert.Equal(t, "b", val) } } + +func TestRemoveKeyspace(t *testing.T) { + stmt, err := NewTestParser().Parse("select 1 from uks.unsharded") + require.NoError(t, err) + RemoveKeyspaceIgnoreSysSchema(stmt) + + require.Equal(t, "select 1 from unsharded", String(stmt)) +} diff --git a/go/vt/vtgate/engine/cached_size.go b/go/vt/vtgate/engine/cached_size.go index e59832cdab5..50d3a4b6bbf 100644 --- a/go/vt/vtgate/engine/cached_size.go +++ b/go/vt/vtgate/engine/cached_size.go @@ -465,7 +465,7 @@ func (cached *Insert) CachedSize(alloc bool) int64 { } size := int64(0) if alloc { - size += int64(240) + size += int64(224) } // field InsertCommon vitess.io/vitess/go/vt/vtgate/engine.InsertCommon size += cached.InsertCommon.CachedSize(false) From e6e67424ae30d3678bc092351e257abac27a2ef5 Mon Sep 17 00:00:00 2001 From: Florent Poinsard Date: Tue, 7 Jan 2025 10:15:31 -0600 Subject: [PATCH 11/13] Fix dml_cases Signed-off-by: Florent Poinsard --- go/vt/vtgate/planbuilder/testdata/dml_cases.json | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/go/vt/vtgate/planbuilder/testdata/dml_cases.json b/go/vt/vtgate/planbuilder/testdata/dml_cases.json index 1efb85f5cdb..e2048273c2f 100644 --- a/go/vt/vtgate/planbuilder/testdata/dml_cases.json +++ b/go/vt/vtgate/planbuilder/testdata/dml_cases.json @@ -2650,10 +2650,10 @@ }, { "comment": "insert using last_insert_id with argument", - "query": "insert into unsharded values(last_insert_id(24), 2)", + "query": "insert into unsharded values(last_insert_id(24), 2, 2, 2, 2, 2)", "plan": { "QueryType": "INSERT", - "Original": "insert into unsharded values(last_insert_id(24), 2)", + "Original": "insert into unsharded values(last_insert_id(24), 2, 2, 2, 2, 2)", "Instructions": { "OperatorType": "Insert", "Variant": "Unsharded", @@ -2663,7 +2663,7 @@ }, "TargetTabletType": "PRIMARY", "FetchLastInsertID": true, - "Query": "insert into unsharded values (last_insert_id(24), 2)", + "Query": "insert into unsharded values (last_insert_id(24), 2, 2, 2, 2, 2)", "TableName": "unsharded" }, "TablesUsed": [ From 192ede01fbd958a616441f9ae21d39fd6184f554 Mon Sep 17 00:00:00 2001 From: Florent Poinsard Date: Tue, 7 Jan 2025 10:19:04 -0600 Subject: [PATCH 12/13] Ignore e2e test in dml_cases Signed-off-by: Florent Poinsard --- go/vt/vtgate/planbuilder/testdata/dml_cases.json | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/go/vt/vtgate/planbuilder/testdata/dml_cases.json b/go/vt/vtgate/planbuilder/testdata/dml_cases.json index e2048273c2f..95cb14e38f5 100644 --- a/go/vt/vtgate/planbuilder/testdata/dml_cases.json +++ b/go/vt/vtgate/planbuilder/testdata/dml_cases.json @@ -2649,11 +2649,11 @@ "skip_e2e": true }, { - "comment": "insert using last_insert_id with argument", - "query": "insert into unsharded values(last_insert_id(24), 2, 2, 2, 2, 2)", + "comment": "insert using last_insert_id with argument (already an e2e test for this plan)", + "query": "insert into unsharded values(last_insert_id(789), 2)", "plan": { "QueryType": "INSERT", - "Original": "insert into unsharded values(last_insert_id(24), 2, 2, 2, 2, 2)", + "Original": "insert into unsharded values(last_insert_id(789), 2)", "Instructions": { "OperatorType": "Insert", "Variant": "Unsharded", @@ -2663,13 +2663,14 @@ }, "TargetTabletType": "PRIMARY", "FetchLastInsertID": true, - "Query": "insert into unsharded values (last_insert_id(24), 2, 2, 2, 2, 2)", + "Query": "insert into unsharded values (last_insert_id(789), 2)", "TableName": "unsharded" }, "TablesUsed": [ "main.unsharded" ] - } + }, + "skip_e2e": true }, { "comment": "update vindex value to null with multiple primary keyspace id", From afde647c76e11fe49bcea9e144dde1418322eb5d Mon Sep 17 00:00:00 2001 From: Andres Taylor Date: Wed, 8 Jan 2025 09:52:51 +0100 Subject: [PATCH 13/13] add to release notes about last_insert_id Signed-off-by: Andres Taylor --- changelog/22.0/22.0.0/summary.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/changelog/22.0/22.0.0/summary.md b/changelog/22.0/22.0.0/summary.md index a54a7a4a9ae..bfc55064148 100644 --- a/changelog/22.0/22.0.0/summary.md +++ b/changelog/22.0/22.0.0/summary.md @@ -10,6 +10,7 @@ - **[VTOrc Config File Changes](#vtorc-config-file-changes)** - **[VTGate Config File Changes](#vtgate-config-file-changes)** - **[Support for More Efficient JSON Replication](#efficient-json-replication)** + - **[Support for LAST_INSERT_ID(x)](#last-insert-id)** - **[Minor Changes](#minor-changes)** - **[VTTablet Flags](#flags-vttablet)** - **[Topology read concurrency behaviour changes](#topo-read-concurrency-changes)** @@ -80,6 +81,12 @@ In [#7345](https://github.com/vitessio/vitess/pull/17345) we added support for [ If you are using MySQL 8.0 or later and using JSON columns, you can now enable this MySQL feature across your Vitess cluster(s) to lower the disk space needed for binary logs and improve the CPU and memory usage in both `mysqld` (standard intrashard MySQL replication) and `vttablet` ([VReplication](https://vitess.io/docs/reference/vreplication/vreplication/)) without losing any capabilities or features. +### Support for `LAST_INSERT_ID(x)` + +In [#17408](https://github.com/vitessio/vitess/pull/17408) and [#17409](https://github.com/vitessio/vitess/pull/17409), we added the ability to use `LAST_INSERT_ID(x)` in Vitess directly at vtgate. This improvement allows certain queries—like `SELECT last_insert_id(123);` or `SELECT last_insert_id(count(*)) ...`—to be handled without relying on MySQL for the final value. + +**Limitations**: +- When using `LAST_INSERT_ID(x)` in ordered queries (e.g., `SELECT last_insert_id(col) FROM table ORDER BY foo`), MySQL sets the session’s last-insert-id value according to the *last row returned*. Vitess does not guarantee the same behavior. ## Minor Changes