tables: basic support for hidden column #13908

wjhuang2016 · 2019-12-04T12:16:30Z

What problem does this PR solve?

Expression index is based on hidden column.
This PR will basicly support hidden column(insert and other functions will be supported in next few PRs):

Hidden column can't be selected.
Hidden column can't be updated or used as update condition.
Hidden column can't be used as delete condition.
Hidden column can't be drop by ALTER TABLE DROP COLUMN statement.
SHOW CREATE TABLE statement wouldn't display hidden columns.

What is changed and how it works?

Add a method VisibleCols for interface table which will ignore hidden column and replace it with Cols in buildDatasourece. Then the hidden columns will not display in plan.schema, and can not be selected, even in order by, group by statement.
(1) Since hidden column is used only by generate column, we check that if the generate column is hidden and be modified. If it is, raise an error.
(2) For where and order by clause in update statement, we can't skip hidden column in writableCols, because we need to fill the assignment and do the update, which need the column infomation of the hidden column. So we skip the hidden column in toColumn of expression_rewriter.
After 1 is done, hidden column can't be used as delete condition.
Replace Cols with VisibleCols in DropColumn, then we can't drop the hidden column.
After read the public columns by tableInfo.Cols(Not the Cols mention before) , skip it if it is a hidden column.

Check List

Tests

Unit test ( Contained in Integration test)
Integration test

Code changes

Has exported variable/fields change

Side effects

Possible performance regression
Increased code complexity
Breaking backward compatibility

Related changes

Release note

wjhuang2016 · 2019-12-05T02:21:30Z

/run-all-tests

table/tables/tables.go

expression/integration_test.go

codecov · 2019-12-05T03:32:06Z

Codecov Report

Merging #13908 into master will not change coverage.
The diff coverage is n/a.

@@             Coverage Diff             @@
##             master     #13908   +/-   ##
===========================================
  Coverage   80.3975%   80.3975%           
===========================================
  Files           483        483           
  Lines        122449     122449           
===========================================
  Hits          98446      98446           
  Misses        16244      16244           
  Partials       7759       7759

table/tables/tables_test.go

wjhuang2016 · 2019-12-05T06:08:13Z

/run-all-tests

wjhuang2016 · 2019-12-05T06:08:35Z

/run-all-tests

wjhuang2016 · 2019-12-05T06:13:25Z

/run-all-tests

djshow832 · 2019-12-11T03:14:43Z

table/tables/tables.go

+)
+
+func (t *TableCommon) getCols(mode getColsMode) []*table.Column {
+	Columns := make([]*table.Column, 0, len(t.Columns))


Suggested change

Columns := make([]*table.Column, 0, len(t.Columns))

columns := make([]*table.Column, 0, len(t.Columns))

zz-jason · 2019-12-11T04:34:17Z

expression/column.go

@@ -201,6 +201,7 @@ type Column struct {
 	VirtualExpr Expression

 	OrigName string
+	Hidden   bool


s/Hidden/IsHidden/

table/tables/tables.go

table/tables/tables_test.go

executor/show.go

table/tables/tables_test.go

Deardrops · 2019-12-11T08:30:04Z

LGTM

zimulala · 2019-12-12T02:46:46Z

table/tables/tables.go

 	for _, col := range t.Columns {
 		if col.State != model.StatePublic {
 			continue
 		}
-		publicColumns[col.Offset] = col
-		if maxOffset < col.Offset {


Why remove these codes?

The previous code imply that public column always before non-public column, and set the maxOffset to the last public column. When we intro hidden column, we can't guarantee the visible columns are continuous.

zimulala · 2019-12-12T02:55:52Z

ddl/ddl_api.go

@@ -2373,7 +2373,7 @@ func (d *ddl) DropColumn(ctx sessionctx.Context, ti ast.Ident, spec *ast.AlterTa

 	// Check whether dropped column has existed.
 	colName := spec.OldColumnName.Name
-	col := table.FindCol(t.Cols(), colName.L)
+	col := table.FindCol(t.VisibleCols(), colName.L)


We can't drop hide columns. Could we do modify column operation to hide columns?

We can't modify the hidden column in theory, I will disable this operation in next PR

sre-bot · 2019-12-14T03:23:12Z

@Deardrops, @djshow832, @zz-jason, @zimulala, @eurekaka, @XuHuaiyu, @wshwsh12, @lzmhhh123, PTAL.

sre-bot · 2019-12-16T03:23:12Z

@Deardrops, @djshow832, @zz-jason, @zimulala, @eurekaka, @XuHuaiyu, @wshwsh12, @lzmhhh123, PTAL.

djshow832 · 2019-12-17T03:13:03Z

LGTM

zimulala

LGTM

sre-bot · 2019-12-17T13:45:49Z

/run-all-tests

AilinKid

nice done.

wjhuang2016 added 7 commits December 3, 2019 19:47

done

52faf27

fix typo

1ddc61d

fix typo

e3c360f

Merge branch 'master' of github.com:pingcap/tidb into cleanCode

61dac19

save

5f17560

merge master

21755c9

done

fe40cb4

wjhuang2016 requested a review from a team as a code owner December 4, 2019 12:16

ghost requested review from wshwsh12 and XuHuaiyu and removed request for a team December 4, 2019 12:16

wjhuang2016 added the type/new-feature label Dec 4, 2019

wjhuang2016 added 3 commits December 4, 2019 20:19

tweak

33552b8

tweak

40dd6d6

tweak

76b13f5

zimulala self-requested a review December 5, 2019 02:42

Deardrops reviewed Dec 5, 2019

View reviewed changes

table/tables/tables.go Show resolved Hide resolved

expression/integration_test.go Outdated Show resolved Hide resolved

address comment

5e7217a

Deardrops reviewed Dec 5, 2019

View reviewed changes

table/tables/tables_test.go Show resolved Hide resolved

table/tables/tables_test.go Show resolved Hide resolved

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

wjhuang2016 added 3 commits December 5, 2019 11:40

address comment

51db782

fix typo

412f5f2

fix test

c7442bf

wjhuang2016 force-pushed the hidden_for_select branch 2 times, most recently from 0f2c2d5 to 5e7217a Compare December 5, 2019 06:05

wjhuang2016 force-pushed the hidden_for_select branch from a347827 to 76b13f5 Compare December 5, 2019 06:13

djshow832 reviewed Dec 11, 2019

View reviewed changes

zz-jason reviewed Dec 11, 2019

View reviewed changes

Deardrops reviewed Dec 11, 2019

View reviewed changes

table/tables/tables.go Show resolved Hide resolved

table/tables/tables_test.go Show resolved Hide resolved

table/tables/tables_test.go Show resolved Hide resolved

executor/show.go Show resolved Hide resolved

wjhuang2016 added 2 commits December 11, 2019 15:43

address comment

69c0e24

Merge remote-tracking branch 'upstream/master' into hidden_for_select

77beff5

wjhuang2016 force-pushed the hidden_for_select branch from 4d57885 to 77beff5 Compare December 11, 2019 07:45

lint

d10ec20

Deardrops reviewed Dec 11, 2019

View reviewed changes

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

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

address comment

2f5c8c2

zimulala added component/DDL-need-LGT3 status/LGT1 Indicates that a PR has LGTM 1. labels Dec 11, 2019

zimulala reviewed Dec 12, 2019

View reviewed changes

wjhuang2016 added 2 commits December 16, 2019 10:46

fix drop column

4bdb258

merge master

bd411a0

wjhuang2016 force-pushed the hidden_for_select branch from b2493b1 to bd411a0 Compare December 16, 2019 02:52

zimulala approved these changes Dec 17, 2019

View reviewed changes

zimulala added status/LGT3 The PR has already had 3 LGTM. status/can-merge Indicates a PR has been approved by a committer. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Dec 17, 2019

Merge branch 'master' into hidden_for_select

0017289

sre-bot merged commit 42adca9 into pingcap:master Dec 17, 2019

AilinKid reviewed Dec 18, 2019

View reviewed changes

XiaTianliang pushed a commit to XiaTianliang/tidb that referenced this pull request Dec 21, 2019

tables: basic support for hidden column (pingcap#13908)

cf3e25e

you06 added the sig/sql-infra SIG: SQL Infra label Mar 4, 2020

wjhuang2016 deleted the hidden_for_select branch November 17, 2022 11:20

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

tables: basic support for hidden column #13908

tables: basic support for hidden column #13908

wjhuang2016 commented Dec 4, 2019 •

edited

Loading

wjhuang2016 commented Dec 5, 2019

codecov bot commented Dec 5, 2019 •

edited

Loading

wjhuang2016 commented Dec 5, 2019

wjhuang2016 commented Dec 5, 2019

wjhuang2016 commented Dec 5, 2019

djshow832 Dec 11, 2019

zz-jason Dec 11, 2019

Deardrops commented Dec 11, 2019

zimulala Dec 12, 2019

wjhuang2016 Dec 12, 2019

zimulala Dec 12, 2019

wjhuang2016 Dec 12, 2019

sre-bot commented Dec 14, 2019

sre-bot commented Dec 16, 2019

djshow832 commented Dec 17, 2019

zimulala left a comment

sre-bot commented Dec 17, 2019

AilinKid left a comment

	Columns := make([]*table.Column, 0, len(t.Columns))
	columns := make([]*table.Column, 0, len(t.Columns))

tables: basic support for hidden column #13908

tables: basic support for hidden column #13908

Conversation

wjhuang2016 commented Dec 4, 2019 • edited Loading

What problem does this PR solve?

What is changed and how it works?

Check List

wjhuang2016 commented Dec 5, 2019

codecov bot commented Dec 5, 2019 • edited Loading

Codecov Report

wjhuang2016 commented Dec 5, 2019

wjhuang2016 commented Dec 5, 2019

wjhuang2016 commented Dec 5, 2019

djshow832 Dec 11, 2019

Choose a reason for hiding this comment

zz-jason Dec 11, 2019

Choose a reason for hiding this comment

Deardrops commented Dec 11, 2019

zimulala Dec 12, 2019

Choose a reason for hiding this comment

wjhuang2016 Dec 12, 2019

Choose a reason for hiding this comment

zimulala Dec 12, 2019

Choose a reason for hiding this comment

wjhuang2016 Dec 12, 2019

Choose a reason for hiding this comment

sre-bot commented Dec 14, 2019

sre-bot commented Dec 16, 2019

djshow832 commented Dec 17, 2019

zimulala left a comment

Choose a reason for hiding this comment

sre-bot commented Dec 17, 2019

AilinKid left a comment

Choose a reason for hiding this comment

wjhuang2016 commented Dec 4, 2019 •

edited

Loading

codecov bot commented Dec 5, 2019 •

edited

Loading