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 index join to support all join types #4944

Merged
merged 13 commits into from Nov 1, 2017

Conversation

Projects
None yet
3 participants
@zz-jason
Member

zz-jason commented Oct 30, 2017

fix #4901

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

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

@zz-jason

This comment has been minimized.

Show comment
Hide comment
@zz-jason

zz-jason Oct 31, 2017

Member

/run-all-test tidb-test=pr/385

Member

zz-jason commented Oct 31, 2017

/run-all-test tidb-test=pr/385

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

@zz-jason

This comment has been minimized.

Show comment
Hide comment
@zz-jason
Member

zz-jason commented Oct 31, 2017

p.SetSchema(expression.MergeSchema(p.Children()[0].Schema(), p.Children()[1].Schema()))
case *PhysicalIndexJoin:

This comment has been minimized.

@hanfei1991

hanfei1991 Oct 31, 2017

Member

This logic is the same as MergeJoin ?

@hanfei1991

hanfei1991 Oct 31, 2017

Member

This logic is the same as MergeJoin ?

This comment has been minimized.

@zz-jason

zz-jason Oct 31, 2017

Member

No, there is a little difference when the join type is InnerJoin, LeftOuterJoin and RightOuterJoin

@zz-jason

zz-jason Oct 31, 2017

Member

No, there is a little difference when the join type is InnerJoin, LeftOuterJoin and RightOuterJoin

Show outdated Hide outdated plan/new_physical_plan_builder.go
Show outdated Hide outdated executor/index_reader.go

zz-jason added some commits Oct 31, 2017

@hanfei1991

This comment has been minimized.

Show comment
Hide comment
@hanfei1991

hanfei1991 Oct 31, 2017

Member

LGTM

Member

hanfei1991 commented Oct 31, 2017

LGTM

@zz-jason

This comment has been minimized.

Show comment
Hide comment
@zz-jason
Member

zz-jason commented Oct 31, 2017

@coocood PTAL

Show outdated Hide outdated executor/index_lookup_join.go
innerKeys []*expression.Column
outerFilter expression.CNFExprs
innerFilter expression.CNFExprs
outerOrderedRows *keyRowBlock

This comment has been minimized.

@winoros

winoros Oct 31, 2017

Member

outerRowBlock is better? It's not ordered when initialized.

@winoros

winoros Oct 31, 2017

Member

outerRowBlock is better? It's not ordered when initialized.

This comment has been minimized.

@zz-jason

zz-jason Oct 31, 2017

Member

I think outerOrderedRows is better, it means we should make it ordered before using it.

@zz-jason

zz-jason Oct 31, 2017

Member

I think outerOrderedRows is better, it means we should make it ordered before using it.

Show outdated Hide outdated executor/index_lookup_join.go
Show outdated Hide outdated executor/index_lookup_join.go

zz-jason added some commits Oct 31, 2017

@zz-jason

This comment has been minimized.

Show comment
Hide comment
@zz-jason

zz-jason Oct 31, 2017

Member

/run-all-test tidb-test=pr/387

Member

zz-jason commented Oct 31, 2017

/run-all-test tidb-test=pr/387

@zz-jason

This comment has been minimized.

Show comment
Hide comment
@zz-jason

zz-jason Oct 31, 2017

Member

/run-unit-test

Member

zz-jason commented Oct 31, 2017

/run-unit-test

@winoros

winoros approved these changes Nov 1, 2017

LGTM

@winoros winoros added status/LGT2 and removed status/LGT1 labels Nov 1, 2017

@hanfei1991 hanfei1991 merged commit 0b24d60 into pingcap:master Nov 1, 2017

5 checks passed

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 71.729%
Details
jenkins-ci-tidb/build Jenkins job succeeded.
Details
license/cla Contributor License Agreement is signed.
Details

@zz-jason zz-jason deleted the zz-jason:dev/semijoin/indexjoin branch Nov 1, 2017

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

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