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 table_id to table and add tidb_indexes table. #9183

Merged
merged 7 commits into from Jan 31, 2019

Conversation

@lysu
Copy link
Member

lysu commented Jan 25, 2019

What problem does this PR solve?

now, we can query table_id and index_id from http-api, but in some enviorment, it'
s more convient to use sql.

final usage can be sawed in test case https://github.com/pingcap/tidb/pull/9183/files#diff-9cb2947f24dfdff2e6f6cec84b53aa56R212

What is changed and how it works?

  • add tidb_table_id column to information_schema.tables
  • add new tidb_indexes table that contains index_id column

let use query table_id and index_id via sql.

Check List

Tests

  • Unit test

Code changes

  • infomation schema: column and table

Side effects

  • N/A

Related changes

  • N/A

This change is Reviewable

@lysu

This comment has been minimized.

Copy link
Member Author

lysu commented Jan 25, 2019

/run-all-tests

@crazycs520
Copy link
Contributor

crazycs520 left a comment

LGTM

@codecov-io

This comment has been minimized.

Copy link

codecov-io commented Jan 27, 2019

Codecov Report

Merging #9183 into master will increase coverage by 0.03%.
The diff coverage is 88.46%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #9183      +/-   ##
==========================================
+ Coverage   67.21%   67.24%   +0.03%     
==========================================
  Files         371      371              
  Lines       77052    77104      +52     
==========================================
+ Hits        51789    51850      +61     
+ Misses      20634    20629       -5     
+ Partials     4629     4625       -4
Impacted Files Coverage Δ
infoschema/tables.go 51.68% <88.46%> (+2.45%) ⬆️
infoschema/infoschema.go 76.31% <0%> (-2.64%) ⬇️
executor/index_lookup_join.go 77.28% <0%> (-0.64%) ⬇️
expression/schema.go 94.95% <0%> (+0.84%) ⬆️
store/tikv/lock_resolver.go 42.65% <0%> (+0.94%) ⬆️
executor/join.go 78.9% <0%> (+1.04%) ⬆️
store/tikv/scan.go 77.31% <0%> (+3.36%) ⬆️
ddl/delete_range.go 80.42% <0%> (+5.29%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b3bdc5d...65c5d55. Read the comment docs.

@jackysp

This comment has been minimized.

Copy link
Member

jackysp commented Jan 28, 2019

/run-all-tests

Show resolved Hide resolved infoschema/tables.go Outdated
tb.Name.O, // TABLE_NAME
0, // NON_UNIQUE
"PRIMARY", // KEY_NAME
1, // SEQ_IN_INDEX

This comment has been minimized.

@lamxTyler

lamxTyler Jan 28, 2019

Member

Is 0 better? Or it may coincide with indices?

This comment has been minimized.

@lysu

lysu Jan 29, 2019

Author Member

it's follow

1, // Seq_in_index
, so there are bug in show index?

This comment has been minimized.

@lysu

lysu Jan 29, 2019

Author Member

maybe we should better let seq start from 1 https://dev.mysql.com/doc/refman/8.0/en/show-index.html

This comment has been minimized.

@lamxTyler

lamxTyler Jan 29, 2019

Member

Ok, so it is better to make the seq_in_index be len(rows)+1 for index.

{"KEY_NAME", mysql.TypeVarchar, 64, 0, nil, nil},
{"SEQ_IN_INDEX", mysql.TypeLonglong, 21, 0, nil, nil},
{"COLUMN_NAME", mysql.TypeVarchar, 64, 0, nil, nil},
{"SUB_PART", mysql.TypeLonglong, 21, 0, nil, nil},

This comment has been minimized.

@XuHuaiyu

XuHuaiyu Jan 28, 2019

Contributor

Why not name it as LENGTH?

This comment has been minimized.

@lysu

lysu Jan 29, 2019

Author Member

because show index use this name

record := types.MakeDatums(
schema.Name.O, // TABLE_SCHEMA
tb.Name.O, // TABLE_NAME
0, // NON_UNIQUE

This comment has been minimized.

@XuHuaiyu

XuHuaiyu Jan 28, 2019

Contributor

May this 0 confuse others?

This comment has been minimized.

@lysu

lysu Jan 29, 2019

Author Member

just follow show index

0, // Non_unique

{"TABLE_SCHEMA", mysql.TypeVarchar, 64, 0, nil, nil},
{"TABLE_NAME", mysql.TypeVarchar, 64, 0, nil, nil},
{"NON_UNIQUE", mysql.TypeLonglong, 21, 0, nil, nil},
{"KEY_NAME", mysql.TypeVarchar, 64, 0, nil, nil},

This comment has been minimized.

@XuHuaiyu

XuHuaiyu Jan 28, 2019

Contributor

put this column before NON_UNIQUE?

This comment has been minimized.

@lysu

lysu Jan 29, 2019

Author Member

it's following show index

Show resolved Hide resolved infoschema/tables.go Outdated
Show resolved Hide resolved infoschema/tables_test.go Outdated

lysu added some commits Jan 29, 2019

Show resolved Hide resolved infoschema/tables_test.go Outdated
@@ -1003,6 +1019,68 @@ func dataForTables(ctx sessionctx.Context, schemas []*model.DBInfo) ([][]types.D
return rows, nil
}

func dataForIndexes(ctx sessionctx.Context, schemas []*model.DBInfo) ([][]types.Datum, error) {

This comment has been minimized.

@eurekaka

eurekaka Jan 29, 2019

Contributor

This looks like duplicate code with fetchShowIndex? can we extract it to a separate function for reuse?

This comment has been minimized.

@lysu

lysu Jan 29, 2019

Author Member

good idea, current show table and tables is same to but maybe we can do this in future?

Show resolved Hide resolved infoschema/tables.go
Show resolved Hide resolved infoschema/tables.go

lysu added some commits Jan 29, 2019

@winkyao
Copy link
Member

winkyao left a comment

LGTM

@jackysp
Copy link
Member

jackysp left a comment

LGTM

@jackysp jackysp merged commit 4c85822 into pingcap:master Jan 31, 2019

5 checks passed

ci/circleci Your tests passed on CircleCI!
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
idc-jenkins-ci-tidb/build Jenkins job succeeded.
Details
idc-jenkins-ci-tidb/check_dev Jenkins job succeeded.
Details
license/cla Contributor License Agreement is signed.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment