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

errno: move the error code from the parser/mysql to tidb/errno #15277

Merged
merged 10 commits into from
Mar 11, 2020

Conversation

tiancaiamao
Copy link
Contributor

@tiancaiamao tiancaiamao commented Mar 11, 2020

What problem does this PR solve?

Some times we need to define/modify the error code.
The error code is defined in the parser repo, so we need to update two repositories in a single PR, that's cumbersome.

I want to move the TiDB customed error back to the TiDB repo.

What is changed and how it works?

The error code is now defined in github/pingcap/tidb/errno

I have a PR to verify TiDB doesn't depend on parser/mysql errors anymore (it's not important to merge the PR or not).

From now on, don't add more error code to parser/mysql error definitions, use tidb/errno instead.
(We can choose to clean the parser repo or not)

Check List

Tests

  • No code

Code changes

  • Has exported variable/fields change

Side effects

Breaking backward compatibility

Related changes

  • Need to cherry-pick to the release branch

It's better to cherry-pick it to reduce conflict in the future.

@tiancaiamao tiancaiamao requested review from a team as code owners March 11, 2020 03:49
@ghost ghost requested review from qw4990, SunRunAway, lzmhhh123 and winoros and removed request for a team March 11, 2020 03:49
@tiancaiamao
Copy link
Contributor Author

PTAL @lysu @jackysp

@tiancaiamao
Copy link
Contributor Author

/run-all-tests

@tiancaiamao
Copy link
Contributor Author

/run-all-tests

@tiancaiamao
Copy link
Contributor Author

/run-all-tests

@tiancaiamao
Copy link
Contributor Author

/run-all-tests

@codecov
Copy link

codecov bot commented Mar 11, 2020

Codecov Report

Merging #15277 into master will not change coverage by %.
The diff coverage is n/a.

@@             Coverage Diff             @@
##             master     #15277   +/-   ##
===========================================
  Coverage   80.4680%   80.4680%           
===========================================
  Files           503        503           
  Lines        133970     133970           
===========================================
  Hits         107803     107803           
  Misses        17736      17736           
  Partials       8431       8431           

@jackysp
Copy link
Member

jackysp commented Mar 11, 2020

Breaking backward compatibility
Why?

Copy link
Member

@jackysp jackysp left a comment

Choose a reason for hiding this comment

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

LGTM

@tiancaiamao
Copy link
Contributor Author

/run-all-tests

Breaking backward compatibility

Not really...

I mean the code compatibility, if we remove the parser exported function, the cherry-pick work would be trouble.

lysu
lysu previously approved these changes Mar 11, 2020
Copy link
Contributor

@lysu lysu left a comment

Choose a reason for hiding this comment

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

LGTM

go.sum Outdated Show resolved Hide resolved
@jackysp
Copy link
Member

jackysp commented Mar 11, 2020

/merge

@sre-bot sre-bot added the status/can-merge Indicates a PR has been approved by a committer. label Mar 11, 2020
@sre-bot
Copy link
Contributor

sre-bot commented Mar 11, 2020

/run-all-tests

@sre-bot
Copy link
Contributor

sre-bot commented Mar 11, 2020

@tiancaiamao merge failed.

@tiancaiamao
Copy link
Contributor Author

/rebuild

@tiancaiamao tiancaiamao merged commit 9f0736e into pingcap:master Mar 11, 2020
@tiancaiamao tiancaiamao deleted the errno branch March 11, 2020 07:40
sre-bot pushed a commit to sre-bot/tidb that referenced this pull request Mar 11, 2020
Signed-off-by: sre-bot <sre-bot@pingcap.com>
@sre-bot
Copy link
Contributor

sre-bot commented Mar 11, 2020

cherry pick to release-3.0 in PR #15283

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/expression sig/execution SIG execution sig/planner SIG: Planner sig/sql-infra SIG: SQL Infra status/can-merge Indicates a PR has been approved by a committer.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants