Skip to content

parser: add config variable to control max display length - #949

Merged
AilinKid merged 1 commit into
pingcap:masterfrom
AilinKid:integerFlenWaring3
Jul 29, 2020
Merged

AilinKid merged 1 commit into
pingcap:masterfrom
AilinKid:integerFlenWaring3

Conversation

@AilinKid

@AilinKid AilinKid commented Jul 29, 2020 •

Copy link
Copy Markdown
Contributor

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

What problem does this PR solve?

Since TiDB and Parser has implemented much tests with int(11), bigint(20) and so on, it better to add the varible as a transition from supported to deprecated.
The benefit is that we don't have to change all the tests implemented with int(11) ,bigint(20) and so on, even some tests in the other repo, like TiDB-Test, Parser...

Also
TiDB #18787 is the pr to fix the integration test (removing the mysql.decimal field), but it's parser is including max-display-length-warning commit which I filed serveral days ago, which will cause test check failed in TiDB Repo.

So this pr is to correct the TiDB test check behavior.

What is changed and how it works?

Add the config variable in parser drived by TiDB config.

Check List

Tests

  • Unit test
  • Integration test

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

codecov Bot commented Jul 29, 2020

Copy link
Copy Markdown

Codecov Report

Merging #949 into master will decrease coverage by 0.01%.
The diff coverage is 80.00%.

@@            Coverage Diff             @@
##           master     #949      +/-   ##
==========================================
- Coverage   78.39%   78.37%   -0.02%     
==========================================
  Files          40       40              
  Lines       14933    14936       +3     
==========================================
  Hits        11706    11706              
- Misses       2536     2538       +2     
- Partials      691      692       +1     

@AilinKid

AilinKid commented Jul 29, 2020 •

Copy link
Copy Markdown
Contributor Author

TiDB #18787 is the pr to fix the integration test (removing the mysql.decimal field), but it's parser is including max-display-length-warning commit which I filed serveral days ago, which will cause test check failed in TiDB Repo.

So this pr is to correct the TiDB test check behavior.
PTAL PTAL PTAL

@AilinKid
AilinKid requested review from crazycs520 and kennytm July 29, 2020 05:29
Comment thread parser.y
x := types.NewFieldType($1.(byte))
x.Flen = $2.(int)
if $2.(int) != types.UnspecifiedLength {
if $2.(int) != types.UnspecifiedLength && types.TiDBStrictIntegerDisplayWidth {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why change here?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

this my previous pr, it will parse int(10) as warnings, it should be under the control of this variable

@crazycs520 crazycs520 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

lgtm

@AilinKid AilinKid added the status/LGT1 LGT1 label Jul 29, 2020

@tangenta tangenta left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@AilinKid
AilinKid merged commit 5fe0b0f into pingcap:master Jul 29, 2020
vitthalaa pushed a commit to vitthalaa/parser that referenced this pull request Oct 14, 2020
tiancaiamao pushed a commit to tiancaiamao/parser that referenced this pull request Apr 27, 2021
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.

4 participants