planner: fix a bug that can't find column in indexLookupJoin with virtual generated column #18516

wjhuang2016 · 2020-07-13T11:47:36Z

Signed-off-by: wjhuang2016 huangwenjun1997@gmail.com

What problem does this PR solve?

Issue Number: close #18515

Problem Summary:

ExpandVirtualColumn is missed.

What is changed and how it works?

let finishCopTask handle ExpandVirtualColumn.
Move (p PointGetPlan) Init to initialize.go

Related changes

Need to cherry-pick to the release branch

Check List

Tests

Unit test

Side effects

Release note

fix a bug that can't find column in indexLookupJoin with virtual generated column

Signed-off-by: wjhuang2016 <huangwenjun1997@gmail.com>

bb7133 · 2020-07-13T14:20:31Z

/run-all-tests

bb7133 · 2020-07-14T02:51:57Z

 ==================
[2020-07-14T02:48:07.003Z] WARNING: DATA RACE
[2020-07-14T02:48:07.003Z] Write at 0x00c005674788 by goroutine 56:
[2020-07-14T02:48:07.003Z]   github.com/pingcap/tidb/executor.ResetContextOfStmt()
[2020-07-14T02:48:07.003Z]       /home/jenkins/agent/workspace/tidb_ghpr_unit_test/go/src/github.com/pingcap/tidb/executor/executor.go:1666 +0xe1d
[2020-07-14T02:48:07.003Z]   github.com/pingcap/tidb/session.(*session).ExecuteStmt()
[2020-07-14T02:48:07.003Z]       /home/jenkins/agent/workspace/tidb_ghpr_unit_test/go/src/github.com/pingcap/tidb/session/session.go:1140 +0x1f1
[2020-07-14T02:48:07.003Z]   github.com/pingcap/tidb/session.(*session).Execute()
[2020-07-14T02:48:07.003Z]       /home/jenkins/agent/workspace/tidb_ghpr_unit_test/go/src/github.com/pingcap/tidb/session/session.go:1081 +0x3ee
[2020-07-14T02:48:07.003Z]   github.com/pingcap/tidb/session.execRestrictedSQL()
[2020-07-14T02:48:07.003Z]       /home/jenkins/agent/workspace/tidb_ghpr_unit_test/go/src/github.com/pingcap/tidb/session/session.go:835 +0x15c
[2020-07-14T02:48:07.003Z]   github.com/pingcap/tidb/session.(*session).ExecRestrictedSQLWithContext()
[2020-07-14T02:48:07.003Z]       /home/jenkins/agent/workspace/tidb_ghpr_unit_test/go/src/github.com/pingcap/tidb/session/session.go:774 +0x25b
[2020-07-14T02:48:07.003Z]   github.com/pingcap/tidb/session.(*session).ExecRestrictedSQL()
[2020-07-14T02:48:07.003Z]       /home/jenkins/agent/workspace/tidb_ghpr_unit_test/go/src/github.com/pingcap/tidb/session/session.go:744 +0x92
[2020-07-14T02:48:07.003Z]   github.com/pingcap/tidb/statistics/handle.(*Handle).Update()
[2020-07-14T02:48:07.003Z]       /home/jenkins/agent/workspace/tidb_ghpr_unit_test/go/src/github.com/pingcap/tidb/statistics/handle/handle.go:162 +0x1eb
[2020-07-14T02:48:07.003Z]   github.com/pingcap/tidb/domain.(*Domain).loadStatsWorker()
[2020-07-14T02:48:07.003Z]       /home/jenkins/agent/workspace/tidb_ghpr_unit_test/go/src/github.com/pingcap/tidb/domain/domain.go:1096 +0x571
[2020-07-14T02:48:07.003Z] 
[2020-07-14T02:48:07.003Z] Previous read at 0x00c005674788 by goroutine 111:
[2020-07-14T02:48:07.003Z]   github.com/pingcap/tidb/executor.(*IndexLookUpExecutor).startIndexWorker.func1()
[2020-07-14T02:48:07.003Z]       /home/jenkins/agent/workspace/tidb_ghpr_unit_test/go/src/github.com/pingcap/tidb/executor/distsql.go:514 +0x477
[2020-07-14T02:48:07.003Z] 
[2020-07-14T02:48:07.003Z] Goroutine 56 (running) created at:
[2020-07-14T02:48:07.003Z]   github.com/pingcap/tidb/domain.(*Domain).UpdateTableStatsLoop()
[2020-07-14T02:48:07.003Z]       /home/jenkins/agent/workspace/tidb_ghpr_unit_test/go/src/github.com/pingcap/tidb/domain/domain.go:1041 +0x332
[2020-07-14T02:48:07.003Z]   github.com/pingcap/tidb/session.BootstrapSession()

Failed UT case

bb7133 · 2020-07-14T02:52:06Z

/run-unit-test

codecov · 2020-07-14T02:54:25Z

Codecov Report

Merging #18516 into master will decrease coverage by 0.0207%.
The diff coverage is 100.0000%.

@@               Coverage Diff                @@
##             master     #18516        +/-   ##
================================================
- Coverage   79.0884%   79.0677%   -0.0208%     
================================================
  Files           549        549                
  Lines        149047     149210       +163     
================================================
+ Hits         117879     117977        +98     
- Misses        21659      21702        +43     
- Partials       9509       9531        +22

wjhuang2016 · 2020-07-14T04:08:56Z

/run-all-tests

wjhuang2016 · 2020-07-14T05:46:43Z

/run-common-test

wjhuang2016 · 2020-07-14T05:47:06Z

/run-common-test

Signed-off-by: wjhuang2016 <huangwenjun1997@gmail.com>

wjhuang2016 · 2020-07-14T09:00:05Z

/run-common-test

wjhuang2016 · 2020-07-20T08:13:38Z

/run-all-tests

qw4990 · 2020-07-24T05:49:00Z

PTAL @wjhuang2016 Please resolve conflicts.

lzmhhh123 · 2020-08-13T03:08:17Z

planner/core/task.go

+	if t.tablePlan != nil {
+		tp := t.tablePlan
+		for len(tp.Children()) > 0 {
+			tp = tp.Children()[0]


Suggested change

tp = tp.Children()[0]

if len(tp.Children()) == 1 {

tp = tp.Children()[0]

} else {

join := tp.(*PhysicalBroadCastJoin)

tp = join.children[1-join.InnerChildIdx]

}

We should consider broadcast join in cop task. And by the way, this check can be moved to https://github.com/pingcap/tidb/pull/18516/files#diff-2f445d8cb9c2f7d8ec4fb0586ae1f510R730

Signed-off-by: wjhuang2016 <huangwenjun1997@gmail.com>

winoros

lgtm

lzmhhh123

LGTM

lzmhhh123 · 2020-08-18T07:13:35Z

/merge

ti-srebot · 2020-08-18T07:13:38Z

Your auto merge job has been accepted, waiting for:

18732
18846
19237
19237
19237
19202
18970
19035

ti-srebot · 2020-08-18T08:05:10Z

/run-all-tests

ti-srebot · 2020-08-18T08:09:09Z

@wjhuang2016 merge failed.

wjhuang2016 · 2020-08-19T08:12:46Z

/run-all-tests

zz-jason · 2020-08-19T15:36:49Z

/merge

ti-srebot · 2020-08-19T15:36:51Z

Your auto merge job has been accepted, waiting for:

19266
19248
19210
19255
19242

ti-srebot · 2020-08-19T16:22:43Z

/run-all-tests

ti-srebot · 2020-08-19T16:34:08Z

@wjhuang2016 merge failed.

wjhuang2016 · 2020-08-25T07:55:57Z

/run-all-tests

Signed-off-by: ti-srebot <ti-srebot@pingcap.com>

ti-srebot · 2020-08-25T08:10:42Z

cherry pick to release-4.0 in PR #19439

…tual generated column (#18516) (#19439) Signed-off-by: ti-srebot <ti-srebot@pingcap.com> Signed-off-by: wjhuang2016 <huangwenjun1997@gmail.com>

done

44c33e6

Signed-off-by: wjhuang2016 <huangwenjun1997@gmail.com>

wjhuang2016 requested review from a team as code owners July 13, 2020 11:47

wjhuang2016 requested review from XuHuaiyu and removed request for a team July 13, 2020 11:47

wjhuang2016 added needs-cherry-pick-4.0 type/bugfix This PR fixes a bug. labels Jul 13, 2020

github-actions bot added the component/expression label Jul 13, 2020

fix

d80653c

Signed-off-by: wjhuang2016 <huangwenjun1997@gmail.com>

wjhuang2016 force-pushed the fix_expandVirtualColumn branch from 73ca629 to d80653c Compare July 14, 2020 08:52

Merge branch 'master' into fix_expandVirtualColumn

59ef250

Merge branch 'master' into fix_expandVirtualColumn

993b340

lzmhhh123 reviewed Aug 13, 2020

View reviewed changes

wjhuang2016 added 2 commits August 13, 2020 16:50

done

9b26ed5

Signed-off-by: wjhuang2016 <huangwenjun1997@gmail.com>

done

8666e5b

Signed-off-by: wjhuang2016 <huangwenjun1997@gmail.com>

wjhuang2016 force-pushed the fix_expandVirtualColumn branch from e329b81 to 8666e5b Compare August 13, 2020 09:25

winoros reviewed Aug 18, 2020

View reviewed changes

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

lzmhhh123 reviewed Aug 18, 2020

View reviewed changes

ti-srebot removed the status/LGT1 Indicates that a PR has LGTM 1. label Aug 18, 2020

ti-srebot approved these changes Aug 18, 2020

View reviewed changes

ti-srebot added the status/LGT2 Indicates that a PR has LGTM 2. label Aug 18, 2020

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

Merge branch 'master' into fix_expandVirtualColumn

73519f7

Merge branch 'master' into fix_expandVirtualColumn

46a4f3a

Merge branch 'master' into fix_expandVirtualColumn

f58690c

wjhuang2016 added 2 commits August 20, 2020 11:18

Merge branch 'master' into fix_expandVirtualColumn

b461fe8

Merge branch 'master' into fix_expandVirtualColumn

ec0e790

wjhuang2016 merged commit 977449f into pingcap:master Aug 25, 2020

ti-srebot pushed a commit to ti-srebot/tidb that referenced this pull request Aug 25, 2020

cherry pick pingcap#18516 to release-4.0

1c96a09

Signed-off-by: ti-srebot <ti-srebot@pingcap.com>

ti-srebot mentioned this pull request Aug 25, 2020

planner: fix a bug that can't find column in indexLookupJoin with virtual generated column (#18516) #19439

Merged

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

planner: fix a bug that can't find column in indexLookupJoin with virtual generated column #18516

planner: fix a bug that can't find column in indexLookupJoin with virtual generated column #18516

wjhuang2016 commented Jul 13, 2020

bb7133 commented Jul 13, 2020

bb7133 commented Jul 14, 2020

bb7133 commented Jul 14, 2020

codecov bot commented Jul 14, 2020 •

edited

Loading

wjhuang2016 commented Jul 14, 2020

wjhuang2016 commented Jul 14, 2020

wjhuang2016 commented Jul 14, 2020

wjhuang2016 commented Jul 14, 2020

wjhuang2016 commented Jul 20, 2020

qw4990 commented Jul 24, 2020

lzmhhh123 Aug 13, 2020

winoros left a comment

lzmhhh123 left a comment

lzmhhh123 commented Aug 18, 2020

ti-srebot commented Aug 18, 2020

ti-srebot commented Aug 18, 2020

ti-srebot commented Aug 18, 2020

wjhuang2016 commented Aug 19, 2020

zz-jason commented Aug 19, 2020

ti-srebot commented Aug 19, 2020

ti-srebot commented Aug 19, 2020

ti-srebot commented Aug 19, 2020

wjhuang2016 commented Aug 25, 2020

ti-srebot commented Aug 25, 2020

-			tp = tp.Children()[0]
+			if len(tp.Children()) == 1 {
+				tp = tp.Children()[0]
+			} else {
+				join := tp.(*PhysicalBroadCastJoin)
+				tp = join.children[1-join.InnerChildIdx]
+			}

planner: fix a bug that can't find column in indexLookupJoin with virtual generated column #18516

planner: fix a bug that can't find column in indexLookupJoin with virtual generated column #18516

Conversation

wjhuang2016 commented Jul 13, 2020

What problem does this PR solve?

What is changed and how it works?

Related changes

Check List

Release note

bb7133 commented Jul 13, 2020

bb7133 commented Jul 14, 2020

bb7133 commented Jul 14, 2020

codecov bot commented Jul 14, 2020 • edited Loading

Codecov Report

wjhuang2016 commented Jul 14, 2020

wjhuang2016 commented Jul 14, 2020

wjhuang2016 commented Jul 14, 2020

wjhuang2016 commented Jul 14, 2020

wjhuang2016 commented Jul 20, 2020

qw4990 commented Jul 24, 2020

lzmhhh123 Aug 13, 2020

Choose a reason for hiding this comment

winoros left a comment

Choose a reason for hiding this comment

lzmhhh123 left a comment

Choose a reason for hiding this comment

lzmhhh123 commented Aug 18, 2020

ti-srebot commented Aug 18, 2020

ti-srebot commented Aug 18, 2020

ti-srebot commented Aug 18, 2020

wjhuang2016 commented Aug 19, 2020

zz-jason commented Aug 19, 2020

ti-srebot commented Aug 19, 2020

ti-srebot commented Aug 19, 2020

ti-srebot commented Aug 19, 2020

wjhuang2016 commented Aug 25, 2020

ti-srebot commented Aug 25, 2020

codecov bot commented Jul 14, 2020 •

edited

Loading