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

executor: support "show create table" for View #8865

Merged
merged 18 commits into from
Jan 15, 2019

Conversation

AndrewDi
Copy link
Contributor

@AndrewDi AndrewDi commented Dec 28, 2018

What problem does this PR solve?

Modify SHOW CREATE TABLE to show view definition

What is changed and how it works?

ref proposal Proposal: Implement View

Check List

Tests

  • Unit test

This change is Reviewable

@AndrewDi AndrewDi mentioned this pull request Dec 28, 2018
19 tasks
@zz-jason zz-jason added contribution This PR is from a community contributor. sig/execution SIG execution labels Dec 29, 2018
@zz-jason
Copy link
Member

zz-jason commented Jan 5, 2019

@AndrewDi Is this PR still work in progress? Can we begin to review it?

@AndrewDi
Copy link
Contributor Author

AndrewDi commented Jan 5, 2019

@zz-jason main code is complete, you can review now.

@codecov-io
Copy link

codecov-io commented Jan 5, 2019

Codecov Report

Merging #8865 into master will decrease coverage by <.01%.
The diff coverage is 64.51%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #8865      +/-   ##
==========================================
- Coverage   67.26%   67.25%   -0.01%     
==========================================
  Files         371      371              
  Lines       76195    76223      +28     
==========================================
+ Hits        51252    51266      +14     
- Misses      20412    20422      +10     
- Partials     4531     4535       +4
Impacted Files Coverage Δ
ddl/ddl_api.go 74.74% <0%> (-0.32%) ⬇️
executor/show.go 42.72% <100%> (+1.33%) ⬆️
planner/core/planbuilder.go 50.04% <55.55%> (-0.09%) ⬇️
ddl/delete_range.go 74.28% <0%> (-4.58%) ⬇️
executor/distsql.go 72.53% <0%> (-0.47%) ⬇️
util/filesort/filesort.go 76.48% <0%> (+0.31%) ⬆️
expression/schema.go 94.95% <0%> (+0.84%) ⬆️
store/tikv/lock_resolver.go 42.65% <0%> (+0.94%) ⬆️
util/systimemon/systime_mon.go 100% <0%> (+20%) ⬆️

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 41838ce...7e6eff5. Read the comment docs.

@AndrewDi AndrewDi changed the title executor: alter show_create_table to support show view definition [DNM] executor: alter show_create_table to support show view definition Jan 5, 2019
@zz-jason zz-jason changed the title executor: alter show_create_table to support show view definition executor: support "show create table" for View Jan 6, 2019
Copy link
Contributor

@XuHuaiyu XuHuaiyu left a comment

Choose a reason for hiding this comment

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

Please add some test cases for this commit.

@zz-jason
Copy link
Member

zz-jason commented Jan 7, 2019

@XuHuaiyu Let's review the main code logic firstly.

executor/show.go Outdated Show resolved Hide resolved
executor/show.go Outdated
}
fmt.Fprintf(&buf, ") AS %s", tb.Meta().View.SelectStmt)
Copy link
Contributor

Choose a reason for hiding this comment

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

This is the output of MySQL:

mysql> show create table v_test;
+--------+-------------------------------------------------------------------------------------------------------------------------------------------+----------------------+----------------------+
| View   | Create View                                                                                                                               | character_set_client | collation_connection |
+--------+-------------------------------------------------------------------------------------------------------------------------------------------+----------------------+----------------------+
| v_test | CREATE ALGORITHM=UNDEFINED DEFINER=`root`@`localhost` SQL SECURITY DEFINER VIEW `v_test` AS select `t`.`a` AS `a`,`t`.`b` AS `b` from `t` | utf8                 | utf8_general_ci      |
+--------+-------------------------------------------------------------------------------------------------------------------------------------------+----------------------+----------------------+
1 row in set (0.00 sec)

mysql> show create table t;
+-------+------------------------------------------------------------------------------------------------------------------+
| Table | Create Table                                                                                                     |
+-------+------------------------------------------------------------------------------------------------------------------+
| t     | CREATE TABLE `t` (
  `a` int(11) DEFAULT NULL,
  `b` int(11) DEFAULT NULL
) ENGINE=InnoDB DEFAULT CHARSET=latin1 |
+-------+------------------------------------------------------------------------------------------------------------------+
1 row in set (0.00 sec)

seems the number of columns and column names are different between normal table and view.

Copy link
Member

Choose a reason for hiding this comment

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

@AndrewDi Please take a look at this comment.

@zz-jason
Copy link
Member

@AndrewDi any update?

@AndrewDi
Copy link
Contributor Author

@zz-jason You advise to exact method to fetchShowCreateTable4View?

@zz-jason
Copy link
Member

@zz-jason You advise to exact method to fetchShowCreateTable4View?

Yes.

@AndrewDi AndrewDi force-pushed the view/show_create_table branch from bdb41af to 0f7158f Compare January 13, 2019 08:23
executor/show.go Outdated Show resolved Hide resolved
@AndrewDi
Copy link
Contributor Author

/run-all-tests

@XuHuaiyu XuHuaiyu requested review from eurekaka and zz-jason January 14, 2019 12:23
Copy link
Contributor

@XuHuaiyu XuHuaiyu left a comment

Choose a reason for hiding this comment

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

LGTM

e.appendRow([]interface{}{tb.Meta().Name.O, buf.String()})
return nil
}

func (e *ShowExec) fetchShowCreateTable4View(tb *model.TableInfo, buf *bytes.Buffer) {
Copy link
Member

Choose a reason for hiding this comment

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

As @eurekaka mentioned in the earlier code reviews. MySQL behaves different between normal table and view on show create table. For a normal table, the output has two columns, namely Table and Create Table:

MySQL(root@localhost:test) > create table t(a bigint, b bigint);
Query OK, 0 rows affected (0.11 sec)

MySQL(root@localhost:test) > show create table t;
+-------+----------------------------------------------------------------------------------------------------------------------------------------------------+
| Table | Create Table                                                                                                                                       |
+-------+----------------------------------------------------------------------------------------------------------------------------------------------------+
| t     | CREATE TABLE `t` (
  `a` bigint(20) DEFAULT NULL,
  `b` bigint(20) DEFAULT NULL
) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_0900_ai_ci |
+-------+----------------------------------------------------------------------------------------------------------------------------------------------------+
1 row in set (0.01 sec)

But for View, the output contains 4 columns, namely View, Create View, character_set_client and collation_connection:

MySQL(root@localhost:test) > create view v as select * from t;
Query OK, 0 rows affected (0.11 sec)

MySQL(root@localhost:test) > show create table v;
+------+---------------------------------------------------------------------------------------------------------------------------------------------------------+----------------------+----------------------+
| View | Create View                                                                                                                                             | character_set_client | collation_connection |
+------+---------------------------------------------------------------------------------------------------------------------------------------------------------+----------------------+----------------------+
| v    | CREATE ALGORITHM=UNDEFINED DEFINER=`skip-grants user`@`skip-grants host` SQL SECURITY DEFINER VIEW `v` AS select `t`.`a` AS `a`,`t`.`b` AS `b` from `t` | utf8mb4              | utf8mb4_0900_ai_ci   |
+------+---------------------------------------------------------------------------------------------------------------------------------------------------------+----------------------+----------------------+
1 row in set (0.00 sec)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

Copy link
Member

@zz-jason zz-jason left a comment

Choose a reason for hiding this comment

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

LGTM

@zz-jason zz-jason added the status/LGT2 Indicates that a PR has LGTM 2. label Jan 15, 2019
@zz-jason
Copy link
Member

/run-all-tests

@zz-jason zz-jason merged commit e6a0eb9 into pingcap:master Jan 15, 2019
@AndrewDi AndrewDi deleted the view/show_create_table branch January 15, 2019 09:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contribution This PR is from a community contributor. sig/execution SIG execution status/LGT2 Indicates that a PR has LGTM 2.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants