parser: fix STATS_AUTO_RECALC syntax

[lauhg]

What problem does this PR solve?

Fix compatibility problem about STATS_AUTO_RECALC syntax

Issue: #472

MySQL Syntax:

type:
...
  | STATS_AUTO_RECALC_SYM opt_equal ternary_option

Bad SQL Case:

create table t (a int) stats_auto_recalc = 0
create table t (a int) stats_persistent = 1, stats_auto_recalc = 1
create table t (a int) stats_auto_recalc = 1, stats_sample_pages = 25
alter table t modify a bigint, ENGINE=InnoDB, stats_auto_recalc = 0

Check List

Tests

• Unit test

[codecov]

Codecov Report

❗ No coverage uploaded for pull request base (master@0c51dce). Click here to learn what that means.
The diff coverage is 100%.

@@            Coverage Diff            @@
##             master     #483   +/-   ##
=========================================
  Coverage          ?   71.56%           
=========================================
  Files             ?       32           
  Lines             ?     7703           
  Branches          ?        0           
=========================================
  Hits              ?     5513           
  Misses            ?     1668           
  Partials          ?      522

Impacted Files	Coverage Δ
parser.go	70.58% <ø> (ø)
misc.go	96.42% <ø> (ø)
ast/ddl.go	80.18% <100%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0c51dce...9294ba9. Read the comment docs.

[lauhg]

@leoppro PTAL

[lauhg]

@leoppro PTAL

[zier-one]

LGTM

[zier-one]

@tangenta PTAL

[tangenta]

@leoppro @lauhg I don't think DEFAULT is the same as 0.

Here is the MySQL 8.0 documentation:

• STATS_AUTO_RECALC
  Specifies whether to automatically recalculate persistent statistics for an InnoDB table. The value DEFAULT causes the persistent statistics setting for the table to be determined by the innodb_stats_auto_recalc configuration option. The value 1 causes statistics to be recalculated when 10% of the data in the table has changed. The value 0 prevents automatic recalculation for this table;

TiDB needs the information to decide whether using innodb_stats_auto_recalc(or something similar) or not.

[lauhg]

@tangenta I agree with you.
@leoppro What do you think?

[zier-one]

fine, I think we should add a feild named default to TableOption

[zier-one]

could fix "STATS_SAMPLE_PAGES" EqOpt "DEFAULT" by the way?

[kennytm]

Could this be parsed into a nil option?

[lauhg]

fine, I think we should add a feild named default to TableOption

I think this is a good way to solve this problem.

[kennytm]

@lauhg Since STATS_SAMPLE_PAGES = DEFAULT is the default the option could just be entirely removed from the AST, so changing this branch to $$ = nil should be possible.

[lauhg]

@kennytm As far as I know, this parser is not only part of TiDB, it will also be used by other projects. In order to be more compatible with mysql, I think it is necessary to know if the default value is explicitly set.

[kennytm]

@lauhg What project would benefit from knowing that the DEFAULT is explicitly set? In MySQL (8.0.17) even if you explicit write STATS_AUTO_RECALC = DEFAULT it will disappear in SHOW CREATE TABLE, showing these are semantically equivalent.

mysql> create table t (a int) stats_auto_recalc = 0;
Query OK, 0 rows affected (0.06 sec)

mysql> show create table t;
+-------+--------------------------------------------------------------------------------------------------------------------------------------+
| Table | Create Table                                                                                                                         |
+-------+--------------------------------------------------------------------------------------------------------------------------------------+
| t     | CREATE TABLE `t` (
  `a` int(11) DEFAULT NULL
) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_general_ci STATS_AUTO_RECALC=0 |
+-------+--------------------------------------------------------------------------------------------------------------------------------------+
1 row in set (0.00 sec)

mysql> drop table t;
Query OK, 0 rows affected (0.02 sec)

mysql> create table t (a int) stats_auto_recalc = default;
Query OK, 0 rows affected (0.01 sec)

mysql> show create table t;
+-------+------------------------------------------------------------------------------------------------------------------+
| Table | Create Table                                                                                                     |
+-------+------------------------------------------------------------------------------------------------------------------+
| t     | CREATE TABLE `t` (
  `a` int(11) DEFAULT NULL
) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_general_ci |
+-------+------------------------------------------------------------------------------------------------------------------+
1 row in set (0.00 sec)

mysql> drop table t;
Query OK, 0 rows affected (0.00 sec)

[lauhg]

@kennytm
In create table case, these are semantically equivalent.
But in alter table case, these are not equivalent.
For example:

create table t (a int) stats_auto_recalc = 1;
alter table t stats_auto_recalc = default;

If parser parses stats_auto_recalc = default to nil, it would be equivalent to
alter table t.
Apparently, it changes semantics.

[lauhg]

I add a feild named Default to TableOption.
@leoppro @tangenta PTAL

[tangenta]

Adding a field this way could lead to every TableOption increases 4 bytes memory usage.
Here are two options:

1. Add a TableOption type named TableOptionStatsAutoRecalcDefault
2. Assign a special value to UIntValue(i.e. math.MaxUint64) in TableOption, which represents default.

How do you think? @leoppro @lauhg

[lauhg]

Or just assign DEFAULT to StrValue?

[tangenta]

@lauhg It's unnecessary to compare a whole string when checking the default property.

[lauhg]

@tangenta ok, I'd like to choose option 2.
@leoppro How do you think?

[zier-one]

Adding a field this way could lead to every TableOption increases 4 bytes memory usage.
Here are two options:

1. Add a TableOption type named TableOptionStatsAutoRecalcDefault
2. Assign a special value to UIntValue(i.e. math.MaxUint64) in TableOption, which represents default.

How do you think? @leoppro @lauhg

I don't think we should reduce the readability of code for save 4 bytes(or 4*n bytes)

[lauhg]

What's the conclusion?

[tangenta]

Fine, keep the current changes is ok. Please resolve the conflicts.

[tangenta]

LGTM

[lauhg]

Resolved.
PTAL @tangenta