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

the process for init_connect = -1 is incompatible with mysql #35324

Closed
seiya-annie opened this issue Jun 13, 2022 · 3 comments · Fixed by #36865
Closed

the process for init_connect = -1 is incompatible with mysql #35324

seiya-annie opened this issue Jun 13, 2022 · 3 comments · Fixed by #36865
Assignees
Labels
help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. severity/minor sig/sql-infra SIG: SQL Infra type/bug The issue is confirmed as a bug.

Comments

@seiya-annie
Copy link

Bug Report

Please answer these questions before submitting your issue. Thanks!

1. Minimal reproduce step (Required)

in tidb v6.1.0

mysql> SET global init_connect = -1;
Query OK, 0 rows affected (0.01 sec)

mysql> show global variables like 'init_connect';
+---------------+-------+
| Variable_name | Value |
+---------------+-------+
| init_connect  | -1    |
+---------------+-------+
1 row in set (0.00 sec)

in MySQL 8.0.29
mysql> SET global init_connect = -1;
ERROR 1232 (42000): Incorrect argument type to variable 'init_connect'

2. What did you expect to see? (Required)

compatible with MySQL

3. What did you see instead (Required)

tidb don't report error for invalid value type

4. What is your TiDB version? (Required)

Git Commit Hash: 1a89dec
Git Branch: heads/refs/tags/v6.1.0
UTC Build Time: 2022-06-05 05:15:11

@seiya-annie seiya-annie added type/bug The issue is confirmed as a bug. sig/sql-infra SIG: SQL Infra severity/moderate labels Jun 13, 2022
@djshow832 djshow832 added the help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. label Jun 14, 2022
@Defined2014
Copy link
Contributor

We will transfer datum to str before validation ref link. So we need to pass datum param for SetGlobalSysVar instead of string to fix this issue.

@likzn
Copy link
Contributor

likzn commented Jun 18, 2022

SysVar has TypeFlag with each system environment. And we also can get input type with v.Expr.Eval(chunk.Row{}) in Datum. What about adding a check with type limit to resolve this issue.

tidb/executor/set.go

Lines 299 to 314 in 9a77892

func (e *SetExecutor) getVarValue(v *expression.VarAssignment, sysVar *variable.SysVar) (value string, err error) {
if v.IsDefault {
// To set a SESSION variable to the GLOBAL value or a GLOBAL value
// to the compiled-in MySQL default value, use the DEFAULT keyword.
// See http://dev.mysql.com/doc/refman/5.7/en/set-statement.html
if sysVar != nil {
return sysVar.Value, nil
}
return variable.GetGlobalSystemVar(e.ctx.GetSessionVars(), v.Name)
}
nativeVal, err := v.Expr.Eval(chunk.Row{})
if err != nil || nativeVal.IsNull() {
return "", err
}
return nativeVal.ToString()
}

@morgo
Copy link
Contributor

morgo commented Jul 6, 2022

There is an easier fix here without having to modify the sysvar framework. Specify a Validation function for InitConnect, and in that func try and parse the new val. If it fails, return the error message that MySQL is returning here and then the SET will fail.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. severity/minor sig/sql-infra SIG: SQL Infra type/bug The issue is confirmed as a bug.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants