TiDB doesn't return an error on set @@SQL_MODE=NULL

[espresso98]

Bug Report

1. Minimal reproduce step

set @@SQL_MODE=NULL;

2. What did you expect to see?

mysql> set @@SQL_MODE=NULL;
ERROR 1231 (42000): Variable 'sql_mode' can't be set to the value of 'NULL'

3. What did you see instead

tidb> set @@SQL_MODE=NULL;
Query OK, 0 rows affected (0.00 sec)

4. What is your TiDB version?

tidb_version(): Release Version: v5.5.0-alpha-210-g11f4ca802
Edition: Community
Git Commit Hash: 11f4ca802083ee38d5972730ba8f9b72395316fb
Git Branch: master
UTC Build Time: 2022-02-24 04:52:01
GoVersion: go1.17.2
Race Enabled: false
TiKV Min Version: v3.0.0-60965b006877ca7234adaced7890d7b029ed1306
Check Table Before Drop: false
1 row in set (0.02 sec)

[djshow832]

According to #32879 (comment), we won't fix this issue, right? @Defined2014

[Defined2014]

According to #32879 (comment), we won't fix this issue, right? @Defined2014

@morgo , What's your opinion about this issue? Document the incompatibility or disallow NULL for all sysvars, which one are you prefer?

[morgo]

According to #32879 (comment), we won't fix this issue, right? @Defined2014

@morgo , What's your opinion about this issue? Document the incompatibility or disallow NULL for all sysvar, which one are you prefer?

I prefer disallowing NULL. For 2 reasons:

• We don't represent it well internally, so it could get us in trouble later down the line.
• It is unlikely to break anything, but a good opportunity to introduce it as a compatibility breaker now (v6.0).

But both behaviors are acceptable to me.

[Defined2014]

Seems Null is useful for some sysvar, like #32987. We have two options,

• Document the incompatibility for sql_mode and other sysvars.
• Add NotNull flag for sysvars. ( If we decided to do this, a lot of API changes will be required.