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: show information about table partitions in information_schema.PARTITIONS #14347

Merged
merged 10 commits into from
Feb 5, 2020

Conversation

tangwz
Copy link
Contributor

@tangwz tangwz commented Jan 5, 2020

What problem does this PR solve?

fixes: #13905

What is changed and how it works?

Check List

Tests

  • Unit test

@sre-bot sre-bot added the contribution This PR is from a community contributor. label Jan 5, 2020
@shenli
Copy link
Member

shenli commented Jan 5, 2020

@tangwz thanks! Could you please add some unit tests in this PR?

infoschema/tables.go Outdated Show resolved Hide resolved
Copy link
Contributor

@qw4990 qw4990 left a comment

Choose a reason for hiding this comment

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

@tangwz Thanks~
Please add some tests for this.
A simple way is adding some integration tests like what we do in expression/integration_test.go, for example:

MustExec("CREATE TABLE ... PARTITION ...")
results := MustQuery("SELECT * FROM INFORMATION_SCHEMA.PARTITIONS")
// check results

infoschema/tables.go Outdated Show resolved Hide resolved
@qw4990
Copy link
Contributor

qw4990 commented Jan 14, 2020

@tangwz Friendly ping, please address these comments.

Copy link
Contributor

@tiancaiamao tiancaiamao left a comment

Choose a reason for hiding this comment

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

I do not quite understand the test code, why do we need to update the statistics before check?

infoschema/tables.go Outdated Show resolved Hide resolved
infoschema/tables_test.go Outdated Show resolved Hide resolved
infoschema/tables_test.go Outdated Show resolved Hide resolved
infoschema/tables_test.go Outdated Show resolved Hide resolved
@sre-bot
Copy link
Contributor

sre-bot commented Jan 22, 2020

@tangwz, please update your pull request.

@sre-bot
Copy link
Contributor

sre-bot commented Jan 29, 2020

@tangwz, please update your pull request.

@qw4990 qw4990 self-requested a review February 3, 2020 03:20
@qw4990
Copy link
Contributor

qw4990 commented Feb 3, 2020

LGTM, PTAL @tiancaiamao

@qw4990 qw4990 added the status/LGT1 Indicates that a PR has LGTM 1. label Feb 3, 2020
@qw4990 qw4990 removed their request for review February 3, 2020 03:29
@tiancaiamao
Copy link
Contributor

LGTM

@tiancaiamao tiancaiamao added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Feb 5, 2020
@tiancaiamao
Copy link
Contributor

/merge

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

sre-bot commented Feb 5, 2020

/run-all-tests

@tiancaiamao tiancaiamao removed the request for review from jackysp February 5, 2020 05:41
@tiancaiamao
Copy link
Contributor

/merge

@sre-bot
Copy link
Contributor

sre-bot commented Feb 5, 2020

Your auto merge job has been accepted, waiting for 14279

@sre-bot
Copy link
Contributor

sre-bot commented Feb 5, 2020

/run-all-tests

@sre-bot sre-bot merged commit b07cf70 into pingcap:master Feb 5, 2020
hsqlu pushed a commit to hsqlu/tidb that referenced this pull request Feb 8, 2020
tiancaiamao added a commit to tiancaiamao/tidb that referenced this pull request Feb 19, 2020
sre-bot pushed a commit that referenced this pull request Feb 25, 2020
@qw4990 qw4990 added the sig/execution SIG execution label Oct 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/infoschema contribution This PR is from a community contributor. sig/execution SIG execution status/can-merge Indicates a PR has been approved by a committer. status/LGT2 Indicates that a PR has LGTM 2.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support information_schema.PARTITIONS
6 participants