admin: refine admin check. #7862

crazycs520 · 2018-10-10T09:04:37Z

What problem does this PR solve?

refine admin check.

What is changed and how it works?

Admin check uses rowDecoder to decode data and evaluates generated column.

Check List

Code changes

Has exported function/method change

Related changes

No need to cherry pick.

crazycs520 · 2018-10-10T09:34:23Z

/run-all-tests

shenli · 2018-10-10T09:44:47Z

Is this PR improve the performance?

crazycs520 · 2018-10-10T10:27:51Z

@shenli

For index on generated columns, may have a little performance improvement. This PR will reduce memory allocation when evaluates virtual generated column.
Normally, this PR have not improve the performance, But may make the code more clearer.

winkyao · 2018-10-10T14:01:42Z

util/admin/admin.go

 	}

-	rowMap, err := tablecodec.DecodeRow(value, colTps, sessCtx.GetSessionVars().Location())
+	rowMap, err := rowDecoder.DecodeAndEvalRowWithMap(sessCtx, value, sessCtx.GetSessionVars().Location(), time.UTC, nil)


Why the time.Location change to time.UTC, not sessCtx.GetSessionVars().Location()?

There is not change.
sessCtx.GetSessionVars().Location() is for tablecodec.DecodeRow to decode data.
time.UTC is for evaluated virtual generated column.

winkyao

LGTM

crazycs520 · 2018-10-11T03:13:11Z

/run-all-tests

ciscoxll · 2018-10-15T15:39:46Z

planner/core/planbuilder.go

@@ -548,8 +548,7 @@ func (b *planBuilder) buildAdminCheckTable(as *ast.AdminStmt) (*CheckTable, erro
 				return nil, errors.Trace(err)
 			}
 			expr = expression.BuildCastFunction(b.ctx, expr, colExpr.GetType())
-			genColumnName := model.GetTableColumnID(tableInfo, column.ColumnInfo)
-			p.GenExprs[genColumnName] = expr
+			p.GenExprs[model.TableColumnID{TableID: tableInfo.ID, ColumnID: column.ColumnInfo.ID}] = expr


~~Will there be a problem here?~~

what problem?

ciscoxll

LGTM

…heck-decoder

ngaut · 2018-10-18T17:06:59Z

/run-all-tests

bugfix fixed pingcap#7518 expression: MySQL compatible current_user function (pingcap#7801) plan: propagate constant over outer join (pingcap#7794) - extract `outerCol = const` from join conditions and filter conditions, substitute `outerCol` in join conditions with `const`; - extract `outerCol = innerCol` from join conditions, derive new join conditions based on this column equal condition and `outerCol` related expressions in join conditions and filter conditions; util/timeutil: fix data race caused by forgetting set stats lease to 0 (pingcap#7901) stats: handle ddl event for partition table (pingcap#7903) plan: implement Operand and Pattern of cascades planner. (pingcap#7910) planner: not convert to TableDual if empty range is derived from deferred constants (pingcap#7808) plan: move projEliminate behind aggEliminate (pingcap#7909) admin: fix admin check table bug of byte compare (pingcap#7887) * admin: remove reflect deepEqual stats: fix panic caused by empty histogram (pingcap#7912) plan: fix panic caused by empty schema of LogicalTableDual (pingcap#7906) * fix drop view if exist error (pingcap#7833) executor: refine `explain analyze` (pingcap#7888) executor: add an variable to compatible with MySQL insert for OGG (pingcap#7863) expression: maintain `DeferredExpr` in aggressive constant folding. (pingcap#7915) stats: fix histogram boundaries overflow error (pingcap#7883) ddl:support the definition of `null` change to `not null` using `alter table` (pingcap#7771) * ddl:support the definition of null change to not null using alter table ddl: add check when create table with foreign key. (pingcap#7885) * ddl: add check when create table with foreign key planner: eliminate if null on non null column (pingcap#7924) executor: fix a bug in point get (pingcap#7934) planner, executor: refine ColumnPrune for LogicalUnionAll (pingcap#7930) executor: fix panic when limit is too large (pingcap#7936) ddl: add TiDB version to metrics (pingcap#7902) stats: limit the length of sample values (pingcap#7931) vendor: update tipb (pingcap#7893) planner: support the Group and GroupExpr for the cascades planner (pingcap#7917) store/tikv: log more information when other err occurs (pingcap#7948) types: fix date time parse (pingcap#7933) ddl: just print error message when ddl job is normal to calcel, to eliminate noisy log (pingcap#7875) stats: update delta info for partition table (pingcap#7947) explaintest: add explain test for partition pruning (pingcap#7505) util: move disjoint set to util package (pingcap#7950) util: add PreAlloc4Row and Insert for Chunk and List (pingcap#7916) executor: add the slow log for commit (pingcap#7951) expression: add builtin json_keys (pingcap#7776) privilege: add USAGE in `show grants` for mysql compatibility (pingcap#7955) ddl: fix invailid ddl job panic (pingcap#7940) *: move ast.NewValueExpr to standalone parser_driver package (pingcap#7952) Make the ast package get rid of the dependency of types.Datum server: allow cors http request (pingcap#7939) *: move `Statement` and `RecordSet` from ast to sqlexec package (pingcap#7970) pr suggestion update executor/aggfuncs: split unit tests to corresponding file (pingcap#7993) store/tikv: fix typo (pingcap#7990) executor, planner: clone proj schema for different children in buildProj4Union (pingcap#7999) executor: let information_schema be the first database in ShowDatabases (pingcap#7938) stats: use local feedback for partition table (pingcap#7963) executor: add unit test for aggfuncs (pingcap#7966) server: add log for binary execute statement (pingcap#7987) admin: refine admin check decoder (pingcap#7862) executor: improve wide table insert & update performance (pingcap#7935) ddl: fix reassigned partition id in `truncate table` does not take effect (pingcap#7919) fix reassigned partition id in truncate table does not take effect add changelog for 2.1.0 rc4 (pingcap#8020) *: make parser package dependency as small as possible (pingcap#7989) parser: support `:=` in the `set` syntax (pingcap#8018) According to MySQL document, `set` use the = assignment operator, but the := assignment operator is also permitted stats: garbage collect stats for partition table (pingcap#7962) docs: add the proposal for the column pool (pingcap#7988) expression: refine built-in func truncate to support uint arg (pingcap#8000) stats: support show stats for partition table (pingcap#8023) stats: update error rate for partition table (pingcap#8022) stats: fix estimation for out of range point queries (pingcap#8015) *: move parser to a separate repository (pingcap#8036) executor: fix wrong result when index join on union scan. (pingcap#8031) Do not modify Plan of dataReaderBuilder directly, because it would impact next batch of outer rows, as well as other concurrent inner workers. Instead, build a local child builder to store the child plan. planner: fix a panic of a cached prepared statement with IndexScan (pingcap#8017) *: fix the issue of executing DDL after executing SQL failure in txn (pingcap#8044) * ddl, executor: fix the issue of executing DDL after executing SQL failure in txn add unit test remove debug info add like evaluator case sensitive test ddl, domain: make schema correct after canceling jobs (pingcap#7997) unit test fix code format proposal: maintaining histograms in plan. (pingcap#7605) support _tidb_rowid for table scan range (pingcap#8047) var rename fix

crazycs520 added status/WIP labels Oct 10, 2018

crazycs520 force-pushed the admin-check-decoder branch from 3c708e7 to ef6ed18 Compare October 10, 2018 09:20

crazycs520 removed the status/WIP label Oct 10, 2018

crazycs520 force-pushed the admin-check-decoder branch from ef6ed18 to 0dd3352 Compare October 10, 2018 09:33

admin: refine admin check decoder

09ee890

crazycs520 force-pushed the admin-check-decoder branch from 0dd3352 to 09ee890 Compare October 10, 2018 13:54

winkyao reviewed Oct 10, 2018

View reviewed changes

crazycs520 added the status/LGT1 Indicates that a PR has LGTM 1. label Oct 11, 2018

crazycs520 added the status/all tests passed label Oct 15, 2018

ciscoxll reviewed Oct 15, 2018

View reviewed changes

ciscoxll reviewed Oct 16, 2018

View reviewed changes

ngaut added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Oct 17, 2018

Merge branch 'master' of https://github.com/pingcap/tidb into admin-c…

ccf9135

…heck-decoder

crazycs520 force-pushed the admin-check-decoder branch from 92a6c34 to ccf9135 Compare October 17, 2018 13:28

Merge branch 'master' into admin-check-decoder

c947045

Merge branch 'master' into admin-check-decoder

f13cbbf

ngaut approved these changes Oct 23, 2018

View reviewed changes

ngaut merged commit 1434fa9 into pingcap:master Oct 23, 2018

crazycs520 added a commit to crazycs520/tidb that referenced this pull request Dec 13, 2018

admin: refine admin check decoder (pingcap#7862)

a221ae0

crazycs520 mentioned this pull request Dec 13, 2018

ddl: fix panic when add index of generated column. (#8620) #8676

Merged

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

admin: refine admin check. #7862

admin: refine admin check. #7862

crazycs520 commented Oct 10, 2018

crazycs520 commented Oct 10, 2018

shenli commented Oct 10, 2018

crazycs520 commented Oct 10, 2018

winkyao Oct 10, 2018

crazycs520 Oct 10, 2018

winkyao left a comment

crazycs520 commented Oct 11, 2018

ciscoxll Oct 15, 2018 •

edited

Loading

crazycs520 Oct 16, 2018

ciscoxll left a comment

ngaut commented Oct 18, 2018

admin: refine admin check. #7862

admin: refine admin check. #7862

Conversation

crazycs520 commented Oct 10, 2018

What problem does this PR solve?

What is changed and how it works?

Check List

crazycs520 commented Oct 10, 2018

shenli commented Oct 10, 2018

crazycs520 commented Oct 10, 2018

winkyao Oct 10, 2018

Choose a reason for hiding this comment

crazycs520 Oct 10, 2018

Choose a reason for hiding this comment

winkyao left a comment

Choose a reason for hiding this comment

crazycs520 commented Oct 11, 2018

ciscoxll Oct 15, 2018 • edited Loading

Choose a reason for hiding this comment

crazycs520 Oct 16, 2018

Choose a reason for hiding this comment

ciscoxll left a comment

Choose a reason for hiding this comment

ngaut commented Oct 18, 2018

ciscoxll Oct 15, 2018 •

edited

Loading