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

parser: create integer type field with specified length should have warings #939

Merged
merged 6 commits into from
Jul 24, 2020

Conversation

AilinKid
Copy link
Contributor

What problem does this PR solve?

before

mysql> create table t(a bigint(2));
Query OK, 0 rows affected (0.01 sec)

after

mysql> create table t(a bigint(2));
Query OK, 0 rows affected, 1 warning (0.01 sec)

mysql> show warnings;
+---------+------+------------------------------------------------------------------------------+
| Level   | Code | Message                                                                      |
+---------+------+------------------------------------------------------------------------------+
| Warning | 1681 | Integer display width is deprecated and will be removed in a future release. |
+---------+------+------------------------------------------------------------------------------+
1 row in set (0.00 sec)


What is changed and how it works?

This is PR is part of the functionality above, it need TiDB PR to check the same warning and rewrite the error code and message.

Check List

Tests

  • Integration test

.
Signed-off-by: AilinKid <314806019@qq.com>
.
Signed-off-by: AilinKid <314806019@qq.com>
.
Signed-off-by: AilinKid <314806019@qq.com>
.
Signed-off-by: AilinKid <314806019@qq.com>
.
Signed-off-by: AilinKid <314806019@qq.com>
@codecov
Copy link

codecov bot commented Jul 24, 2020

Codecov Report

Merging #939 into master will decrease coverage by 0.08%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #939      +/-   ##
==========================================
- Coverage   78.46%   78.37%   -0.09%     
==========================================
  Files          40       40              
  Lines       14913    14786     -127     
==========================================
- Hits        11701    11589     -112     
+ Misses       2521     2509      -12     
+ Partials      691      688       -3     

@@ -9634,6 +9634,10 @@ NumericType:
// TODO: check flen 0
x := types.NewFieldType($1.(byte))
x.Flen = $2.(int)
if $2.(int) != types.UnspecifiedLength {
yylex.AppendError(yylex.Errorf("Integer display width is deprecated and will be removed in a future release."))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we use ErrWarnDeprecatedIntegerDisplayWidth directly?

Copy link
Contributor Author

@AilinKid AilinKid Jul 24, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it can't use a terror class directly in scanner here

ErrWarnDeprecatedIntegerDisplayWidth is used in TiDB to append warnings in statement context to replace the raw warning, it contains the error code 1681, error message.

the raw message is like: "You have an error in your SQL syntax; check the manual that corresponds to your TiDB version for the right syntax to use near ''%s" Integer display width is deprecated and will be removed in a future release" and the fault error code is 1064

tangenta
tangenta previously approved these changes Jul 24, 2020
Copy link
Contributor

@tangenta tangenta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@AilinKid
Copy link
Contributor Author

Cause ast.UnionStmt is removed in newer parser, but TiDB still referred the old version parser. That's why there is integation error.

Copy link
Contributor

@kennytm kennytm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@kennytm kennytm added status/LGT2 LGT2 and removed status/LGT1 LGT1 labels Jul 24, 2020
@kennytm kennytm merged commit 4bb6b1f into pingcap:master Jul 24, 2020
tiancaiamao pushed a commit to tiancaiamao/parser that referenced this pull request Apr 27, 2021
…arings (pingcap#939)

* .

Signed-off-by: AilinKid <314806019@qq.com>

* .

Signed-off-by: AilinKid <314806019@qq.com>

* .

Signed-off-by: AilinKid <314806019@qq.com>

* .

Signed-off-by: AilinKid <314806019@qq.com>

* .

Signed-off-by: AilinKid <314806019@qq.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants