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

*: add sys schema, sys.SCHEMA_UNUSED_INDEXES view and sys.SCHEMA_INDEX_USAGE view #22126

Open
wants to merge 15 commits into
base: master
Choose a base branch
from

Conversation

rebelice
Copy link
Contributor

@rebelice rebelice commented Dec 31, 2020

What problem does this PR solve?

Issue Number: #19209

What is changed and how it works?

This is one step of #19209.

Add sys schema, sys.SCHEMA_UNUSED_INDEXES view and sys.SCHEMA_INDEX_USAGE view.

The sys schema is reference sys schema in MySQL.

And sys.SCHEMA_UNUSED_INDEXES view is also reference sys.SCHEMA_UNUSED_INDEXES in MySQL

The difference between sys.SCHEMA_INDEX_USAGE and mysql.SCHEMA_INDEX_USAGE is that access to sys.SCHEMA_INDEX_USAGE does not need to modify any permissions, and the content of sys.SCHEMA_INDEX_USAGE has been managed with permissions, only the index information of the table with the SELECT permission can be obtained. This is the same for sys.SCHEMA_UNUSED_INDEXES.

Related changes

  • PR to update pingcap/docs/pingcap/docs-cn:

Check List

Tests

  • Unit test

Side effects

Release note

  • add sys schema, sys.SCHEMA_UNUSED_INDEXES view and sys.SCHEMA_INDEX_USAGE view

@rebelice
Copy link
Contributor Author

/label status/WIP

@rebelice
Copy link
Contributor Author

I'll add tests for this PR later.

@rebelice rebelice mentioned this pull request Dec 31, 2020
12 tasks
@rebelice rebelice requested a review from a team as a code owner January 5, 2021 06:37
@rebelice rebelice requested review from hanfei1991 and removed request for a team January 5, 2021 06:37
@rebelice
Copy link
Contributor Author

rebelice commented Jan 5, 2021

/label sig/planner

@ti-srebot ti-srebot added the sig/planner SIG: Planner label Jan 5, 2021
@rebelice
Copy link
Contributor Author

rebelice commented Jan 5, 2021

/label component/statisitic

@ti-srebot
Copy link
Contributor

These labels are not found component/statisitic.

@rebelice
Copy link
Contributor Author

rebelice commented Jan 5, 2021

/label component/statistic

@ti-srebot
Copy link
Contributor

These labels are not found component/statistic.

@rebelice
Copy link
Contributor Author

rebelice commented Jan 5, 2021

/label component/statistics

@rebelice rebelice changed the title *: add sys schema and sys.SCHEMA_UNUSED_INDEXES view *: add sys schema and sys.SCHEMA_UNUSED_INDEXES view, sys.SCHEMA_INDEX_USAGE view Jan 5, 2021
@rebelice rebelice changed the title *: add sys schema and sys.SCHEMA_UNUSED_INDEXES view, sys.SCHEMA_INDEX_USAGE view *: add sys schema, sys.SCHEMA_UNUSED_INDEXES view and sys.SCHEMA_INDEX_USAGE view Jan 5, 2021
@rebelice rebelice requested a review from a team as a code owner January 5, 2021 08:17
@rebelice rebelice requested review from lzmhhh123 and removed request for a team January 5, 2021 08:17
@rebelice
Copy link
Contributor Author

rebelice commented Jan 5, 2021

/unlabel status/WIP

@rebelice
Copy link
Contributor Author

rebelice commented Jan 5, 2021

/cc @qw4990

@ti-srebot ti-srebot requested a review from qw4990 January 5, 2021 08:20
@github-actions github-actions bot added the sig/execution SIG execution label Jan 5, 2021
var pluginTable = make(map[string]func(autoid.Allocators, *model.TableInfo) (table.Table, error))

// RegisterTable registers a new table into TiDB.
func RegisterTable(tableName, sql string,
Copy link
Contributor

Choose a reason for hiding this comment

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

This function is not used.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated.

}

const viewUnusedIndexes = "CREATE VIEW sys." + viewNameUnusedIndexes +
` AS SELECT DISTINCT i.table_schema AS table_schema, i.table_name AS table_name, i.key_name AS index_name
Copy link
Contributor

Choose a reason for hiding this comment

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

Why use DISTINCT in these two SQLs?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Consider a situation, table t has an index idx_ab(a, b). There will be two rows of data in the table information_schema.tidb_indexes because this index covers two columns. So there will be two lines of the same (table_schema, t, idx_ab). DISTINCT is to deal with this situation.

@@ -1923,6 +1923,52 @@ func (d *ddl) RecoverTable(ctx sessionctx.Context, recoverInfo *RecoverInfo) (er
return errors.Trace(err)
}

// BuildTableInfoFromCreateViewAST builds model.TableInfo from a CreateViewStmt statement.
func BuildTableInfoFromCreateViewAST(s *ast.CreateViewStmt) (*model.TableInfo, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

How about renaming it to BuildViewInfoFromAST to correspond to BuildTableInfoFromAST?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don’t think this is a good name, because this function does not build ViewInfo, but TableInfo.

@@ -0,0 +1,49 @@
// Copyright 2016 PingCAP, Inc.
Copy link
Contributor

Choose a reason for hiding this comment

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

Please change 2016 to 2021.

@@ -0,0 +1,76 @@
// Copyright 2016 PingCAP, Inc.
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto

@@ -0,0 +1,101 @@
// Copyright 2017 PingCAP, Inc.
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto

@@ -0,0 +1,63 @@
// Copyright 2016 PingCAP, Inc.
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated all.

@@ -46,6 +46,7 @@ const (
PerformanceSchemaDBID int64 = SystemSchemaIDFlag | 10000
// MetricSchemaDBID is the metrics_schema schema id, it's exported for test.
MetricSchemaDBID int64 = SystemSchemaIDFlag | 20000
SysSchemaDBID int64 = SystemSchemaIDFlag | 40000
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add some comments. Does the number 40000 is a magic number, just need to be different from others?

Copy link
Contributor Author

@rebelice rebelice Jan 6, 2021

Choose a reason for hiding this comment

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

OK.

Does the number 40000 is a magic number, just need to be different from others?

Yes.

@@ -193,6 +193,8 @@ var (
PerformanceSchemaName = model.NewCIStr("PERFORMANCE_SCHEMA")
// MetricSchemaName is the `METRICS_SCHEMA` database name.
MetricSchemaName = model.NewCIStr("METRICS_SCHEMA")
// SysSchemaName is the `sys` database name.
SysSchemaName = model.NewCIStr("sys")
Copy link
Contributor

Choose a reason for hiding this comment

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

If some users have used sys as the database name before, how to solve it here? Should we consider this situation? @qw4990 @rebelice

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good question, I didn’t think about it before. I want to know how the information_schema or MySQL 8.0 was introduced at that time. Do you have any advice? @qw4990

@github-actions github-actions bot added the sig/sql-infra SIG: SQL Infra label Jan 7, 2021
Copy link
Member

@wjhuang2016 wjhuang2016 left a comment

Choose a reason for hiding this comment

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

IsMemDB also needs to modify.

@ti-chi-bot ti-chi-bot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. and removed sig/infra labels Feb 22, 2021
@ti-chi-bot
Copy link
Member

@rebelice: PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@lzmhhh123 lzmhhh123 removed their request for review June 28, 2021 02:38
@codecov
Copy link

codecov bot commented Jul 22, 2023

Codecov Report

Merging #22126 (3f29a08) into master (4a4acbe) will decrease coverage by 0.0029%.
Report is 9938 commits behind head on master.
The diff coverage is 63.2911%.

Additional details and impacted files
@@               Coverage Diff                @@
##             master     #22126        +/-   ##
================================================
- Coverage   77.0764%   77.0736%   -0.0029%     
================================================
  Files           573        575         +2     
  Lines        145715     145793        +78     
================================================
+ Hits         112312     112368        +56     
- Misses        23286      23305        +19     
- Partials      10117      10120         +3     

@ti-chi-bot ti-chi-bot bot deleted a comment from ti-chi-bot Dec 19, 2023
Copy link

ti-chi-bot bot commented Nov 18, 2024

@rebelice: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
idc-jenkins-ci-tidb/check_dev_2 3f29a08 link true /test check-dev2
idc-jenkins-ci-tidb/mysql-test 3f29a08 link true /test mysql-test
idc-jenkins-ci-tidb/check_dev 3f29a08 link true /test check-dev
pull-mysql-client-test 3f29a08 link true /test pull-mysql-client-test
pull-integration-ddl-test 3f29a08 link true /test pull-integration-ddl-test
idc-jenkins-ci-tidb/unit-test 3f29a08 link true /test unit-test
idc-jenkins-ci-tidb/build 3f29a08 link true /test build
pull-br-integration-test 3f29a08 link true /test pull-br-integration-test
pull-lightning-integration-test 3f29a08 link true /test pull-lightning-integration-test
pull-integration-e2e-test 3f29a08 link true /test pull-integration-e2e-test

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/statistics needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. sig/execution SIG execution sig/planner SIG: Planner sig/sql-infra SIG: SQL Infra
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants