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

Fix Set Statement in Tablet when executed with bindvars #7431

Merged
merged 4 commits into from
Feb 3, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
51 changes: 45 additions & 6 deletions go/test/endtoend/vtgate/reservedconn/udv_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,11 +49,9 @@ func TestSetUDV(t *testing.T) {
query: "select @foo",
expectedRows: "[[NULL]]", rowsAffected: 1,
}, {
query: "set @foo = 'abc', @bar = 42, @baz = 30.5, @tablet = concat('foo','bar')",
expectedRows: "", rowsAffected: 0,
query: "set @foo = 'abc', @bar = 42, @baz = 30.5, @tablet = concat('foo','bar')",
}, {
query: "/*!40101 SET @OLD_SQL_MODE=@@SQL_MODE */",
expectedRows: "", rowsAffected: 0,
query: "/*!40101 SET @OLD_SQL_MODE=@@SQL_MODE */",
}, { // This is handled at vtgate.
query: "select @foo, @bar, @baz, @tablet",
expectedRows: `[[VARBINARY("abc") INT64(42) FLOAT64(30.5) VARBINARY("foobar")]]`, rowsAffected: 1,
Expand Down Expand Up @@ -83,7 +81,7 @@ func TestSetUDV(t *testing.T) {
expectedRows: ``, rowsAffected: 1,
}, {
query: "select id, val2 from test where val2=@bar",
expectedRows: ``, rowsAffected: 0,
expectedRows: ``,
}, {
query: "update test set val2 = @bar where val1 = @foo",
expectedRows: ``, rowsAffected: 1,
Expand All @@ -98,7 +96,7 @@ func TestSetUDV(t *testing.T) {
expectedRows: `[[INT64(42) VARCHAR("foobar")]]`, rowsAffected: 1,
}, {
query: "set @foo = now(), @bar = now(), @dd = date('2020-10-20'), @tt = time('10:15')",
expectedRows: `[]`, rowsAffected: 0,
expectedRows: `[]`,
}, {
query: "select @foo = @bar, @dd, @tt",
expectedRows: `[[INT64(1) VARCHAR("2020-10-20") VARCHAR("10:15:00")]]`, rowsAffected: 1,
Expand All @@ -125,6 +123,47 @@ func TestSetUDV(t *testing.T) {
}
}

func TestMysqlDumpInitialLog(t *testing.T) {
defer cluster.PanicHandler(t)
ctx := context.Background()
vtParams := mysql.ConnParams{
Host: "localhost",
Port: clusterInstance.VtgateMySQLPort,
}
conn, err := mysql.Connect(ctx, &vtParams)
require.NoError(t, err)
defer conn.Close()

queries := []string{
"/*!40101 SET @OLD_CHARACTER_SET_CLIENT=@@CHARACTER_SET_CLIENT */;",
"/*!40101 SET @OLD_CHARACTER_SET_RESULTS=@@CHARACTER_SET_RESULTS */;",
"/*!40101 SET @OLD_COLLATION_CONNECTION=@@COLLATION_CONNECTION */;",
"/*!50503 SET NAMES utf8mb4 */;",
"/*!40103 SET @OLD_TIME_ZONE=@@TIME_ZONE */;",
"/*!40103 SET TIME_ZONE='+00:00' */;",
"/*!40014 SET @OLD_UNIQUE_CHECKS=@@UNIQUE_CHECKS, UNIQUE_CHECKS=0 */;",
"/*!40014 SET @OLD_FOREIGN_KEY_CHECKS=@@FOREIGN_KEY_CHECKS, FOREIGN_KEY_CHECKS=0 */;",
"/*!40101 SET @OLD_SQL_MODE=@@SQL_MODE, SQL_MODE='NO_AUTO_VALUE_ON_ZERO' */;",
"/*!40111 SET @OLD_SQL_NOTES=@@SQL_NOTES, SQL_NOTES=0 */;",
"/*!40103 SET TIME_ZONE=@OLD_TIME_ZONE */;",
"/*!40101 SET SQL_MODE=@OLD_SQL_MODE */;",
"/*!40014 SET FOREIGN_KEY_CHECKS=@OLD_FOREIGN_KEY_CHECKS */;",
"/*!40014 SET UNIQUE_CHECKS=@OLD_UNIQUE_CHECKS */;",
"/*!40101 SET CHARACTER_SET_CLIENT=@OLD_CHARACTER_SET_CLIENT */;",
"/*!40101 SET CHARACTER_SET_RESULTS=@OLD_CHARACTER_SET_RESULTS */;",
"/*!40101 SET COLLATION_CONNECTION=@OLD_COLLATION_CONNECTION */;",
"/*!40111 SET SQL_NOTES=@OLD_SQL_NOTES */;",
}

for _, query := range queries {
t.Run(query, func(t *testing.T) {
_, more, err := conn.ExecuteFetchMulti(query, 1000, true)
require.NoError(t, err)
require.False(t, more)
})
}
}

func TestUserDefinedVariableResolvedAtTablet(t *testing.T) {
ctx := context.Background()
vtParams := mysql.ConnParams{
Expand Down
6 changes: 1 addition & 5 deletions go/vt/vttablet/tabletserver/planbuilder/plan.go
Original file line number Diff line number Diff line change
Expand Up @@ -294,11 +294,7 @@ func checkForPoolingUnsafeConstructs(expr sqlparser.SQLNode) error {
return sqlparser.Walk(func(in sqlparser.SQLNode) (kontinue bool, err error) {
switch node := in.(type) {
case *sqlparser.Set:
for _, setExpr := range node.Exprs {
if setExpr.Name.AtCount() > 0 {
return false, genError(node)
}
}
return false, genError(node)
case *sqlparser.FuncExpr:
if sqlparser.IsLockingFunc(node) {
return false, genError(node)
Expand Down
32 changes: 0 additions & 32 deletions go/vt/vttablet/tabletserver/planbuilder/testdata/exec_cases.txt
Original file line number Diff line number Diff line change
Expand Up @@ -595,38 +595,6 @@ options:PassthroughDMLs
"FullQuery": "delete from a limit 10"
}

# int
"set a=1"
{
"PlanID": "Set",
"TableName": "",
"FullQuery": "set a = 1"
}

# float
"set a=1.2"
{
"PlanID": "Set",
"TableName": "",
"FullQuery": "set a = 1.2"
}

# string
"set a='b'"
{
"PlanID": "Set",
"TableName": "",
"FullQuery": "set a = 'b'"
}

# multi
"set a=1, b=2"
{
"PlanID": "Set",
"TableName": "",
"FullQuery": "set a = 1, b = 2"
}

# create
"create table a(a int, b varchar(8))"
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,5 +19,13 @@
"release_lock('foo') not allowed without a reserved connections"

# setting system variables must happen inside reserved connections
"set @sql_safe_updates = false"
"set @sql_safe_updates = false not allowed without a reserved connections"
"set sql_safe_updates = false"
"set sql_safe_updates = false not allowed without a reserved connections"

# setting system variables must happen inside reserved connections
"set @@sql_safe_updates = false"
"set @@sql_safe_updates = false not allowed without a reserved connections"

# setting system variables must happen inside reserved connections
"set @udv = false"
"set @udv = false not allowed without a reserved connections"
6 changes: 3 additions & 3 deletions go/vt/vttablet/tabletserver/query_executor.go
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ func (qre *QueryExecutor) Execute() (reply *sqltypes.Result, err error) {
return qr, nil
case p.PlanSelectLock:
return nil, vterrors.Errorf(vtrpcpb.Code_FAILED_PRECONDITION, "%s disallowed outside transaction", qre.plan.PlanID.String())
case p.PlanSet, p.PlanOtherRead, p.PlanOtherAdmin, p.PlanFlush:
case p.PlanOtherRead, p.PlanOtherAdmin, p.PlanFlush:
return qre.execOther()
case p.PlanSavepoint, p.PlanRelease, p.PlanSRollback:
return qre.execOther()
Expand Down Expand Up @@ -197,14 +197,14 @@ func (qre *QueryExecutor) execAsTransaction(f func(conn *StatefulConnection) (*s

func (qre *QueryExecutor) txConnExec(conn *StatefulConnection) (*sqltypes.Result, error) {
switch qre.plan.PlanID {
case p.PlanInsert, p.PlanUpdate, p.PlanDelete:
case p.PlanInsert, p.PlanUpdate, p.PlanDelete, p.PlanSet:
return qre.txFetch(conn, true)
case p.PlanInsertMessage:
qre.bindVars["#time_now"] = sqltypes.Int64BindVariable(time.Now().UnixNano())
return qre.txFetch(conn, true)
case p.PlanUpdateLimit, p.PlanDeleteLimit:
return qre.execDMLLimit(conn)
case p.PlanSet, p.PlanOtherRead, p.PlanOtherAdmin, p.PlanFlush:
case p.PlanOtherRead, p.PlanOtherAdmin, p.PlanFlush:
return qre.execStatefulConn(conn, qre.query, true)
case p.PlanSavepoint, p.PlanRelease, p.PlanSRollback:
return qre.execStatefulConn(conn, qre.query, true)
Expand Down
13 changes: 2 additions & 11 deletions go/vt/vttablet/tabletserver/query_executor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,15 +112,6 @@ func TestQueryExecutorPlans(t *testing.T) {
// Because the fields would have been cached before, the field query will
// not get re-executed.
inTxWant: "select * from t limit 1",
}, {
input: "set a=1",
dbResponses: []dbResponse{{
query: "set a=1",
result: dmlResult,
}},
resultWant: dmlResult,
planWant: "Set",
logWant: "set a=1",
}, {
input: "show engines",
dbResponses: []dbResponse{{
Expand Down Expand Up @@ -262,7 +253,7 @@ func TestQueryExecutorPlans(t *testing.T) {
inTxWant: "RELEASE savepoint a",
}}
for _, tcase := range testcases {
func() {
t.Run(tcase.input, func(t *testing.T) {
db := setUpQueryExecutorTest(t)
defer db.Close()
for _, dbr := range tcase.dbResponses {
Expand Down Expand Up @@ -300,7 +291,7 @@ func TestQueryExecutorPlans(t *testing.T) {
want = tcase.inTxWant
}
assert.Equal(t, want, qre.logStats.RewrittenSQL(), "in tx: %v", tcase.input)
}()
})
}
}

Expand Down