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

ddl: the 'max-index-length' check does not respect non-restricted sql_mode #34931

Closed
bb7133 opened this issue May 25, 2022 · 7 comments · Fixed by #35008
Closed

ddl: the 'max-index-length' check does not respect non-restricted sql_mode #34931

bb7133 opened this issue May 25, 2022 · 7 comments · Fixed by #35008
Assignees
Labels
help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. severity/moderate sig/sql-infra SIG: SQL Infra type/bug The issue is confirmed as a bug. type/compatibility

Comments

@bb7133
Copy link
Member

bb7133 commented May 25, 2022

Bug Report

Please answer these questions before submitting your issue. Thanks!

1. Minimal reproduce step (Required)

drop table if exists t;
set @@sql_mode='';
create table t (id int, name varchar(2048), index(name)) charset=utf8;

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

In MySQL 5.7 & MySQL 8:

mysql> set @@sql_mode='';
Query OK, 0 rows affected (0.00 sec)

mysql> create table t (id int, name varchar(2048), index(name)) charset=utf8;
Query OK, 0 rows affected, 2 warnings (0.02 sec)

mysql> show warnings;
+---------+------+-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
| Level   | Code | Message                                                                                                                                                                     |
+---------+------+-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
| Warning | 3719 | 'utf8' is currently an alias for the character set UTF8MB3, but will be an alias for UTF8MB4 in a future release. Please consider using UTF8MB4 in order to be unambiguous. |
| Warning | 1071 | Specified key was too long; max key length is 3072 bytes                                                                                                                    |
+---------+------+-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
2 rows in set (0.00 sec)

mysql> show create table t;
+-------+-------------------------------------------------------------------------------------------------------------------------------------------------------+
| Table | Create Table                                                                                                                                          |
+-------+-------------------------------------------------------------------------------------------------------------------------------------------------------+
| t     | CREATE TABLE `t` (
  `id` int(11) DEFAULT NULL,
  `name` varchar(2048) DEFAULT NULL,
  KEY `name` (`name`(1024))
) ENGINE=InnoDB DEFAULT CHARSET=utf8 |
+-------+-------------------------------------------------------------------------------------------------------------------------------------------------------+
1 row in set (0.00 sec)

Notice that in non-strict SQL mode, MySQL converted the error to warning and truncated the index name as a prefix one("name(1024)"), which met the maximum length of index(3072 bytes / 3 bytes per utf8 encoding = 1024).

3. What did you see instead (Required)

In TiDB(all versions with default configuration):

mysql> set @@sql_mode='';
Query OK, 0 rows affected (0.00 sec)

tidb> create table t (id int, name varchar(2048), index(name)) charset=utf8;
ERROR 1071 (42000): Specified key was too long; max key length is 3072 bytes

4. What is your TiDB version? (Required)

tidb> select version();
+--------------------+
| version()          |
+--------------------+
| 5.7.25-TiDB-v6.0.0 |
+--------------------+
1 row in set (0.00 sec)

I am using v6.0 but it applies to all existing versions.

@bb7133 bb7133 added type/bug The issue is confirmed as a bug. type/compatibility sig/sql-infra SIG: SQL Infra help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. labels May 25, 2022
@bb7133 bb7133 changed the title ddl: the max-index-length check does not respect sql_mode ddl: the max-index-length check does not respect non-restricted sql_mode May 25, 2022
@bb7133 bb7133 changed the title ddl: the max-index-length check does not respect non-restricted sql_mode ddl: the 'max-index-length' check does not respect non-restricted sql_mode May 25, 2022
@e1ijah1
Copy link
Contributor

e1ijah1 commented May 27, 2022

@CbcWestwolf @bb7133 Has this issue been resolved? I wanna try it.

@CbcWestwolf
Copy link
Member

@CbcWestwolf @bb7133 Has this issue been resolved? I wanna try it.

Sure :-) Please have a try.

@bb7133
Copy link
Member Author

bb7133 commented May 28, 2022

@CbcWestwolf @bb7133 Has this issue been resolved? I wanna try it.

Thank you! I will reassign this issue to you, glad to hear that.

@bb7133 bb7133 assigned e1ijah1 and unassigned CbcWestwolf May 28, 2022
@bb7133
Copy link
Member Author

bb7133 commented May 28, 2022

@e11jah , just a kind reminder:

The `max-index-length' in MySQL(with InnoDB) is hard-coded to 3072, but it is configurable in TiDB because of some histroical reason:

https://docs.pingcap.com/tidb/dev/tidb-configuration-file#max-index-length

@e1ijah1
Copy link
Contributor

e1ijah1 commented May 28, 2022

@e11jah , just a kind reminder:

The `max-index-length' in MySQL(with InnoDB) is hard-coded to 3072, but it is configurable in TiDB because of some histroical reason:

https://docs.pingcap.com/tidb/dev/tidb-configuration-file#max-index-length

@bb7133 @CbcWestwolf Is the implicit truncation behavior in non-restrict sql mode reasonable? Coz I had found that the index truncation behavior in mysql looks like only working with a single varchar column index (not unique). And the multiple column index in which the length sum exceeds the maximum size will return an error instead produce a warning.

mysql> select version();
+-------------------------+
| version()               |
+-------------------------+
| 8.0.29-0ubuntu0.20.04.3 |
+-------------------------+
1 row in set (0.00 sec)

mysql> create table t (id int(32), name varchar(512), alias varchar(1024),index(name,alias)) charset utf8;
ERROR 1071 (42000): Specified key was too long; max key length is 3072 bytes
mysql> set @@sql_mode='';
Query OK, 0 rows affected (0.00 sec)

mysql> create table t (id int(32), name varchar(512), alias varchar(1024),index(name,alias)) charset utf8;
ERROR 1071 (42000): Specified key was too long; max key length is 3072 bytes

And the documentation doesn't clearly describe this.

If a specified index prefix exceeds the maximum column data type size, CREATE INDEX handles the index as follows:

For a nonunique index, either an error occurs (if strict SQL mode is enabled), or the index length is reduced to lie within the maximum column data type size and a warning is produced (if strict SQL mode is not enabled).

For a unique index, an error occurs regardless of SQL mode because reducing the index length might enable insertion of nonunique entries that do not meet the specified uniqueness requirement.

https://dev.mysql.com/doc/refman/8.0/en/create-index.html

@CbcWestwolf
Copy link
Member

Is the implicit truncation behavior in non-restrict sql mode reasonable?

According to the documentation posted here, I think we can do such truncation on non-unique index, regardless of one-column index or multi-columns index. In func buildIndexColumns the calling mysql.HasUniKeyFlag(col.GetFlag()) may help.

@bb7133
Copy link
Member Author

bb7133 commented May 30, 2022

I think we can do such truncation on non-unique index, regardless of one-column index or multi-columns index.

@e11jah @CbcWestwolf I think we should follow the behavior of MySQL, TiDB aims to be 100% compatible with MySQL(although it is not possible for all the cases).

So for multi columns, please report an error still.

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/moderate sig/sql-infra SIG: SQL Infra type/bug The issue is confirmed as a bug. type/compatibility
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants