planner: donot prune all columns for Projection #24024

guo-shaoge · 2021-04-14T10:04:11Z

Signed-off-by: guo-shaoge shaoge1994@163.com

What problem does this PR solve?

Issue Number: close #23887

Problem Summary:

Hash Join (eq("abc", tbl1.col1))
  |                     |
Projection     DataSource(tbl1)
  |
Index Join

We prune all columns in Projection
Resolve indices for col1, Its index is 0(because Projection's output columns are all pruned)
Projection is eliminated, because its output columns are all pruned. But Index Join still has output columns
Got panic when calc eq("abc", tbl1.col1). Because use index 0 to eval tbl1.col1

What is changed and how it works?

What's Changed:

We donot prune all columns in Projection. If Projection is pruned to empty, we make it a Projection 1 (just like count(1))
Delete some code in rule_eliminate_projection.go, because Projection cannot be empty. Check here

Related changes

PR to update pingcap/docs/pingcap/docs-cn:
Need to cherry-pick to the release branch

Check List

Tests

Integration test

Side effects: no

Release note

donot prune all columns for Projection

guo-shaoge · 2021-04-15T06:49:54Z

/run-all-tests

guo-shaoge · 2021-04-15T09:03:25Z

@winoros @lzmhhh123 PTAL

guo-shaoge · 2021-04-15T09:55:31Z

As @lzmhhh123 suggested, I'm trying to make it Projection(1), just like Count(1) does. Good for performance.

guo-shaoge · 2021-04-15T13:47:00Z

/run-all-tests

lzmhhh123

How about adding some tests? BTW, can we avoid pruning projection in tryEliminateProjection?

lzmhhh123 · 2021-04-16T02:29:01Z

@winoros PTAL the PR. Do you have any better way to solve the issue?

guo-shaoge · 2021-04-16T02:35:45Z

tryEliminateProjection

You mean we do not eliminate Projection whose output column is empty?

Maybe we can not do that because this PR

lzmhhh123

tryEliminateProjection

You mean we do not eliminate Projection whose output column is empty?

Maybe we can not do that because this PR

No, we can check whether a join above it.

guo-shaoge · 2021-04-16T02:42:27Z

tryEliminateProjection

You mean we do not eliminate Projection whose output column is empty?
Maybe we can not do that because this PR

No, we can check whether a join above it.

I think this bug is not only for Join, for example, Apply may also got same error.

Apply
 |       |
DS   Projection
         |
        xxx

You can repeat this bug use following SQL(I will add it to testcase):

drop table if exists t, s;
create table t(a int, b int);
insert into t values(1, 2), (3, 4);
select (2) in (select b from t) from (select t.a < (select t.a from t t1 limit 1) from t) t;

planner/core/rule_column_pruning.go

guo-shaoge · 2021-04-16T07:51:21Z

/run-cla

Signed-off-by: guo-shaoge <shaoge1994@163.com>

XuHuaiyu · 2021-04-16T09:00:29Z

/merge

ti-chi-bot · 2021-04-16T09:00:32Z

This pull request has been accepted and is ready to merge.

Commit hash: 037b140

guo-shaoge · 2021-04-16T09:17:12Z

/run-unit-test

guo-shaoge · 2021-04-16T09:17:39Z

/run-all-tests

guo-shaoge · 2021-04-16T10:14:06Z

/run-mybatis-test

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

ti-srebot · 2021-04-16T10:18:38Z

cherry pick to release-4.0 in PR #24092

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

ti-srebot · 2021-04-16T10:19:43Z

cherry pick to release-5.0 in PR #24093

… (pingcap#24093)" This reverts commit f5cac30. Signed-off-by: guo-shaoge <shaoge1994@163.com>

)" (#24202)

This reverts commit 4cd7fb0.

…4180)

guo-shaoge requested a review from a team as a code owner April 14, 2021 10:04

guo-shaoge requested review from lzmhhh123 and removed request for a team April 14, 2021 10:04

ti-chi-bot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Apr 14, 2021

ichn-hu mentioned this pull request Apr 14, 2021

Welcome to contribute #20804

Closed

guo-shaoge force-pushed the 23887_tiflash_hash_join branch from cdc7df2 to 46f1532 Compare April 14, 2021 15:29

ti-chi-bot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Apr 14, 2021

XuHuaiyu added needs-cherry-pick-release-5.0 type/bugfix This PR fixes a bug. labels Apr 15, 2021

guo-shaoge force-pushed the 23887_tiflash_hash_join branch from 7188b06 to 57d4fa3 Compare April 15, 2021 06:30

guo-shaoge force-pushed the 23887_tiflash_hash_join branch from 9f1aaf7 to 5e14d5a Compare April 15, 2021 08:34

ti-chi-bot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Apr 15, 2021

guo-shaoge changed the title ~~planner: do physical projection eliminatoin before resolve column indices~~ planner: donot prune all columns for Projection Apr 15, 2021

guo-shaoge force-pushed the 23887_tiflash_hash_join branch from 5e14d5a to 6e33207 Compare April 15, 2021 11:54

ti-chi-bot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Apr 15, 2021

lzmhhh123 reviewed Apr 16, 2021

View reviewed changes

winoros reviewed Apr 16, 2021

View reviewed changes

planner/core/rule_column_pruning.go Outdated Show resolved Hide resolved

planner/core/rule_column_pruning.go Show resolved Hide resolved

guo-shaoge force-pushed the 23887_tiflash_hash_join branch from 2154047 to b6cd2e5 Compare April 16, 2021 07:11

ti-chi-bot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Apr 16, 2021

trivial change

037b140

Signed-off-by: guo-shaoge <shaoge1994@163.com>

ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Apr 16, 2021

Merge branch 'master' into 23887_tiflash_hash_join

df1ab2b

ti-chi-bot merged commit 4cd7fb0 into pingcap:master Apr 16, 2021

ti-srebot pushed a commit to ti-srebot/tidb that referenced this pull request Apr 16, 2021

cherry pick pingcap#24024 to release-4.0

0009bc4

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

ti-srebot mentioned this pull request Apr 16, 2021

planner: do not prune all columns for Projection (#24024) #24092

Closed

ti-srebot mentioned this pull request Apr 16, 2021

planner: donot prune all columns for Projection (#24024) #24093

Merged

ti-srebot pushed a commit to ti-srebot/tidb that referenced this pull request Apr 16, 2021

cherry pick pingcap#24024 to release-5.0

242466f

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

wshwsh12 mentioned this pull request Apr 16, 2021

executor: wrong results for not in clause #24071

Closed

hanfei1991 mentioned this pull request Apr 16, 2021

tidb reports error if there is a leftsemijoin on the top but refers no columns on probe side on which runs a mpp join pingcap/tiflash#1718

Closed

wshwsh12 mentioned this pull request Apr 16, 2021

executor: wrong results for a outer join #23904

Closed

ti-chi-bot pushed a commit that referenced this pull request Apr 16, 2021

planner: donot prune all columns for Projection (#24024) (#24093)

f5cac30

lzmhhh123 mentioned this pull request Apr 20, 2021

v5.0.1-pre Inconsistent select count(1) #24149

Closed

XuHuaiyu added the sig/planner SIG: Planner label Apr 22, 2021

guo-shaoge added a commit to guo-shaoge/tidb that referenced this pull request Apr 22, 2021

Revert "planner: donot prune all columns for Projection (pingcap#24024)…

476c6e4

… (pingcap#24093)" This reverts commit f5cac30. Signed-off-by: guo-shaoge <shaoge1994@163.com>

ti-chi-bot pushed a commit that referenced this pull request Apr 22, 2021

Revert "planner: donot prune all columns for Projection (#24024) (#24093

1145e34

)" (#24202)

guo-shaoge added a commit to guo-shaoge/tidb that referenced this pull request Apr 23, 2021

Revert "planner: donot prune all columns for Projection (pingcap#24024)"

5229998

This reverts commit 4cd7fb0.

ti-chi-bot pushed a commit that referenced this pull request Apr 25, 2021

Revert "planner: donot prune all columns for Projection (#24024)" (#2…

fb676b7

…4180)

guo-shaoge mentioned this pull request Apr 29, 2021

planner: fix column pruning bug for Apply and Join #24369

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

planner: donot prune all columns for Projection #24024

planner: donot prune all columns for Projection #24024

guo-shaoge commented Apr 14, 2021 •

edited

Loading

guo-shaoge commented Apr 15, 2021

guo-shaoge commented Apr 15, 2021

guo-shaoge commented Apr 15, 2021 •

edited

Loading

guo-shaoge commented Apr 15, 2021

lzmhhh123 left a comment

lzmhhh123 commented Apr 16, 2021

guo-shaoge commented Apr 16, 2021

lzmhhh123 left a comment

guo-shaoge commented Apr 16, 2021 •

edited

Loading

guo-shaoge commented Apr 16, 2021

XuHuaiyu commented Apr 16, 2021

ti-chi-bot commented Apr 16, 2021

guo-shaoge commented Apr 16, 2021

guo-shaoge commented Apr 16, 2021

guo-shaoge commented Apr 16, 2021

ti-srebot commented Apr 16, 2021

ti-srebot commented Apr 16, 2021

planner: donot prune all columns for Projection #24024

planner: donot prune all columns for Projection #24024

Conversation

guo-shaoge commented Apr 14, 2021 • edited Loading

What problem does this PR solve?

What is changed and how it works?

Related changes

Check List

Release note

guo-shaoge commented Apr 15, 2021

guo-shaoge commented Apr 15, 2021

guo-shaoge commented Apr 15, 2021 • edited Loading

guo-shaoge commented Apr 15, 2021

lzmhhh123 left a comment

Choose a reason for hiding this comment

lzmhhh123 commented Apr 16, 2021

guo-shaoge commented Apr 16, 2021

lzmhhh123 left a comment

Choose a reason for hiding this comment

guo-shaoge commented Apr 16, 2021 • edited Loading

guo-shaoge commented Apr 16, 2021

XuHuaiyu commented Apr 16, 2021

ti-chi-bot commented Apr 16, 2021

guo-shaoge commented Apr 16, 2021

guo-shaoge commented Apr 16, 2021

guo-shaoge commented Apr 16, 2021

ti-srebot commented Apr 16, 2021

ti-srebot commented Apr 16, 2021

guo-shaoge commented Apr 14, 2021 •

edited

Loading

guo-shaoge commented Apr 15, 2021 •

edited

Loading

guo-shaoge commented Apr 16, 2021 •

edited

Loading