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

infoschema: add TIDB_ROW_ID_SHARDING_INFO for table table. #13418

Merged
merged 3 commits into from Nov 14, 2019

Conversation

bb7133
Copy link
Member

@bb7133 bb7133 commented Nov 13, 2019

What problem does this PR solve?

This PR adds an extra column TIDB_ROW_ID_SHARDING_INFO to information_schema.tables table, which gives a brief description of the sharding information of the table:

The description may be:

  • "NOT_SHARDED" for tables that SHARD_ROW_ID_BITS is not specified.
  • "NOT_SHARDED(PK_IS_HANDLE)" for tables that primary key is part of row id(aka PK_IS_HANDLE).
  • "SHARD_BITS={bit_number}": for tables that are defined with SHARD_ROW_ID_BITS.
  • nil: for tables that are not suitable(views, system tables).

What is changed and how it works?

A GetShardingInfo is added to retrieve such information.

Check List

Tests

  • Unit test
  • Integration test

Code changes

  • Has exported function/method change

Side effects

  • Breaking backward compatibility

Related changes

  • Need to update the documentation(4.0 new feature)

// - "SHARD_BITS={bit_number}": for tables that with SHARD_ROW_ID_BITS.
// The returned nil indicates that sharding information is not suitable for the table(for example, when the table is a View).
// This function is exported for unit test.
func GetShardingInfo(dbInfo *model.DBInfo, tableInfo *model.TableInfo) interface{} {
Copy link
Member Author

Choose a reason for hiding this comment

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

@djshow832 I suggest changing the return type to string and returning "NIL" if it's not suitable.

Why this is better?

I still prefer to nil because it will be converted as NULL of MySQL, but string "nil" will not.

@bb7133
Copy link
Member Author

bb7133 commented Nov 13, 2019

PTAL @djshow832 @zimulala @lonng

@codecov
Copy link

codecov bot commented Nov 13, 2019

Codecov Report

Merging #13418 into master will not change coverage.
The diff coverage is n/a.

@@            Coverage Diff            @@
##            master    #13418   +/-   ##
=========================================
  Coverage   80.483%   80.483%           
=========================================
  Files          472       472           
  Lines       115920    115920           
=========================================
  Hits         93296     93296           
  Misses       15440     15440           
  Partials      7184      7184

@zz-jason
Copy link
Member

zz-jason commented Nov 13, 2019

@bb7133 please add some proper labels for this PR.

@bb7133
Copy link
Member Author

bb7133 commented Nov 14, 2019

@bb7133 please add some proper labels for this PR.

Done, thanks for reminding

Copy link
Contributor

@djshow832 djshow832 left a comment

Choose a reason for hiding this comment

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

LGTM

@djshow832 djshow832 added the status/LGT2 Indicates that a PR has LGTM 2. label Nov 14, 2019
@bb7133 bb7133 added status/LGT1 Indicates that a PR has LGTM 1. and removed status/LGT2 Indicates that a PR has LGTM 2. labels Nov 14, 2019
Copy link
Contributor

@crazycs520 crazycs520 left a comment

Choose a reason for hiding this comment

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

LGTM

@crazycs520 crazycs520 added status/LGT3 The PR has already had 3 LGTM. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Nov 14, 2019
@bb7133 bb7133 added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT3 The PR has already had 3 LGTM. labels Nov 14, 2019
@crazycs520
Copy link
Contributor

/run-all-tests

@bb7133 bb7133 added the status/can-merge Indicates a PR has been approved by a committer. label Nov 14, 2019
@sre-bot
Copy link
Contributor

sre-bot commented Nov 14, 2019

/run-all-tests

@sre-bot
Copy link
Contributor

sre-bot commented Nov 14, 2019

@bb7133 merge failed.

Copy link
Contributor

@AilinKid AilinKid 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 AilinKid removed the status/LGT2 Indicates that a PR has LGTM 2. label Nov 14, 2019
@AilinKid AilinKid added the status/LGT3 The PR has already had 3 LGTM. label Nov 14, 2019
@AilinKid
Copy link
Contributor

/run-all-tests

@bb7133
Copy link
Member Author

bb7133 commented Nov 14, 2019

/run-unit-test

@bb7133 bb7133 merged commit c24b416 into pingcap:master Nov 14, 2019
@bb7133 bb7133 deleted the bb7133/add_shard_info branch December 5, 2019 09:42
XiaTianliang pushed a commit to XiaTianliang/tidb that referenced this pull request Dec 21, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status/can-merge Indicates a PR has been approved by a committer. status/LGT3 The PR has already had 3 LGTM. type/enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants