New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

plan, executor: improve merge join to support all join types #4869

Merged
merged 41 commits into from Oct 26, 2017

Conversation

Projects
5 participants
@zz-jason
Member

zz-jason commented Oct 23, 2017

fix #4822

main changes:

  1. Extracted a joinResultGenerator to file executor/join_result_generators.go to emit join results according to the concrete join type (SemiJoin, AntiSemiJoin, LeftOuterJoin, InnerJoin ...), some branch predicating is removed
  2. Extracted a joinBuilder to file executor/join_builder.go
  3. Implemented SemiJoin, Anti SemiJoin, LeftOuterSemiJoin, Anti LeftOuterSemiJoin in MergeJoinExec
  4. Rewrited buildSchema() to handle SemiJoin and LeftOuterSemiJoin in PhysicalMergeJoin

@zz-jason zz-jason added the status/WIP label Oct 23, 2017

@zz-jason zz-jason added this to the 1.1 milestone Oct 23, 2017

@zz-jason zz-jason added this to Join in Runtime Oct 23, 2017

zz-jason added some commits Oct 23, 2017

"TableReader_14 MergeJoin_27 root data:TableScan_13 8000",
"IndexScan_21 cop table:t2, index:c1, range:[<nil>,+inf], out of order:false 8000",
"IndexReader_22 MergeJoin_27 root index:IndexScan_21 8000",
"MergeJoin_27 StreamAgg_9 TableReader_14,IndexReader_22 root left outer semi join, equal:[eq(test.t1.c1, test.t2.c1)], asc, left key:test.t1.c1, right key:test.t2.c1 8000",

This comment has been minimized.

@winoros

winoros Oct 23, 2017

Member

Distinguish MergeSemiJoin from MergeJoin

@winoros

winoros Oct 23, 2017

Member

Distinguish MergeSemiJoin from MergeJoin

This comment has been minimized.

@zz-jason

zz-jason Oct 25, 2017

Member

I distinguished them by operator name in plan/stringer.go. In explain info we already have join type to distinguish them, so I keep the operator name to "MergeJoin"

@zz-jason

zz-jason Oct 25, 2017

Member

I distinguished them by operator name in plan/stringer.go. In explain info we already have join type to distinguish them, so I keep the operator name to "MergeJoin"

Show outdated Hide outdated plan/expression_rewriter.go

@zz-jason zz-jason removed the status/WIP label Oct 24, 2017

zz-jason added some commits Oct 24, 2017

@zz-jason

This comment has been minimized.

Show comment
Hide comment
@zz-jason
Member

zz-jason commented Oct 24, 2017

Show outdated Hide outdated plan/physical_plans.go
Show outdated Hide outdated executor/join_result_outputers.go
Show outdated Hide outdated executor/merge_join.go
Show outdated Hide outdated executor/merge_join.go

zz-jason added some commits Oct 25, 2017

@hanfei1991

This comment has been minimized.

Show comment
Hide comment
@hanfei1991

hanfei1991 Oct 25, 2017

Member

executor/join_result_outputers.go , the file name also need to change.

Member

hanfei1991 commented Oct 25, 2017

executor/join_result_outputers.go , the file name also need to change.

@hanfei1991 hanfei1991 closed this Oct 25, 2017

@hanfei1991 hanfei1991 reopened this Oct 25, 2017

@zz-jason zz-jason changed the title from plan, executor: support SemiJoin&&LeftOuterSemiJoin in merge join to plan, executor: improve merge join to support all join types Oct 25, 2017

@zz-jason

This comment has been minimized.

Show comment
Hide comment
@zz-jason
Member

zz-jason commented Oct 26, 2017

@coocood PTAL

zz-jason added some commits Oct 26, 2017

@winoros

This comment has been minimized.

Show comment
Hide comment
@winoros

winoros Oct 26, 2017

Member

What about change joinType to bit representation in the future? We can use a bit to simply represent whether it's semi join and another bit to represent which side is its outer table. Since we have index semi join, index anti semi join and so one in the future.
LGTM

Member

winoros commented Oct 26, 2017

What about change joinType to bit representation in the future? We can use a bit to simply represent whether it's semi join and another bit to represent which side is its outer table. Since we have index semi join, index anti semi join and so one in the future.
LGTM

@coocood

This comment has been minimized.

Show comment
Hide comment
@coocood

coocood Oct 26, 2017

Member

LGTM

Nice refactor!

Member

coocood commented Oct 26, 2017

LGTM

Nice refactor!

@coocood

This comment has been minimized.

Show comment
Hide comment
@coocood

coocood Oct 26, 2017

Member

/run-all-test

Member

coocood commented Oct 26, 2017

/run-all-test

@zz-jason

This comment has been minimized.

Show comment
Hide comment
@zz-jason

zz-jason Oct 26, 2017

Member

this pr should be merged together with: #375

Member

zz-jason commented Oct 26, 2017

this pr should be merged together with: #375

@zz-jason zz-jason added status/LGT2 and removed status/LGT1 labels Oct 26, 2017

@hanfei1991 hanfei1991 merged commit d036f0e into pingcap:master Oct 26, 2017

10 of 12 checks passed

jenkins-ci-tidb/common-test Jenkins job failed
Details
jenkins-ci-tidb/integration-common-test Jenkins job failed
Details
ci/circleci Your tests passed on CircleCI!
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls First build on master at 72.4%
Details
jenkins-ci-tidb/build Jenkins job succeeded.
Details
jenkins-ci-tidb/integration-compatibility-test Jenkins job succeeded.
Details
jenkins-ci-tidb/integration-ddl-test Jenkins job succeeded.
Details
jenkins-ci-tidb/mybatis-test Jenkins job succeeded.
Details
jenkins-ci-tidb/sqllogic-test Jenkins job succeeded.
Details
jenkins-ci-tidb/unit-test Jenkins job succeeded.
Details
license/cla Contributor License Agreement is signed.
Details

@zz-jason zz-jason deleted the zz-jason:semijoin/mergejoin branch Oct 26, 2017

dbjoa added a commit to cloud-pi/tidb that referenced this pull request Oct 27, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment