proposal: add more detail view implement proposal #8416
Conversation
|
Hi contributor, thanks for your PR. This patch needs to be approved by someone of admins. They should reply with "/ok-to-test" to accept this PR for running test automatically. |
|
@XuHuaiyu PTAL |
shenli
added
component/docs
contribution
| ## SubTask Schedule | ||
| |Action |Priority|Deadline|Notes| | ||
| | ------ | ------ | ------ |-----| | ||
| |`CREATE [OR REPLACE] VIEW view_name [(column_list)] AS select_statement`|P1|2019/01/15|后续的所有工作都依赖这项| |
|
/run-common-test tidb-test=pr/658 |
@zhexuany I didn't do any code change, why test fail... |
|
@AndrewDi Please ignored the error. I am adding the new test case in our internal test framework. |
|
/run-common-test tidb-test=pr/658 |
|
/run-common-test |
|
/run-common-test tidb-test=pr/668 |
| Security SecurityType `json:"view_security""` | ||
| SelectStmt string `json:"view_select"` | ||
| CheckOption CheckOption `json:"view_checkoption"` | ||
| Cols []string `json:"view_cols"` |
There was a problem hiding this comment.
Why do we need this?
Seems duplicate with TableInfo.ColumnName.
There was a problem hiding this comment.
Use TableInfo.ColumnName to store origin column names, and use View.Cols to store column alias name
There was a problem hiding this comment.
s/Cols []string/ Cols []model.CIStr
| * IsUpdatable ref https://dev.mysql.com/doc/refman/5.7/en/view-updatability.html | ||
| This parameter mark if this view is updatable. | ||
| * SelectStmt | ||
| This string is the select sql statement after sql rewriter. |
There was a problem hiding this comment.
- We only store the origin sql of create_view now.
- Add an example here to show the expected format.
| |Add some test cases for CreateView and Select … From View(port from MySQL test)|P1|2019/03/30|--| | ||
| |UPDATE VIEW|P2| |Difficult| | ||
| |INSERT VIEW|P2| |Difficult, dependent on UPDATE VIEW)| | ||
| |SHOW CREATE [VIEW | TABLE]|P2| | | |
There was a problem hiding this comment.
We should add a task like support CREATE_VIEW_PRIV check with priority P2.
| Cols []string `json:"view_cols"` | ||
| } | ||
| ``` | ||
| * AlgorithmType ref https://dev.mysql.com/doc/refman/5.7/en/view-algorithms.html |
There was a problem hiding this comment.
use syntax:
[AlgorithmType](https://dev.mysql.com/doc/refman/5.7/en/view-algorithms.html)
| ``` | ||
| * AlgorithmType ref https://dev.mysql.com/doc/refman/5.7/en/view-algorithms.html | ||
| The view SQL AlGORITHM characteristic. The value is one of UNDEFINED、MERGE OR TEMPTABLE, if no ALGORITHM clause is present, UNDEFINED is the default algorithm. | ||
| We will implement Algorithm=UNDEFINED only now. |
There was a problem hiding this comment.
We will implement MERGE not UNDEFINED, and regard UNDEFINED as MERGE now.
As the MySQL manual shows:
If no ALGORITHM clause is present, UNDEFINED is the default algorithm prior to MySQL 5.7.6. As of 5.7.6, the default algorithm is determined by the value of the derived_merge flag of the optimizer_switch system variable. For additional discussion, see Section 8.2.2.3, “Optimizing Derived Tables and View References”.
| * SelectStmt | ||
| This string is the origin select sql statement. | ||
| * Cols | ||
| This string array is the view's column alias names. |
| ``` | ||
| 1. Parse the create view statement and build a logical plan for select cause part. If any grammar error occurs, return errors to parser. | ||
| 2. Examine view definer's privileges. Definer should own both `CREATE_VIEW` and base table's `SELECT` privileges. | ||
| 3. Examine create view statement, If ViewFieldList cause part is empty, then we should generate view column names from SelectStmt cause. Otherwise check len(ViewFieldList) == len(Columns from SelectStmt). And then we save column names to `TableInfo.Columns` . | ||
| 2. Drop a view |
There was a problem hiding this comment.
s/ Drop a view/ DROP VIEW
| 2. Drop a view | ||
| Implement DROP VIEW grammar, and delete the existing view tableinfo object. | ||
| Implement `DROP VIEW` grammar, and delete the existing view tableinfo object. This function should reuse `DROP TABLE` code logical | ||
| 3. Select from a view |
There was a problem hiding this comment.
s/ Select from a view/ SELECT FROM VIEW
| select * from t; | ||
| ``` | ||
| Once we query from view `v`, database will rewrite view's `SelectStmt` from `select * from t` into **`select a as a,b as b from t`** | ||
|
|
There was a problem hiding this comment.
Add a line here:
But, if we alter the schema of t like this:
| drop table t; | ||
| create table t(c int,d int); | ||
| ``` | ||
| If we rebuild table `v` from sql above and query from view `v` again, database will rewrite view's `SelectStmt` from `select * from t` into **`select c as c,d as d from t`** |
There was a problem hiding this comment.
change line 92~93 to:
Executing select * from v now is equivalent to select c as c, d as d from t.The result of this query will be incompatible with the character of VIEW that a VIEW should be "frozen" after defined.
Thanks for your reminder... |
| In order to solve the problem described above, we must build a `Projection` at the top of original select's `LogicalPlan`, just like we rewrite view's `SelectStmt` from `select * from t` into **`select a as a,b as b from (select * from t)`**. | ||
| This is a temporary fix and we will implement TiDB to rewrite SQL with replacing all wildcard finally. | ||
| 4. Show table status | ||
| Modify `SHOW TABLE STATUS` function to support show view status, and we use this command to check if `CREATE VIEW` and `DROP VIEW` operation is successful. To reuse `SHOW TABLE STATUS` code logical is preferred. |
There was a problem hiding this comment.
s/ code logical/ code logic?
|
PTAL @zz-jason @lamxTyler |
|
The effect of SQL_MODE should also be taken into consideration. We need to list it as a P2 or P3 priority task. |
I think P3 would be nice |
There was a problem hiding this comment.
Reviewable status: 0 of 1 files reviewed, 12 unresolved discussions (waiting on @lamxTyler, @XuHuaiyu, and @zz-jason)
There was a problem hiding this comment.
Reviewed 1 of 1 files at r16.
Reviewable status: all files reviewed, 11 unresolved discussions (waiting on @XuHuaiyu, @zz-jason, and @lamxTyler)
| ``` | ||
| * [Algorithm](https://dev.mysql.com/doc/refman/5.7/en/view-algorithms.html) | ||
| The view SQL `AlGORITHM` characteristic. The value should be one of `UNDEFINED`, `MERGE` OR `TEMPTABLE`. If no `ALGORITHM` clause is present, `UNDEFINED` is the default algorithm. | ||
| Currently, we will only implement Algorithm=MERGE algorithm. |
There was a problem hiding this comment.
| Currently, we will only implement Algorithm=MERGE algorithm. | |
| Currently, we will only implement the `MERGE` algorithm. |
| * [Security](https://dev.mysql.com/doc/refman/5.7/en/create-view.html) | ||
| The view SQL `SECURITY` characteristic. The value is one of `DEFINER` or `INVOKER`. | ||
| * [CheckOption](https://dev.mysql.com/doc/refman/5.7/en/view-check-option.html) | ||
| The `WITH CHECK OPTION` clause can be given to an updatable view to prevent inserts to rows for which the WHERE clause in the select_statement is not true. It also prevents updates to rows for which the WHERE clause is true but the update would cause it to be not true (in other words, it prevents visible rows from being updated to nonvisible rows). |
There was a problem hiding this comment.
| The `WITH CHECK OPTION` clause can be given to an updatable view to prevent inserts to rows for which the WHERE clause in the select_statement is not true. It also prevents updates to rows for which the WHERE clause is true but the update would cause it to be not true (in other words, it prevents visible rows from being updated to nonvisible rows). | |
| The `WITH CHECK OPTION` clause can be given to an updatable view to prevent inserts to rows for which the `WHERE` clause in the select statement is not true. It also prevents updating rows for which the `WHERE` clause is true but the update operation would cause it to be false. In other words, it prevents visible rows from being updated to nonvisible rows. |
There was a problem hiding this comment.
"updates to rows" would be more precisely.
| The view SQL `SECURITY` characteristic. The value is one of `DEFINER` or `INVOKER`. | ||
| * [CheckOption](https://dev.mysql.com/doc/refman/5.7/en/view-check-option.html) | ||
| The `WITH CHECK OPTION` clause can be given to an updatable view to prevent inserts to rows for which the WHERE clause in the select_statement is not true. It also prevents updates to rows for which the WHERE clause is true but the update would cause it to be not true (in other words, it prevents visible rows from being updated to nonvisible rows). | ||
| In a `WITH CHECK OPTION` clause for an updatable view, the `LOCAL` and `CASCADED` keywords determine the scope of check testing when the view is defined in terms of another view. When neither keyword is given, the default is CASCADED. |
There was a problem hiding this comment.
| In a `WITH CHECK OPTION` clause for an updatable view, the `LOCAL` and `CASCADED` keywords determine the scope of check testing when the view is defined in terms of another view. When neither keyword is given, the default is CASCADED. | |
| In a `WITH CHECK OPTION` clause for an updatable view, the `LOCAL` and `CASCADED` keywords determine the scope of check testing when the view is defined in terms of another view. When neither keyword is given, the default is `CASCADED`. |
| * SelectStmt | ||
| This string is the origin `SELECT` SQL statement. | ||
| * Cols | ||
| This model.CIStr array stores view's column alias names. |
There was a problem hiding this comment.
| This model.CIStr array stores view's column alias names. | |
| This `model.CIStr` array stores view's column alias names. |
| ``` | ||
| 1. Parse the create view statement and build a logical plan for select clause part. If any grammar error occurs, return errors to parser. | ||
| 2. Examine view definer's privileges. Definer should own both `CREATE_VIEW_PRIV` privilege and base table's `SELECT` privilege. We will reuse `CREATE_PRIV` privilege when implement `CREATE VIEW` feature and will support `CREATE_VIEW_PRIV` check later. | ||
| 3. Examine create view statement. If the ViewFieldList clause part is empty, then we should generate view column names from SelectStmt clause. Otherwise check len(ViewFieldList) == len(Columns from SelectStmt). And then we save column names to `TableInfo.Columns`. |
There was a problem hiding this comment.
| 3. Examine create view statement. If the ViewFieldList clause part is empty, then we should generate view column names from SelectStmt clause. Otherwise check len(ViewFieldList) == len(Columns from SelectStmt). And then we save column names to `TableInfo.Columns`. | |
| 3. Examine create view statement. If the `ViewFieldList` clause part is empty, then we should generate view column names from the `SelectStmt` clause. Otherwise check `len(ViewFieldList) == len(Columns from SelectStmt)`. And then we save column names to `TableInfo.Columns`. |
|
|
||
| ## Compatibility | ||
| It makes TiDB more compatible with MySQL. | ||
| Add TiDB support for the basic VIEW feature without affecting other existing functions, and make TiDB more compatible with MySQL. |
There was a problem hiding this comment.
| Add TiDB support for the basic VIEW feature without affecting other existing functions, and make TiDB more compatible with MySQL. | |
| Support the basic VIEW functionality without affecting other existing functionalities in TiDB. And make TiDB be more compatible with MySQL. |
| We convert `expression.columns` into `table.Column` and reuse function `buildTableInfo` to build view | ||
| 4. logical_plan_builder.go | ||
| Every time we make a `SELECT` statement within a view, modify the `buildDataSource` function to rewrite the view table to select `LogicalPlan`. | ||
| |Action |Priority|Deadline|Notes| |
There was a problem hiding this comment.
| |Action |Priority|Deadline|Notes| | |
| Here is the work items list: | |
| |Action |Priority|Deadline|Notes| |
There was a problem hiding this comment.
Reviewed 1 of 1 files at r16.
Reviewable status: all files reviewed, 16 unresolved discussions (waiting on @zz-jason and @lamxTyler)
|
Another thing is: |
|
@zz-jason PTAL again |
There was a problem hiding this comment.
LGTM
Reviewed 1 of 1 files at r17.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @lamxTyler)
There was a problem hiding this comment.
Reviewed 1 of 1 files at r17.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @lamxTyler)
There was a problem hiding this comment.
Reviewable status:
complete! all files reviewed, all discussions resolved
What problem does this PR solve?
Proposal view implement.
What is changed and how it works?
After discuss about the view implement detail, add more details to document.
Check List
Tests
Side effects
Increased code complexity
Related changes
Need to update the documentation
This change is