*: fix a bug causes indexed virtual generated column return wrong value and refine admin check table #18408

wjhuang2016 · 2020-07-07T07:15:12Z

What problem does this PR solve?

Issue Number: close #17989

Problem Summary:

Previously, we try to use column substitution to decode a multi-level virtual generated column, which is buggy.
Since admin check table uses tableReader, and tableReader can output rows with a virtual generated column already, we don't need to compute it again. Besides, the previous implementation also suffered the above problem.
I’m

What is changed and how it works?

Build virtual expression for virtual generated column in ColumnInfos2ColumnsAndNames.
Remove genExprs in checkIndexValue.
In compareData, don't compute the virtual generated column again.
Export a function rewriteAstExpr, so that we can use rewrite in package expression.
Refine buildPhysicalIndexLookUpReader, make it simple.
Move table/tables/gen_expr_test.go → util/generatedexpr/gen_expr_test.go. to avoid circular reference.

What's Changed:

How it Works:

Related changes

Need to cherry-pick to the release branch

Check List

Tests

Unit test

Side effects

Release note

Fix a bug causes indexed virtual generated column return wrong value and refine admin check table

Signed-off-by: wjhuang2016 <[email protected]>

…ex_vgc

Signed-off-by: wjhuang2016 <[email protected]>

util/admin/admin.go

util/rowDecoder/decoder.go

Signed-off-by: wjhuang2016 <[email protected]>

…ex_vgc

wjhuang2016 · 2020-07-15T09:41:27Z

/run-all-tests

codecov · 2020-07-15T09:50:59Z

Codecov Report

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

@@             Coverage Diff             @@
##             master     #18408   +/-   ##
===========================================
  Coverage   79.6853%   79.6853%           
===========================================
  Files           543        543           
  Lines        148922     148922           
===========================================
  Hits         118669     118669           
  Misses        20916      20916           
  Partials       9337       9337

Signed-off-by: wjhuang2016 <[email protected]>

wjhuang2016 · 2020-07-15T16:33:51Z

/run-all-tests

expression/expression.go

djshow832 · 2020-07-20T10:23:34Z

expression/expression.go

+	save := ctx.GetSessionVars().StmtCtx.IgnoreTruncate
+	defer func() {
+		ctx.GetSessionVars().StmtCtx.IgnoreTruncate = save
+	}()
+	ctx.GetSessionVars().StmtCtx.IgnoreTruncate = true


Please add a comment for this.

djshow832 · 2020-07-20T10:40:53Z

planner/core/expression_rewriter.go

@@ -46,17 +46,30 @@ func evalAstExpr(sctx sessionctx.Context, expr ast.ExprNode) (types.Datum, error
 	if val, ok := expr.(*driver.ValueExpr); ok {
 		return val.Datum, nil
 	}
+	NewExpr, err := rewriteAstExpr(sctx, expr, nil, nil)


Suggested change

NewExpr, err := rewriteAstExpr(sctx, expr, nil, nil)

newExpr, err := rewriteAstExpr(sctx, expr, nil, nil)

planner/core/expression_rewriter.go

planner/core/planbuilder.go

util/rowDecoder/decoder.go

Co-authored-by: djshow832 <[email protected]>

djshow832

Please address comments and involve a member from executor SIG. Rest LGTM.

bb7133 · 2020-07-24T03:11:24Z

LGTM

bb7133 · 2020-07-24T03:11:31Z

/run-all-tests

Signed-off-by: wjhuang2016 <[email protected]>

…ex_vgc

djshow832

LGTM

qw4990

LGTM

qw4990 · 2020-07-24T03:15:43Z

/merge

ti-srebot · 2020-07-24T03:16:00Z

/run-all-tests

ti-srebot · 2020-07-24T03:19:03Z

@wjhuang2016 merge failed.

bb7133 · 2020-07-24T04:47:24Z

/merge

ti-srebot · 2020-07-24T04:49:33Z

/run-all-tests

…ue and refine admin check table (#18408) (#19062) * save Signed-off-by: wjhuang2016 <[email protected]> * done Signed-off-by: wjhuang2016 <[email protected]> * 1 Signed-off-by: wjhuang2016 <[email protected]> * 1 Signed-off-by: wjhuang2016 <[email protected]> Co-authored-by: ti-srebot <[email protected]>

wjhuang2016 added 5 commits June 15, 2020 13:08

save

42407a0

Signed-off-by: wjhuang2016 <[email protected]>

Merge branch 'master' of https://github.com/pingcap/tidb into fix_ind…

3f98b61

…ex_vgc

Merge branch 'master' of https://github.com/pingcap/tidb into fix_ind…

a6f2c76

…ex_vgc

Merge branch 'master' of https://github.com/pingcap/tidb into fix_ind…

9ff9694

…ex_vgc

done

8068838

Signed-off-by: wjhuang2016 <[email protected]>

wjhuang2016 requested review from a team as code owners July 7, 2020 07:15

wjhuang2016 requested review from qw4990 and XuHuaiyu and removed request for a team July 7, 2020 07:15

wjhuang2016 added the status/WIP label Jul 7, 2020

clean

6ca6f4a

Signed-off-by: wjhuang2016 <[email protected]>

github-actions bot added sig/sql-infra SIG: SQL Infra sig/execution SIG execution component/expression labels Jul 7, 2020

wjhuang2016 added 5 commits July 7, 2020 15:26

clean

8376b4e

Signed-off-by: wjhuang2016 <[email protected]>

clean

2cee529

Signed-off-by: wjhuang2016 <[email protected]>

clean

0acd870

Signed-off-by: wjhuang2016 <[email protected]>

done

dae1ecb

Signed-off-by: wjhuang2016 <[email protected]>

done

6cd1dda

Signed-off-by: wjhuang2016 <[email protected]>

wjhuang2016 force-pushed the fix_index_vgc branch from a0b2328 to 6cd1dda Compare July 9, 2020 05:13

wjhuang2016 removed the status/WIP label Jul 10, 2020

tangenta reviewed Jul 14, 2020

View reviewed changes

util/admin/admin.go Show resolved Hide resolved

util/rowDecoder/decoder.go Show resolved Hide resolved

wjhuang2016 added 2 commits July 15, 2020 14:14

fix

88cfd10

Signed-off-by: wjhuang2016 <[email protected]>

Merge branch 'master' of https://github.com/pingcap/tidb into fix_ind…

0ca784e

…ex_vgc

done

81a0bac

Signed-off-by: wjhuang2016 <[email protected]>

tangenta reviewed Jul 17, 2020

View reviewed changes

expression/expression.go Show resolved Hide resolved

djshow832 reviewed Jul 20, 2020

View reviewed changes

wjhuang2016 and others added 3 commits July 22, 2020 15:21

Update expression/expression.go

6f66a09

Co-authored-by: djshow832 <[email protected]>

Update planner/core/expression_rewriter.go

1a74932

Co-authored-by: djshow832 <[email protected]>

Update util/rowDecoder/decoder.go

5ffa4d7

Co-authored-by: djshow832 <[email protected]>

djshow832 reviewed Jul 23, 2020

View reviewed changes

ti-srebot added the status/LGT1 Indicates that a PR has LGTM 1. label Jul 24, 2020

address comment

8063f60

Signed-off-by: wjhuang2016 <[email protected]>

bb7133 added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Jul 24, 2020

Merge branch 'master' of https://github.com/pingcap/tidb into fix_ind…

12dc085

…ex_vgc

wjhuang2016 requested review from djshow832 and tangenta July 24, 2020 03:14

djshow832 reviewed Jul 24, 2020

View reviewed changes

ti-srebot approved these changes Jul 24, 2020

View reviewed changes

qw4990 approved these changes Jul 24, 2020

View reviewed changes

ti-srebot added status/LGT3 The PR has already had 3 LGTM. and removed status/LGT2 Indicates that a PR has LGTM 2. labels Jul 24, 2020

ti-srebot added the status/can-merge Indicates a PR has been approved by a committer. label Jul 24, 2020

Merge branch 'master' into fix_index_vgc

73ab039

bb7133 merged commit baf6c99 into pingcap:master Jul 24, 2020

wjhuang2016 deleted the fix_index_vgc branch November 17, 2022 11:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

*: fix a bug causes indexed virtual generated column return wrong value and refine admin check table #18408

*: fix a bug causes indexed virtual generated column return wrong value and refine admin check table #18408

wjhuang2016 commented Jul 7, 2020 •

edited

Loading

wjhuang2016 commented Jul 15, 2020

codecov bot commented Jul 15, 2020 •

edited

Loading

wjhuang2016 commented Jul 15, 2020

djshow832 Jul 20, 2020

djshow832 Jul 20, 2020

djshow832 left a comment

bb7133 commented Jul 24, 2020

bb7133 commented Jul 24, 2020

djshow832 left a comment

qw4990 left a comment

qw4990 commented Jul 24, 2020

ti-srebot commented Jul 24, 2020

ti-srebot commented Jul 24, 2020

bb7133 commented Jul 24, 2020

ti-srebot commented Jul 24, 2020

	NewExpr, err := rewriteAstExpr(sctx, expr, nil, nil)
	newExpr, err := rewriteAstExpr(sctx, expr, nil, nil)

*: fix a bug causes indexed virtual generated column return wrong value and refine admin check table #18408

*: fix a bug causes indexed virtual generated column return wrong value and refine admin check table #18408

Conversation

wjhuang2016 commented Jul 7, 2020 • edited Loading

What problem does this PR solve?

What is changed and how it works?

Related changes

Check List

Release note

wjhuang2016 commented Jul 15, 2020

codecov bot commented Jul 15, 2020 • edited Loading

Codecov Report

wjhuang2016 commented Jul 15, 2020

djshow832 Jul 20, 2020

Choose a reason for hiding this comment

djshow832 Jul 20, 2020

Choose a reason for hiding this comment

djshow832 left a comment

Choose a reason for hiding this comment

bb7133 commented Jul 24, 2020

bb7133 commented Jul 24, 2020

djshow832 left a comment

Choose a reason for hiding this comment

qw4990 left a comment

Choose a reason for hiding this comment

qw4990 commented Jul 24, 2020

ti-srebot commented Jul 24, 2020

ti-srebot commented Jul 24, 2020

bb7133 commented Jul 24, 2020

ti-srebot commented Jul 24, 2020

wjhuang2016 commented Jul 7, 2020 •

edited

Loading

codecov bot commented Jul 15, 2020 •

edited

Loading