Skip to content

parser: support using dash('-') in config item names#821

Merged
zz-jason merged 1 commit intopingcap:masterfrom
qw4990:fix-configitemname
Apr 22, 2020
Merged

parser: support using dash('-') in config item names#821
zz-jason merged 1 commit intopingcap:masterfrom
qw4990:fix-configitemname

Conversation

@qw4990
Copy link
Contributor

@qw4990 qw4990 commented Apr 22, 2020

What problem does this PR solve?

Close #820
After this issue(#768), the parser supports set config name=value syntax, but the name doesn't support using -.
We should support it since there are some config items like label-property.reject-leader, auto-compaction which use - in their names.

What is changed and how it works?

Introduce a new token ConfigItemName to support -.

Check List

Tests

  • Unit test

@qw4990 qw4990 added the type/enhancement New feature or request label Apr 22, 2020
@qw4990 qw4990 requested a review from a team April 22, 2020 06:47
@ghost ghost requested review from kennytm and removed request for a team April 22, 2020 06:47
@qw4990 qw4990 requested review from SunRunAway, XuHuaiyu, rleungx, wshwsh12 and zz-jason and removed request for SunRunAway April 22, 2020 06:47
@codecov
Copy link

codecov bot commented Apr 22, 2020

Codecov Report

Merging #821 into master will increase coverage by 0.00%.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #821   +/-   ##
=======================================
  Coverage   78.26%   78.27%           
=======================================
  Files          40       40           
  Lines       14719    14723    +4     
=======================================
+ Hits        11520    11524    +4     
  Misses       2517     2517           
  Partials      682      682           

Copy link
Member

@zz-jason zz-jason left a comment

Choose a reason for hiding this comment

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

LGTM

@zz-jason zz-jason added the status/LGT1 LGT1 label Apr 22, 2020
Copy link
Member

@rleungx rleungx left a comment

Choose a reason for hiding this comment

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

LGTM

@qw4990 qw4990 added status/LGT2 LGT2 and removed status/LGT1 LGT1 labels Apr 22, 2020
@zz-jason zz-jason merged commit 304bbd3 into pingcap:master Apr 22, 2020
@SunRunAway
Copy link
Contributor

Should it be cherry-picked to release-4.0 branch and update the dependency of tidb repo?

@tiancaiamao
Copy link
Collaborator

Dash(-) is not a valid char as identifier
When used, it should be warp with `

`sth-sth`

https://dev.mysql.com/doc/refman/5.7/en/identifier-qualifiers.html

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

status/LGT2 LGT2 type/enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support using dash('-') in config item names

5 participants